Re: [eigen] a few more points...

["Gael Guennebaud" <gael.guennebaud@xxxxxxxxx>]

On Fri, Jan 2, 2009 at 10:28 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> Hi List,
>
> 1) Should we rename Eigen/Regression to Eigen/LeastSquares?
> after all, regression analysis is only one of the things that one can
> do with least squares, currently this module is essentially providing
> raw least squares, no fancy (non-linear) regression analysis; moreover
> other modules like LU are named after the basic algo not after the
> common use case -- so naming that module Regression is a bit like
> calling the LU module "Solving" and the QR module "Spectral" etc... We
> could keep Regression as a trivial, deprecated header for now until 2.0...

I'm ok with that.

> 2) Is it a bug that ei_traits<Block>::Flags doesn't set the AlignedBit
> even if the ForceAligned option is used?
> I have a local change like this:
> -    Flags = (MatrixType::Flags & (HereditaryBits |
> MaskPacketAccessBit | DirectAccessBit)) | FlagsLinearAccessBit,
> +    Flags = (MatrixType::Flags & (HereditaryBits |
> MaskPacketAccessBit | DirectAccessBit)) | FlagsLinearAccessBit
> +          | (int(PacketAccess)==ForceAligned ? AlignedBit : 0),
> Is this OK with you?

hm... actually ForceAligned is used when we can guarantee that the
read/write accesses to the data will be aligned even though the first
element is not aligned (e.g., m.block(...)  += ... ;). The AlignedBit
means that the first element is aligned. So they are 2 different
things, and I don't think that's safe to add AlignedBit if we have
ForceAligned, eg:

Matrix4d m1;
m1.block<2,2>(1,0) *= 2;

will take the "LinearVectorization with CompleteUnrolling" path and crash.

But I see your point, ForceAligned is currently only useful for
relatively large matrices for which there is no unrolling. Perhaps,
what you would like is a way to tell Block that our sub matrix
expression preserve alignment ?

> I've come across this issue while trying to get
> the 4x4 matrix inversion to really take advantage of vectorization.
> Recall it's working on 2x2 blocks. By the way, the block() methods
> don't currently expose the ForceAligned stuff, don't know yet if we
> want to expose that there or keep it internal.

I'd keep it internal for now....

> 3) I'm adding a StorageOrder enum in Matrix, to fix wrong code in LU.h
> that passed Flags as 4th templ param of Matrix. One more little thing
> to change when/if you do the change with the Compact option.

I won't have time to code till Monday evening....

> 4) I think we need a detailed wiki page explaining what we guarantee
> API stability for and what we don't -- indeed it's nontrivial to
> separate public from private stuff in a template lib. I think we
> should plainly exclude asDiagonal() from the API stability guarantee
> in 2.0 to leave us as much wiggling space as possible.

Yes, good idea, again no time to help till Monday evening...

gael.

---