Re: [eigen] Indexes: why signed instead of unsigned?

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

Conclusion, and executive summary for Gael:

Let's implement ei_index_type like this:

// we agreed that ptrdiff_t was a good default
// however we can also make it easy for the user to override this default
// by defining a pp symbol, if you want.
template<typename StorageKind> struct ei_index_type
{ typedef ptrdiff_t type; };

OK now we can import that typedef e.g. in EIGEN_DENSE_PUBLIC_INTERFACE
and whatever is the equivalent in Sparse, so that all our expression
types have a typedef Index, say

  typedef typename ei_index_type<
    typename ei_traits<Derived>::StorageKind
  >::type Index;

So, just like we already have the Scalar typedef everywhere, we now have Index.

So far, this is only a very few changes. Just a few lines of code.

Then the big deal is:
* replace int by Index everywhere appropriate

Since we're staying signed, honestly it's not such a big deal. Just a
big search & replace.

At this stage we're good to release eigen 3.0, we're future-proof.

And later (not necessarily now):
* handle properly the mixing of different index types. Maybe we should
do like for the mixing of different Scalar types: just refuse it, but
offer an easy casting method allowing the user to explicitly cast.
Like,

   matrix.index_cast<NewIndexType>()

Does that sound reasonable?

Benoit

2010/5/11 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> 2010/5/11 Mark Borgerding <mark@xxxxxxxxxxxxxx>:
>> Is there any reason why sparse matrices backed by distributed storage
>
> This distributed storage idea was for dense, not sparse matrices, IIRC.
>
>> need
>> to use the same indexing integer data type as the more common dense storage
>> in memory?
>
> They shouldn't have to. Right now, due to a limitation in Eigen's
> design, the 'int' type is hardcoded everywhere. The idea of letting
> that either be defined in the ei_traits, or let that be inferred from
> the StorageKind (as Joel suggests, and I agree with him it's the
> better way), changes that, so they will be decoupled. Distributed
> storage will be able to use its own index type, regardless of the
> default.
>
>>
>> Beware bending a design to meet the edge case.
>
> Sure is good to keep in mind!
>
> Benoit
>
>>
>>
>> Benoit Jacob wrote:
>>>
>>> Note: the distribute storage case is an example where one might want
>>> 64bit indices even if the CPU architecture is 32bit. Indeed, since the
>>> array doesn't need anymore to fit in RAM...
>>>
>>> So it's an example where ptrdiff_t may not be the right choice.
>>>
>>> Benoit
>>>
>>> 2010/5/11 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
>>>
>>>>
>>>> grrr, i had forgotten but a while ago someone inquired about the
>>>> possibility to use eigen with distributed storage, and we took that
>>>> into account when doing the big class-hierarchy-refactorings, to
>>>> ensure it's possible to plug in a new storage system, see
>>>> StorageKind... so it would be a bit too bad to restrict the indices
>>>> types to int.
>>>>
>>>> This actually suggests that no matter what the default is, the indices
>>>> type should be something that the Storage can control. This would also
>>>> allow Sparse matrices to use different index types. We could make it a
>>>> typedef in ei_traits, that is propagated to nested expressions, ...
>>>>
>>>> Gael, what's your opinion?
>>>>
>>>> Benoit
>>>>
>>>> 2010/5/11 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
>>>>
>>>>>
>>>>> 2010/5/11 Rui Maciel <rui.maciel@xxxxxxxxx>:
>>>>>
>>>>>>
>>>>>> On 11 May 2010 16:47, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>>>>>>
>>>>>>>
>>>>>>> I just had a diabolic idea, if we agree that bigger-than-2^31-support
>>>>>>> is not a top priority and will at most be used by relatively few
>>>>>>> people, then we can keep things unchanged for now, so keep 'int' in
>>>>>>> Eigen 3.0, and later on, when actual people actually need it (perhaps
>>>>>>> in eigen 3.45), add a non-default compile-time option
>>>>>>> EIGEN_USE_LONG_INDICES. Such an option would then break the ABI of
>>>>>>> Eigen classes, so it couldn't become default.
>>>>>>>
>>>>>>> Question - is BLAS/LAPACK using int or ptrdiff_t ?
>>>>>>>
>>>>>>> Benoit
>>>>>>>
>>>>>>
>>>>>> As this move will break the ABI then why not implement it right on the
>>>>>> 3.0 release instead of pushing it to a point release in the future?
>>>>>>
>>>>>
>>>>> Because we've a lot on our plates already now.
>>>>>
>>>>> Benoit
>>>>>
>>>>>
>>>>>>
>>>>>> Rui Maciel
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>
>>>
>>
>>
>>
>>
>