Skip to content

Fix recall computation for beta-filtered search in benchmark#1238

Merged
magdalendobson merged 25 commits into
mainfrom
users/magdalen/fix_recall_beta_search
Jul 10, 2026
Merged

Fix recall computation for beta-filtered search in benchmark#1238
magdalendobson merged 25 commits into
mainfrom
users/magdalen/fix_recall_beta_search

Conversation

@magdalendobson

@magdalendobson magdalendobson commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

It turns out that when we implemented the fix to recall computation for filtered search in #1069, the fix did not propagate to beta search. This happened because beta search used the same runner as regular knn search in search/knn.rs, so the wrong GroundTruthMode was passed in. This PR fixes that problem by including GroundTruthMode in SearchSteps so it can be passed in earlier in the stack.

@codecov-commenter

codecov-commenter commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.11%. Comparing base (ee7dbaa) to head (860ccae).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
diskann-benchmark/src/index/benchmarks.rs 66.66% 3 Missing ⚠️
diskann-benchmark/src/index/search/knn.rs 80.00% 3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (76.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1238      +/-   ##
==========================================
- Coverage   91.01%   90.11%   -0.90%     
==========================================
  Files         501      504       +3     
  Lines       95572    96074     +502     
==========================================
- Hits        86988    86581     -407     
- Misses       8584     9493     +909     
Flag Coverage Δ
miri 90.11% <76.00%> (-0.90%) ⬇️
unittests 89.78% <76.00%> (-1.20%) ⬇️

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

Files with missing lines Coverage Δ
diskann-benchmark/src/index/inmem/spherical.rs 100.00% <ø> (ø)
...nn-benchmark/src/index/streaming/full_precision.rs 93.61% <100.00%> (+0.06%) ⬆️
diskann-benchmark/src/utils/recall.rs 57.14% <ø> (ø)
diskann-benchmark/src/index/benchmarks.rs 70.77% <66.66%> (+4.11%) ⬆️
diskann-benchmark/src/index/search/knn.rs 78.21% <80.00%> (+0.44%) ⬆️

... and 56 files with indirect coverage changes

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

@magdalendobson magdalendobson marked this pull request as ready for review July 8, 2026 13:22
@magdalendobson magdalendobson requested review from a team and Copilot July 8, 2026 13:22

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 recall computation for beta-filtered (and other range-groundtruth) search benchmarks by ensuring the correct GroundTruthMode is threaded into the KNN runner earlier in the call stack, avoiding the previous hardcoded mode mismatch.

Changes:

  • Add groundtruth_mode: GroundTruthMode to SearchSteps (defaulting to Fixed) and pass it through Knn::search_all.
  • Update KNN runner implementations to accept a groundtruth_mode parameter instead of hardcoding Fixed/Flexible.
  • Set GroundTruthMode::Flexible at beta-filtered / multihop / inline-filter benchmark call sites that load range groundtruth.

Reviewed changes

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

File Description
diskann-benchmark/src/index/search/knn.rs Threads GroundTruthMode via SearchSteps into the KNN search runner/aggregator.
diskann-benchmark/src/index/inmem/spherical.rs Marks beta-filtered/multihop/inline-filter in-memory phases as GroundTruthMode::Flexible.
diskann-benchmark/src/index/benchmarks.rs Marks beta-filtered/multihop/inline-filter benchmark phases as GroundTruthMode::Flexible.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@harsha-simhadri

Copy link
Copy Markdown
Contributor

Does the change the results reported in the wiki comparing different filtered search algos?

@magdalendobson

Copy link
Copy Markdown
Contributor Author

Does the change the results reported in the wiki comparing different filtered search algos?

No, this error doesn't lead to incorrect recall, but rather to throwing an error if any query point has fewer than k groundtruth results. This wasn't the case for the datasets I tested in the Wiki post. I think I found the error originally when working with smaller datasets where small selectivity led to fewer than k valid results for some points.

@magdalendobson magdalendobson merged commit 053830b into main Jul 10, 2026
24 checks passed
@magdalendobson magdalendobson deleted the users/magdalen/fix_recall_beta_search branch July 10, 2026 14:41
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.

6 participants