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 CAGRA InnerProduct test and fix a bug #595

Merged
merged 9 commits into from
Jan 24, 2025
Merged

Add CAGRA InnerProduct test and fix a bug #595

merged 9 commits into from
Jan 24, 2025

Conversation

enp1s0
Copy link
Member

@enp1s0 enp1s0 commented Jan 21, 2025 •
edited
Loading

There is no CAGRA InnerProduct test in cuVS while there was in RAFT. This PR adds the test.

Also, this PR

  • fixes an error that occurs when building a CAGRA graph with NN Descent + InnerProduct (cpp/src/neighbors/nn_descent_*.cu)
    • Could not pass the idx.metric() == params.metric validation in the nn_descent::detail::build function.
  • updates RAFT_EXPECT in the nn_descent::detail::build function for better error understanding
  • fixes the NN Descent API sample codes

@enp1s0 enp1s0 requested a review from a team as a code owner January 21, 2025 06:18
@enp1s0 enp1s0 self-assigned this Jan 21, 2025
@github-actions github-actions bot added the cpp label Jan 21, 2025
@enp1s0 enp1s0 added bug Something isn't working non-breaking Introduces a non-breaking change doc Improvements or additions to documentation testing cpp and removed cpp testing doc Improvements or additions to documentation labels Jan 21, 2025
@enp1s0 enp1s0 changed the title Add CAGRA InnerProduct test Add CAGRA InnerProduct test and fix a bug Jan 21, 2025
cjnolet
cjnolet approved these changes Jan 23, 2025
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

Thanks for this fix @enp1s0!

@cjnolet
Copy link
Member

cjnolet commented Jan 24, 2025

/merge

@rapids-bot rapids-bot bot merged commit 2f18645 into rapidsai:branch-25.02 Jan 24, 2025
61 checks passed
@bdice bdice mentioned this pull request Jan 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees

@enp1s0 enp1s0

Labels
bug Something isn't working cpp non-breaking Introduces a 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.
2 participants
@enp1s0 @cjnolet