Skip to content

[benchamrk] Remove DispatchRule#1064

Open
hildebrandmw wants to merge 4 commits into
mainfrom
mhildebr/remove-dispatch-rule
Open

[benchamrk] Remove DispatchRule#1064
hildebrandmw wants to merge 4 commits into
mainfrom
mhildebr/remove-dispatch-rule

Conversation

@hildebrandmw
Copy link
Copy Markdown
Contributor

Remove the module diskann_benchmark_runner::dispatcher and all its associated items except for MatchScore and FailureScore, which get moved to diskann_benchmark_runner::benchmark.

The current users of DispatchRule are almost universally focused on matching against DataType. This PR introduces a much simpler AsDataType trait as a replacement, removing the need for the Type auxiliary struct altogether.

PR #865 changed how benchmark matching worked and rendered DispatchRule obsolete, this continues that clean up in an effort to make the benchmark API friendlier.

Suggested Review Order

  • diskann_benchmark_runner:
    • src/benchmark.rs: New home for MatchScore/FailureScore.
    • src/utils/datatype.rs: New AsDataType trait for matching against the DataType enum. Importantly, matching only returns true/false, not a MatchScore or FailureScore. This moves the ranking of matches to the caller rather than baking it in internally.
    • src/any.rs: Remove DispatchRule related inherent methods of Any as these are not longer useful. The type-erasure infrastructure in benchmark.rs has been responsible for proper downcasting and reporting since Cleanup diskann-benchmark-runner and friends. #865.
  • diskann_benchmark:
    • src/utils/mod.rs: Since type-based matching is now the caller's responsibility, a small match_data_type helper is added to replace the current uses of Type<T>: DispatchRule<DataType>.
    • Everything Else: Adapting type level assignment to AsDataType.
  • diskann_benchmark_simd: Mainly updating to AsDataType. Architecture dispatch gets a similar treatment to AsDataType, which is simpler now that AsArch is a local trait allowing us to skip the Identity new-type wrapper.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 removes the obsolete diskann_benchmark_runner::dispatcher module and replaces most type-matching usage with a simpler AsDataType trait, while relocating MatchScore/FailureScore into diskann_benchmark_runner::benchmark and updating downstream crates accordingly. It continues the cleanup started in PR #865 to make benchmark matching and diagnostics simpler and more caller-driven.

Changes:

  • Remove dispatcher / DispatchRule infrastructure and migrate MatchScore/FailureScore into benchmark.
  • Introduce AsDataType (+ Describe) for boolean type matching and human-readable mismatch descriptions.
  • Update diskann-benchmark and diskann-benchmark-simd to use AsDataType (and a new local AsArch in SIMD) instead of dispatch rules.

Reviewed changes

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

Show a summary per file
File Description
diskann-benchmark/src/utils/mod.rs Adds match_data_type helper; updates imports to new benchmark::{MatchScore, FailureScore} path.
diskann-benchmark/src/backend/index/spherical.rs Replaces DispatchRule/Type<T> matching with AsDataType::is_match checks.
diskann-benchmark/src/backend/index/scalar.rs Migrates type matching + description formatting to AsDataType constants/descriptions.
diskann-benchmark/src/backend/index/product.rs Uses utils::match_data_type and AsDataType::describe for matching/diagnostics.
diskann-benchmark/src/backend/index/benchmarks.rs Updates full-precision benchmarks to new data type matching + description logic.
diskann-benchmark/src/backend/filters/benchmark.rs Updates imports to use benchmark::{MatchScore, FailureScore}.
diskann-benchmark/src/backend/exhaustive/spherical.rs Updates imports to use benchmark::{MatchScore, FailureScore}.
diskann-benchmark/src/backend/exhaustive/product.rs Updates imports to use benchmark::{MatchScore, FailureScore}.
diskann-benchmark/src/backend/exhaustive/minmax.rs Updates imports to use benchmark::{MatchScore, FailureScore}.
diskann-benchmark/src/backend/disk_index/benchmarks.rs Replaces Type<T>: DispatchRule<DataType> constraints with T: AsDataType and new matching/description logic.
diskann-benchmark-simd/src/lib.rs Replaces DispatchRule for data types + arch dispatch with AsDataType and new local AsArch.
diskann-benchmark-runner/src/utils/datatype.rs Removes Type<T>/DispatchRule matching and introduces AsDataType + Describe.
diskann-benchmark-runner/src/test/typed.rs Updates test benchmarks to match on AsDataType and use new description formatting.
diskann-benchmark-runner/src/test/dim.rs Updates imports to new benchmark::{MatchScore, FailureScore} path.
diskann-benchmark-runner/src/registry.rs Updates imports to new benchmark::{MatchScore, FailureScore} path.
diskann-benchmark-runner/src/lib.rs Stops exporting the removed dispatcher module (left as a commented line).
diskann-benchmark-runner/src/dispatcher/mod.rs Deleted (obsolete module removed).
diskann-benchmark-runner/src/dispatcher/api.rs Deleted (obsolete module removed).
diskann-benchmark-runner/src/benchmark.rs Adds MatchScore/FailureScore types directly to benchmark module.
diskann-benchmark-runner/src/any.rs Removes DispatchRule-based matching/conversion helpers from Any.

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

pub struct Describe(DescribeInner);

impl Describe {
/// Return `true` is the data type match was successful.
Comment thread diskann-benchmark-runner/src/lib.rs Outdated
pub mod any;
pub mod app;
pub mod dispatcher;
// pub mod dispatcher;
@@ -579,7 +555,7 @@ where
_: diskann_benchmark_runner::Checkpoint<'_>,
mut output: &mut dyn diskann_benchmark_runner::Output,
) -> anyhow::Result<Self::Output> {
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

❌ Patch coverage is 61.40351% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.51%. Comparing base (d00cac0) to head (77540ba).

Files with missing lines Patch % Lines
diskann-benchmark-simd/src/lib.rs 38.29% 29 Missing ⚠️
diskann-benchmark/src/backend/index/benchmarks.rs 11.11% 8 Missing ⚠️
diskann-benchmark-runner/src/benchmark.rs 0.00% 6 Missing ⚠️
diskann-benchmark-runner/src/test/typed.rs 92.85% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1064      +/-   ##
==========================================
- Coverage   89.51%   89.51%   -0.01%     
==========================================
  Files         461      459       -2     
  Lines       85920    85642     -278     
==========================================
- Hits        76914    76659     -255     
+ Misses       9006     8983      -23     
Flag Coverage Δ
miri 89.51% <61.40%> (-0.01%) ⬇️
unittests 89.13% <61.40%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
diskann-benchmark-runner/src/any.rs 100.00% <ø> (ø)
diskann-benchmark-runner/src/registry.rs 88.26% <ø> (ø)
diskann-benchmark-runner/src/test/dim.rs 89.21% <ø> (ø)
diskann-benchmark-runner/src/utils/datatype.rs 100.00% <100.00%> (+1.01%) ⬆️
diskann-benchmark/src/backend/exhaustive/minmax.rs 100.00% <ø> (ø)
...iskann-benchmark/src/backend/exhaustive/product.rs 100.00% <ø> (ø)
...kann-benchmark/src/backend/exhaustive/spherical.rs 100.00% <ø> (ø)
diskann-benchmark/src/backend/filters/benchmark.rs 86.14% <ø> (ø)
diskann-benchmark/src/backend/index/product.rs 100.00% <ø> (ø)
diskann-benchmark/src/backend/index/scalar.rs 100.00% <ø> (ø)
... and 6 more

... and 1 file 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