Migrate from thrust::tuple to cuda::std::tuple#5226
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
ChuckHastings
added
improvement
| if (group_lasts[i] - group_firsts[i] == 2) { | ||
| auto kv_pair_first = | ||
| thrust::make_zip_iterator(thrust::make_tuple(key_firsts[i], value_firsts[i])); | ||
| thrust::make_zip_iterator(cuda::std::make_tuple(key_firsts[i], value_firsts[i])); |
There was a problem hiding this comment.
cuda::std::make_tuple here is unnecessary.
There was a problem hiding this comment.
Guess I missed a few... will fix.
There was a problem hiding this comment.
Just for curiosity, is this you missing a few or cursor missing a few?
There was a problem hiding this comment.
Cursor missed these, although I think it didn't check the include directory. I asked it to list all cases where thrust::make_zip_iterator and either thrust::make_tuple or cuda::std::make_tuple together. It offered a list that only contained things in src and test directory.
| auto kv_pair_first = thrust::make_zip_iterator(thrust::make_tuple(key_first, value_first)); | ||
| auto output_chunk_first = thrust::make_zip_iterator(thrust::make_tuple( | ||
| auto kv_pair_first = thrust::make_zip_iterator(cuda::std::make_tuple(key_first, value_first)); | ||
| auto output_chunk_first = thrust::make_zip_iterator(cuda::std::make_tuple( |
There was a problem hiding this comment.
cuda::std::make_tuple here is unnecessary.
| auto num_chunks = (num_elements + max_elements_per_iteration - 1) / max_elements_per_iteration; | ||
| auto kv_pair_first = thrust::make_zip_iterator(thrust::make_tuple(key_first, value_first)); | ||
| auto output_chunk_first = thrust::make_zip_iterator(thrust::make_tuple( | ||
| auto kv_pair_first = thrust::make_zip_iterator(cuda::std::make_tuple(key_first, value_first)); |
There was a problem hiding this comment.
cuda::std::make_tuple here is unnecessary.
| rmm::device_uvector<size_t> d_tx_value_counts(d_tx_dst_ranks.size(), stream_view); | ||
| auto rank_count_pair_first = thrust::make_zip_iterator( | ||
| thrust::make_tuple(d_tx_dst_ranks.begin(), d_tx_value_counts.begin())); | ||
| cuda::std::make_tuple(d_tx_dst_ranks.begin(), d_tx_value_counts.begin())); |
There was a problem hiding this comment.
cuda::std::make_tuple here is unnecessary.
| rmm::device_uvector<size_t> d_tx_value_counts(d_tx_dst_ranks.size(), stream_view); | ||
| auto rank_count_pair_first = thrust::make_zip_iterator( | ||
| thrust::make_tuple(d_tx_dst_ranks.begin(), d_tx_value_counts.begin())); | ||
| cuda::std::make_tuple(d_tx_dst_ranks.begin(), d_tx_value_counts.begin())); |
There was a problem hiding this comment.
cuda::std::make_tuple here is unnecessary.
| // a temporary function to emulate thrust::tuple_cat (should retire once thrust::tuple is replaced | ||
| // with cuda::std::tuple) | ||
| // a temporary function to emulate cuda::std::tuple_cat (should retire once cuda::std::tuple is | ||
| // replaced with cuda::std::tuple) |
There was a problem hiding this comment.
This comment is now a bit weird. Does cuda::std::tuple_cat work? If yes, we may update this to a FIXME statement saying we need to replace this with cuda::std::tuple_cat.
There was a problem hiding this comment.
Updated to a FIXME to test/use cuda::std::tuple_cat
seunghwak
left a comment
seunghwak
left a comment
There was a problem hiding this comment.
LGTM except for the above issues.
|
/merge |
…:tuple (#5235) Graph 500 BFS updates and replace thrust::tuple, thrust::make_tuple, thrust::get with cuda::std::tuple, cuda::std::make_tuple, cuda::std::get (#5226 did this migration but there were multiple remaining ones). Authors: - Seunghwa Kang (https://github.com/seunghwak) Approvers: - Chuck Hastings (https://github.com/ChuckHastings) URL: #5235
2 participants
CCCL has deprecated use of thrust::tuple, this PR eliminates references to
thrust::tupleandthrust::make_tuple. It also eliminates the use ofmake_tuplewiththrust::make_zip_iterator, as this is no longer required.