Re: [eigen] ideas about the corner types

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

ok to add this, but i don't have time, so, patch welcome.

The closest existing thing in Eigen is topRows(), leftCols(), etc.

Someone implementing this should stay close to the current design:
 - implement dynamic version returning a RowsBlockXpr
 - implement fixed-size version returning a NRowsBlockXpr<N>
 - expand the corners.cpp unit tests

Benoit

2010/4/22 Manoj Rajagopalan <rmanoj@xxxxxxxxx>:
>
>
>
> On Thursday 22 April 2010 02:12:58 pm Benoit Jacob wrote:
>> 2010/2/5 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:
>>
>> I chose slightly longer method names to avoid any risk of confusion /
>> pollution.
>>
>> > m.topLeft()
>>
>> it's m.topLeftCorner(int r, int c)
>> etc.
>>
>> > ...
>> > m.bottom()
>> > m.left()
>>
>> m.topRows(n)
>> m.bottomRows(n)
>> m.leftCols(n)
>> m.rightCols(n)
>>
>> Benoit
>>
>> > 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.
>
> I might be a little late in joining this discussion but I would find utility
> functions to extract a block of rows or columns from the middle useful in
> addition to the corners. For example,
>
>  for(int i=0; i<n; i+=BLOCK) {
>      matrix.rows(i,i+BLOCK-1) = ...; // all cols
>      matrix.cols(i,i+BLOCK-1) = ...; // all rows
>   }
>
> These constructs are useful in block-partitioned matrix arithmetic, esp. in
> solution of PDEs on a spatial grid where mesh-points in spatial "slices" are
> related only to nearest neighbours in-slice and in immediately neighbouring
> slices.
>
> If the DenseBase<>::rows() and ::cols() names are potentially confusing then
> I'd suggest something like ::rowBlock() or colBlock()
>
> Thanks,
> Manoj
>
>
>