Skip to content

Inherit thrust::transform_iterator traversal from base iterator traversal#5883

Merged
bernhardmgruber merged 3 commits intoNVIDIA:mainfrom
bernhardmgruber:transform_traversal_fix
Sep 16, 2025
Merged

Inherit thrust::transform_iterator traversal from base iterator traversal#5883
bernhardmgruber merged 3 commits intoNVIDIA:mainfrom
bernhardmgruber:transform_traversal_fix

Conversation

@bernhardmgruber
Copy link
Contributor

@bernhardmgruber bernhardmgruber commented Sep 15, 2025

Instead of from the base iterator's iterator category.

Fixes: #5860

@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Sep 15, 2025

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.

@bernhardmgruber
Copy link
Contributor Author

/ok to test

@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Progress in CCCL Sep 15, 2025
@bernhardmgruber
Copy link
Contributor Author

/ok to test cf00e5e

use_default,
typename ::cuda::std::iterator_traits<Iterator>::iterator_category,
use_default, // pick system from Iterator
use_default, // pick traversal from Iterator
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is the fix to #5860 and I also think it's an overall cleaner behavior. I don't know though why the old behavior was chosen. Need to do some digging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found a hint: https://github.com/NVIDIA/cccl/pull/1915/files#diff-866ad8d2535a5dd3e954db56809b4673fcce2b064fc630fe24c872c5e8195a81L61-L67

  typedef thrust::iterator_adaptor<transform_iterator<UnaryFunc, Iterator, Reference, Value>,
                                   Iterator,
                                   value_type,
                                   thrust::use_default // Leave the system alone
                                                       //, thrust::use_default   // Leave the traversal alone
                                                       // use the Iterator's category to let any system iterators remain
                                                       // random access even though transform_iterator's reference type
                                                       // may not be a reference
                                                       // XXX figure out why only iterators whose reference types are
                                                       // true references are random access
                                   ,
                                   typename thrust::iterator_traits<Iterator>::iterator_category,
                                   reference>

It seems category was used to work around any system iterators loosing their random accessness. Let's see whether this problem still exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tests to check whether the properties of a random access any system iterator are retained through a transform iterator. And they are. So I think the fix is legit.

@github-project-automation github-project-automation bot moved this from In Progress to In Review in CCCL Sep 15, 2025
@miscco
Copy link
Contributor

miscco commented Sep 15, 2025

/ok to test

@github-actions

This comment has been minimized.

@bernhardmgruber bernhardmgruber force-pushed the transform_traversal_fix branch 2 times, most recently from 8c71eea to 1fa341e Compare September 15, 2025 21:19
@bernhardmgruber bernhardmgruber marked this pull request as ready for review September 15, 2025 21:44
@bernhardmgruber bernhardmgruber requested a review from a team as a code owner September 15, 2025 21:44
@github-actions

This comment has been minimized.

@bernhardmgruber bernhardmgruber enabled auto-merge (squash) September 16, 2025 07:17
@github-actions
Copy link
Contributor

🥳 CI Workflow Results

🟩 Finished in 3h 11m: Pass: 100%/159 | Total: 6d 01h | Max: 3h 02m | Hits: 65%/186335

See results here.

@bernhardmgruber bernhardmgruber merged commit ca57b79 into NVIDIA:main Sep 16, 2025
170 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Sep 16, 2025
@bernhardmgruber bernhardmgruber deleted the transform_traversal_fix branch September 16, 2025 14:38
pciolkosz pushed a commit to pciolkosz/cccl that referenced this pull request Sep 24, 2025
…versal (NVIDIA#5883)

Instead of from the base iterator's iterator category.

Fixes: NVIDIA#5860
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[BUG]: thrust::transform_iterator deduces cpp system from cuda::counting_iterator instead of any_system_tag

2 participants