Port thrust::min|max_element to CUB - #8291
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. |
This comment has been minimized.
This comment has been minimized.
c7152f8 to
c998b21
Compare
ecd6b31 to
b60c572
Compare
This comment has been minimized.
This comment has been minimized.
|
Looks like we are not returning the right values: |
b60c572 to
b8b1c55
Compare
This may have been an issue in #8285, where |
|
I tried adding |
This comment has been minimized.
This comment has been minimized.
| cub_min_element(execution_policy<Derived>& policy, ItemsIt first, ItemsIt last, BinaryPred binary_pred) | ||
| { | ||
| if (first == last) | ||
| cudaStream_t stream = cuda_cub::stream(policy); |
There was a problem hiding this comment.
stream_ref? Or does CUB not understand this?
There was a problem hiding this comment.
CUB would be fine, but we use the raw cudaStream_t everywhere in Thrust currently. cuda_cub::stream is also part of the public API. I think this should be refactored as a separate PR in the future.
b8b1c55 to
079080d
Compare
🥳 CI Workflow Results🟩 Finished in 2h 25m: Pass: 100%/118 | Total: 3d 11h | Max: 2h 17m | Hits: 89%/179912See results here. |
Pulled out from: #4970
DeviceReduce::ArgMin|Max#8285Added a new thrust benchmark. Performance is overwhelmingly good, except for a few runs.
Fixes part of: #1626