Improving Csr::strategy_type #320
Comments
|
The strategy strategy creates a CUDA handle to take the parameter of GPU code This implementation leads that |
|
I think it would be better if we had a kernel call to |
{{ message }}
Labels
is:interface-breaking
This change may break interface compatibility. Can only be done through a major release.
is:proposal
Maybe we should do something this way.
mod:core
This is related to the core module.
mod:cuda
This is related to the CUDA module.
type:matrix-format
This is related to the Matrix formats
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, all strategies for CSR are tailored for CUDA, which is fine for the most part.
However, also the names are CUDA specific (see
Csr::cusparse), which should be changed in my opinion since we want to support multiple platforms. I would prefer to go with a more neutral name likeCsr::sparse_libraryin that case (maybe there is even an OpenMP sparse library we could use for that).Also,
automaticalonly works on CUDA devices and even requires aCudaExecutorto work. I would prefer a solution where it is possible to also adapt to certain OpenMP properties (which we should have as soon as we have a more sophisticated SpMV there).Additionally, I am not sure why we use an
std::shared_ptrfor these strategies. Currently, we always have to callstd::make_sharedto generate a strategy, which is both not intuitive and not necessary since there is not much stored inside a strategy object (at most anstd::stringand anint64_t). I think copying the object would be faster than allocating memory on the heap, although it should not really matter much (the more important part for me is the intuitiveness).We could also encapsulate the strategies in a class named
strategy, so it is clear thatCsr::spmv_strategy::automaticalis an SpMV strategy.In summary, I think the following changes should be introduced:
cusparse->sparse_libraryautomaticalto actually be automatical and dependent on the executor (CUDA vs. OpenMP vs. Reference) without requiring a CudaExecutorstd::shared_ptrto just a plain object since the most one of these objects contain is anint64_tand anstd::stringspmv_strategy(or similar), so you call it withCsr::spmv_strategy::automatical, which is more descriptiveAdditionally, some functionality/performance changes can also be incorporated into the strategies:
make_srowto keep data persistent over manyapplycalls optimizing the apply calls. See discussion here.The text was updated successfully, but these errors were encountered: