Re: [eigen] [Review] Pull request 66, Huge Tensor module improvements

[Christian Seiler <christian@xxxxxxxx>]

Hi,

> > I generally like your new names, but I dont think I can reuse
> > DenseBase at the moment as a base class, since EigenBase,
> > DenseCoeffsBase and DenseBase currently only support two indices.
> > Thats why I suggested copying but not reusing Eigens own class
> > hierarchy.
>
> I dont see why this is a problem: MultiDimBase would add the needed
> multi-dim accessor overloads.

Let me just a list a couple of methods of these classes:

EigenBase:

  - cols()
  - rows()

DenseCoeffsBase:

  - rowIndexByOuterInner()
  - colIndexByOuterInner()
  - coeff(row, col)
  - coeffByOuterInner(outer, inner)
  - operator()(row, col)
  - packet(row, col)
  - packetByOuterInner(outer, inner)
  - coeffRef(row, col)
  - writePacket(row, col, val)
  - writePacketByOuterInner(outer, inner, val)
  - copyCoeff(row, col, other)
  - copyCoeffByOuterInner(outer, inner, other)
  - copyPacket(row, col, other)
  - copyPacketByOuterInner(outer, inner, other)
  - innerStride()
  - outerStride()
  - stride()
  - rowStride()
  - colStride()
  - use of {Outer,Inner}StrideAtCompileTime

DenseBase:

  - RowsAtCompileTime
  - ColsAtCompileTime
  - MaxRowsAtCompileTime
  - MaxColsAtCompileTime
  - InnerStrideAtCompileTime
  - OuterStrideAtCompileTime
  - outerSize()
  - innerSize()
  - static constructors
  - trace()
        (btw. should that really be in DenseBase?
         shouldn't that go to MatrixBase? array.trace()
         doesn't make sense to me)
  - select()
  - replicate()
  - minCoeff(IndexType*, IndexType*)
  - maxCoeff(IndexType*, IndexType*)
  - block()

Probably I missed some. But all these methods have in common that they
assume that there are only two indices for row and column, and that
vectors are a special case where one of the sizes is 1 (but not
non-existent, since there's a difference between a row- and a column-
vector).

Or to put it that way: except for the 1-index methods for linear access
I don't see anything in the EigenBase/DenseCoeffsBase that can actually
be reused for tensors directly. So I don't see the point of needlessly
deriving from DenseBase. Actually, it probably makes my life harder,
since I'll have to be really careful that in no instance these methods
are called in the wrong place.

I do think that in the long term, one could think of making the two-
index variant a special case of a more generic variant. But at least in
my eyes for now it's better to keep it separate until the generic is
stable enough.

Regards,
Christian