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

[Christian Seiler <christian@xxxxxxxx>]

Hi,

> 5. API stuff:
>
>    a. "Tensor" class prefix. For the internal classes you obviously
>       went the TensorXXX route, i.e. TensorEvaluator, 
> TensorCwiseXXXOp
>       etc. Since I raise this as a question in this thread, I thought
>       about it some more and it's probably the easiest solution.
>
>       That said, I really don't like names like TensorMap<> etc. that
>       people will then use from the public API. Also, later on 
> there's
>       probably going to be something like Ref<> etc. I think it 
> should
>       be better to leverage the same names for the Tensor module, and 
> I
>       think I have an idea how to do that without making the rest of
>       Eigen fall apart, but I have to try to see if it works first.

So here I'd like to propose a non-invasive way to expand Map<> and
Ref<> so that they can also support Tensors. The reason why they work
with both Array and Matrix in Core is that they both inherit the same
base class.

Instead of playing games with that, I've drawn up what I think could
work in this regard:

// changes in core:

enum {
  EigenSubsystemCore = 0,
};

template...
struct traits<...>
{
  typedef typename Stride<0,0> DefaultStrideType;
  enum {
    SubsystemType = EigenSubsystemCore
  };
};

template<
  typename MatrixType,
  int MapOptions=Unaligned,
  typename StrideType =
     typename internal::traits<MatrixType>::DefaultStrideType,
  int SubsystemType = internal::traits<MatrixType>::SubsystemType
> class Map;

// in Tensor:

enum {
  EigenSubsystemTensor = 1,
};

template...
struct traits<...>
{
  // until we implement strides for tensors, then change this
  typedef void DefaultStrideType;
  enum {
    SubsystemType = EigenSubsystemTensor
  };
};

template<typename TensorType, int MapOptions>
class Map<TensorType, MapOptions, void, EigenSubsystemTensor>
{
  // ...
};

The same thing for Ref, and then one should be able to do
Eigen::Map<Tensor<double, 3> > and it will just work. This doesn't
remove the requirement for duplicating the logic somewhat for tensors,
but it will allow us to reuse the Map/Ref names. Also, I don't see
anything in the tensor class that would require different tenmplate
parameters, so this creates a really nice consistent API across all
of Eigen.

Also, maybe the same logic be used to provide Ref for sparse matrices,
(Map probably doesn't make much sense for sparse) just have a
EigenSubsystemSparse or something. (I haven't looked at the Sparse
module in detail, so maybe it doesn't apply, I'm just throwing an idea
out there.)

Thoughts?

Best Regards,
Christian