Skip to content

Implement filter search for disk path using AdpativeL#1173

Open
dyhyfu wants to merge 4 commits into
mainfrom
u/yaohongdeng/impl_adaptiveL_disk
Open

Implement filter search for disk path using AdpativeL#1173
dyhyfu wants to merge 4 commits into
mainfrom
u/yaohongdeng/impl_adaptiveL_disk

Conversation

@dyhyfu

@dyhyfu dyhyfu commented Jun 16, 2026

Copy link
Copy Markdown

This PR is to add filter search to disk path using AdpativeL filter algorithm.

Reference PR: #1131

The main changes:

  1. New filter_search() method on DiskIndexSearcher (parallel to flat_search) that constructs InlineFilterSearch::new(Knn, label_provider, adaptive_l) and hands it to the same self.index.search(...) engine as the plain Knn path.
  2. New PredicateLabelProvider that adapts the existing &dyn Fn(&u32) -> bool to QueryLabelProvider
  3. search() and search_internal() gain adaptive_l: Option. When None, behavior is identical to today. When Some(_), the graph path routes through filter_search.
  4. Three-way dispatch in search_internal: flat scan → graph + adaptive_l → plain Knn.

@dyhyfu dyhyfu requested review from a team and Copilot June 16, 2026 03:06

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 adds an AdaptiveL-enabled, inline label-filtered graph-search path for disk index search, wiring it through DiskIndexSearcher while keeping existing behavior unchanged when AdaptiveL is not provided.

Changes:

  • Add adaptive_l: Option<AdaptiveL> to DiskIndexSearcher::search / search_internal and dispatch to an InlineFilterSearch path when present.
  • Introduce a small PredicateLabelProvider adapter to bridge the existing &dyn Fn(&u32) -> bool filter predicate into QueryLabelProvider<u32>.
  • Add disk-provider tests covering (1) no-op equivalence vs plain Knn and (2) selective predicates returning only matching IDs; update callers to pass None until CLI/JSON exposes AdaptiveL.

Reviewed changes

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

File Description
diskann-tools/src/utils/search_disk_index.rs Passes None for the new adaptive_l argument (not yet exposed via CLI).
diskann-disk/src/search/provider/disk_provider.rs Implements filter_search via InlineFilterSearch + AdaptiveL, adds adapter + tests, and extends search APIs with adaptive_l.
diskann-disk/src/build/builder/core.rs Updates test call sites to include the new adaptive_l parameter.
diskann-benchmark/src/backend/disk_index/search.rs Passes None for the new adaptive_l argument (not yet exposed via benchmark JSON).

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

Comment thread diskann-disk/src/search/provider/disk_provider.rs Outdated
@magdalendobson

Copy link
Copy Markdown
Contributor

You may want to take a look at the changes that are coming to filters with #1141 and make sure your changes are compatible

Comment thread diskann-benchmark/src/backend/disk_index/search.rs
Comment thread diskann-tools/src/utils/search_disk_index.rs
@magdalendobson magdalendobson linked an issue Jun 16, 2026 that may be closed by this pull request
Comment thread diskann-disk/src/search/provider/disk_provider.rs Outdated
Comment thread diskann-disk/src/search/provider/disk_provider.rs Outdated
Comment thread diskann-disk/src/search/provider/disk_provider.rs Outdated
Comment thread diskann-disk/src/search/provider/disk_provider.rs Outdated
@@ -8,8 +8,12 @@ use std::{collections::HashSet, sync::atomic::AtomicBool, time::Instant};
use diskann::utils::IntoUsize;

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.

Worth noting this whole module diskann-tools/src/utils/search_disk_index.r looks dead. pub fn search_disk_index / SearchDiskIndexParameters aren't referenced from any binary.
I recommend deleting search_disk_index.rs (and its pub mod / pub use in utils/mod.rs) in a follow-up rather than carry the filter search migration.

/// at visit time (not just during rerank). `adaptive_l = Some(_)` grows the
/// beam mid-search if the observed match specificity is low.

pub enum SearchPlan<'a> {

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.

nit: Plan  tends to read as a query/execution plan: something the engine generates and optimize. Consdier naming SearchMode  or  SearchKind  which would line up more directly with what the variants express

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.

Filtered search support on disk path

4 participants