[FEA] Implement unravel_index for row-major array. #723
Conversation
trivialfis
added
feature request
|
rerun tests |
cpp/include/raft/core/mdarray.hpp
Outdated
| */ | ||
| template <typename LayoutPolicy, std::size_t... Exts> | ||
| MDSPAN_INLINE_FUNCTION auto unravel_index(size_t idx, |
There was a problem hiding this comment.
std::size_t idx as even extents are expressed as that type?
There was a problem hiding this comment.
I haven't tested the updated mdspan and its new extent type yet.
There was a problem hiding this comment.
Fair. We'll update it when we update mdspan
There was a problem hiding this comment.
Could this parameter possibly be optional with a default type/value? For those wishing to use the template param directly?
There was a problem hiding this comment.
Yes. I will update the parameter type
There was a problem hiding this comment.
I changed the parameter type of std::size_t idx into template Idx idx
There was a problem hiding this comment.
LGTM. Thanks @trivialfis for the change to use std::tuple instead of exposing thrust.
|
@gpucibot merge |
Revised version of #631 with
thrust::tuplereplaced bystd::tupleand customapplyfunction replaced bystd::apply.Related: #629
The implementation is mostly copied from XGBoost https://github.com/dmlc/xgboost/blob/fdf533f2b9af9c068cddba50839574c6abb58dc3/include/xgboost/linalg.h#L523 .
In the tests, I have used both
__host__ __device__and__device__lambdas to make surestd::tupleandstd::applyare working correctly with CUDA kernel.