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

[ Thread Index | Date Index | More Archives ]


Regarding class names, Id would suggest "Tensor" for the
generalization of matrices and "MultiDimArray" for the generalization
of arrays.

Thank you! I was looking for a better name than 'TensorArray' but it
appears I missed the forest for the trees.

Ideally, Id even merge these last two notions to make Array
multi-dimensional, or in other words if everything does well, at the
end MultiDimArray would be renamed Array, and the current Array would
go to /dev/null.

Yes, this seems to me to be a good idea in the long term. However, I'd
first like to stabilize the current experimental module and slowly
implement all the features that are present in Array so far, before we
should be discussing a merge.

Im pretty sure that this is  possible without
breaking the API. Not 100% sure about the ABI though.

I'll look into that. OTOH, this is something that's probably not going
to happen in the immediate future, so maybe this might be a thing for a
future (and currently not even talked about) Eigen 4, where I don't
think ABI breakage is going to be a problem.

Regarding the
class hierarchy, this implies something like:

  <- MatrixBase
  <- MultiDimBase
     <- MultiDimArrayBase
      <- MultiDimStorageBase
        <- MultiDimArray
    <- TensorBase
       <- MultiDimStorageBase
        <- Tensor

I generally like your new names, but I don't think I can reuse
DenseBase at the moment as a base class, since EigenBase,
DenseCoeffsBase and DenseBase currently only support two indices.
That's why I suggested copying but not reusing Eigen's own class

But with your new names, the following could probably make sense:

Eigen Matrix/Array                      Tensor/MultiDimArray
EigenBase                               EigenMultiDimBase
  <- DenseCoeffsBase                      <- MultiDimDenseCoeffsBase
    <- DenseBase                            <- MultiDimBase
      <- MatrixBase                           <- TensorBase
        <- PlainObjectBase                      <- MultiDimStorageBase
          <- Matrix                               <- Tensor
      <- ArrayBase                            <- MultiDimArrayBase
        <- PlainObjectBase                      <- MultiDimStorageBase
          <- Array                                <- MultiDimArray

If at a later point Array/MultiDimArray are merged, we may think about
making EigenMultiDimBase the new EigenBase and add some specializations
for the case of one and two indices. But while the Tensor module is
still very much in flux, I think it's best to keep it separate for now.

Im also wondering whether 1D and 2D tensors should not behave as for
the current vectors and matrices regarding operator* and the likes?
operator* would be coefficient-wise for multi-dim arrays of arbitrary
dimension, for tensors it would be restricted to 1D and 2D objects.

My original idea was to add .vector() / .matrix() to 1d and 2d tensors,
such that the following code is possible:

VectorXd = tensor2d.matrix().eigenvalues();
or perhaps:
VectorXd = tensor2d.asMatrix().eigenvalues();

I don't like treating a 2d tensor as a matrix automatically, since
I think both are different perspectives on the same object.

I think there is a good argument to be made for the following
distinctions (the following diagram assumes that at some point Array
and MultiDimArray are merged):

      Matrix       ---------------->           Tensor
        ^|            .matrix()                  ^|
        ||                                       ||
        ||                                       ||
        ||.array()                    .tensor()  ||
        |+--------> (MultiDim)Array -------------+|
        +----------                 <-------------+
         .matrix()                    .array()

Or, as long as they are not merged:

         Matrix       <----------------          Tensor
           ^|                                       ^|
           ||                                       ||
 .matrix() || .array()                    .tensor() || .array()
           |v                                       |v

         Array                                MultiDimArray


Mail converted by MHonArc 2.6.19+