[juchen-ms (partychen)]

• Does this PR have a descriptive title that could go in our release notes?
• Does this PR add any new dependencies?
• Does this PR modify any existing APIs?
• Is the change to the API backwards compatible?
• Should this result in any changes to our documentation, either updating existing docs or adding new ones?

Reference Issues/PRs

Follow-up to #1494, which introduced the standalone MinMax matrix kernel.

What does this implement/fix? Briefly explain your changes.

Expose the MinMax matrix kernel through build_minmax_max_sim, returning a concrete MinMaxMaxSim<B_BITS> that owns the prepared query and can be reused across documents.

• Accept existing QueryMatRef<MinMaxMeta<A_BITS>> inputs, with independent query/document widths from 1 through 8.
• Instantiate fixed bit-width/backend combinations inside diskann-quantization, removing the caller-provided visitor and BoxErase argument.
• Select backends through an internal architecture enum. Support Auto, Scalar, x86-64 V3/V4, Neon and an independent scalar Reference implementation; unavailable explicit ISAs return an error.
• Store the Reference query copy in rowmajor::Owned<u8>.
• Return one negated maximum inner product per query row. Preserve empty-input and nonfinite-score behavior, and check dimensions/output lengths before modifying scores. Reuse BuildMaxSimError, with Overflow for storage-size failures.

No new dependencies. Existing full-precision factory signatures and pairwise APIs remain unchanged.

Usage example: MinMax8 query against MinMax4 documents

The query and documents are already quantized and have matching intrinsic dimensions. Build once, then reuse the kernel and score buffer:

use diskann_quantization::{
    minmax::{MinMaxMeta, build_minmax_max_sim},
    multi_vector::{Mat, MaxSimIsa, QueryMatRef},
};

fn score_documents(
    query: QueryMatRef<'_, MinMaxMeta<8>>,
    documents: &[Mat<MinMaxMeta<4>>],
) -> Result<Vec<Vec<f32>>, Box<dyn std::error::Error>> {
    let kernel = build_minmax_max_sim::<8, 4>(MaxSimIsa::Auto, query)?;
    let mut scores = vec![0.0; kernel.nrows()];
    let mut results = Vec::with_capacity(documents.len());

    for document in documents {
        kernel.compute_max_sim(document.as_view(), &mut scores)?;
        results.push(scores.clone());
    }
    Ok(results)
}

For an owned query, pass QueryMatRef::from(query.as_view()). Each call overwrites scores; the example clones it only to retain each document’s result. Lower scores indicate greater similarity.

Any other comments?

The updated usage example was compile-checked against the current API. Tests cover all 64 bit-width combinations, panel tails, backend/layout selection, query ownership, invalid inputs, nonfinite scores and storage overflow. See the PR checks for current CI results.

[Copilot]

🔵 Needs a closer look

Unsafe storage cloning and runtime-selected SIMD paths warrant human review with cross-platform validation results, which were not verified here.

No open issues 🎉

What changed in this PR

Integrates the MinMax matrix kernel into DiskANN’s multi-vector distance API through an owned-query factory, preserving QueryMatRef inputs.

Changes:

• Adds runtime backend selection, prepared kernels, and an independent scalar reference.
• Introduces shared construction errors and owned MinMax query cloning.
• Adds coverage for bit widths, backend layouts, ownership, validation, and edge-case scores.

File	Description
diskann-quantization/​src/​multi_vector/​distance/​factory.rs	Adds overflow to shared construction errors.
diskann-quantization/​src/​minmax/​multi/​reference.rs	Adds an independent scalar scoring baseline.
diskann-quantization/​src/​minmax/​multi/​prepared.rs	Wraps packed kernels with validation and distance conversion.
diskann-quantization/​src/​minmax/​multi/​mod.rs	Registers and exports the new factory interfaces.
diskann-quantization/​src/​minmax/​multi/​meta.rs	Implements owned query cloning and cloning tests.
diskann-quantization/​src/​minmax/​multi/​kernel.rs	Defines kernel and erasure traits with shared validation.
diskann-quantization/​src/​minmax/​multi/​factory.rs	Implements backend selection, construction checks, and factory tests.
diskann-quantization/​src/​minmax/​mod.rs	Exports the public MinMax factory API.
diskann-quantization/​src/​matrix_kernels/​maxsim/​packed_minmax_x_unpacked_minmax.rs	Exposes the accumulator limit for factory validation.
diskann-quantization/​src/​matrix_kernels/​maxsim/​mod.rs	Removes the integrated kernel’s dead-code suppression.

_{Review effort: Balanced}

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

❌ Patch coverage is 97.83654% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.08%. Comparing base (8a9ff2d) to head (023d2dd).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
diskann-quantization/src/minmax/multi/factory.rs	96.85%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1508      +/-   ##
==========================================
- Coverage   92.16%   91.08%   -1.09%     
==========================================
  Files         592      596       +4     
  Lines      123582   124073     +491     
==========================================
- Hits       113901   113006     -895     
- Misses       9681    11067    +1386     

Flag	Coverage Δ
miri	91.08% <97.83%> (-1.09%)	⬇️
unittests	90.78% <97.81%> (-1.33%)	⬇️

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

Files with missing lines	Coverage Δ
..._kernels/maxsim/packed_minmax_x_unpacked_minmax.rs	94.63% <100.00%> (-3.56%)	⬇️
diskann-quantization/src/minmax/multi/kernel.rs	100.00% <100.00%> (ø)
diskann-quantization/src/minmax/multi/prepared.rs	100.00% <100.00%> (ø)
diskann-quantization/src/minmax/multi/reference.rs	100.00% <100.00%> (ø)
...-quantization/src/multi_vector/distance/factory.rs	77.51% <ø> (ø)
diskann-quantization/src/minmax/multi/factory.rs	96.85% <96.85%> (ø)

... and 60 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Mark Hildebrand (hildebrandmw)]

Thanks for working on this. I think trying to follow the pattern established by the full-precision kernels is doubling down on patterns we should be trying to fix instead.

Instead, I would encourage you to take an approach of (1) returning a concrete object from the factory APIs and (2) seeing if you can find an approach that is more ergonomic to use and implement than the complexity somewhat inherent in the factory design of the full precision kernels. Bonus points if you can figure out a way to tie the full-precision and min-max abstractions together (though don't feel the need to do so, particularly if makes the interfaces less clear).

[juchen-ms (partychen)]

Thanks for the review, Mark Hildebrand (@hildebrandmw). Updated in 09f3d47: the factory now returns MinMaxMaxSim<B_BITS> without a visitor, with concrete constructors in the library. RuntimeArch and fixed-layout implementations simplify the dispatch and bounds.

I tried sharing the full-precision and MinMax abstractions, but the existing interface is tied to Standard<T>, while MinMax needs packed codes, per-vector metadata, and independent query/document bit widths. I'd keep that broader API change separate from this PR. They still share the ISA selector and error types.

[Mark Hildebrand (hildebrandmw)]

Thanks - this is significantly cleaner, but now it forces us to address the compilation cost of these kernels.

The easiest thing to do is to just compile the one kernel we care about. Thinking more broadly, though, about the problem is about specializing the entire loopnest for all bit combinations and all architectures compounds the code generation cost with probably a dubious ROI for that specialization.

Thinking abstractly, the most important bit to specialize is probably at the panel kernel level. Maybe a tiny bit higher. Everything else could be moved behind dynamically dispatched interfaces (maybe), which would avoid some of the cost. I don't know how all that would end up working out, though. Just a thought.

[Mark Hildebrand (hildebrandmw)]

It's generally a good idea to call these like inner_type_id or something. type_id can get too easily conflated with std::any::Any::type_id, which (in the case of a trait object for example), would accidentally return the type ID of Box<dyn Trait> instead of the type-id of the wrapped type.

[Mark Hildebrand (hildebrandmw)]

There should really be a shared helper for this arithmetic instead of duplicating it in multiple places.

[Mark Hildebrand (hildebrandmw)]

This is what I was worried was hiding under the previous generic interface. With all of these now being compiled into diskann-quantization, the time for a release compilation of the crate goes from around 7 seconds on my machine to 23 seconds! That's a 3x increase.

That's way too much. If you are set on materializing all these bit specializations, something needs to be done on the backend to avoid monomorphizing the entire stack for all bits, especially if the logical underlying code remains the same.

Otherwise, please just pick the one that we actually care about to try to limit code bloat.

This also ties into my concern on this line of work about the impact on test code generation. Test code is usually run at much lower optimization level, but testing all of these has a compilation cost as well.