kokkos / kokkos-kernels

Kokkos C++ Performance Portability Programming Ecosystem: Math Kernels - Provides BLAS, Sparse BLAS and Graph Kernels
Other
302 stars 96 forks source link

inconsistent default size type between `CrsMatrix` and `StaticCrsGraph` #2336

Open maartenarnst opened 1 week ago

maartenarnst commented 1 week ago

@brian-kelley @romintomasetti

In the following PR:

the default size types of KokkosSparse::CrsMatrix and several other matrix types were changed to default_size_type. This is typically int or size_t depending on the configuration of Kokkos-kernels.

However, in Kokkos::StaticCrsGraph, the default size type is taken from a ViewTraits instance. Typically, that size type is in turn a size type of a space, and it need not be int or size_t.

The issue is thus that these default size types are not necessarily the same. And because KokkosSparse::CrsMatrix and Kokkos::StaticCrsGraph are so interrelated, this discrepancy can lead to problems when using eg. the KokkosSparse::CrsMatrix constructor that takes a Kokkos::StaticCrsGraph among its arguments.

This issue is thus a suggestion to consider whether further consistency across the size types may be worth pursuing.

maartenarnst commented 1 week ago

@lucbv

Looks like it will be solved :)

lucbv commented 1 week ago

Yes, it was good timing you reminded me about this, I was able to "jump on it" effectively during the meeting. Since we are the biggest user of this container I think it is reasonable to move it : )