Skip to content

Deduplicate virtual start point edges during disk serialization - #1350

Merged
juchen-ms (partychen) merged 3 commits into
microsoft:mainfrom
partychen:fix/virtual-start-serialization
Aug 25, 2026
Merged

Deduplicate virtual start point edges during disk serialization#1350
juchen-ms (partychen) merged 3 commits into
microsoft:mainfrom
partychen:fix/virtual-start-serialization

Conversation

@partychen

@partychen juchen-ms (partychen) commented Aug 24, 2026

Copy link
Copy Markdown
Contributor
  • Does this PR have a descriptive title that could go in our release notes?
  • Does this PR add any new dependencies? No.
  • Does this PR modify any existing APIs? No.
  • Is the change to the API backwards compatible? Yes.
  • Should this result in any documentation changes? No.

Reference Issues/PRs

N/A

What does this implement/fix? Briefly explain your changes.

The in-memory graph uses a virtual start point ID, while the serialized disk graph must replace it with the actual medoid ID.

Previously, this replacement happened after adjacency-list uniqueness had already been established. If a list contained both the virtual start point and the actual medoid, the remapping produced a duplicate medoid edge. On the medoid's own adjacency list, it could also produce a self-loop.

This PR updates DiskAdaptor to:

  • remap the virtual start point to the actual medoid;
  • preserve the original neighbor order;
  • retain only the first resulting medoid edge;
  • remove self-loops after remapping.

It does not backfill removed entries with arbitrary neighbors, so affected adjacency lists may contain one fewer unique edge.

The regression test covers both a remapping collision and the medoid self-loop case. The expected disk-index fixture is updated for the corrected serialized graph.

Any other comments?

The in-memory adjacency list is already unique before serialization, so only the alias introduced by virtual-start-point remapping needs post-remap deduplication. This keeps the change limited to disk serialization and does not alter graph construction or pruning behavior.

Copilot AI left a comment

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.

Pull request overview

This PR fixes a disk-serialization edge case in diskann-providers where remapping the in-memory “virtual start point” ID to the actual medoid ID could introduce duplicate medoid edges and (on the medoid’s own adjacency list) create a self-loop in the serialized disk graph. The change is scoped to the disk-graph adaptor used during serialization and is validated by a new regression test plus an updated disk-index fixture.

Changes:

  • Update DiskAdaptor::get_adjacency_list to remap the virtual start point to the actual start point while preserving neighbor order, deduplicating the resulting medoid edge (keep first), and removing self-loops after remapping.
  • Add a unit test covering both the remapping collision (duplicate medoid) and medoid self-loop scenarios.
  • Update the expected disk-index fixture (Git LFS pointer) to match the corrected serialized graph output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs Adjusts disk-serialization adjacency list remapping to deduplicate the remapped medoid edge and remove self-loops; adds a regression test for both failure modes.
test_data/disk_index_build/truth_sift_learn_R4_L50_disk.index Updates the expected on-disk index fixture pointer to reflect the corrected serialization output.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.53%. Comparing base (860cf47) to head (408ccd3).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1350      +/-   ##
==========================================
+ Coverage   91.55%   92.53%   +0.97%     
==========================================
  Files         521      521              
  Lines      100347   100371      +24     
==========================================
+ Hits        91877    92881    +1004     
+ Misses       8470     7490     -980     
Flag Coverage Δ
miri 92.53% <100.00%> (+0.97%) ⬆️
unittests 92.50% <100.00%> (+1.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../graph/provider/async_/simple_neighbor_provider.rs 96.62% <100.00%> (-0.09%) ⬇️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs Outdated
Comment thread diskann-providers/src/model/graph/provider/async_/simple_neighbor_provider.rs Outdated
juchen-ms (partychen) and others added 2 commits August 25, 2026 10:36
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@partychen
juchen-ms (partychen) enabled auto-merge (squash) August 25, 2026 02:55
@partychen
juchen-ms (partychen) merged commit 07af709 into microsoft:main Aug 25, 2026
30 checks passed
@partychen
juchen-ms (partychen) deleted the fix/virtual-start-serialization branch August 25, 2026 03:08
MustafaIdrisArm pushed a commit to MustafaIdrisArm/DiskANN that referenced this pull request Sep 11, 2026
# Breaking Changes

Flat search visitors are now query-aware (microsoft#1359)
* `flat::FlatIndex`  has been removed. The search entry point is now the
free function `flat::knn_search`, and the `DistancesUnordered` visitor
is constructed per-query rather than reused. The `ElementRef`,
`QueryComputer`, and `QueryComputerError` associated types and the
visitor GAT have also been removed. `DistancesUnordered` now yields
`(id, distance)` pairs directly.

Migration: Callers of the old `FlatIndex`/visitor API should:

1. Drop `FlatIndex` and construct your `DistancesUnordered` visitor
directly for the query being searched.
2. Replace calls into the removed wrapper with `flat::knn_search(&mut
visitor, k, processor, query, &mut output)`.
3. Remove any `ElementRef`/`QueryComputer` implementation, fuse scanning
and distance computation directly in your
`DistancesUnordered::distances_unordered` implementation.

# All Changes

* Make `UnalignedSlice` Send and Sync. by @hildebrandmw in
microsoft#1348
* Bump actions/checkout from 4.4.0 to 7.0.1 in the github-actions group
by @dependabot[bot] in microsoft#1344
* Deduplicate virtual start point edges during disk serialization by
@partychen in microsoft#1350
* Fix alpha pruning documentation by @xinyuwen2 in
microsoft#1351
* Bump the github-actions group with 4 updates by @dependabot[bot] in
microsoft#1356
* Add Neon Inner product u4*u4 kernel by @pfoxARM in
microsoft#1353
* Strengthen arguments to `robust_prune`. by @hildebrandmw in
microsoft#1358
* Allow inspection of the paged search accessor. by @hildebrandmw in
microsoft#1364
* Make flat search visitors query-aware by @partychen in
microsoft#1359
* Add Neon inner-product kernel for USlice<2> with spherical wiring by
@pfoxARM in microsoft#1363

## New Contributors
* @pfoxARM made their first contribution in
microsoft#1353

**Full Changelog**:
microsoft/DiskANN@v0.56.0...v0.57.0

Co-authored-by: Mark Hildebrand <mhildebrand@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants