Re: [eigen] Tensor Module: Index mapping support

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


I have submitted your patch. I'll monitor how well it fares on various compilers by looking at the eigen dashboard (http://manao.inria.fr/CDash/index.php?project=Eigen&display=project) over the next few days.

In the case of the coeff and coeffRef accessors, the only types that make sense are collections of integers. It's therefore ok to try to interpret unknown types as collection of indices. In the case of a constructor, we will want to support more types in the future. In particular, it would make a lot of sense to be able to construct a tensor from an Eigen matrix. This means that in the case of the constructor, we should find a way to write
EIGEN_SFINAE_ENABLE_IF(isCollectionOfIndices<CustomDimension>::value). I'll try to find a way to do this, but if you have a solution feel free to send another patch.

On Wed, Oct 14, 2015 at 4:41 AM, Gabriel <gnuetzi@xxxxxxxxx> wrote:
Cool, thanks a lot for your effort.

Some thoughts:

templated constructor conflicts with the copy constructor :

Can the SFINAE not be enhanced, such that

EIGEN_SFINAE_ENABLE_IF( !(isOfNormalIndex<CustomDimension>::value)  &&  IS_NOT_DERIVED_OF_TENSOR_BASE )

so when ever things enter a constructor which are neither a custom index nor derived from TensorBase
this custom index function is enabled.
Will that help? Or make things worse?


Thanks a lot :-)





On 10/14/2015 12:43 AM, Benoit Steiner wrote:
I tried running the other tensor tests with your patch and there are a few failures. The main issue is that the templated constructor conflicts with the copy constructor, so we need to remove it. 

I'll cleanup the patch and submit it, hopefully in the next few days.



On Mon, Oct 12, 2015 at 9:50 AM, Gabriel <gnuetzi@xxxxxxxxx> wrote:
Ahh for sure, thats a good point.

Maybee one could write another template recursion without variadic templates, for other compilers ?
Shall I try to correct that in another patch, or are you already able to integrate the patch ? :-)

Let me know :)

BR







On 10/12/2015 06:32 PM, Benoit Steiner wrote:
Thanks for the patch. It looks really good to me. The only issue I noticed is that the index conversion code depends on variadic templates, which are not well supported by many compilers yet. The solution is to wrap the code with the corresponding code using the EIGEN_HAS_VARIADIC_TEMPLATES define: this ensures that most of the tensor functionality is preserved for users who can't upgrade their toolchain.



On Fri, Oct 9, 2015 at 3:37 PM, Gabriel <gnuetzi@xxxxxxxxx> wrote:
Benoit, the correct patch is here :-)

http://eigen.tuxfamily.org/bz/show_bug.cgi?id=1084

BR


On 10/08/2015 07:49 PM, Benoit Steiner wrote:
It is currently not possible to use custom index types to extract coefficients from a tensor. If you want to add the functionality I'll be happy to review a patch/pull request.

On Thu, Oct 8, 2015 at 6:52 AM, Gabriel <gnuetzi@xxxxxxxxx> wrote:
If the user of the tensor component in eigen
uses its own indices lets say

Eigen::Array<int,3> idx(1,2,3);

this is currently not possible:

Eigen::Tensor<double,3> t(4,4,4)
t( idx ) =  5;


We could implement this maybe with the following wrappers
(here for the resize function and CXX11 support of course as example):

                template<typename IndexType>
            void resize( const IndexType & dimensions ){

                resize(dimensions, std::make_index_sequence<NTensorIndices>{} );

            }

            template<typename IndexType, std::size_t... Is>
            void resize(const IndexType & dimensions, std::index_sequence<Is...> )
            {
                m_tensor.resize( dimensions(Is)... );
            }



Or is this already provided in the tensor module such that one can use own index types ?


Thanks a lot :-)

BR Gabriel












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