[dev][VectorSets] Update diskann-garnet and wire up metric types (#1688)#1692
Merged
Merged
Conversation
vazois
approved these changes
Apr 10, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Garnet’s DiskANN integration to support distance metric selection end-to-end, along with synthetic recall tests to validate expected nearest-neighbor behavior after updating the diskann-garnet dependency.
Changes:
- Bump
diskann-garnetNuGet to1.0.26and add an explicit reference in the test project. - Wire
VectorDistanceMetricTypeinto the nativecreate_indexP/Invoke and managedDiskANNService.CreateIndexcall path. - Replace the old ignored grid test with new synthetic recall tests (grid for L2, circle for cosine).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Directory.Packages.props |
Updates central package version for diskann-garnet to 1.0.26. |
libs/server/Resp/Vector/DiskANNService.cs |
Passes the distance metric into create_index; adds external-id validation P/Invoke surface. |
test/Garnet.test/Garnet.test.csproj |
Adds diskann-garnet package reference for test runtime/assets availability. |
test/Garnet.test/DiskANN/DiskANNSyntheticRecallTests.cs |
Adds new synthetic recall tests for grid (L2) and circle (cosine) datasets. |
test/Garnet.test/DiskANN/DiskANNServiceTests.cs |
Updates native create_index callsites to include the new metricType parameter. |
test/Garnet.test/DiskANN/DiskANNGridTests.cs |
Removes the old ignored grid search test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tiagonapoli
force-pushed
the
tiagonapoli/cherry-pick-diskann-update-to-dev
branch
from
c280773 to
35c0271
Compare
* update diskann garnet Signed-off-by: Tiago Napoli <tiagonapoli@microsoft.com> * 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 <223556219+Copilot@users.noreply.github.com> * 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 <223556219+Copilot@users.noreply.github.com> * Fix formatting: remove trailing newline Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Merge RunRecallTest_FP32 and RunRecallTest_XB8 into single generic RunRecallTest<TVec> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * 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 <223556219+Copilot@users.noreply.github.com> * 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 <223556219+Copilot@users.noreply.github.com> * Remove toDistanceKey parameter; hardcode Round(d*100) bucketing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Signed-off-by: Tiago Napoli <tiagonapoli@microsoft.com> Co-authored-by: Tiago Napoli <tiagonapoli@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
tiagonapoli
force-pushed
the
tiagonapoli/cherry-pick-diskann-update-to-dev
branch
from
35c0271 to
9992e27
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.