Skip to content

Vamana half type instantiation#1943

Open
viclafargue wants to merge 2 commits intorapidsai:mainfrom
viclafargue:vamana-half-instantiation
Open

Vamana half type instantiation#1943
viclafargue wants to merge 2 commits intorapidsai:mainfrom
viclafargue:vamana-half-instantiation

Conversation

@viclafargue
Copy link
Copy Markdown
Contributor

Closes #1753.

@viclafargue viclafargue requested review from a team as code owners March 24, 2026 14:37
@tarang-jain tarang-jain added the non-breaking Introduces a non-breaking change label Mar 24, 2026
Copy link
Copy Markdown
Contributor

@tarang-jain tarang-jain 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 adding this instantiation. Have you compared with https://github.com/microsoft/DiskANN/blob/cpp_main/src/index.cpp? Serializing a half dataset might not work for DiskANN search (when include_dataset is set to True). Perhaps we must add a note, warning the user about this when they attempt to include the dataset while serializing the index.

Comment thread cpp/tests/neighbors/ann_vamana.cuh Outdated
@tarang-jain tarang-jain added the feature request New feature or request label Mar 24, 2026
@viclafargue
Copy link
Copy Markdown
Contributor Author

Thanks for adding this instantiation. Have you compared with https://github.com/microsoft/DiskANN/blob/cpp_main/src/index.cpp? Serializing a half dataset might not work for DiskANN search (when include_dataset is set to True). Perhaps we must add a note, warning the user about this when they attempt to include the dataset while serializing the index.

Thanks @tarang-jain, good catch. I added a note in the doc and a warning in the serialization function.

if (include_dataset) {
RAFT_LOG_WARN(
"Serializing a half-precision (float16) dataset is not compatible with DiskANN search. "
"The serialized .data file uses raw half values with no type metadata, so DiskANN (which "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure there is no type metadata? Does DiskANN break with a "half" .data file or does it give garbage results? Just want to make sure that the wording of the warning is accurate.

Copy link
Copy Markdown
Contributor

@tarang-jain tarang-jain left a comment

Choose a reason for hiding this comment

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

I am ready to approve this PR barring my comment above. However, I am not sure about the utility of this feature yet (I am guessing once we have built the graph on half data which theoretically should be faster, we want to search using DiskANN with an fp32 dataset?). Can you also please report the binary size increase in libcuvs due to the new instantiation?

@cjnolet
Copy link
Copy Markdown
Member

cjnolet commented Mar 24, 2026

Tagging @bkarsin for more insight- I know half type in vamana has come up a lot. Does DiskANN support half precision? Does building a graph in half precision make sense? Does it make sense to do mixed (build in half an dsearch in full)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request non-breaking Introduces a non-breaking change

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

[FEA] Add float16 Instantiations for Vamana index

3 participants