Re: [eigen] Index types change pushed

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2010/6/2 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>
>
> On Wed, Jun 2, 2010 at 7:48 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
> wrote:
>>
>> 2010/6/2 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>> >
>> >
>> > On Wed, Jun 2, 2010 at 6:39 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> > wrote:
>> >>
>> >> 2010/6/2 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>> >> >
>> >> > if you don't mind I would slightly change the implementation details
>> >> > to
>> >> > propagate the Index type trough the traits structure. Indeed, this is
>> >> > needed
>> >> > to add such a template paramter to the sparse matrix.
>> >>
>> >> I don't mind if you have to, but the ei_traits already propagates
>> >> StorageKind, from which you can get the Index type via ei_index, see
>> >> how it's done in e.g. DenseBase:
>> >>
>> >>    typedef typename ei_traits<Derived>::StorageKind StorageKind;
>> >>    typedef typename ei_index<StorageKind>::type Index;
>> >>
>> >> Do you think that this doesn't cover your needs in the Sparse module?
>> >>
>> >> Note that if you want to allow the index types to me more configurable
>> >> in the Sparse module, you can introduce more storage kinds. You could
>> >> even make them templated. Currently we have a single Sparse type for
>> >> StorageKind, but that could become Sparse<SomeIndexType> for example.
>> >> So I was considering the current system to be very flexible.
>> >
>> > this is a bit overkill and it actually does not work because the
>> > template
>> > parameter allowing to specialize the expression implementations has to
>> > be a
>> > type not a templated struct...
>>
>> I meant pass the type Sparse<short int>, not the template<typename
>> Index> Sparse. So this would work!
>
> look at that example:
>
> template<typename UnaryOp, typename MatrixType>
> class CwiseUnaryOpImpl<UnaryOp,MatrixType,Sparse>
>   : public SparseMatrixBase<CwiseUnaryOp<UnaryOp, MatrixType> >
> {
> ...
> };
>
> we don't want to have to specialize CwiseUnaryOpImpl for each supported
> index types....

ok ok, I hadn't thought of that one!

>> Do as you think is right :)
>
> already half done ;)

Thanks!
Benoit

>
> gael
>
>>
>> Benoit
>>
>> >
>> > Propagating the Index type as the scalar type sounds more natural to me.
>> >
>> > gael
>> >
>> >>
>> >> Benoit
>> >>
>> >> >
>> >> > gael
>> >> >
>> >> > On Sun, May 30, 2010 at 10:10 PM, Benoit Jacob
>> >> > <jacob.benoit.1@xxxxxxxxx>
>> >> > wrote:
>> >> >>
>> >> >> Hi list,
>> >> >>
>> >> >> The huge "index types" change is pushed to the development branch.
>> >> >>
>> >> >> *** What changed ***
>> >> >>
>> >> >> Before, we used type int for all indices. A recent huge thread on
>> >> >> this
>> >> >> list showed that while the choice of a signed type was sensible, it
>> >> >> would be good to make it more adaptable. This is what this change
>> >> >> does:
>> >> >>  - dense matrices now default to std::ptrdiff_t, which means that
>> >> >> you're sure to be able to address half the size of your address
>> >> >> space
>> >> >> (half, because it's signed).
>> >> >>  - sparse matrices are still using int, because the sizeof() matters
>> >> >> a
>> >> >> lot there (since we are story arrays of indices).
>> >> >>
>> >> >> You can change that at will by defining
>> >> >> EIGEN_DEFAULT_DENSE_INDEX_TYPE
>> >> >> and EIGEN_DEFAULT_SPARSE_INDEX_TYPE. Of course, this breaks ABI
>> >> >> compatibility.
>> >> >>
>> >> >> In the future we'll probably add an option to Sparse matrix types to
>> >> >> specify different index types. On the other hand, for Dense
>> >> >> matrices,
>> >> >> I don't think there can ever be a valid reason to use something else
>> >> >> than ptrdiff_t. Notice that for fixed-size matrices, this doesn't
>> >> >> have
>> >> >> any cost as the sizes aren't stored as runtime variables.
>> >> >>
>> >> >> *** How this works ***
>> >> >>
>> >> >> This works per-StorageKind. So if in the future we add a new
>> >> >> StorageKind for "Dense matrix stored over distributed storage",
>> >> >> we're
>> >> >> free to specify e.g. long long int for them, without affecting the
>> >> >> rest.
>> >> >>
>> >> >> See in XprHelper.h, the new struct ei_index, see this code:
>> >> >>
>> >> >>
>> >> >> template<typename StorageKind> struct ei_index {};
>> >> >>
>> >> >> template<>
>> >> >> struct ei_index<Dense>
>> >> >> { typedef EIGEN_DEFAULT_DENSE_INDEX_TYPE type; };
>> >> >>
>> >> >> typedef typename ei_index<Dense>::type DenseIndex;
>> >> >>
>> >> >>
>> >> >> Then in the macro EIGEN_DENSE_PUBLIC_INTERFACE, we now have:
>> >> >>
>> >> >>  typedef typename Eigen::ei_traits<Derived>::StorageKind
>> >> >> StorageKind; \
>> >> >>  typedef typename Eigen::ei_index<StorageKind>::type Index; \
>> >> >>
>> >> >> this means that all expressions have a Index typedef that means the
>> >> >> right thing. Normally, we propagate this recursively as appropriate.
>> >> >> However there are some contexts where we know we are only dealing
>> >> >> with
>> >> >> Dense expressions, and don't have a MatrixType to work with (e.g. in
>> >> >> some public functions). There, it's OK to specify explicitly
>> >> >> DenseIndex.
>> >> >>
>> >> >> Then, we have the same thing in the Sparse module, in SparseUtil.h:
>> >> >>
>> >> >> template<>
>> >> >> struct ei_index<Sparse>
>> >> >> { typedef EIGEN_DEFAULT_SPARSE_INDEX_TYPE type; };
>> >> >>
>> >> >> typedef typename ei_index<Sparse>::type SparseIndex;
>> >> >>
>> >> >>
>> >> >> etc, etc.
>> >> >>
>> >> >> Benoit
>> >> >>
>> >> >>
>> >> >
>> >> >
>> >>
>> >>
>> >
>> >
>>
>>
>
>