Re: [eigen] New indexing/slicing API: almost ready to be merged

[ Thread Index | Date Index | More Archives ]

On Fri, Jan 13, 2017 at 3:18 AM, Gael Guennebaud <gael.guennebaud@xxxxxxxxx> wrote:

On Thu, Jan 12, 2017 at 9:54 PM, Yuanchen Zhu <yuanchen.zhu@xxxxxxxxx> wrote:
Great work. Looks really advanced.

​One small suggestion: seq(a, b, d) should allow the case where (b-a+d)/d < 0, and clamp the length to 0. seqN on the other hand should treat N < 0 as a compile time or run time error.

I'm not sure, it does not look right to call say, seq(13,2,2). The current code gives some tolerance to define an empty range with inclusive bounds from: b=a-1 to b=a-d (for d>0).

​But this is basically the convention of ​matlab and numpy (dont know about fortran). 

A good reason for allowing it is that you can represent an empty sequence using seq, i.e., seq(n, m, 1) with m < n. In the current version, seq(n, n-1, 1) is indeed an empty sequence, but seq(n, n-2, 1) is runtime error.


On Wed, Jan 11, 2017 at 12:09 PM, Gael Guennebaud <gael.guennebaud@xxxxxxxxx> wrote:

Hi everyone,

my implementation of a generic operator() for sub-matrix indexing and slicing is pretty well advanced, and nearly OK to be merged. The diff is quite huge, about 1600 lines of code and comments:

Of particular interest is the respective unit test that demonstrate its use and guarantees:

The main missing part is to update the user manual to present this new feature and API in an intelligible way. It would be of great help if someone would volunteer on this task.

On the user side, it provides the following static variables and functions:

  fix<N> // for fixed size parameters
  seq(start, stop [, incr])
  seqN(start, size [, incr])
  placeholders::end = last+1

PLEASE, do everyone agree on this? From the last discussions, I've introduced a placeholders namespace to ease importing the whole Eigen namespace. The user can then cherry pick the placeholders they frequently use:

  using Eigen::placeholders::last;

or all of them, or make aliases... Perhaps we can find a shorter name for this namespace?

Each parameter of the new DenseBase::operator() is compatible with:
  - a single integer
  - Eigen::all
  - any return-type of seq and seqN
  - any type providing a size() and operator[](integer) members
  - plain C array like A({2,3,4}) in c++11 (and not too old clang)

Some remarks:

 - expressions of booleans (aka masking) is not supported yet. It could be by first converting it to an array of indices if there is a high demand.

- The last and end symbols can be used as the start, stop, and size parameters of seq and seqN

- They support any arithmetic operations, including, e.g.,  end/2+1, and supporting sqrt(end) is a matter of 5 lines of code max.

- fix<N> can be used as the size and incr parameters of seq and seqN

Regarding some internal implementation details:

 - The return type of operator() is a old good Eigen::Block if possible. This means that A.tail(a) is strictly equivalent to A(seq(a,last)), there are more equivalence example in the unit test.

 - seq(a,b,d) is just an alias to seqN(a,(b-a+d)/d,d) where (b-a+d)/d can be a symbolic _expression_ if a and/or b are based on the last/end symbols. (the c++11 implementation of seq is trivial, the c++98 is horrible)

 - to support arbitrary expressions on last/end symbols I've implemented a minimalistic _expression_ template engine specialized for expressions of Index types. In C++14 it can support an arbitrary number of symbols, might become handy in the future. There is an example there:


Mail converted by MHonArc 2.6.19+