-
Notifications
You must be signed in to change notification settings - Fork 150
Use HNSW GPU Hierarchy by Default #1617
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
base: main
Are you sure you want to change the base?
Conversation
tfeher
left a comment
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 Julian for fixing this, LGTM!
|
@tfeher this is a breaking change, and it's also missing an update to the C API default. |
divyegala
left a comment
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.
Instead of a segmentation fault, can you put an error checker to fail gracefully if user tries to search a NONE hierarchy HNSW index?
I have changed the C API default as well.
Sorry to not me more specific. Our internal search works but a separate search with Hnswlib fails. Changing the default would enable the interoperability out of the box. |
Thanks @divyegala for catching my mistake. Indeed, users might be relying on the default value, so this is a breaking change. I have updated the labels. |
13e735d to
5cf9913
Compare
Change the default HNSW hierarchy from
NONEtoGPUto enable search with Hnswlib. Searching an HNSW index built withHnswHierarchy::NONEusing Hnswlib fails with a segmentation fault. This change should go into 25.12 to prevent such issues and improve the interoperability between cuVS and Hnswlib.