Skip to content

[MOD-17189] Share SQ8 training state for RAM and disk indexes - #1078

Open
dor-forer wants to merge 1 commit into
mainfrom
mod-17189-sq8-accumulator
Open

dor-forer wants to merge 1 commit into
mainfrom
mod-17189-sq8-accumulator

Conversation

@dor-forer

@dor-forer dor-forer commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Describe the changes in the pull request

Share SQ8TrainingState between RAM tiered HNSW and the optional trained disk implementation. It owns the capped training threshold, FP64 running sums, live-vector count and mean calculation. RAM keeps its existing scheduling and training behavior; disk supplies its own storage and jobs.

Add TieredHNSWDiskParams::QuantNormalizationSetSize for downstream parameter forwarding. This PR does not change disk defaults: downstream disk HNSW keeps regular SQ8 unless COMPRESSION SQ8 is explicitly requested. TRAINING_THRESHOLD retains RAM semantics, including zero to skip learning.

Which issues this PR fixes

Prerequisite for MOD-17189. Merge order: VecSim → RediSearch integration → RSE disk integration. Downstream dependencies pin this published branch commit until it merges.

Validation

Code remains at 30983555573524436edc8c55960e18606965ad89: all six SQ8TrainingState component tests and all 162 HNSWTieredIndexTestSQ8*.* tests passed on dorer-intel. The optional disk behavior and its integration validation belong to the downstream PRs. This revision updates the description; it adds no library code.

Mark if applicable

  • This PR introduces API changes: an internal disk-tiered training-size parameter.
  • This PR introduces serialization changes.

Note

Medium Risk
Changes SQ8 training bookkeeping and finalize preconditions for tiered HNSW; behavior is intended to be equivalent but mistakes would affect quantization mean and backend initialization.

Overview
Introduces shared SQ8TrainingState for SQ8 training while vectors accumulate in the flat buffer: capped normalization threshold, FP64 running sums, live vector count, add/subtract, finalize check, and mean calculation.

Tiered RAM HNSW drops its nested SQAccumulationState and calculateQuantizationMean; it delegates to SQ8TrainingState and adds asserts that training count matches flat index size. Finalization now uses shouldFinalize() and calculateMean().

TieredHNSWDiskParams replaces the placeholder with QuantNormalizationSetSize (same semantics as tiered RAM HNSW) so disk tiered indexes can share the same training rules in follow-up work.

Adds component tests for SQ8TrainingState (float/float16, threshold cap, FP64 accumulation) and updates tiered SQ8 test accessors to use the new getters.

Reviewed by Cursor Bugbot for commit 3098355. Bugbot is set up for automated code reviews on this repo. Configure here.

@dor-forer
dor-forer marked this pull request as draft October 18, 2026 10:45
@codecov

codecov Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.57%. Comparing base (0348684) to head (3098355).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1078      +/-   ##
==========================================
- Coverage   97.58%   97.57%   -0.01%     
==========================================
  Files         142      143       +1     
  Lines        9444     9456      +12     
==========================================
+ Hits         9216     9227      +11     
- Misses        228      229       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@dor-forer
dor-forer force-pushed the mod-17189-sq8-accumulator branch from a4438c1 to 3098355 Compare October 18, 2026 12:33
@dor-forer dor-forer changed the title [MOD-17189] Extract reusable SQ8 mean accumulator [MOD-17189] Share SQ8 training state for RAM and disk indexes Oct 18, 2026
@dor-forer
dor-forer marked this pull request as ready for review October 18, 2026 12:38
@dor-forer
dor-forer requested a review from lerman25 October 26, 2026 07:52
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.

1 participant