Skip to content

Device, Host, Managed Accessor Types for mdspan#776

Merged
rapids-bot[bot] merged 9 commits intorapidsai:branch-22.10from
divyegala:fea-accessor-type
Sep 1, 2022
Merged

Device, Host, Managed Accessor Types for mdspan#776
rapids-bot[bot] merged 9 commits intorapidsai:branch-22.10from
divyegala:fea-accessor-type

Conversation

@divyegala
Copy link
Member

No description provided.

@divyegala divyegala requested review from a team as code owners August 2, 2022 20:55
@divyegala divyegala changed the base branch from branch-22.08 to branch-22.10 August 2, 2022 20:56
@cjnolet cjnolet added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 2, 2022
@trivialfis
Copy link
Member

Hi, I got something very primitive here: https://github.com/trivialfis/raft/tree/fea-restricted-accessor for annotating the access function with __device__ and __host__ tag. It doesn't compile as we access the device mdarray (not span) on host for tests. (see the device_reference class). There are a few options we can proceed with this:

  • Don't annotate the access function. The type should provide sufficient information/protection for most use cases. It's c++, we are not going to have 100% safety.
  • Remove the device_reference class and disable the operator() for device_mdarray completely. Since we can't access an owning object in the kernel so the operator will be inaccessible anywhere. But less convenient for testing and debugging.
  • Create different accessors for mdarray and mdspan and annotate only the latter. The code is more complicated.

My personal preference is for the first option, I think annotating it with __device__ is a great idea for safety, however, I also think the current type system is safe "enough" for most cases.

@ajschmidt8 ajschmidt8 removed the request for review from a team August 4, 2022 20:05
@cjnolet
Copy link
Member

cjnolet commented Sep 1, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c2e7e90 into rapidsai:branch-22.10 Sep 1, 2022
loulankxh pushed a commit to loulankxh/raft that referenced this pull request Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants