Skip to content

[MOD-14955] Add DistanceCalculatorWithNorm#1002

Open
dor-forer wants to merge 4 commits into
dor-forer-MOD-14952-support-quant-normalizationfrom
dor-forer-MOD-14955-distance-calculator-with-norm
Open

[MOD-14955] Add DistanceCalculatorWithNorm#1002
dor-forer wants to merge 4 commits into
dor-forer-MOD-14952-support-quant-normalizationfrom
dor-forer-MOD-14955-distance-calculator-with-norm

Conversation

@dor-forer

@dor-forer dor-forer commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Describe the changes in the pull request

This PR is stacked on #1000. Review only the incremental diff against dor-forer-MOD-14952-support-quant-normalization.

Cherry-picks the two commits from ARM-software/VectorSimilarity-for-Arm#3: 493ee7a8 and c27401d6.

This PR adds DistanceCalculatorWithNorm, which invokes the selected SQ8 kernel and applies the mean-normalization correction terms needed to recover the distance between the original vectors. The follow-up commit makes metadata reads safe for unaligned storage.

The calculator integrates with the construction-time dispatch introduced in #999:

  • HNSW caches a non-virtual callback and a minimal immutable context containing only the stored/query kernels and mean_sum_squares.
  • The candidate loop does not call IndexCalculatorInterface::calcDistance or calcDistanceForQuery, so it does not restore the per-distance vtable lookup removed by MOD-14916 Devirtualize HNSW / brute-force search hot path #937.
  • Existing calculators retain the stateless raw-kernel path. Stored-to-stored normalized L2 also uses that path because its mean terms cancel.

Tests cover FP32/FP16, L2/IP, symmetric/asymmetric, zero-mean, randomized-vector, and stateless/stateful dispatch behavior.

Which issues this PR fixes

  1. MOD-14955

Main objects this PR modified

  1. DistanceCalculatorWithNorm
  2. Normalized-distance component unit tests

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

Validation

  • make format
  • make check-format
  • Release build of test_components and test_hnsw
  • test_components: 40/40 passed
  • test_hnsw excluding the two external serialization fixtures: 168/168 passed
  • git diff --check dor-forer-MOD-14952-support-quant-normalization...HEAD
  • Focused stateless-dispatch microbenchmark: 36 alternating-order rounds, 20M calls per path per round; cached raw kernel averaged 2.75 ns/call and DistanceDispatch averaged 2.69 ns/call (no measurable regression within run-to-run noise)

Note

Medium Risk
Changes core vector distance math for normalized SQ8 indices; incorrect formulas would skew search results, though coverage is strong and L2 symmetric path stays on the raw kernel.

Overview
Adds DistanceCalculatorWithNorm, a distance calculator for mean-normalized SQ8 indices where stored vectors are quantized from x' = x - mean. It runs the existing SQ8 kernels and applies analytical corrections using x_mean_ip / y_mean_ip metadata and mean_sum_squares so L2/IP distances match the original (unshifted) vectors for symmetric SQ8–SQ8 and asymmetric SQ8–query paths.

getDistanceDispatch keeps a stateless fast path for stored-to-stored L2 (mean terms cancel); other modes use stateful callbacks with a cached context (kernels + mean_sum_squares) for the HNSW hot path. Metadata is read via load_unaligned.

Unit tests in test_components.cpp cover FP32/FP16, L2/IP, symmetric/asymmetric distances, zero-mean equivalence to raw kernels, random vectors vs brute force, and dispatch stateless vs stateful behavior.

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

*
* @return the distance between the vectors.
*/
DistType calcDistance(const void *vector_data1, const void *vector_data2) const {

@ofiryanai ofiryanai Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This method calls indexCalculator->calcDistance, restoring the per-distance IndexCalculatorInterface vtable fallback removed by PR #937. DistanceCalculatorWithNorm always returns nullptr for the query kernel, so every visited candidate now takes a virtual call followed by the raw kernel call; existing calculators also pay the new branch. Cache a non-virtual state-aware callback or specialize the normalized dispatch outside the candidate loop, and benchmark against the #937 path before merging

@dor-forer
dor-forer force-pushed the dor-forer-MOD-14952-support-quant-normalization branch from f438a59 to d37077d Compare July 23, 2026 11:59
@dor-forer
dor-forer force-pushed the dor-forer-MOD-14955-distance-calculator-with-norm branch from 4deae2b to a0be70d Compare July 23, 2026 12:14
@dor-forer
dor-forer marked this pull request as ready for review July 23, 2026 12:21
@dor-forer
dor-forer requested a review from ofiryanai July 23, 2026 12:22

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a0be70d. Configure here.

Comment thread src/VecSim/spaces/computer/calculator.h Outdated
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.12%. Comparing base (d37077d) to head (c559e31).

Additional details and impacted files
@@                                 Coverage Diff                                 @@
##           dor-forer-MOD-14952-support-quant-normalization    #1002      +/-   ##
===================================================================================
+ Coverage                                            97.11%   97.12%   +0.01%     
===================================================================================
  Files                                                  141      141              
  Lines                                                 8296     8326      +30     
===================================================================================
+ Hits                                                  8057     8087      +30     
  Misses                                                 239      239              

☔ 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.

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.

2 participants