-
Notifications
You must be signed in to change notification settings - Fork 197
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
CAGRA-Q search #2206
CAGRA-Q search #2206
Conversation
/ok to test |
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.
Thanks, @enp1s0, for working on this!
I love your modular approach of using dataset descriptor to abstract away the distance computation code. I think we'll have to think a bit later whether it's possible to unify/backport this to IVF methods.
I'm trying now integrate the index building (compression) part, and I want to figure out how much of the functionality to implement at first.
As far as I see now, both the ported code and the original prototype support only pq_bits = 8
, is that correct? There's probably some limitation on possible pq_dim
values as well.
Could you please write down these and possibly other limitations of the prototype/initial port (search part only) in the description of the PR?
/ok to test |
…moved into detail namespace in rapidsai#2206
/ok to test |
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.
Thanks @enp1s0 and @achirkin for your work. The PR looks good to me. There are a few additional issues that we can handle in a follow up PR.
Additionally, the test seem to compile too long, I will check whether there is any unintended template instantiation:
file | compile time | binary size |
---|---|---|
ann_cagra/test_half_uint32_t.cu.o | 31:37 min | 91.827 MB |
ann_cagra/test_int8_t_uint32_t.cu.o | 31:11 min | 91.846 MB |
ann_cagra/test_float_uint32_t.cu.o | 30:41 min | 91.709 MB |
ann_cagra/test_uint8_t_uint32_t.cu.o | 30:22 min | 91.874 MB |
ann_cagra_vpq/test_float_int64_t.cu.o | 17:40 min | 46.242 MB |
ann_cagra/test_half_int64_t.cu.o | 17:27 min | 46.422 MB |
ann_cagra/test_float_int64_t.cu.o | 15:31 min | 37.865 MB |
bench/cagra_float_uint32_t.cu.o | 14:09 min | 47.566 MB |
test_filter_float_int64_t.cu.o | 10:49 min | 19.376 MB |
ann_cagra_vpq/test_float_uint32_t.cu.o | 7:52 min | 11.921 MB |
Co-authored-by: Artem M. Chirkin <[email protected]>
cpp/include/raft/neighbors/detail/cagra/compute_distance_vpq.cuh
Outdated
Show resolved
Hide resolved
cpp/include/raft/neighbors/detail/cagra/compute_distance_vpq.cuh
Outdated
Show resolved
Hide resolved
/merge |
Add the relevant options to the CAGRA parameter parser and refinement to the CAGRA ANN benchmark. No changes to the library code. NB: the new option won't work correctly until #2206 is merged. Authors: - Artem M. Chirkin (https://github.com/achirkin) Approvers: - Tamas Bela Feher (https://github.com/tfeher) URL: #2233
Rel: #1889
Limitations