Conversation
tarang-jain
left a comment
There was a problem hiding this comment.
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 " |
There was a problem hiding this comment.
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.
tarang-jain
left a comment
There was a problem hiding this comment.
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?
|
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)? |
Closes #1753.