Re: [eigen] ideas about the corner types

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

On Fri, Feb 5, 2010 at 9:55 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> 2010/2/5 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>>
>>
>> On Fri, Feb 5, 2010 at 4:01 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> wrote:
>>>
>>> 2010/2/5 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>>> >
>>> >
>>> > On Fri, Feb 5, 2010 at 11:43 AM, Thomas Capricelli
>>> > <orzel@xxxxxxxxxxxxxxx>
>>> > wrote:
>>> >>
>>> >>
>>> >> Something i dont understand: why do we need to use a trick to make a
>>> >> compile-time parameter to look like a runtime one ?
>>> >> Why not (as is done in other parts of eigen) do corner<TopLeft>(3,3)
>>> >> and
>>> >> makes this explicit ?
>>> >
>>> > sure, ideally I would prefer your suggestion but c++ is such that the
>>> > trick
>>> > has the advantage of not needing the template prefix (in template code):
>>> >
>>> > m.corner(TopLeft,3,3)
>>> >
>>> > vs
>>> >
>>> > m.template corner<TopLeft>(3,3)
>>>
>>> Still, Thomas has a point since elsewhere in Eigen we're just using
>>> template parameters. Perhaps we should be consistent. I guess that the
>>> biggest argument in favor of my corner() proposal here is to keep the
>>> same API as Eigen2... as you say, one could keep the current function
>>> unchanged in              Eigen2support and move to an "officially"
>>> templated one...
>>>
>>> Notice that _every_ templated method could be replaced by an
>>> _implicitly templated one in this way_, general int parameters could
>>> be wrapped in a CompileTime<int> empty struct, etc... so we just have
>>> to make a choice, to use such tricks or to use plain template methods,
>>> and be consistent with that... thanks for the reminder.
>>
>> right.
>>
>> there is also a naming issue to add Bottom, Top as we cannot reuse corner
>> for that.
>>
>> So let me suggest a completely different API for all the sub matrices:
>>
>> m(Bottom(rows))
>> m(Bottom<Rows>())
>> m(TopLeft(rows,cols))
>> m(TopLeft<Rows,Cols>())
>> m(Block(i,j,rows,cols))
>> m(Block<Rows,Cols>(i,j))
>>
>> if TopLeft, Bottom, Block (yes there is a name clash with the Block expr but
>> that's a detail), etc. inherit the same base class (CRTP powered), we can
>> have a single overload of operator() for these cases. The computation of the
>> actual block parameters would be respectively done by the Bottom, TopLeft,
>> Block, etc. classes. So it seems to me that would even simplify the
>> implementation.
>>
>> This way we could even add Ranges:
>> m(Range(i_start,i_end), Range(j_start,j_end))
>>
>> and Indexes (this one requires a special expression, but it's just to show
>> the API consistency).
>> vec(Indexes(some_vector_of_integer))
>>
>> This is just a rough idea which is btw closer to what you would do with
>> MatLab.
>>
>> Let's see our your reaction...
>
> Why not, but not now. Let's do that after 3.0. I thought about similar
> changes but I would like to consider them an additional feature to add
> in the future. Even if that means that we eventually have 2 concurrent
> syntaxes for blocks. That wouldn't even necessarily be a bad thing, as
> both approaches have their advantages.
>
> Notice one thing: for people who dont 'using namespace Eigen', the
> approach that you describe is very wordy, because it requires a lot of
> Eigen:: all over the place.

very good point, and this also apply to the current corner API.

> On the other hand, it is of course very
> attractive. So i propose to treat it as a new feature for 3.1.
>
> For now, let's stick to minimal changes. Right now we have a little
> problem: corner() doesn't guarantee that it resolves the problem at
> compile time. Let's just fix this problem:
>
> Two solutions:
>  1) the solution that I proposed. It's API stable (or essentially so).
>  2) keep corner(TopLeft, m,n) as deprecated or in Eigen2Support and
> introduce template corner<TopLeft>(m,n).
>
> Also about this:
>
>> there is also a naming issue to add Bottom
>
> How about avoiding the too polluting names Bottom, Top, Left, Right
> (especially Left and Right are super polluting) and instead do
> BottomHalf, LeftHalf, ...

hm.. and what about getting rid of corner() and instead use a set of
individual functions:


m.topLeft()
....
m.bottom()
m.left()

No namespace verbosity or pollution. Regarding the implementation,
corner is currently a big switch so I don't really see any drawback,
but feel free to correct me.


gael

> Benoit
>
>
>>
>> gael
>>
>>>
>>> Benoit
>>>
>>> >
>>> > gael.
>>> >
>>> >>
>>> >> (The trick could still be used in the eigen2 compatibility stuff to
>>> >> keep
>>> >> the old api)
>>> >>
>>> >> Thomas
>>> >> --
>>> >> Thomas Capricelli <orzel@xxxxxxxxxxxxxxx>
>>> >> http://www.freehackers.org/thomas
>>> >>
>>> >> In data venerdì 05 febbraio 2010 10:45:21, Benoit Jacob ha scritto:
>>> >> > >> Currently matrix.corner(TopLeft,3,3) takes TopLeft as a runtime
>>> >> > >> parameter. This is based around the assumption that the compiler
>>> >> > >> resolves this at compile time. In practice that seems to work
>>> >> > >> although
>>> >> > >> I only checked GCC. Here's an idea to resolve this at runtime: let
>>> >> > >> TopLeft be an object of type CornerType<TopLeft_t>, etc.. taking a
>>> >> > >> common template CornerType type allows to write the function only
>>> >> > >> once, yet have a guarantee that that stuff resolves at compile
>>> >> > >> time.
>>> >> > >
>>> >> > > Sounds good, and that does not change the API right ?
>>> >> >
>>> >> > No API change, assuming that nobody was perverse enough to actually
>>> >> > pass a runtime variable, whose value actually isn't known at compile
>>> >> > time, as the corner type.
>>> >>
>>> >>
>>> >
>>>
>>>
>>
>>
>>
>>
>
>
>