Skip to content

Fix serialization round trip for QuantizationType::SQ without standard deviation - #1461

Merged
Alex Razumov (arrayka) merged 1 commit into
mainfrom
u/arrayka/sq_serialization
Oct 5, 2026
Merged

Alex Razumov (arrayka) merged 1 commit into
mainfrom
u/arrayka/sq_serialization

Conversation

@arrayka

@arrayka Alex Razumov (arrayka) commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Why

QuantizationType::SQ with no explicit standard deviation did not round-trip through serialization:

QuantizationType::SQ {
    nbits: <N>,
    standard_deviation: None,
}

It serialized as SQ_N_None, which the deserializer rejected.

What

Serialize SQ configs without an explicit standard deviation as SQ_N and cover the case in round-trip tests.

Align the serialized spelling for SQ's default standard deviation with the parser's canonical SQ_N form.
@arrayka Alex Razumov (arrayka) changed the title Fix scalar quantization serialization round trips Fix the scalar quantization serialization round trip for SQ_N Oct 5, 2026
@arrayka
Alex Razumov (arrayka) requested a balanced review from Copilot October 5, 2026 18:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The serialization format now matches the existing parser and is adequately tested.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes SQ_N serialization so scalar quantization configurations without an explicit standard deviation round-trip correctly.

Changes:

  • Serializes SQ without standard deviation as SQ_N.
  • Adds formatting and serialization round-trip coverage.
File Description
diskann-disk/​src/​build/​configuration/​quantization_types.rs Aligns SQ formatting with parsing and updates tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.86%. Comparing base (31589c3) to head (90ece37).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1461      +/-   ##
==========================================
- Coverage   91.90%   90.86%   -1.04%     
==========================================
  Files         582      582              
  Lines      114947   114950       +3     
==========================================
- Hits       105640   104452    -1188     
- Misses       9307    10498    +1191     
Flag Coverage Δ
miri 90.86% <100.00%> (-1.04%) ⬇️
unittests 90.63% <100.00%> (-1.22%) ⬇️

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

Files with missing lines Coverage Δ
...disk/src/build/configuration/quantization_types.rs 98.14% <100.00%> (+0.03%) ⬆️

... and 47 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arrayka Alex Razumov (arrayka) changed the title Fix the scalar quantization serialization round trip for SQ_N Fix serialization round trip for QuantizationType::SQ without standard deviation Oct 5, 2026
@arrayka
Alex Razumov (arrayka) marked this pull request as ready for review October 5, 2026 20:14
@arrayka
Alex Razumov (arrayka) requested a review from a team October 5, 2026 20:14
@arrayka
Alex Razumov (arrayka) enabled auto-merge (squash) October 5, 2026 21:22
@arrayka
Alex Razumov (arrayka) merged commit 4830bd1 into main Oct 5, 2026
32 checks passed
@arrayka
Alex Razumov (arrayka) deleted the u/arrayka/sq_serialization branch October 5, 2026 22:17
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.

5 participants