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

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]




My proposal for this issue was, to reiterate it in a bit more detail:

 - merge the pull request
 - create unsupported/Eigen/CXX11/Emulated module that includes the
   required emulation stuff
 - create unsupported/Eigen/CXX11/CoreOrEmulated that either includes
   CXX11/Core or CXX11/Emulated depending on compiler version
 - move tensor module to unsupported/Eigen/Tensor (one dir up)
 - change tensor module to include
   unsupported/Eigen/CXX11/CoreOrEmulated
 - leave TensorSymmetry underneath CXX11 and have it include CXX11/Core
   but not CoreOrEmulated

Sounds good to me.
 
This has the consequence that:

 - Tensor module can now be used without C++11 support
   #include <unsupported/Eigen/Tensor>
 - Tensor module will automatically load advanced C++11 version if the
   compiler supports it, but will fall back to the C++03 version if it
   doesn't. Differences between both versions will be:
       * C++03: maximum number of indices, C++11: unlimited
         as I said before, we should choose a sensible limit, I think
         that Fortran's seven (7) is a good idea (current limit for
         Benoit's code is five (5))
 
I'll need 8 in the near future, so let's go broke ;)
 
       * otherwise, I don't see anything in the feature set that's
         specific to C++11 that doesn't require the user to actively
         use C++11 themselves (such as initializer list constructors
         etc.) 
 - TensorSymmetry module still needs C++11
   #include <unsupported/Eigen/CXX11/TensorSymmetry>

But this is definitely something that can be done after the pull
request has been merged.

So the only case somebody uses something that's C++11, it's going to be
more than MAX_INDICES incides in a tensor. If we don't make MAX_INDICES
too small, I don't think this is going to be a huge issue.


    c. Eigen::array is a bad name decision for emulating std::array,
       since array kind-of-clashes with Eigen::Array - it's going to be
       hell for people using Eigen's traditional capital-A Array
       template if they accidentally have a typo.

Is there actually a practical difference between Eigen::Array<Type,
size, 1> and std::array<Type, size>? I.e., why not allow indexing with
the already existing Eigen::Array and optionally implement a direct
mapping from std::array if that is available? You'd also don't need to
re-implement constructors for fixed sized arrays (up to size 4 at the
moment, but we could easily increase that to 6 or 8, maybe)

In C++11 std::array has a constexpr constructor and in C++14 its
accessors are also constexpr. CXX11Workarounds.h contains some
workarounds that allows C++11 compilers to have constexpr access to
array elements. On the other hand, this does not apply to Eigen::Array,
and I'm not sure how difficult it would be to do this change and be
vary of regressions later.

If it is the case that both constructor and accessors are constexpr,
then the compiler can easily optimize away a lot of things w.r.t. to
the calculation of the linear index from a set of indices and the such.
For example, the internal::array_prod I added is reduced to the correct
number of multiplication assembly instructions when calculating the
total number of elements in a tensor constructor if this is constexpr.

So, from a C++03 version of the Tensor module, where constexpr is not
available, there is indeed no reason not to use Eigen::Array. From a
C++11 standpoint, using Eigen::Array is probably far from optimal.

I use std::array in order to encode ordered lists of dimensions. I am not convinced that using Eigen::Array instead is a good choice:
 * Eigen Array is a 2d object. Users will always wonder if they need to order their coefficients in a row or a column. 
 * The Array class lacks nice to have cxx11 features such as initialization lists. This is syntactic sugar, but if I can use a recent compiler I'd rather write:
    Tensor<float, 3> T({1,2,3})
than
    Array<ptrdiff_t, 3, 1> dims;
    coeffs << 1, 2, 3;   
    Tensor<float, 3> T(dims);
 * I'm being optimistic here but a tensor is a generalization of the functionality provided by the Array class. In the distant future it might be possible to reimplement the Array class as a special case of Tensor, as long as the tensor API doesn't depend on the Array class itself.
 

--
Benoit


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/