[VectorSets] Update diskann-garnet and wire up metric types#1688
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Garnet’s DiskANN integration to align with a newer diskann-garnet package and expands test coverage around vector-search recall.
Changes:
- Bump
diskann-garnetNuGet package version to1.0.26. - Update DiskANN native interop to pass the distance metric into
create_indexand add an external-id validity check. - Refresh DiskANN tests: adjust
create_indexcall sites, replace the old (ignored) grid test with synthetic recall tests, and add an explicit test-project package reference.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Garnet.test/Garnet.test.csproj | Adds a direct diskann-garnet package reference for the test project. |
| test/Garnet.test/DiskANN/DiskANNSyntheticRecallTests.cs | Introduces new synthetic recall tests exercising VADD/VSIM for FP32/XB8 and quantization modes. |
| test/Garnet.test/DiskANN/DiskANNServiceTests.cs | Updates native create_index invocations to include the distance metric argument. |
| test/Garnet.test/DiskANN/DiskANNGridTests.cs | Removes the prior grid test that was effectively disabled via Ignore. |
| libs/server/Resp/Vector/DiskANNService.cs | Updates P/Invoke signature and CreateIndex call to include distance metric; adds check_external_id_valid interop and wrapper. |
| Directory.Packages.props | Updates central package version for diskann-garnet to 1.0.26. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tiagonapoli
force-pushed
the
tiagonapoli/update-diskann-garnet
branch
3 times, most recently
from
ca59d2b to
d393c6e
Compare
tiagonapoli
changed the title
tiagonapoli
force-pushed
the
tiagonapoli/update-diskann-garnet
branch
2 times, most recently
from
6f65133 to
23d49e1
Compare
Signed-off-by: Tiago Napoli <[email protected]>
tiagonapoli
force-pushed
the
tiagonapoli/update-diskann-garnet
branch
from
23d49e1 to
029fca7
Compare
…thods Replace 4 type-specific methods (BruteForceNearestNeighbors_FP32, BruteForceNearestNeighbors_XB8, CountPerDistance_FP32, CountPerDistance_XB8) with 2 generic methods that accept Func delegates for ID extraction and distance computation. Co-authored-by: Copilot <[email protected]>
…lTest - Replace FP32VectorEntry and XB8VectorEntry with generic VectorEntry<TVec> - Extract common recall loop into RunRecallTest<TEntry> with lambdas for distance, distance-key conversion, and VSIM querying - RunRecallTest_FP32 and RunRecallTest_XB8 become thin wrappers Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
…nRecallTest<TVec> Co-authored-by: Copilot <[email protected]>
Instead of filtering entries by ID set internally, CountPerDistance now receives the target vector and an enumerable of neighbor vectors, plus the distance and bucketing functions. Callers filter with LINQ. Co-authored-by: Copilot <[email protected]>
BruteForceNearestNeighbors now takes targetVector + candidates and returns List<TVec> directly. VsimQuery IDs are mapped to vectors via idToVector dictionary. Both flow as vector lists into CountPerDistance. Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
kevin-montrose
approved these changes
Apr 10, 2026
vazois
approved these changes
Apr 10, 2026
tiagonapoli
added a commit
that referenced
this pull request
Apr 10, 2026
* update diskann garnet Signed-off-by: Tiago Napoli <[email protected]> * Unify BruteForceNearestNeighbors and CountPerDistance into generic methods Replace 4 type-specific methods (BruteForceNearestNeighbors_FP32, BruteForceNearestNeighbors_XB8, CountPerDistance_FP32, CountPerDistance_XB8) with 2 generic methods that accept Func delegates for ID extraction and distance computation. Co-authored-by: Copilot <[email protected]> * Refactor recall tests: unify entry types and extract generic RunRecallTest - Replace FP32VectorEntry and XB8VectorEntry with generic VectorEntry<TVec> - Extract common recall loop into RunRecallTest<TEntry> with lambdas for distance, distance-key conversion, and VSIM querying - RunRecallTest_FP32 and RunRecallTest_XB8 become thin wrappers Co-authored-by: Copilot <[email protected]> * Fix formatting: remove trailing newline Co-authored-by: Copilot <[email protected]> * Merge RunRecallTest_FP32 and RunRecallTest_XB8 into single generic RunRecallTest<TVec> Co-authored-by: Copilot <[email protected]> * Simplify CountPerDistance to take targetVector + otherVectors directly Instead of filtering entries by ID set internally, CountPerDistance now receives the target vector and an enumerable of neighbor vectors, plus the distance and bucketing functions. Callers filter with LINQ. Co-authored-by: Copilot <[email protected]> * Return vectors instead of IDs from BruteForce and VsimQuery mapping BruteForceNearestNeighbors now takes targetVector + candidates and returns List<TVec> directly. VsimQuery IDs are mapped to vectors via idToVector dictionary. Both flow as vector lists into CountPerDistance. Co-authored-by: Copilot <[email protected]> * Remove toDistanceKey parameter; hardcode Round(d*100) bucketing Co-authored-by: Copilot <[email protected]> --------- Signed-off-by: Tiago Napoli <[email protected]> Co-authored-by: Tiago Napoli <[email protected]> Co-authored-by: Copilot <[email protected]>
tiagonapoli
added a commit
that referenced
this pull request
Apr 10, 2026
* update diskann garnet Signed-off-by: Tiago Napoli <[email protected]> * Unify BruteForceNearestNeighbors and CountPerDistance into generic methods Replace 4 type-specific methods (BruteForceNearestNeighbors_FP32, BruteForceNearestNeighbors_XB8, CountPerDistance_FP32, CountPerDistance_XB8) with 2 generic methods that accept Func delegates for ID extraction and distance computation. Co-authored-by: Copilot <[email protected]> * Refactor recall tests: unify entry types and extract generic RunRecallTest - Replace FP32VectorEntry and XB8VectorEntry with generic VectorEntry<TVec> - Extract common recall loop into RunRecallTest<TEntry> with lambdas for distance, distance-key conversion, and VSIM querying - RunRecallTest_FP32 and RunRecallTest_XB8 become thin wrappers Co-authored-by: Copilot <[email protected]> * Fix formatting: remove trailing newline Co-authored-by: Copilot <[email protected]> * Merge RunRecallTest_FP32 and RunRecallTest_XB8 into single generic RunRecallTest<TVec> Co-authored-by: Copilot <[email protected]> * Simplify CountPerDistance to take targetVector + otherVectors directly Instead of filtering entries by ID set internally, CountPerDistance now receives the target vector and an enumerable of neighbor vectors, plus the distance and bucketing functions. Callers filter with LINQ. Co-authored-by: Copilot <[email protected]> * Return vectors instead of IDs from BruteForce and VsimQuery mapping BruteForceNearestNeighbors now takes targetVector + candidates and returns List<TVec> directly. VsimQuery IDs are mapped to vectors via idToVector dictionary. Both flow as vector lists into CountPerDistance. Co-authored-by: Copilot <[email protected]> * Remove toDistanceKey parameter; hardcode Round(d*100) bucketing Co-authored-by: Copilot <[email protected]> --------- Signed-off-by: Tiago Napoli <[email protected]> Co-authored-by: Tiago Napoli <[email protected]> Co-authored-by: Copilot <[email protected]>
tiagonapoli
added a commit
that referenced
this pull request
Apr 11, 2026
* update diskann garnet Signed-off-by: Tiago Napoli <[email protected]> * Unify BruteForceNearestNeighbors and CountPerDistance into generic methods Replace 4 type-specific methods (BruteForceNearestNeighbors_FP32, BruteForceNearestNeighbors_XB8, CountPerDistance_FP32, CountPerDistance_XB8) with 2 generic methods that accept Func delegates for ID extraction and distance computation. Co-authored-by: Copilot <[email protected]> * Refactor recall tests: unify entry types and extract generic RunRecallTest - Replace FP32VectorEntry and XB8VectorEntry with generic VectorEntry<TVec> - Extract common recall loop into RunRecallTest<TEntry> with lambdas for distance, distance-key conversion, and VSIM querying - RunRecallTest_FP32 and RunRecallTest_XB8 become thin wrappers Co-authored-by: Copilot <[email protected]> * Fix formatting: remove trailing newline Co-authored-by: Copilot <[email protected]> * Merge RunRecallTest_FP32 and RunRecallTest_XB8 into single generic RunRecallTest<TVec> Co-authored-by: Copilot <[email protected]> * Simplify CountPerDistance to take targetVector + otherVectors directly Instead of filtering entries by ID set internally, CountPerDistance now receives the target vector and an enumerable of neighbor vectors, plus the distance and bucketing functions. Callers filter with LINQ. Co-authored-by: Copilot <[email protected]> * Return vectors instead of IDs from BruteForce and VsimQuery mapping BruteForceNearestNeighbors now takes targetVector + candidates and returns List<TVec> directly. VsimQuery IDs are mapped to vectors via idToVector dictionary. Both flow as vector lists into CountPerDistance. Co-authored-by: Copilot <[email protected]> * Remove toDistanceKey parameter; hardcode Round(d*100) bucketing Co-authored-by: Copilot <[email protected]> --------- Signed-off-by: Tiago Napoli <[email protected]> Co-authored-by: Tiago Napoli <[email protected]> Co-authored-by: Copilot <[email protected]>
vazois
pushed a commit
that referenced
this pull request
Apr 11, 2026
…1692) * update diskann garnet * Unify BruteForceNearestNeighbors and CountPerDistance into generic methods Replace 4 type-specific methods (BruteForceNearestNeighbors_FP32, BruteForceNearestNeighbors_XB8, CountPerDistance_FP32, CountPerDistance_XB8) with 2 generic methods that accept Func delegates for ID extraction and distance computation. * Refactor recall tests: unify entry types and extract generic RunRecallTest - Replace FP32VectorEntry and XB8VectorEntry with generic VectorEntry<TVec> - Extract common recall loop into RunRecallTest<TEntry> with lambdas for distance, distance-key conversion, and VSIM querying - RunRecallTest_FP32 and RunRecallTest_XB8 become thin wrappers * Fix formatting: remove trailing newline * Merge RunRecallTest_FP32 and RunRecallTest_XB8 into single generic RunRecallTest<TVec> * Simplify CountPerDistance to take targetVector + otherVectors directly Instead of filtering entries by ID set internally, CountPerDistance now receives the target vector and an enumerable of neighbor vectors, plus the distance and bucketing functions. Callers filter with LINQ. * Return vectors instead of IDs from BruteForce and VsimQuery mapping BruteForceNearestNeighbors now takes targetVector + candidates and returns List<TVec> directly. VsimQuery IDs are mapped to vectors via idToVector dictionary. Both flow as vector lists into CountPerDistance. * Remove toDistanceKey parameter; hardcode Round(d*100) bucketing --------- Signed-off-by: Tiago Napoli <[email protected]> Co-authored-by: Tiago Napoli <[email protected]> Co-authored-by: Copilot <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.