Skip to content

Alpaka: Vecmem Interface #960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

CrossR
Copy link
Contributor

@CrossR CrossR commented May 2, 2025

Based on the code in #959, just with a few small changes:

  • Test + examples updated (where appropriate) to use the new code.
  • ifdef guard around async_copy init in the CUDA || HIP bit, just whilst we are still on an older vecmem release. Can be updated when we update that, though no rush as the async copy is not used in Alpaka yet.
  • I swapped the alpaka::Queue to default to device 0 in cases where there is no device given. Any opinions on this welcomed. I didn't / couldn't go down the CUDA route (which is if device == INVALID_DEVICE get_current_device()), as Alpaka has no concept of the current device. Could instead just remove the default and force a device ID to be given? I think either is preferable to just silently failing.

I've deleted the previous code now as well, which removes Alpaka from the traccc::alpaka public headers once again.

krasznaa and others added 4 commits May 2, 2025 09:35
Meant as an example/guidance for Ryan Cross.
Needs testing on both HIP and SYCL still.
Can be removed when vecmem is updated.
Alpaka, unlike CUDA, doesn't have an equivalent of "Get Current Device",
such that we can't really use that.

This is hopefully a reasonable middle-ground, as if a user does care
about what device they are using, they should pass a device over.

Otherwise, they get a reasonable default (and per device environment
arguments can be used to change this, as they would for native CUDA, HIP
& SYCL code.
@CrossR
Copy link
Contributor Author

CrossR commented May 2, 2025

Tested with my async branch too, and after a few minor changes to the Queue using code, works there too.

@CrossR CrossR marked this pull request as ready for review May 6, 2025 08:41
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how things look, but let's do one other kind of cleanup as part of this PR!

Now that traccc::alpaka takes care of everything internally, practically all executables should become "simple C++ code". I.e.

https://github.com/acts-project/traccc/blob/main/examples/run/alpaka/CMakeLists.txt#L12-L13

should no longer be needed I believe. (But you'll need to test this.)

@@ -38,7 +35,7 @@ add_library( traccc_examples_alpaka STATIC
"full_chain_algorithm.hpp"
"full_chain_algorithm.cpp" )
target_link_libraries( traccc_examples_alpaka
PUBLIC alpaka::alpaka vecmem::core detray::core detray::detectors
PUBLIC vecmem::core detray::core detray::detectors
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alpaka calls now removed from the examples CMakeLists.

@CrossR CrossR force-pushed the AlpakaVecmemOrganization-main-20250501 branch from 4e6c1e9 to b29c2c2 Compare May 8, 2025 11:11
@CrossR CrossR force-pushed the AlpakaVecmemOrganization-main-20250501 branch from 3905ebe to 2c4e954 Compare May 9, 2025 11:14
Copy link

sonarqubecloud bot commented May 9, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants