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.

Sign up for GitHub

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

Jump to bottom

Add RAFT ANN benchmark for CAGRA #1552

Merged
merged 17 commits into from
Jun 23, 2023
Merged

Add RAFT ANN benchmark for CAGRA #1552

merged 17 commits into from
Jun 23, 2023

Conversation

enp1s0
Copy link
Member

@enp1s0 enp1s0 commented May 29, 2023

This PR adds the RAFT ANN benchmark for CAGRA

@enp1s0 enp1s0 requested review from a team as code owners May 29, 2023 12:12
@enp1s0 enp1s0 self-assigned this May 29, 2023
@enp1s0 enp1s0 added 5 - DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed cpp CMake labels May 29, 2023
tfeher
tfeher requested changes Jun 2, 2023
View reviewed changes
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 @enp1s0 for adding this benchmarking functionality! Looks great, just a few small comments.

cpp/bench/ann/src/common/benchmark.hpp Outdated Show resolved Hide resolved
cpp/bench/ann/src/raft/raft_cagra_wrapper.h Show resolved Hide resolved
cpp/bench/ann/src/raft/raft_cagra_wrapper.h Outdated Show resolved Hide resolved
cpp/bench/ann/src/raft/raft_cagra_wrapper.h Outdated Show resolved Hide resolved
cpp/bench/ann/src/raft/raft_cagra_wrapper.h Outdated Show resolved Hide resolved
cpp/bench/ann/src/raft/raft_benchmark.cu Outdated Show resolved Hide resolved
@enp1s0
Copy link
Member Author

enp1s0 commented Jun 7, 2023 •
edited
Loading

TODO:

  • Add support for deep-100M and sift-128-euclidean, that use L2-norm for similarity

@enp1s0 enp1s0 removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jun 7, 2023
@cjnolet cjnolet mentioned this pull request Jun 22, 2023
Closed
9 tasks
tfeher
tfeher approved these changes Jun 23, 2023
View reviewed changes
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 @enp1s0 for addressing the issues, the PR looks good to me!

@enp1s0
Copy link
Member Author

enp1s0 commented Jun 23, 2023

Thanks @tfeher for checking the code.

@cjnolet
Copy link
Member

cjnolet commented Jun 23, 2023

/merge

@rapids-bot rapids-bot bot merged commit 185e933 into rapidsai:branch-23.08 Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfeher tfeher tfeher approved these changes

Assignees

@enp1s0 enp1s0

Labels
CMake cpp improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
VS/ML/DM Primitives Release Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@enp1s0 @cjnolet @tfeher