Repository navigation
test: cover Bits1_5 in TurboQuant test matrices - #10448
Merged
Merged
Conversation
Nine bit-width matrices in `turboquant::quantization` omitted `TQBits::Bits1_5`, so the variant went unexercised there. This is the gap qdrant#10390 closed for `quantize_output_byte_length`, where a `quantized_size` double-padding bug had slipped through a hardcoded [Bits1, Bits2, Bits4] list. Add `Bits1_5` to the seven matrices that accept it, including the rstest cases of `score_precomputed_dispatches_all_bit_widths`, whose name already claims to cover every bit width. The two `Unpadded` rotation tests keep their lists: `TurboQuantizer::new` asserts "Bits1_5 requires TQRotation::Padded", since `Bits1_5` rotates into its x1.5 padding and an unpadded rotation would leave half the codes carrying nothing. Each list now names that invariant, so the omission reads as deliberate rather than as the same oversight. `unpadded_rotation_matches_padded_for_padding_free_dims` also claimed multiples of 8 are padding-free "for every supported bit width", which `Bits1_5` falsified: `padded_dim(8)` is 16. Co-Authored-By: Claude Opus 5 <[email protected]>
IvanPleshkov
approved these changes
Sep 3, 2026
timvisee
pushed a commit
that referenced
this pull request
Sep 3, 2026
Nine bit-width matrices in `turboquant::quantization` omitted `TQBits::Bits1_5`, so the variant went unexercised there. This is the gap #10390 closed for `quantize_output_byte_length`, where a `quantized_size` double-padding bug had slipped through a hardcoded [Bits1, Bits2, Bits4] list. Add `Bits1_5` to the seven matrices that accept it, including the rstest cases of `score_precomputed_dispatches_all_bit_widths`, whose name already claims to cover every bit width. The two `Unpadded` rotation tests keep their lists: `TurboQuantizer::new` asserts "Bits1_5 requires TQRotation::Padded", since `Bits1_5` rotates into its x1.5 padding and an unpadded rotation would leave half the codes carrying nothing. Each list now names that invariant, so the omission reads as deliberate rather than as the same oversight. `unpadded_rotation_matches_padded_for_padding_free_dims` also claimed multiples of 8 are padding-free "for every supported bit width", which `Bits1_5` falsified: `padded_dim(8)` is 16. Co-authored-by: Claude Opus 5 <[email protected]>
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
Nine bit-width matrices in
lib/quantization/src/turboquant/quantization.rsomitTQBits::Bits1_5, so that variant is never exercised by them.This is the same enumeration gap #10390 closed for
quantize_output_byte_length: thequantized_sizedouble-padding bug was invisible because the one test that would have caught it did not list the affected variant. The rest of the module still has that gap, whilescore_precomputed_batch_matches_singlealready enumeratesBits1_5— so the inconsistency sits inside a single file.Test-only change; no production code is touched.
Changes
Bits1_5added to the seven matrices where it applies:quantize_extreme_valuesquantize_deterministicquantize_zero_vectorquantize_non_power_of_two_dimspack_unpack_vector_roundtrippack_unpack_vector_uniform_indicesscore_precomputed_dispatches_all_bit_widths— the name already claims to cover all bit widthsThe two
Unpaddedrotation tests cannot take it, and now say so:unpadded_rotation_keeps_padding_zeroandunpadded_rotation_matches_padded_for_padding_free_dimsboth build quantizers withTQRotation::Unpadded, andTurboQuantizer::newassertsBits1_5 requires TQRotation::Padded—Bits1_5rotates into its x1.5 padding, so an unpadded rotation would leave half the codes carrying nothing. Adding it there panics; each list now carries a one-line comment naming the invariant, so the omission reads as deliberate.unpadded_rotation_matches_padded_for_padding_free_dimsalso claimed multiples of 8 are padding-free "for every supported bit width". That stopped being true whenBits1_5was added —padded_dim(8)is 16 — so the comment now names the bit widths it actually covers.Left alone as out of scope:
higher_bits_reduce_errorasserts the orderingMAE(Bits4) <= MAE(Bits2) <= MAE(Bits1). PlacingBits1_5in that chain is an accuracy claim that deserves its own justification rather than being folded into a coverage change.Verification
Run on stable 1.98.0, the toolchain
.github/workflows/rust.ymluses.cargo test -p quantization --lib turboquantcargo +nightly fmt --all -- --checkcargo clippy -p quantization --all-features --all-targetsBefore the change the same filter reports 89 tests; the extra one is the new
score_precomputed_dispatches_all_bit_widths::bits1_5case.AI contribution
Prepared with Claude Code: it audited the bit-width matrices in the module, made the edits, and ran the commands above.
The first attempt added
Bits1_5tounpadded_rotation_keeps_padding_zeroas well; the test run caught theBits1_5 requires TQRotation::Paddedassertion inTurboQuantizer::new, which is what established that bothUnpaddedtests must keep their current lists. The scope call onhigher_bits_reduce_errorwas made againstTQBits::bit_sizeand the centroid tables.All Submissions:
devbranch (notmaster) and my branch was created fromdev.Changes to Core Features: