Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode#1232
Open
dyhyfu wants to merge 3 commits into
Open
Refactor(diskann-benchmark): consolidate disk search config under DiskSearchMode#1232dyhyfu wants to merge 3 commits into
dyhyfu wants to merge 3 commits into
Conversation
added 2 commits
July 7, 2026 17:03
Contributor
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.
Comment on lines
112
to
116
| pub(crate) recall_at: u32, | ||
| #[cfg(feature = "disk-index")] | ||
| #[serde(default)] | ||
| pub(crate) search_mode: DiskSearchMode, | ||
| // Backward compatibility for older benchmark inputs that used | ||
| // `is_flat_search` directly at the search-phase level. | ||
| #[cfg(feature = "disk-index")] | ||
| #[serde(default, skip_serializing)] | ||
| pub(crate) is_flat_search: Option<bool>, | ||
| #[cfg(not(feature = "disk-index"))] | ||
| pub(crate) is_flat_search: bool, | ||
| pub(crate) distance: SimilarityMeasure, | ||
| pub(crate) vector_filters_file: Option<InputFile>, | ||
| pub(crate) num_nodes_to_cache: Option<usize>, |
Comment on lines
218
to
221
|
|
||
| #[cfg(feature = "disk-index")] | ||
| if let Some(is_flat_search) = self.is_flat_search { | ||
| self.search_mode.is_flat_search = is_flat_search; | ||
| } | ||
|
|
||
| #[cfg(feature = "disk-index")] | ||
| self.search_mode | ||
| .validate(checker) | ||
| .context("invalid disk search mode")?; |
Comment on lines
+322
to
326
| let has_filter = search_params.search_mode.vector_filters_file.is_some(); | ||
| let mode: SearchMode<'_> = build_search_mode( | ||
| &search_params.search_mode, | ||
| has_filter, | ||
| vf, |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (15.38%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1232 +/- ##
========================================
Coverage 90.01% 90.02%
========================================
Files 501 504 +3
Lines 95584 95976 +392
========================================
+ Hits 86044 86404 +360
- Misses 9540 9572 +32
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
hildebrandmw
reviewed
Jul 7, 2026
| @@ -6,27 +6,18 @@ | |||
| use std::{fmt, num::NonZeroUsize, path::Path}; | |||
|
|
|||
| use anyhow::Context; | |||
Contributor
There was a problem hiding this comment.
One request I have (either in this PR or in a follow-up) is that running
cargo test --package diskann-benchmark --features disk-index --profile ci
generates the following files
diskann-benchmark/siftsmall_index_filter_flat_disk.index
diskann-benchmark/siftsmall_index_filter_flat_pq_compressed.bin
diskann-benchmark/siftsmall_index_filter_flat_pq_pivots.bin
diskann-benchmark/siftsmall_index_filter_graph_disk.index
diskann-benchmark/siftsmall_index_filter_graph_pq_compressed.bin
diskann-benchmark/siftsmall_index_filter_graph_pq_pivots.bin
Can this be fixed to properly use a tempdir or a virtual file system to avoid polluting the workspace? Thanks!
Contributor
Author
There was a problem hiding this comment.
Fixed, thanks for discovering this problem.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.