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

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

Thanks a lot Ricard for the patch, and Gael for the reviewing!!

2009/2/6 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
> for the record:
>
> - I added the vectorization of reverse and did the simplification at
> the same time.
> - the vectorized version of reverse is twice faster for float and x1.5
> for double
>
> gael.
>
> On Fri, Feb 6, 2009 at 12:45 PM, Gael Guennebaud
> <gael.guennebaud@xxxxxxxxx> wrote:
>> On Fri, Feb 6, 2009 at 10:23 AM, Ricard Marxer Piñón
>> <email@xxxxxxxxxxxxxxxx> wrote:
>>>> 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?
>>
>> ah, sorry your are right. if Direction == BothDirections then we
>> should enable the LinearAccessBit as well.
>>
>>
>>> 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
>>>> >
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> ricard
>>> http://www.ricardmarxer.com
>>> http://www.caligraft.com
>>>
>>
>
>
>