Skip to content

Provide specialization of AccessTraits for std::vector#1262

Open
aprokop wants to merge 3 commits intoarborx:masterfrom
aprokop:access_traits_std_vector
Open

Provide specialization of AccessTraits for std::vector#1262
aprokop wants to merge 3 commits intoarborx:masterfrom
aprokop:access_traits_std_vector

Conversation

@aprokop
Copy link
Copy Markdown
Contributor

@aprokop aprokop commented Jun 9, 2025

This PR provides a specialization of AccessTraits for std::vector for user convenience.

In general, we should document somewhere that we reserve the right to provide those specializations for something user's don't control in their libraries (like, things from std::). If users want to provide custom specializations for those, they should wrap inside a holder.

@aprokop aprokop requested a review from dalg24 June 9, 2025 21:18
@aprokop aprokop added the API User visible interface modifications label Jun 9, 2025
@dalg24
Copy link
Copy Markdown
Contributor

dalg24 commented Jun 9, 2025

Not sure it is a good idea

@Rombur
Copy link
Copy Markdown
Collaborator

Rombur commented Jun 10, 2025

Not sure it is a good idea

When updating the wrappers for deal.II, we realized that there is a lot of boiler code to write with the new interface. It makes it unnecessary hard to use ArborX. With ArborX 1.X, we only needed to write access_traits. Now to do the same thing, we need to write an ExtractIndex structure, the acces_traits, and the IndexableGetter. For most people, only the IndexableGetter uses user data.

@aprokop
Copy link
Copy Markdown
Contributor Author

aprokop commented Jun 10, 2025

Not sure it is a good idea

What do you see as the difference between Kokkos::View and std::vector specializations?

@dalg24
Copy link
Copy Markdown
Contributor

dalg24 commented Jun 10, 2025

Views are fit for accelerators. What would we want to do about host device annotations on the access functions?

@aprokop
Copy link
Copy Markdown
Contributor Author

aprokop commented Jun 10, 2025

What would we want to do about host device annotations on the access functions?

Not use them. Everything will be HostSpace, so we won't need annotations.

@dalg24
Copy link
Copy Markdown
Contributor

dalg24 commented Jun 10, 2025

It is not about what memory space we tag it with, we'd run into issues when CUDA or HIP are enabled.
Also, I am not sure how it interacts with NVHPC when stdpar is enabled.

@Rombur
Copy link
Copy Markdown
Collaborator

Rombur commented Jun 10, 2025

It is not about what memory space we tag it with, we'd run into issues when CUDA or HIP are enabled.

Why? It worked correctly with the previous API. I have been doing that for years in adamantine.

@aprokop aprokop force-pushed the access_traits_std_vector branch from 9550e65 to 79da7b1 Compare March 6, 2026 02:37
@aprokop
Copy link
Copy Markdown
Contributor Author

aprokop commented Mar 6, 2026

After a long hiatus, I'm coming back to this PR.

Also, I am not sure how it interacts with NVHPC when stdpar is enabled.

My understanding is that when using nvc++ compiler with -stdpar=gpu option, it replaces the default std::allocator with a managed memory allocator. In that case, the data lives in the Unified Memory space, and migrates based on page faults.

For hip, with --hipstdpar, it can use HMM.

I think in either case, the memory would be migrated to the host if we access it there. If we run into issues later, we could do cudaPointerGetAttributes to check where the pointer lives.

It is not about what memory space we tag it with, we'd run into issues when CUDA or HIP are enabled.

@dalg24 Can you please elaborate? I don't understand what you mean here.

@dalg24
Copy link
Copy Markdown
Contributor

dalg24 commented Mar 6, 2026

AccessTraits::get defined as a host-only function that will be called from a host device function.

@aprokop
Copy link
Copy Markdown
Contributor Author

aprokop commented Mar 6, 2026

AccessTraits::get defined as a host-only function that will be called from a host device function.

It will result in a warning, yes, but in a correct execution, as the corresponding hierarchy will be on the host.

@aprokop
Copy link
Copy Markdown
Contributor Author

aprokop commented Mar 6, 2026

@dalg24 requested that we produce no warning in this scenario. Will need to see how we can hide it.

@aprokop aprokop force-pushed the access_traits_std_vector branch from 79da7b1 to 17980fd Compare March 24, 2026 02:03
@aprokop aprokop force-pushed the access_traits_std_vector branch from 17980fd to 1632018 Compare March 24, 2026 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API User visible interface modifications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants