-
Notifications
You must be signed in to change notification settings - Fork 69
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
Port C++ CAGRA runtime from RAFT #8
Conversation
Signed-off-by: Mickael Ide <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great so far! I wanted to share some initial feedback early.
I think we should go ahead and remove all of the headers from the detail
namespace and any of the public headers that we're not using. I had originally copied the entire RAFT repo over thinking it might be easy just to rename the VSS bits but it proved harder than I had hoped.
cpp/test/CMakeLists.txt
Outdated
src/neighbors/detail/cagra/search_single_cta_float_uint64_dim512_t32.cu | ||
src/neighbors/detail/cagra/search_single_cta_float_uint64_dim1024_t32.cu | ||
#test/neighbors/ann_cagra/test_float_int64_t.cu | ||
#src/neighbors/detail/cagra/search_multi_cta_float_uint64_dim128_t8.cu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move the relevant tests over so that we can test end-to-end w/ the new cuvs::neighbors public API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kept test/neighbors/ann_cagra
to test end-to-end CAGRA
Signed-off-by: Mickael Ide <[email protected]>
Changes have been merged into #27. Closing this one for now. |
This PR is porting the CAGRA runtime API from RAFT to cuVS.