Drop usage of thrust iterators#2254
Conversation
We can use the more modern cuda iterators instead
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaced Thrust iterator utilities with CUDA C++ iterator equivalents across benchmarks, headers, and tests; updated iterator type aliases in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cpp/include/rmm/mr/fixed_size_memory_resource.hpp`:
- Line 17: Remove the now-unused include of <rmm/detail/thrust_namespace.h> from
cpp/include/rmm/mr/fixed_size_memory_resource.hpp (it was used previously for
Thrust iterator aliases but is no longer needed after switching to
<cuda/iterator> and cuda::make_transform_iterator /
cuda::make_counting_iterator); locate the `#include`
"<rmm/detail/thrust_namespace.h>" and delete that line and then verify no other
references to symbols from rmm::detail::thrust_namespace remain in the file
(e.g., usages around FixedSizeMemoryResource or related helper functions).
In `@cpp/tests/logger_tests.cpp`:
- Around line 99-102: The tests use the non-existent helper
cuda::make_zip_iterator; replace its usage by directly constructing
cuda::zip_iterator for the begin and end iterators so the compiler can find the
correct API. Specifically, update the creation of begin and end that currently
reference cuda::make_zip_iterator to construct cuda::zip_iterator objects using
expected_events.begin()/end() and actual_events.begin()/end() respectively (the
variables named begin and end and the containers expected_events and
actual_events). Ensure both begin and end use the direct cuda::zip_iterator
constructor syntax so the code compiles.
bdice
left a comment
There was a problem hiding this comment.
@miscco If you don’t mind, please look at the CodeRabbit suggestions and comment on whether they are useful/true/false. If there are incorrect comments, I am interested in the “Analysis Chain” and where it is right or wrong. I’m refining the AI review prompts and I might need to give it more context on how to consult against CCCL.
00079aa to
012a63a
Compare
|
/merge |
We can use the more modern cuda iterators instead