Skip to content

Tweak behavior of adaptive L - #1354

Open
Magdalen Dobson Manohar (magdalendobson) wants to merge 6 commits into
mainfrom
users/magdalen/adaptive_l_improvement
Open

Tweak behavior of adaptive L#1354
Magdalen Dobson Manohar (magdalendobson) wants to merge 6 commits into
mainfrom
users/magdalen/adaptive_l_improvement

Conversation

@magdalendobson

Copy link
Copy Markdown
Contributor

Adaptive L search had a bit of strange behavior at the margins with the interaction of sample_count and resizing. If no matching samples were found when a resize was attempted, it would default to the maximum multiplier, meaning that if sample_count is relatively low, and selectivity is low but not nearly low enough to prompt resizing to the maximum multiplier, you could see an unnecessary, very large increase in L_search. This PR fixes that issue by resizing to 1/sample_count if no matching elements were found, and then allowing more than one opportunity for the queue to resize, allowing a resize attempt each time the number of comparisons doubled. This allows for more graceful behavior, and makes it harder to enact a pathological mismatch between sampling and resizing.

Along the way, after Mark noticed some issues with nondeterminism in log10, it adds some tolerance to the unit tests that depend on that function.

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 refines DiskANN’s inline filtered search behavior when using adaptive-L, aiming to avoid pathological over-expansion of L_search when sampling finds zero matches and to allow more gradual resizing as sampling progresses. It also updates the inline graph test baselines and expectations to reflect the adjusted adaptive-L behavior, and makes the adaptive-L unit tests more tolerant to minor floating-point variability in log10-driven calculations.

Changes:

  • Update adaptive-L to treat “0 matches” as an estimated specificity of 1 / visited, avoiding immediate jumps to the max multiplier.
  • Allow repeated adaptive-L resize opportunities at doubling sample thresholds (N, 2N, 4N, …) instead of only once.
  • Refresh inline search test expectations/baselines and add tolerance in adaptive-L unit tests to reduce flakiness from floating-point nondeterminism.

Reviewed changes

Copilot reviewed 2 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
diskann/src/graph/search/inline_filter_search.rs Adjusts adaptive-L sampling/resizing logic; updates adaptive-L unit tests for tolerance.
diskann/src/graph/test/cases/inline.rs Updates test expectations and test naming/docs to match the new adaptive-L behavior.
diskann/test/generated/graph/test/cases/inline/inline_search_three_level_no_adaptive_l_with_l2_finds_no_matches.json Updates generated baseline for the renamed/adjusted three-level non-adaptive-L test (L=2).
diskann/test/generated/graph/test/cases/inline/inline_search_three_level_adaptive_l_with_l2_finds_matches.json Updates generated baseline for the renamed/adjusted three-level adaptive-L test (L=2).
diskann/test/generated/graph/test/cases/inline/inline_adaptive_l_max.json Updates generated baseline to reflect new incremental growth behavior (fewer results returned).
diskann/test/generated/graph/test/cases/inline/inline_adaptive_l_logarithmic.json Updates generated baseline values consistent with the revised adaptive-L behavior.
diskann/test/generated/graph/test/cases/inline/inline_adaptive_l_linear.json Updates generated baseline values and expected IDs consistent with the revised adaptive-L behavior.

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

Comment on lines +267 to 269
if new_l > scratch.best.capacity() {
scratch.resize(new_l);
}
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1354   +/-   ##
=======================================
  Coverage   91.55%   91.56%           
=======================================
  Files         521      521           
  Lines      100347   100390   +43     
=======================================
+ Hits        91877    91922   +45     
+ Misses       8470     8468    -2     
Flag Coverage Δ
miri 91.56% <100.00%> (+<0.01%) ⬆️
unittests 91.24% <100.00%> (+<0.01%) ⬆️

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

Files with missing lines Coverage Δ
diskann/src/graph/search/inline_filter_search.rs 99.51% <100.00%> (+0.04%) ⬆️
diskann/src/graph/test/cases/inline.rs 99.75% <100.00%> (ø)

... and 3 files with indirect coverage changes

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

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.

3 participants