Re: [eigen] Re: [PATCH] Reverse expression (trying again)

On Fri, Feb 6, 2009 at 10:16 AM, Gael Guennebaud <gael.guennebaud@xxxxxxxxx> wrote:

Hi,

thanks for the patches. I applied them with some minor modifications:
- move Reverse to the Array module

makes sense.

- update the computation of the flags:
* remove: LinearAccessBit, PacketAccessBit, AlignedAccessBit, etc.

yep, you're right these are not needed.. Although maybe LinearAccessBit can be interesting when Direction == BothDirections?

* keep the original storage order (to my understanding the storage
order does not change)

yes the original storage order should be kept.

- I commented part which should not be currently used (packet functions)

and again, yes, maybe at some point it can be interesting to write up these, but by now it's simpler without them.

Since the Reverse _expression_ cannot have the LinearAccessBit flags,
the index based coeff*(int) functions will be called on vector
expressions only, and so they can be significantly simplified.

I'll look into simplifying them this evening, taking into account that they will only be used in matrices.
The LinearAccessBit will never be set for Reverse because the obviously the coeff*(int) are not linear, right? Or maybe it should only be set when Direction == BothDirections? And for that case it is interesting to have the coeff*(int) of BothDirections that we have now, no?

ricard

gael

On Thu, Feb 5, 2009 at 8:48 PM, Ricard Marxer Piñón

<email@xxxxxxxxxxxxxxxx> wrote:
> Ok here is part 2.
>
> I tried to tackle the index based coeff, coeffRef, packet and writePacket.
>
> I'm not sure it is right, please review it.
>
> I have also made a small change to the doc snippet of PartialRedux.
>
> Let me know if I'm on the right track.
>
> ricard
>
> On Thu, Feb 5, 2009 at 3:35 PM, Ricard Marxer Piñón <email@xxxxxxxxxxxxxxxx>
> wrote:
>>
>> Hi again,
>>
>> This time I think I've got it right. Or at least righter..
>>
>> I have added tests and as Gael suggested I have added a template parameter
>> to Reverse so that PartialRedux returns an _expression_ (however I still don't
>> know if I've got that part right).
>>
>> In the tests there is one commented part, which I would like to pass in
>> the future but I think it is a bit more complicated.
>>
>> The patch as been generated using git:
>> git format-patch git-svn
>>
>> So to apply to svn it you will need to do (I think):
>> patch -p1 -i 0001-adding-Reverse.patch
>>
>> The coeff(int index) and coeffRef(int index) are still not handled
>> (reversed). That will come in the next patch.
>>
>> Ricard
>>
>> --
>> ricard
>> http://www.ricardmarxer.com
>> http://www.caligraft.com
>
>
>
> --
> ricard
> http://www.ricardmarxer.com
> http://www.caligraft.com
>