Re: [eigen] ideas about the corner types

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

2010/2/5 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> 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. 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).

And thinking about this, maybe 2) is the most reasonable. It's more in
line with the rest of the API, it'll be time to do fancy stuff when we
develop 3.1, and it keeps TopLeft and friends as mere enums.

Benoit


>
> 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, ...
>
> 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.
>>> >>
>>> >>
>>> >
>>>
>>>
>>
>>
>>
>>
>