Strengthen arguments to robust_prune. - #1358
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens the graph::internal::prune::robust_prune API by requiring its candidate input to be a SortedNeighbors, making the sorted-by-distance invariant structural rather than a debug assertion. It also removes the lingering Eq bound from SortedNeighbors (left over from #1273) and introduces a SortedNeighbors::map_in helper to project a sorted neighbor list into another storage buffer while preserving sortedness.
Changes:
- Change
robust_pruneto acceptSortedNeighbors<'_, Option<V>>instead of a raw&[(f32, Option<V>)], removing the runtime sortedness assertion. - Refactor
SortedNeighborsto drop theEqbound and addmap_into project IDs into a separate, linearly accessible cache while preserving order. - Extend
VerboseEqsupport in tests (notably forstrand references).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| diskann/src/test/cmp.rs | Adds VerboseEq impl for references and expands leaf VerboseEq coverage (incl. str). |
| diskann/src/graph/internal/sorted_neighbors.rs | Drops Eq constraint, makes SortedNeighbors Copy/Clone, adds map_in, and adds tests for mapping behavior. |
| diskann/src/graph/internal/prune.rs | Updates robust_prune to accept SortedNeighbors candidates and adapts indexing/lookup logic accordingly. |
| diskann/src/graph/index.rs | Uses SortedNeighbors::map_in to build the prune candidate cache and updates the robust_prune call site. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1358 +/- ##
==========================================
- Coverage 91.55% 91.55% -0.01%
==========================================
Files 521 521
Lines 100371 100410 +39
==========================================
+ Hits 91898 91927 +29
- Misses 8473 8483 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
# 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>
Make the
sorted_cacheargument aSortedNeighborsinstead to structurally require sortedness. This removes theEqbound fromSortedNeighbors, which is left over from #1273, and provides a helpermap_infor projectingSortedNeighbors.