Repository navigation
[multi-vector] Add MinMax MaxSim kernel factory - #1508
Open
juchen-ms (partychen) wants to merge 1 commit into
Open
juchen-ms (partychen) wants to merge 1 commit into
juchen-ms (partychen) wants to merge 1 commit into
Conversation
Integrate the merged MinMax matrix kernel with an owned-query factory, runtime ISA selection, an independent scalar reference, and shared construction errors. Preserve QueryMatRef inputs and add focused factory and query-cloning coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 94d30f43-fadc-4a6b-889b-406de424a1ab
juchen-ms (partychen)
requested review from
a team
and
a balanced review from Copilot
October 23, 2026 05:05
Contributor
There was a problem hiding this comment.
🔵 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 Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1508 +/- ##
==========================================
- Coverage 92.16% 91.14% -1.02%
==========================================
Files 592 596 +4
Lines 123582 124019 +437
==========================================
- Hits 113901 113039 -862
- Misses 9681 10980 +1299
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This branch has not been deployed
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.
Reference Issues/PRs
Follow-up to #1494, which introduced the standalone MinMax matrix kernel. This PR is based on its merged commit and adds the factory integration without repeating the kernel's original commits.
What does this implement/fix? Briefly explain your changes.
Expose the MinMax matrix kernel through an owned-query MaxSim factory:
build_minmax_max_sim, acceptingQueryMatRef<MinMaxMeta<A_BITS>>. Query and document bit widths independently support 1 through 8. Prepare the query once and reuse the kernel across borrowed documents and output buffers.ReferenceKernelas an independent scalar correctness baseline. It does not delegate to the SIMD-capable pairwise API; existing pairwise registrations andMaxSimIsasemantics remain unchanged.BuildMaxSimError, addingOverflowfor storage-size failures. ImplementNewClonedforMinMaxMetaso the reference kernel uses the existingto_owned()mechanism.No new dependencies. Existing factory signatures remain unchanged; the new entry points and the additional variant on the already non-exhaustive
BuildMaxSimErrorare additive. Rustdoc includes the new API's contract and a usage example.Usage example: MinMax8 query against MinMax4 documents
The inputs below are already quantized and must have matching intrinsic dimensions. Build once per query and reuse both the kernel and score buffer across documents:
If the query is an owned
Mat<MinMaxMeta<8>>, passQueryMatRef::from(query.as_view()).Autoselects the available backend;BoxErasepackages it behind a common kernel interface. Each result contains one negated maximum inner product per query row (lower is better). Each call overwritesscores; the example clones the buffer only to retain each document's result.Any other comments?
Validated locally on Windows x86-64 at commit
9b16ba1f8aa050e5706272f6328ed5c7dc4cd691:cargo fmt --allcargo clippy --workspace --all-targets -- -D warningscargo test -p diskann-quantization --lib minmax --quietcargo test -p diskann-quantization --lib --config 'profile.test.debug-assertions=false' --config 'profile.test.overflow-checks=false' minmax::multi::factory --quietcargo test -p diskann-quantization --no-default-features --lib minmax::multi::factory --quietcargo test -p diskann-quantization --doc build_minmax_max_sim --quietcargo check -p diskann-quantization --tests --target aarch64-pc-windows-msvc --quietgit diff --checkRuntime validation covers only the backends available on the local host. AArch64 execution, ISA emulation, and coverage reporting were not run locally; CI results should be checked separately.