Skip to content

[diskann-inmem] Spherical Quantization Round 2 - #1485

Open
Mark Hildebrand (hildebrandmw) wants to merge 5 commits into
mainfrom
mhildebr/inmem-spherical
Open

Mark Hildebrand (hildebrandmw) wants to merge 5 commits into
mainfrom
mhildebr/inmem-spherical

Conversation

@hildebrandmw

@hildebrandmw Mark Hildebrand (hildebrandmw) commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Turns out with the work done in #1469 and #1484, we can simplify the implementation of spherical quantization in diskann-inmem to also be behind Quantized. This does that and gets rid of the custom Spherical representation.

Since we now have a lot Quantizer monomorphizations, this also introduces quantized::Backed as kind of an intermediate type-erased quantizer to store in quantized::Config. The relationship goes like this:

  • The Quantized trait has an into_backend method for making a Backend. This can make use of private constructors and so is largely unimplementable by user code (they could do something weird by swapping out the quantizer, but that wouldn't cause us any issues since we call into_backend before calling any methods.
  • The IntoBackend trait is used to construct a backend. There is a blanket implementation for any Quantizer, delegating to Quantizer::into_backend. There is also an implementation for Backend which simply returns itself. Since Backend does not implement Quantizer, there is no coherence issue.
  • Config can then be constructed from a concrete Quantizer or from a Backend. The latter allows quantizers to be type-erased before constructing a Config, which simplifies a lot of set up code.
    Now, the reason to go with an into_backend method on the Quantizer trait is to give quantizers defined within diskann-inmem an escape hatch for potentially specializing inner::Quantizer, keeping the door open for hybrid PQ pruning if absolutely necessary.

@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.66667% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.07%. Comparing base (2a26c7b) to head (61b1bdd).

Files with missing lines Patch % Lines
diskann-inmem/src/repr/quantized/spherical.rs 97.33% 6 Missing ⚠️
diskann-benchmark/src/index/inmem2.rs 70.58% 5 Missing ⚠️
diskann-inmem/integration/index/runner.rs 64.28% 5 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1485      +/-   ##
==========================================
- Coverage   92.08%   91.07%   -1.02%     
==========================================
  Files         591      591              
  Lines      122196   121845     -351     
==========================================
- Hits       112523   110969    -1554     
- Misses       9673    10876    +1203     
Flag Coverage Δ
miri 91.07% <94.66%> (-1.02%) ⬇️
unittests 90.77% <94.66%> (-1.26%) ⬇️

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

Files with missing lines Coverage Δ
diskann-inmem/integration/index/object.rs 99.00% <ø> (ø)
diskann-inmem/src/repr/internal/intrusive.rs 100.00% <ø> (ø)
...ann-inmem/src/repr/internal/quantization/rerank.rs 98.75% <100.00%> (+1.22%) ⬆️
diskann-inmem/src/repr/mod.rs 93.75% <ø> (ø)
diskann-inmem/src/repr/quantized/mod.rs 98.42% <100.00%> (+0.04%) ⬆️
diskann-inmem/src/store/intrusive.rs 93.97% <100.00%> (-0.07%) ⬇️
diskann-quantization/src/spherical/quantizer.rs 98.29% <100.00%> (+0.01%) ⬆️
diskann-benchmark/src/index/inmem2.rs 62.10% <70.58%> (+0.97%) ⬆️
diskann-inmem/integration/index/runner.rs 91.78% <64.28%> (+3.21%) ⬆️
diskann-inmem/src/repr/quantized/spherical.rs 97.33% <97.33%> (ø)

... and 48 files with indirect coverage changes

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

Base automatically changed from mhildebr/inmem-scalar to main October 21, 2026 19:15
@hildebrandmw
Mark Hildebrand (hildebrandmw) marked this pull request as ready for review October 21, 2026 19:43
@hildebrandmw
Mark Hildebrand (hildebrandmw) requested review from a team and a balanced review from Copilot October 21, 2026 19:43

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.

🟢 Approval recommended

Static review identified no blocking issues in the migration or backend conversions; project tests were not run.

No open issues 🎉

What changed in this PR

Unifies spherical quantization with the shared Quantized representation, following the product and scalar quantization work.

Changes:

  • Adds a spherical adapter and type-erased Backend configuration.
  • Migrates integration and benchmark callers, removing the standalone representation.
  • Handles zero-norm rescaling and adds adapter tests.
File Description
diskann-quantization/​src/​spherical/​quantizer.rs Guards zero-norm rescaling; exposes concrete quantizers.
diskann-inmem/​src/​store/​intrusive.rs Removes guard access while retaining ownership.
diskann-inmem/​src/​repr/​spherical.rs Deletes the standalone spherical representation.
diskann-inmem/​src/​repr/​quantized/​spherical.rs Adds the spherical adapter and tests.
diskann-inmem/​src/​repr/​quantized/​mod.rs Adds Backend, conversions, and spherical exports.
diskann-inmem/​src/​repr/​mod.rs Removes standalone spherical exports.
diskann-inmem/​src/​repr/​internal/​quantization/​rerank.rs Removes the unused disabled configuration variant.
diskann-inmem/​src/​repr/​internal/​intrusive.rs Removes the unused guard accessor.
diskann-inmem/​integration/​index/​runner.rs Migrates spherical and scalar setup to backends.
diskann-inmem/​integration/​index/​object.rs Removes standalone spherical index registration.
diskann-benchmark/​src/​index/​inmem2.rs Migrates benchmark setup to shared configuration.

Review effort: Balanced

This branch has not been deployed

No deployments
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.

4 participants