Re: [eigen] Re: yet another deep change wrt storage orders...

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

Hi Marton,

2010/7/1 Márton Danóczy <marton78@xxxxxxxxx>:
> Hi Benoit,
>
> I assume you've merged it in, because code that used to work earlier
> doesn't compile now.

Yes, it's been merged 2 or 3 months ago.

> For a data acquisition module I use arrays of the
> form Array<Scalar, NCHAN, Dynamic, 0, NCHAN, MAXDATA>, where NCHAN and
> MAXDATA are #defined in a config.h file and depend on the version of
> the hardware I compile the code for. Now, if NCHAN==1, compilation
> breaks, as Eigen expects me to provide the RowMajor flag. However, my
> code relies on the data being column major.
>
> Any ideas how to fix this?

Yes, this is very easy. Let's first explain the change made in Eigen
at that time, and the reason for it. For vectors (either 1 row or 1
column), the storage order absolutely doesn't matter. Elements of the
vector are stored sequentially, anyway. So having separate row-major
and col-major vector types serves absolutely no purpose. It even has a
big disadvantage: it means that 2x more vector types can be
instantiated, potentially giving 2x more bloated binary code. So we
decided to force vector types to have the "right" storage order, that
is: col-vectors must be col-major, row-vectors must be row-major.

So this code:

    Array<Scalar, NCHAN, Dynamic, 0, NCHAN, MAXDATA>

fails when NCHAN==1, because in that case you have a row-vector (only
1 row) so it should be row-major, and your Options field being equal
to 0 means column-major order. When you specify only the first 3
template params, Eigen takes care of that for you, but here, it can't,
because you're specifying the remaining template params.

The fix is to write:

    Array<Scalar, NCHAN, Dynamic, NCHAN==1 ? RowMajor : ColMajor,
NCHAN, MAXDATA>

I understand that you said that your code relies on the data being
col-major, that's why in the above line I keep ColMajor as soon as
NCHAN!=1. In the case NCHAN==1, you have a vector anyway, so the
storage order is useless abstract nonsense, so it doesn't cost
anything to call it RowMajor.

Cheers,
Benoit



 Is there a strong reason to not support
> 1-major matrices? Can I simply take out the respective assert in
> DenseStorageBase or will something terrible happen?
>
> Thanks,
> Marton
>
>
> On 9 March 2010 07:16, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> the fork is there,
>> https://bitbucket.org/bjacob/eigen-storageorders/
>>
>> the basic change is done, but i wont merge it back until all the
>> current issues are resolved.
>> Benoit
>>
>> 2010/3/8 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
>>> as if we didn't have enough... ever since the RowMajor fixes from
>>> january, we have quite some instability in the development branch:
>>> there's always a little thing broken somewhere and fixing it results
>>> in something else getting broken. Currently, what's broken is certain
>>> matrix algorithms applied to 1x1 matrices.
>>>
>>> To summarize a little discussion that happened today over IRC:
>>>
>>> [17:30] <bjacob> the bottom of the problem is that our idea of
>>> vector-specific functions that work seamlessly for row-vecs and
>>> col-vecs, detecting 1xN vs. Nx1, completely fails on the 1x1 case
>>> [17:30] <gael_> yep
>>> [...]
>>> [17:40] <gael_> but cannot the RowMajor/ColMajor mean RowVector
>>> ColumnVector for the vector case ?
>>>
>>> With that gael stepped over one of my big taboos so i initially
>>> psychically rejected his idea... then I realized he was right, this
>>> actually simplifies a lot of code we have for vectors, and moreover:
>>>
>>> [19:26] <bjacob> gael_: wow, here's a very very good reason for it
>>> too: let's constrain the Options to Matrix, to have the right
>>> RowMajorBit. This prevents having 2 different vector types that differ
>>> only by the (unused) RowMajorBit !!
>>>
>>> So i'll start that in a fork and report here...
>>>
>>> Benoit
>>>
>>
>>
>>
>
>
>