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

Add generic reduction functions and separate reductions/warp_primitives #1470

Merged
merged 31 commits into from
May 18, 2023

Conversation

akifcorduk
Copy link
Member

This PR adds bunch of new device reduction functions such as:

  • Generic device reductions that takes reduction operator as argument.
  • Ranked reductions to return the index/rank of the reduced value.
  • Weighted random reduction to have probabilistic reduction using conditional probability.
  • Binary reduction to reduce binary values more efficiently.

There are tests implemented for all device reduction operations.

This PR also separates warp primitives to the warp_primitives.cuh.
All reduction functions are moved to reduction.cuh

@akifcorduk akifcorduk requested review from a team as code owners April 27, 2023 16:08
@akifcorduk akifcorduk added non-breaking Non-breaking change 3 - Ready for Review enhancement New feature or request improvement Improvement / enhancement to an existing function and removed cpp CMake labels Apr 27, 2023
Copy link
Contributor

@ahendriksen ahendriksen left a comment

Choose a reason for hiding this comment

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

Thank you @akifcorduk: it's a good idea to split up cuda_utils and add tests. I have added some comments.

cpp/include/raft/util/pow2_utils.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/cuda_utils.cuh Show resolved Hide resolved
cpp/include/raft/util/cuda_utils.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/device_loads_stores.cuh Outdated Show resolved Hide resolved
cpp/test/util/reduction.cu Outdated Show resolved Hide resolved
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks @akifcorduk for this work! Most of it looks good, but I have a few questions below.

cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
@akifcorduk akifcorduk requested a review from tfeher May 4, 2023 08:14
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks Akif for the updates, a few additional comments.

cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
Copy link
Contributor

@tfeher tfeher left a comment

Choose a reason for hiding this comment

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

Thanks @akifcorduk for addressing the issues. Apart from one issue with the right home for the new random primitives it looks good to me, therefore pre-approving.

cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
cpp/include/raft/util/reduction.cuh Outdated Show resolved Hide resolved
@cjnolet
Copy link
Member

cjnolet commented May 17, 2023

/merge

@rapids-bot rapids-bot bot merged commit 8e412b4 into rapidsai:branch-23.06 May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review CMake cpp enhancement New feature or request improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
Development

Successfully merging this pull request may close these issues.

4 participants