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

[ Thread Index | Date Index | More Archives ]


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,

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

  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,

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.)


Best Regards,

Mail converted by MHonArc 2.6.19+