Skip to content
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

Fix thrust namespace for CUDA 12.6 #428

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

miwechner
Copy link

@miwechner miwechner commented Aug 17, 2024

Affecting compilation issue mentioned here: #417 (comment). The other device-code compilation errors with CUDA 12.5 mentioned in the issue are not adressed with this PR.

Thrust changed the namespace for its implementations from thrust:: to thrust::<<ABI_IDENTIFIER>>:: since thrust 2.3.0.

For the few template specializations in iterator_impl, its necessary to to use the correct namespaces. Using the THRUST_NAMESPACE_BEGIN/END marcos should make sure that we always use the correct namespace, even with older CUDA Toolkit versions, but I did not exhaustively test that.

Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.34%. Comparing base (3b7d712) to head (4d6bb20).
Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #428   +/-   ##
=======================================
  Coverage   97.34%   97.34%           
=======================================
  Files          32       32           
  Lines        2524     2524           
=======================================
  Hits         2457     2457           
  Misses         67       67           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stotko
Copy link
Owner

stotko commented Aug 19, 2024

Thanks for working on a fix for CUDA 12.6. For reference, the respective namespace macros were introduced in thrust 1.13.1 and CUDA 11.5. We technically also support older CUDA version down to 11.0, so a fallback would actually be needed for those. However, I do not believe that such old versions are still in use, so just bumping the minimum requirements should be sufficient. I will handle the respective documentation changes. Other than that, LGTM. Thanks again for the contribution!

@stotko stotko merged commit 1d29bb6 into stotko:master Aug 19, 2024
61 checks passed
@stotko stotko added this to the 2.0.0 milestone Aug 19, 2024
@miwechner miwechner deleted the fix-thrust-namespace branch August 19, 2024 13:22
@stotko stotko added the bug label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants