Skip to content

Vamana half type instantiation#1943

Open
viclafargue wants to merge 3 commits into
rapidsai:mainfrom
viclafargue:vamana-half-instantiation
Open

Vamana half type instantiation#1943
viclafargue wants to merge 3 commits into
rapidsai: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)?

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 07038e0e-1697-489f-9278-4bebbfd99b62

📥 Commits

Reviewing files that changed from the base of the PR and between 63ed6ec and 21d0fcc.

📒 Files selected for processing (8)
  • cpp/CMakeLists.txt
  • cpp/include/cuvs/neighbors/vamana.hpp
  • cpp/src/neighbors/detail/vamana/vamana_serialize.cuh
  • cpp/src/neighbors/vamana_build_half.cu
  • cpp/src/neighbors/vamana_serialize_half.cu
  • cpp/tests/CMakeLists.txt
  • cpp/tests/neighbors/ann_vamana.cuh
  • cpp/tests/neighbors/ann_vamana/test_half_uint32_t.cu

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added half-precision (FP16) support for Vamana neighbor search index building and serialization.
  • Documentation

    • Added compatibility warnings noting that half-precision serialized datasets are not compatible with external systems and recommending include_dataset=false for improved interoperability.

Walkthrough

This pull request extends the Vamana neighbor index in cuVS to support half-precision floating-point data. It declares new public API overloads, provides explicit template instantiations for build and serialize operations, updates test infrastructure to generate half-precision test data, and integrates the new sources into the build system. DiskANN compatibility warnings are emitted during serialization of half-precision datasets.

Changes

Half-precision Vamana support

Layer / File(s) Summary
Public API declarations for half-precision
cpp/include/cuvs/neighbors/vamana.hpp
Declares two vamana::build() overloads accepting half device and host matrices, and one vamana::serialize() overload for half indices. Documentation warns that half-precision serialized datasets are incompatible with DiskANN and recommends include_dataset=false for cross-compatibility.
Build template instantiation for half
cpp/src/neighbors/vamana_build_half.cu
Introduces forwarding macro RAFT_INST_VAMANA_BUILD to generate explicit build() overloads for half type with uint32_t index type, supporting both device and host row-major matrix views.
Serialization implementation and DiskANN warnings
cpp/src/neighbors/detail/vamana/vamana_serialize.cuh, cpp/src/neighbors/vamana_serialize_half.cu
Adds compile-time warnings in the serialize path when include_dataset or sector_aligned is enabled for half data due to DiskANN incompatibility. New translation unit explicitly instantiates serialization for half.
Test fixture updates for half-precision data
cpp/tests/neighbors/ann_vamana.cuh
Updates test harness to generate random half test data by casting float buffers using raft::linalg::map. Adds cuda_fp16.h and reorganizes includes to support half-specific operations.
Test case and build registration
cpp/tests/CMakeLists.txt, cpp/tests/neighbors/ann_vamana/test_half_uint32_t.cu
Registers new test source file and instantiates parameterized test AnnVamanaTest<float, half, uint32_t> that validates the half-precision Vamana implementation.
Build system integration
cpp/CMakeLists.txt
Adds vamana_build_half.cu and vamana_serialize_half.cu to cuvs_objs compilation sources.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

improvement

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Vamana half type instantiation' clearly and concisely describes the main change: adding half-precision support to the Vamana index.
Description check ✅ Passed The description 'Closes #1753' is related to the changeset, referencing the feature request issue that this PR addresses.
Linked Issues check ✅ Passed The PR fully implements half-precision instantiation for Vamana as requested in #1753, including build and serialize overloads, compilation units, and comprehensive tests.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing half-precision support for Vamana: new compilation units, API overloads, serialization warnings, and tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

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