[xinyuwen2]

• 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

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

Optimizes SphericalQ encoding without changing the quantization algorithm or encoded output:

• Avoids redundant transform allocation and copying. When no subsampling is required, DoubleHadamard operates directly on the existing destination buffer instead of allocating a temporary buffer and copying the result.
• Simplifies comparisons in scale search. Nonnegative, non-NaN critical values are ordered using their unsigned floating-point bit representations. NaN inputs are explicitly rejected before entering the heap loop.

Any other comments?

Windows x86-64, Rust 1.97.1, opt-level=3, codegen-units=1, target-cpu=x86-64-v3. Each configuration uses 512 deterministic synthetic vectors with components sampled in (-1, 1), one warmup pass and three timed passes. Three process pairs alternate float/integer execution order. The table is the median of the three per-process medians; times are nanoseconds per vector. All nine scale-result fingerprints match across comparators and runs.

Dimension	Bits	Float ns/vector	Integer ns/vector	Time reduction
128	2	5869.9	4700.6	19.9%
128	3	16031.8	12759.8	20.4%
128	4	35973.4	28927.1	19.6%
768	2	44791.2	33444.9	25.3%
768	3	128015.0	96599.4	24.5%
768	4	284202.3	217329.1	23.5%
1536	2	93603.5	69594.7	25.6%
1536	3	272758.8	205962.9	24.5%
1536	4	609417.4	462919.9	24.0%

Wikipedia-Cohere: 1M × 768 float32 vectors, inner product, the same 50K training sample, 16 encoding threads, release build.
All times are in seconds; medians of three runs.

Configuration	Quantizer training	In-memory encoding (excluding I/O)	Streamed encoding (including I/O)	Training + streamed encoding
PQ, 2 dimensions/chunk	2.238	2.700	4.539	6.753
SphericalQ2, before	0.044	3.578	5.859	5.904
SphericalQ2, after	0.044	2.715	4.566	4.610
SphericalQ3, before	0.045	9.265	11.535	11.581
SphericalQ3, after	0.043	7.302	9.752	9.795
SphericalQ4, before	0.044	20.201	23.096	23.139
SphericalQ4, after	0.043	16.020	18.705	18.748

[Copilot]

🟢 Approval recommended

The optimized paths preserve existing behavior and are covered by focused equivalence and edge-case tests.

0 open findings

What changed in this PR

Optimizes spherical quantization encoding while preserving encoded output.

Changes:

• Uses integer bit ordering for non-NaN scale-search heap values.
• Reuses the destination buffer for non-subsampled Double Hadamard transforms.
• Adds equivalence and allocation-path tests.

File	Description
diskann-quantization/​src/​spherical/​quantizer.rs	Optimizes critical-value ordering and tests behavioral equivalence.
diskann-quantization/​src/​algorithms/​transforms/​double_hadamard.rs	Avoids temporary allocation when subsampling is unnecessary.

🧠 Review effort: Balanced

---

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

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

❌ Patch coverage is 98.91304% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.98%. Comparing base (907d405) to head (0237d26).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
diskann-quantization/src/spherical/quantizer.rs	97.91%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1478      +/-   ##
==========================================
+ Coverage   91.97%   91.98%   +0.01%     
==========================================
  Files         586      586              
  Lines      118173   118391     +218     
==========================================
+ Hits       108688   108903     +215     
- Misses       9485     9488       +3     

Flag	Coverage Δ
miri	91.98% <98.91%> (+0.01%)	⬆️
unittests	91.92% <98.91%> (+0.01%)	⬆️

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

Files with missing lines	Coverage Δ
...ation/src/algorithms/transforms/double_hadamard.rs	99.43% <100.00%> (+0.05%)	⬆️
diskann-quantization/src/test_util.rs	85.38% <100.00%> (+0.26%)	⬆️
diskann-quantization/src/spherical/quantizer.rs	98.20% <97.91%> (+0.05%)	⬆️

... and 14 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 - I think both of these are useful. Please see the inline comments. For the ordering, I'd rather we just embrace integer based ordering. Is there a noticeable improvement there? When done in isolation?

[Mark Hildebrand (hildebrandmw)]

Thanks!!