Re: [eigen] Initial implementation of tensor support

[Christian Seiler <christian@xxxxxxxx>]

Hi there,

> A Tensor module would of course be very welcome: it is part of the
> features that we planed since the beginning of Eigen, so I'm very glad
> there is a concrete starting point.

Thanks!

> Looking at your code, a C++98
> only version would look much more complicated while it's currently only
> about storage. So I'm not against importing it into unsupported to give
> the visibility it deserve and make it maturing more rapidly (hopefully).

Thanks!

> However, it is important to make sure C++11 features cannot be used by
> mistake. It has been suggested to ask for the user to define a new
> preprocessor token (EIGEN_ENABLE_CXX11) for that purpose. Since I'm
> trying to limit the amount of compilation options, I'd like to suggest
> two other solutions:
> 
> - Put "CXX11" in the module name, e.g., Eigen/CXX11/Tensor and all CXX11
> modules could go in Eigen/CXX11 more like "Eigen/CXX11" was a "side"
> project.
> - Add a new namespace for C++11 features, e.g., Eigen::CXX11 or simply
> Eigen11 or ????
> 
> Note that these two options are not exclusive.

Personally, I wouldn't like having two different namespaces, since the
class really should be called Eigen::Tensor IMHO, but having an extra
directory in the include path works fine for me. Especially since it
makes it immediately clear to anybody who reads the code that that
particular feature requires C++11 support just from the #include line.

This also would have two other advantages:

 1) my Tensor/util/Meta.h has some nifty little things that people
    writing other modules might find useful; and if that other module
    would also require C++11, then this could actually now be renamed
    to something like unsupported/CXX11/Core/util/Meta.h.

 2) if at some point (2-3 years or so?) the Tensor module is considered
    stable enough, it'd be really easy to merge it from unsupported/
    into Eigen's main directory, without causing too much confusion as
    to the C++ version support of Eigen.

So I personally think Eigen/CXX11 is a good idea, now that you mention it.

OTOH, for the internal:: stuff, having internal::cxx11:: is probably a
good idea; I currently use internal::tensor::, but I use that also for
not directly tensor related stuff, so in that case, a namespace would
probably be a good idea.

> Anyway, this also means the Tensor module will never be included by
> Eigen/Dense or Eigen/Eigen.

Never say never, perhaps in 10 or 15 years the compiler landscape has
changed sufficiently so that you may feel comfortable saying that C++11
will be required for Eigen 42.0. ;-)

But I agree that this is not going to happen in the foreseeable future.

> Regarding the feature set, assignment and slices are of course one of
> the most important.

Yes, definitely.

> Fixed sizes and tensor products have not been
> mentioned yet, but are very important too!

I have them on my internal TODO list. The Tensor class itself doesn't
have any support for fixed sizes yet, but if you look at the
TensorStorage class ([1], analogous to DenseStorage), the signature
already contains information about compile-time sizes, although only the
specialization for Dynamic sizes is implemented at the moment. So the
basic ideas are already there.

> Tensor products is more challenging
> regarding the API!

Yes, one of the reasons I haven't done more with the Tensor class just
yet is also the fact that I'd like to do some brainstorming about the
most useful API before writing code and wasting time.

> I've also a question regarding the ColMajor/RowMajor option. I guess
> they rather mean something like forward/backward storage order, but is
> that option really needed??

People might want to use the new tensor module to TensorMap<> (also on
my TODO list ;-)) some binary data that is already in some file they
have, that was previously written with an own tensor implementation or
some other third-party software that has one or the other order. So I
think having that option is definitely going to be useful.

So, to recap:

 * you're in favor of adding my Tensor module to Eigen
 * you want some kind of separation of C++11 features
       - I think Eigen/CXX11/ is a good idea for that
 * we both want to see quite a few features implemented in the Tensor
   module

Where do we go from here?

1) I don't really care whether you merge the existing code now and I
contribute to Eigen master or I continue working on it on my own fork
first and it's merged once you think it's ready enough. (Now that we've
established that you do intend to merge it at some point. ;-)) It all
depends on what kind of workflow you prefer. I just need to know.

2) I'm unsure about what API design for the more advanced features
should be used. For this reason, I'd like to have some kind of
brainstorming session (perhaps on IRC?) with interested parties and then
write an API design document of what kind of API is wanted in the long
run. Once that's fixed (probably after posting a first draft to the
mailing list first), there'd be a goal that I can write code towards
(and perhaps other people would be interested in contributing).

Cheers,
Christian

[1]
https://bitbucket.org/chris-se/eigen/src/760259d391f33466e36d19795ee4bc84040dce3f/unsupported/Eigen/src/Tensor/TensorStorage.h?at=tensor