Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode#1232
Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode#1232dyhyfu wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the diskann-benchmark disk-index benchmark input schema to consolidate disk search configuration under DiskSearchMode, and moves diskann_disk::SearchMode construction out of the JSON schema layer into the disk search execution path.
Changes:
- Nested
vector_filters_fileandpost_processorunderDiskSearchMode, alongsideis_flat_searchandadaptive_l, and moved validation accordingly. - Added a
build_search_modehelper indisk_index/search.rsto construct backendSearchModeat execution time. - Updated benchmark JSON fixtures (examples + perf inputs) to the new nested
search_modeformat.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| diskann-benchmark/src/inputs/disk.rs | Refactors disk-index JSON schema to centralize mode-specific config/validation in DiskSearchMode. |
| diskann-benchmark/src/disk_index/search.rs | Builds backend SearchMode during execution using new helper; updates access paths to nested config. |
| diskann-benchmark/perf_test_inputs/wikipedia-100K-disk-index.json | Migrates perf input to nested search_mode object. |
| diskann-benchmark/perf_test_inputs/openai-100K-disk-index.json | Migrates perf input to nested search_mode object. |
| diskann-benchmark/example/disk-index.json | Migrates example input to nested search_mode object. |
| diskann-benchmark/example/disk-index-filter.json | Migrates filter example to nested search_mode.vector_filters_file. |
| diskann-benchmark/example/disk-index-determinant-diversity.json | Migrates post-processor example to nested search_mode.post_processor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1232 +/- ##
==========================================
- Coverage 91.16% 90.15% -1.01%
==========================================
Files 509 509
Lines 97093 97160 +67
==========================================
- Hits 88510 87598 -912
- Misses 8583 9562 +979
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| #[derive(Debug, Serialize, Deserialize, Default)] | ||
| pub(crate) struct DiskSearchMode { | ||
| pub(crate) is_flat_search: bool, | ||
| #[serde(default)] | ||
| pub(crate) adaptive_l: Option<AdaptiveL>, | ||
| #[serde(default)] | ||
| pub(crate) vector_filters_file: Option<InputFile>, | ||
| #[serde(default)] | ||
| pub(crate) post_processor: Option<TopkPostProcessor>, | ||
| } |
There was a problem hiding this comment.
Can we use a enum here to avoid impossible combinations? Something similar and in spirit of:
pub enum SearchMode<'a> {
FlatScan {
filter: Option<SearchPredicate<'a>>,
},
Graph {
filter: Option<SearchPredicate<'a>>,
},
InlineFilter {
filter: Box<dyn QueryLabelProvider<u32> + 'a>,
adaptive_l: Option<AdaptiveL>,
},
DiverseGraph {
filter: Option<SearchPredicate<'a>>,
params: DeterminantDiversityParams,
},
}I understand SearchMode is per query and has relevant types, but can we have something similiar on the benchmark input level eliminating the impossible scenarios?
Summary
Consolidates disk-index search configuration under
DiskSearchModeand decouples the benchmark's JSON input schema from the optionaldiskann-diskbackend.Previously, the input schema both defined the config and constructed
diskann_disk::SearchMode, forcing several#[cfg(feature = "disk-index")]gates and scattering related fields (vector_filters_file,post_processor) acrossDiskSearchPhase. This PR moves the backend-specific construction into the search execution path and groups the search-mode fields together.Changes
SearchModeconstruction to the backend. The match logic that buildsdiskann_disk::SearchModenow lives in abuild_search_modehelper in the search execution module, so the input schema is pure config data with no dependency on the disk backend'sSearchModetype.DiskSearchMode.vector_filters_fileandpost_processorare nested insideDiskSearchModealongsideis_flat_searchandadaptive_l. Validation andDisplaymoved accordingly.DiskSearchModeand its fields no longer need#[cfg(feature = "disk-index")]. The only remaining gates are forQuantizationType, which is adiskann-disktype (intentionally left as-is — see below).search_modeformat.