Re: [eigen] Index types change pushed

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

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