[MOD-17526] Make the SQ8 metadata describe the reconstruction - #1011
Open
dor-forer wants to merge 2 commits into
Open
[MOD-17526] Make the SQ8 metadata describe the reconstruction#1011dor-forer wants to merge 2 commits into
dor-forer wants to merge 2 commits into
Conversation
dor-forer
marked this pull request as ready for review
August 10, 2026 14:42
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1011 +/- ##
=======================================
Coverage 97.16% 97.16%
=======================================
Files 141 141
Lines 8361 8373 +12
=======================================
+ Hits 8124 8136 +12
Misses 237 237 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
lerman25
reviewed
Aug 13, 2026
lerman25
left a comment
lerman25
left a comment
Collaborator
There was a problem hiding this comment.
Nice - 1 blocking comment
lerman25
reviewed
Aug 13, 2026
| // FP32 arithmetic could leave the representable range for input that is entirely valid. | ||
| // [-FLT_MAX, +FLT_MAX] made max - min overflow to inf, then delta inf, inv_delta 0, and | ||
| // finally inf * 0 = NaN, whose conversion to an integer is undefined behaviour. Doubles | ||
| // cannot overflow for any pair of finite floats, so the whole class disappears rather than |
Collaborator
There was a problem hiding this comment.
Blocking: WithNorm can still make min_val/max_val non-finite before this double range calculation. Both find_min_max() and transformed_value() compute input[i] - mean[i] in FP32. For example, finite FP32 input [FLT_MAX, 0] with finite mean [-FLT_MAX, 0] centers to [+Inf, 0]; this then gives diff = Inf, delta = Inf, and inv_delta = 0, so to_byte(+Inf) evaluates Inf * 0 as NaN. std::clamp preserves NaN, and the following conversion to uint32_t is undefined behavior. This is reachable by the mean-centred SQ8 configuration introduced by #1007, so the finite-input safety claim is incomplete. Please perform/check centering in a representation that cannot overflow here (or reject non-finite derived values before quantization) and add a UBSan regression for this case.
Collaborator
Author
There was a problem hiding this comment.
Agreed, and fixed in 6c8d6e0. You are right that the double range does not help when the value is already lost upstream in FP32.
find_min_max now clamps its endpoints to the float range. That is also a storage constraint rather than only a guard: min_val is stored as FP32, so a non-finite endpoint could not be stored under any arithmetic. Only the endpoints are clamped, so the per-element loop stays FP32 and pays nothing.
With min_val finite and inv_delta finite and positive, NaN is unreachable in to_byte: a centered inf element gives inf * finite = inf, not 0 * inf, and lands on 255. I also moved the bound from std::clamp to std::fmin/std::fmax, since clamp propagates NaN, so the conversion is defined without relying on a proof that spans two functions.
I went with clamping rather than centering in double. The tradeoff: it puts overflowing elements at the ends instead of placing them proportionally. Happy to switch to double centering if you want proportionality, but it costs a conversion per element and the FP32 min slot still cannot hold the range.
UBSan regression added: QuantizationHandlesNonRepresentableCenteredRange, with your exact input.
This was referenced Aug 16, 2026
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
The uint8 kernels accumulate products or squared differences of bytes, so
the total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold
that, and all of them are on the plain int8/uint8 index paths that ship
today.
* IP.cpp / L2.cpp: ret_t for a 1-byte element type was int, so the
scalar UINT8_InnerProduct and UINT8_L2Sqr executed signed-overflow UB
from dimension 33,026, while the comment claimed support to 2^16.
ret_t is now 64-bit for every element type. Keeping it signed means the
"1 - ip" in the wrappers stays signed arithmetic and cannot underflow,
and the int8 paths are unaffected. The L2 comment still carried the old
"at least 2 bytes wider" rationale and is corrected.
* UINT8_InnerProductImp returned float on NEON and SVE, which
accumulated exactly in integer lanes and then discarded it, exact only
to dimension 258 since 2^24 / 65025 = 258.
* AVX512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a
signed int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned
horizontal reduce back into a signed int, so the distance went negative
from the same dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were
already unsigned and are unchanged.
Note the accumulation itself was never the problem. The SIMD adds wrap
modulo 2^32 and are bit-exact, so the bit pattern was already correct; the
top bit was being read as a sign. An unsigned 32-bit reduce therefore costs
nothing over the original and is exact through dimension 66,051, twice the
old signed limit of 33,025.
Above 66,051 a 32-bit total genuinely does run out, so each kernel gains a
`bool Wide` template parameter selecting the epilogue: the narrow unsigned
32-bit reduce, or a widening one that zero-extends the lanes to 64 bits
first and cannot wrap at any dimension. The lanes are accumulated
identically either way. The choosers pick once per index, so no branch
enters the kernel.
Widening unconditionally would have been simpler and was measured rather
than assumed. On an Ice Lake-SP Xeon it costs 4 extra uops in the epilogue:
dim 32 +20%
dim 55-200 +8 to +11%
dim 256 +7%
dim 900-1024 +4 to +5%
15 repetitions, pinned core, two passes with the A/B order reversed; sign
and magnitude hold across both. The loop bodies are instruction-for-
instruction identical with the loop tops at the same 32-byte offset, so
this is the epilogue alone. Instruction count understates it, because the
widening reduce lengthens a dependency chain rather than adding throughput
work; that is also why the absolute delta grows at high dim, where fewer
calls overlap to hide the latency.
Selecting per dimension keeps that cost off every ordinary index. The price
is instantiating both variants: on the AVX512F_BW_VL_VNNI translation unit
at -O2, object size goes from 514,792 to 657,592 bytes, +27.7%, with 197
extra exported symbols. The narrow instantiation is unchanged at 40
instructions for residual 32, before and after, so the common case keeps
the full benefit.
To avoid a second case ladder, CHOOSE_IMPLEMENTATION now forwards trailing
arguments as further template arguments using __VA_OPT__, so the same
ladder serves kernels templated on <residual> and on <residual, Wide>
alike and every existing call site is untouched.
CHOOSE_UINT8_IMPLEMENTATION wraps the dimension test so each of the 15
uint8 call sites is a one-word change, and
CHOOSE_SVE_UINT8_IMPLEMENTATION does the same for the SVE ladder.
The SQ8-to-SQ8 inner product kernels reuse this helper, on main as much as
here, so they now pass Wide explicitly. They pass false: SQ8 is capped at
the same dimension independently, because its q_sum_squares metadata slot
is a uint32 holding 65025 * dim, so a widening reduce there would exceed
what the metadata itself can represent.
Split out of #1011 because none of this depends on the SQ8 metadata
contract that PR is changing, while all of it affects code reachable today.
#1011 does depend on this, through the helper above.
The regressions use all-255 bytes, the worst case, which makes the expected
value an exact integer: dimensions 33,026 and 40,000 for the narrow path,
which the old signed reduce got wrong, and 66,052 and 80,000 for the wide
path. The existing UINT8 suites stop at dimension 128, which is why all of
this went unseen; being SIMD-versus-scalar comparisons they would also have
agreed with each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with
delete and stored the trailing norms through unaligned float casts, so
measurements taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
QuantPreprocessor::quantize could execute undefined behaviour for input
the API accepts, in three ways that all end at the same cast.
The scale was derived entirely in FP32. For [-FLT_MAX, +FLT_MAX], every
component finite and accepted without validation, max - min overflowed to
inf, delta became inf, inv_delta 0, and the per-element product
inf * 0 = NaN, whose conversion to an integer is undefined. UBSan:
"-nan is outside the range of representable values of type 'unsigned
char'". The existing diff == 0 guard covers equal values, not overflow of
the subtraction. (MOD-17528)
Two more paths reach the same cast and were found in review of the
follow-up work:
* With WithNorm, centering is an FP32 subtraction, so finite input
against a finite mean can produce inf before any range is computed:
FLT_MAX against mean -FLT_MAX centers to 6.8e38. Widening the range
to double does not help, because the value is already lost upstream.
* delta is stored as FP32, and (float)(diff / 255) underflows to zero
for any diff below about 1.8e-43 while diff itself is nonzero, so
testing diff does not catch it. 1/delta was then inf, and the minimum
element, whose numerator is exactly zero, scaled to 0 * inf.
There is also no rejection path: nothing in VecSim validates finiteness,
and AddVector has no way to report "unquantizable", so the contract has to
be saturation rather than an error.
All three are closed by normalizing the endpoints once, immediately after
find_min_max, covering both the plain and WithNorm branches. Both
endpoints get a two-sided clamp behind an order check that catches NaN. A
one-sided clamp is not enough: for an all-+inf vector inf <= inf passes
the order check, so std::max(+inf, -FLT_MAX) would leave min at +inf and
store it. min is stored as FP32, so a non-finite endpoint could not be
represented under any arithmetic; this is the storage limit as much as a
guard.
That bounds diff at 6.8e38, so delta can be neither inf nor NaN and only
the underflow guard remains, as one comparison. inv_delta stays double,
not for precision, which needs only +/-0.5 in 255, but because an FP32
reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top
element to 255.
The per-element bound is written by hand rather than with std::clamp or
std::fmin/std::fmax. Each of those breaks something: std::clamp is
comparisons and propagates NaN into the cast, while fmin/fmax are
NaN-correct but compile to two out-of-line libm calls per element at this
translation unit's baseline. Measured at -O3: 8 instructions for this
form against 9 for std::clamp and 10 plus two calls for fmin/fmax. It
also subsumes the std::round that was there, since bounding first makes
+0.5 and truncation equivalent, and round() is likewise out-of-line here.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven domain matrix covering constant vectors positive negative and
zero, a subnormal but representable delta, a range that underflows and
collapses, the full FP32 range, all +inf, all -inf, mixed infinities, and
NaN first, middle and last. Expected bytes and metadata are asserted
rather than a range check, which is vacuous for uint8_t. The three NaN
cases pin that position matters: std::minmax_element compares with < and
every comparison against NaN is false, so a NaN at either end reaches an
endpoint and trips the order check while one in the middle is skipped and
the finite values set a real range. Expectations were derived by
simulating the pipeline, which corrected three of them.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
The uint8 kernels accumulate products or squared differences of bytes, so
the total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold
that, and all of them are on the plain int8/uint8 index paths that ship
today.
* IP.cpp / L2.cpp: ret_t for a 1-byte element type was int, so the
scalar UINT8_InnerProduct and UINT8_L2Sqr executed signed-overflow UB
from dimension 33,026, while the comment claimed support to 2^16.
ret_t is now 64-bit for every element type. Keeping it signed means the
"1 - ip" in the wrappers stays signed arithmetic and cannot underflow,
and the int8 paths are unaffected. The L2 comment still carried the old
"at least 2 bytes wider" rationale and is corrected.
* UINT8_InnerProductImp returned float on NEON and SVE, which
accumulated exactly in integer lanes and then discarded it, exact only
to dimension 258 since 2^24 / 65025 = 258.
* AVX512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a
signed int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned
horizontal reduce back into a signed int, so the distance went negative
from the same dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were
already unsigned and are unchanged.
The accumulation itself was never the problem. The SIMD adds wrap modulo
2^32 and are bit-exact, so the bit pattern was already correct; the top bit
was being read as a sign. An unsigned 32-bit reduce therefore costs nothing
over the original and is exact through dimension 66,051, twice the old
signed limit of 33,025. Verified: the AVX512 object is byte-for-byte the
same size as before at 514,792, with the same 33 and 40 instructions for
residual 0 and 32.
Above 66,051 the choosers hand back the scalar kernel, which after the
ret_t change is exact to roughly dimension 2.8e14. That is one comparison
at index creation, reusing the "if (dim < 32) return ret_dist_func" idiom
the choosers already had, and it leaves every kernel untouched.
Two alternatives were explored and rejected, both recorded on the constant:
* Widening the horizontal reduce. Measured on an Ice Lake-SP Xeon it
costs 4 extra uops in the epilogue: +20% at dimension 32, +8-11% across
55-200, +4-5% at 900-1024, on byte-identical loop code. It also only
moves the limit, and to a different place per ISA, since NEON combines
four accumulators with vaddq_u32 in 32 bits before any widening reduce
sees them, capping it at 264,204 rather than the 1,056,816 AVX512 gets.
* Chunking the accumulation and flushing into a 64-bit total. Exact at
any dimension, and cheap when the chunk loop lives in the wrapper
rather than the kernel: +2 instructions on the fast path against +12 to
+21 when placed inside. Deferred rather than dismissed, since it is
only worth the restructuring if such dimensions become real.
Nothing comparable supports that range today, which is what settles it.
Lucene caps its scalar-quantized format at 1,024 dimensions and
Elasticsearch caps dense vectors at 4,096, both keeping a 32-bit
accumulator safe by contract. Faiss's QT_8bit_direct accumulates
full-range bytes into 32-bit lanes with no widening and carries the same
theoretical limit. Qdrant quantizes to 0..127, lowering the per-element cap
to 16,129, and its raw uint8 metric still sums into i32. The scalar
fallback here is already stricter than any of them.
Split out of #1011 because none of this depends on the SQ8 metadata
contract that PR is changing, while all of it affects code reachable today.
#1011 does depend on this, since its SQ8_SQ8 kernels call
UINT8_InnerProductImp. Note SQ8 is independently capped at the same
dimension, because q_sum_squares is a uint32 slot holding 65025 * dim, so
that PR needs its own fence regardless.
The regressions use all-255 bytes, the worst case, which makes the expected
value an exact integer, at dimensions 33,026 and 40,000 for the SIMD path
and 66,052 for the fallback. The fallback test asserts the returned
function pointer, not just the distance: on a host with no uint8 SIMD tier
the value comparison would pass either way, but the pointer identity would
not. The existing UINT8 suites stop at dimension 128, which is why all of
this went unseen; being SIMD-versus-scalar comparisons they would also have
agreed with each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with
delete and stored the trailing norms through unaligned float casts, so
measurements taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
QuantPreprocessor::quantize could execute undefined behaviour for input
the API accepts, in three ways that all end at the same cast.
The scale was derived entirely in FP32. For [-FLT_MAX, +FLT_MAX], every
component finite and accepted without validation, max - min overflowed to
inf, delta became inf, inv_delta 0, and the per-element product
inf * 0 = NaN, whose conversion to an integer is undefined. UBSan:
"-nan is outside the range of representable values of type 'unsigned
char'". The existing diff == 0 guard covers equal values, not overflow of
the subtraction. (MOD-17528)
Two more paths reach the same cast and were found in review of the
follow-up work:
* With WithNorm, centering is an FP32 subtraction, so finite input
against a finite mean can produce inf before any range is computed:
FLT_MAX against mean -FLT_MAX centers to 6.8e38. Widening the range
to double does not help, because the value is already lost upstream.
* delta is stored as FP32, and (float)(diff / 255) underflows to zero
for any diff below about 1.8e-43 while diff itself is nonzero, so
testing diff does not catch it. 1/delta was then inf, and the minimum
element, whose numerator is exactly zero, scaled to 0 * inf.
There is also no rejection path: nothing in VecSim validates finiteness,
and AddVector has no way to report "unquantizable", so the contract has to
be saturation rather than an error.
All three are closed by normalizing the endpoints once, immediately after
find_min_max, covering both the plain and WithNorm branches. Both
endpoints get a two-sided clamp behind an order check that catches NaN. A
one-sided clamp is not enough: for an all-+inf vector inf <= inf passes
the order check, so std::max(+inf, -FLT_MAX) would leave min at +inf and
store it. min is stored as FP32, so a non-finite endpoint could not be
represented under any arithmetic; this is the storage limit as much as a
guard.
That bounds diff at 6.8e38, so delta can be neither inf nor NaN and only
the underflow guard remains, as one comparison. inv_delta stays double,
not for precision, which needs only +/-0.5 in 255, but because an FP32
reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top
element to 255.
The per-element bound is written by hand rather than with std::clamp or
std::fmin/std::fmax. Each of those breaks something: std::clamp is
comparisons and propagates NaN into the cast, while fmin/fmax are
NaN-correct but compile to two out-of-line libm calls per element at this
translation unit's baseline. Measured at -O3: 8 instructions for this
form against 9 for std::clamp and 10 plus two calls for fmin/fmax. It
also subsumes the std::round that was there, since bounding first makes
+0.5 and truncation equivalent, and round() is likewise out-of-line here.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven domain matrix covering constant vectors positive negative and
zero, a subnormal but representable delta, a range that underflows and
collapses, the full FP32 range, all +inf, all -inf, mixed infinities, and
NaN first, middle and last. Expected bytes and metadata are asserted
rather than a range check, which is vacuous for uint8_t. The three NaN
cases pin that position matters: std::minmax_element compares with < and
every comparison against NaN is false, so a NaN at either end reaches an
endpoint and trips the order check while one in the middle is skipped and
the finite values set a real range. Expectations were derived by
simulating the pipeline, which corrected three of them.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
…nput QuantPreprocessor::quantize could execute undefined behaviour for input the API accepts. Three paths, all ending at the same conversion to a byte, and all reachable with entirely finite components. The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range overflowed: max - min became inf, delta inf, inv_delta 0, and the per-element product inf * 0 = NaN, whose conversion to an integer is undefined. UBSan: "-nan is outside the range of representable values of type 'unsigned char'". The existing diff == 0 guard covers equal values, not overflow of the subtraction. (MOD-17528) With WithNorm, centering is an FP32 subtraction, so finite input against a finite mean reaches inf before any range is computed: FLT_MAX against mean -FLT_MAX centers to 6.8e38. Widening the range does not help, because the value is already lost upstream. And delta is stored as FP32, so (float)(diff / 255) underflows to zero while diff itself is nonzero, which leaves 1/delta inf and scales the minimum element, numerator exactly zero, to 0 * inf. All three are closed by normalizing the endpoints once, after find_min_max, covering the plain and WithNorm branches together, after which a single delta comparison suffices. Both endpoints get a two-sided clamp: for an all-+inf vector inf <= inf passes the order check, so a one-sided std::max would leave min at +inf and store it. min is stored as FP32, so a non-finite endpoint could not be represented under any arithmetic. inv_delta stays double, not for precision, which needs only +/-0.5 in 255, but because an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta 2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element to 255. The per-element bound and rounding are written by hand. That replaces std::round, an out-of-line libm call at this translation unit's baseline that ran once per element: bounding first makes adding 0.5 and truncating equivalent for non-negative values. Measured at -O3 it is 8 instructions against 9 for std::clamp plus std::round's call. Deliberately NOT claimed: that the function is defined for non-finite components. It is not, and cannot be made so here. find_min_max uses std::minmax_element, whose precondition is that the comparison induce a strict weak ordering, and floating-point < is not one once a NaN is present: incomparability must be transitive, yet 1.0 is incomparable with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. The undefined behaviour is therefore inside that algorithm, before quantize() ever sees a range, so no normalization afterwards can define a portable result. A partial contract would also be misleading, since x_mean_ip, the quantized sums and the whole query metadata path are untouched and can still produce non-finite values. Non-finite components are treated as unsupported. The order check remains as a defensive fallback so that a NaN endpoint cannot be stored, which degrades a caller error into meaningless-but-finite metadata rather than poisoning every distance computed against that vector. Validating at the public ingestion boundary belongs in its own change; nothing in VecSim does it today. This matches how comparable systems handle it. Lucene validates vector components and throws on NaN or infinity, and Elasticsearch rejects NaN, infinity and magnitudes that overflow before delegating to Lucene. Faiss guards only an exactly zero range and assumes finite input otherwise, with the same float-to-integer concern at its final cast. Qdrant gets defined bytes from Rust's saturating cast, which C++ does not have, and can still store an unusable scale. Tests: the MOD-17528 reproduction, the WithNorm centering case, and a table-driven domain matrix over constant vectors positive negative and zero, a single element, a subnormal but representable delta, a range that underflows and collapses, the full FP32 range, and all-+inf, all--inf and mixed infinities. Infinities are pinned exactly, since < remains a strict weak ordering over finites and +/-inf; only NaN breaks it. Expected bytes and metadata are asserted rather than a range check, which is vacuous for uint8_t, and every expectation was derived by simulating the pipeline rather than predicted. NaN input gets one test that asserts only that the stored min and delta stay finite and delta positive, which is position-independent and portable, and which under UBSan also covers the conversion. Metadata meaning is unchanged: the sums are still FP32 over the input values. Making them exact integer sums over the quantized bytes is a storage-contract change that has to move together with every kernel that reads them, and stays in #1011. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
…nput QuantPreprocessor::quantize could execute undefined behaviour for input the API accepts. Three paths, all ending at the same conversion to a byte, and all reachable with entirely finite components. The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range overflowed: max - min became inf, delta inf, inv_delta 0, and the per-element product inf * 0 = NaN, whose conversion to an integer is undefined. UBSan: "-nan is outside the range of representable values of type 'unsigned char'". The existing diff == 0 guard covers equal values, not overflow of the subtraction. (MOD-17528) With WithNorm, centering is an FP32 subtraction, so finite input against a finite mean reaches inf before any range is computed: FLT_MAX against mean -FLT_MAX centers to 6.8e38. Widening the range does not help, because the value is already lost upstream. And delta is stored as FP32, so (float)(diff / 255) underflows to zero while diff itself is nonzero, which leaves 1/delta inf and scales the minimum element, numerator exactly zero, to 0 * inf. find_min_max now guarantees that both endpoints are finite and ordered, and everything else follows from that. The guarantee lives there rather than at the call site because that is where it can be broken: the WithNorm branch creates the inf itself, from two valid operands, and the plain branch passes through whatever the input holds. Both endpoints get a two-sided clamp, since for an all-+inf vector inf <= inf passes the order check and a one-sided std::max would leave min at +inf. Clamping to the float range is not merely defensive either: min is stored as an FP32 field, so a non-finite endpoint could not be represented under any arithmetic. With that established, quantize needs one delta comparison, and inv_delta stays double. Not for precision, which needs only +/-0.5 in 255, but because an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta 2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element to 255. The per-element bound and rounding are written by hand. That replaces std::round, an out-of-line libm call at this translation unit's baseline that ran once per element: bounding first makes adding 0.5 and truncating equivalent for non-negative values. Measured at -O3 it is 8 instructions against 9 for std::clamp plus std::round's call. Deliberately NOT claimed: that the function is defined for non-finite components. It is not, and cannot be made so here. find_min_max uses std::minmax_element, whose precondition is that the comparison induce a strict weak ordering, and floating-point < is not one once a NaN is present: incomparability must be transitive, yet 1.0 is incomparable with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. The undefined behaviour is therefore inside that algorithm, before any range exists, so no normalization afterwards can define a portable result. A partial contract would also be misleading, since x_mean_ip, the quantized sums and the whole query metadata path are untouched and can still produce non-finite values. Non-finite components are treated as unsupported. The order check remains as a defensive fallback so that a NaN endpoint cannot be stored, which degrades a caller error into meaningless-but-finite metadata rather than poisoning every distance computed against that vector. Validating at the public ingestion boundary belongs in its own change; nothing in VecSim does it today. This matches how comparable systems handle it. Lucene validates vector components and throws on NaN or infinity, and Elasticsearch rejects NaN, infinity and magnitudes that overflow before delegating to Lucene. Faiss guards only an exactly zero range and assumes finite input otherwise, with the same float-to-integer concern at its final cast. Qdrant gets defined bytes from Rust's saturating cast, which C++ does not have, and can still store an unusable scale. Tests: the MOD-17528 reproduction, the WithNorm centering case, and a table-driven domain matrix over constant vectors positive negative and zero, a single element, a subnormal but representable delta, a range that underflows and collapses, the full FP32 range, and all-+inf, all--inf and mixed infinities. Infinities are pinned exactly, since < remains a strict weak ordering over finites and +/-inf; only NaN breaks it. Expected bytes and metadata are asserted rather than a range check, which is vacuous for uint8_t, and every expectation was derived by simulating the pipeline rather than predicted. NaN input gets one test that asserts only that the stored min and delta stay finite and delta positive, which is position-independent and portable, and which under UBSan also covers the conversion. Metadata meaning is unchanged: the sums are still FP32 over the input values. Making them exact integer sums over the quantized bytes is a storage-contract change that has to move together with every kernel that reads them, and stays in #1011. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 16, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
… free
The uint8 kernels accumulate products or squared differences of bytes, so the
total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold that,
all of them on the plain int8/uint8 index paths that ship today.
* IP.cpp / L2.cpp: ret_t for a 1-byte element type was int, so the scalar
UINT8_InnerProduct and UINT8_L2Sqr executed signed-overflow UB from
dimension 33,026, while the comment claimed support to 2^16. The
conditional was also dead: only int8_t and uint8_t instantiate these, both
1 byte, so it always selected int. ret_t is now 64-bit for every element
type, which also covers int8 at dimension 131,072. Keeping it signed means
the "1 - ip" in the wrappers stays signed arithmetic and cannot underflow.
The L2 comment still carried the old byte-counting rationale and is fixed.
* UINT8_InnerProductImp returned float on NEON and SVE, accumulating exactly
in integer lanes and then discarding it, exact only to dimension 258 since
2^24 / 65025 = 258.
* AVX-512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a signed
int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned horizontal
reduce back into a signed int, so the distance went negative from the same
dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were already unsigned.
The accumulation was never the problem. The SIMD adds wrap modulo 2^32 and are
bit-exact, so the bit pattern was already correct; the top bit was being read
as a sign. An unsigned 32-bit reduce therefore costs nothing over the original
and is exact through dimension 66,051, twice the old signed limit of 33,025.
Two bounds follow, because the horizontal total and the lanes run out at
different points:
* UINT8_NARROW_REDUCE_MAX_DIM = 66,051 bounds the 32-bit total, which is
floor(UINT32_MAX / 65,025). Past it the reduce is widened to 64 bits.
* UINT8_SIMD_MAX_DIM = 4 * 66,051 bounds the lanes, which widening the total
does not protect. NEON is the limiting ISA: it combines four accumulators
with vaddq_u32 in 32 bits before any widening reduce sees them, so its
capacity is four lanes' worth. AVX-512 accumulates into 16 lanes from one
accumulator and reaches roughly 1,056,816, and SVE depends on its vector
length, so NEON sets the shared bound for IP, Cosine and L2 alike. Above
it the choosers hand back the scalar kernel, exact by the ret_t change.
Only AVX-512 carries both reduce forms. On ARM widening is free, since
vaddlvq_u32 (UADDLV) and svaddv_u32 are single instructions already producing
64 bits, so those kernels always widen and need no variant.
The AVX-512 pair is two named wrappers, X and X_Wide, rather than a template
argument threaded through the chooser macros. implementation_chooser.h is
shared with every other element type, so keeping a uint8 concern out of it
avoids blast radius, and the narrow wrapper stays byte-for-byte what it was.
That matters, and was measured: putting a runtime branch in the epilogue
instead cost 0.4 to 0.6 ns per call, +20% at dimension 32 and +8% across
55..200 on an Ice Lake-SP Xeon, because the fatter function lost its inlining
in 31 of the Cosine wrappers and grew .text by 18.4%. Two names keep the
choice at index creation and both kernels branch-free. Selection lives inside
the per-ISA Choose_* functions, which already take dim, so no header changes
and no new exported names.
Verified against the narrow-only version: the narrow wrappers are unchanged at
33, 40 and 47 instructions for IP at residual 0, 32 and 33, and 37, 43 and 50
for Cosine, with zero calls in the 33..63 band and the out-of-line Imp count
unchanged at 7. The object file grows 514,792 to 656,120 for the extra 192
instantiations, which is the intended trade.
The SQ8_SQ8 kernels reuse this helper, on main as much as here. They now take
the result as uint64_t and pass Wide as false: SQ8 is capped at the same
66,051 independently, by its uint32 q_sum_squares metadata slot. Previously
the AVX-512 one assigned it to int, which wrapped past 33,025, and the three
ARM ones to float, which lost exactness past 258. Note the SQ8_SQ8 choosers
have no dimension guard, unlike the uint8 ones, so the fence belongs with SQ8
index creation in #1007; on main nothing constructs an SQ8 index.
Split out of #1011 because none of this depends on the SQ8 metadata contract
that PR is changing, while all of it affects code reachable today. #1011
depends on this, through the helper above.
Tests walk all four boundaries, at each bound and one past it, through the
dispatched function so selection is covered as well as arithmetic. All-255
against all-0 is the worst case and keeps every expectation an exact integer.
The top boundary is also asserted by pointer identity, since on a host without
a uint8 SIMD tier the value comparisons would pass either way, and the narrow
and widened dispatch results are asserted to differ so the selection is
exercised rather than assumed. The existing UINT8 suites stop at dimension
128, which is why all of this went unseen; being SIMD-versus-scalar
comparisons they would also have agreed with each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with delete
and stored the trailing norms through unaligned float casts, so measurements
taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
The uint8 kernels accumulate products or squared differences of bytes, so the
total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold that,
all of them on the plain int8/uint8 index paths that ship today.
* IP.cpp / L2.cpp: ret_t for a 1-byte element type was int, so the scalar
UINT8_InnerProduct and UINT8_L2Sqr executed signed-overflow UB from
dimension 33,026, while the comment claimed support to 2^16. The
conditional was also dead: only int8_t and uint8_t instantiate these, both
1 byte, so it always selected int. ret_t is now 64-bit for every element
type, which also covers int8 at dimension 131,072. Kept signed so the
"1 - ip" in the wrappers stays signed arithmetic and cannot underflow. The
L2 comment still carried the old byte-counting rationale and is fixed.
* UINT8_InnerProductImp returned float on NEON and SVE, accumulating exactly
in integer lanes and then discarding it, exact only to dimension 258 since
2^24 / 65025 = 258.
* AVX-512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a signed
int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned horizontal
reduce back into a signed int, so the distance went negative from the same
dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were already unsigned.
The accumulation was never the problem. The SIMD adds wrap modulo 2^32 and are
bit-exact, so the bit pattern was already correct; the top bit was being read
as a sign. An unsigned 32-bit reduce therefore costs nothing over the original
and is exact through dimension 66,051 = floor(UINT32_MAX / 65,025), twice the
old signed limit of 33,025. Verified: the AVX-512 object is byte-for-byte the
same size as before at 514,792, with the same 33 and 40 instructions for
residual 0 and 32.
Above 66,051 the choosers hand back the scalar kernel, which after the ret_t
change is exact to roughly dimension 2.8e14. One comparison at index creation,
reusing the "if (dim < 32) return ret_dist_func" idiom the choosers already
had, and it leaves every kernel untouched.
Three alternatives were tried and rejected, each on evidence:
* Widening the horizontal reduce unconditionally. Measured on an Ice Lake-SP
Xeon it costs 4 extra uops in the epilogue: +20% at dimension 32, +8-11%
across 55-200, +4-5% at 900-1024, on byte-identical loop code.
* A runtime branch selecting the width per call. Measured at +0.4 to 0.6 ns
per call, +15% at dimension 32, and it cost 31 of the 65 Cosine wrappers
their inlining, growing .text by 18.4%.
* Compile-time selection between two named wrappers, extending SIMD to a
second bound of 4 * 66,051. This one is free on the narrow path, verified:
the narrow wrappers stayed byte-identical and the out-of-line count
unchanged. It was rejected for correctness, not cost. That bound assumes
products spread evenly across the four uint32 lanes after NEON's 32-bit
vaddq_u32 merge, and the even case already lands within 1,020 of
UINT32_MAX, while the masked residual load can add up to 16 products, or
1,040,400, into specific lanes. So lanes wrap before the widened reduce
sees them, and a correct bound would have to be derived per kernel from its
accumulator count and residual distribution. The narrow reduce needs none
of that: its bound is on the horizontal total, which does not depend on how
products land in lanes.
Recorded for whoever revisits this: on ARM the widening reduce is free
instruction-for-instruction. Cross-compiling with
clang++ --target=aarch64-linux-gnu -O2 emits addv/fmov w/ucvtf against
uaddlv/fmov x/ucvtf, three instructions either way. So the obstacle to a wider
band is the lane bound, not the reduce.
Nothing comparable supports that range regardless. Lucene caps its
scalar-quantized format at 1,024 dimensions and Elasticsearch caps dense
vectors at 4,096, both keeping a 32-bit accumulator safe by contract. Faiss's
QT_8bit_direct accumulates full-range bytes into 32-bit lanes with no widening
and carries the same theoretical limit. Qdrant quantizes to 0..127, lowering
the per-element cap to 16,129, and its raw uint8 metric still sums into i32.
The SQ8_SQ8 kernels reuse this helper, on main as much as here, so they now
take its result as uint32_t. Previously the AVX-512 one assigned it to int,
which wrapped past 33,025 once the helper stopped returning int, and the three
ARM ones to float, which lost exactness past 258. Note the SQ8_SQ8 choosers
have no dimension guard, unlike the uint8 ones, so the fence belongs with SQ8
index creation in #1007; on main nothing constructs an SQ8 index.
Split out of #1011 because none of this depends on the SQ8 metadata contract
that PR is changing, while all of it affects code reachable today. #1011
depends on this, through the helper above.
The regressions use all-255 bytes, the worst case, which makes the expected
value an exact integer, at dimensions 33,026 and 40,000 for the SIMD path and
66,052 for the fallback. The fallback test asserts the returned function
pointer, not just the distance: on a host with no uint8 SIMD tier the value
comparison would pass either way, but the pointer identity would not. The
existing UINT8 suites stop at dimension 128, which is why all of this went
unseen; being SIMD-versus-scalar comparisons they would also have agreed with
each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with delete
and stored the trailing norms through unaligned float casts, so measurements
taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
The uint8 SIMD kernels accumulate products or squared differences of bytes, so
the total reaches 255 * 255 * dim = 65025 * dim. Three paths discarded or
wrapped it:
* UINT8_InnerProductImp returned float on NEON and SVE, accumulating exactly
in integer lanes and then throwing that away, exact only to dimension 258
since 2^24 / 65025 = 258.
* AVX-512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a signed
int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned horizontal
reduce back into a signed int, so the distance went negative from the same
dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were already unsigned.
The accumulation was never wrong. The SIMD adds wrap modulo 2^32 and are
bit-exact, so the bit pattern was already correct; the top bit was being read
as a sign. Reading it unsigned therefore costs nothing, and doubles the exact
range from dimension 33,025 to 66,051 = floor(UINT32_MAX / 65,025). Verified:
the AVX-512 object file is byte-for-byte the same size as before at 514,792,
with the same 33 and 40 instructions for residual 0 and 32.
spaces::MAX_EXACT_UINT8_SIMD_DIM records that bound. It is documentation, not
a fence: above it these kernels still wrap, as they do on main, only twice as
far out. Two things are deliberately not done here.
Routing past the bound to the scalar kernel would need that kernel's
accumulator widened first. Its ret_t is std::conditional_t<sizeof(int_elem_t)
== 1, int, long long>, which for uint8 is int and therefore executes
signed-overflow UB from dimension 33,026 itself, so it is not a safe fallback
as it stands. That is a defect on the scalar path rather than in the SIMD
reduces this change is about, and it is filed separately.
Widening the SIMD reduce past the bound was implemented and measured, then
dropped. Unconditional widening cost +20% at dimension 32 on an Ice Lake-SP
Xeon, +8-11% across 55-200, on byte-identical loop code. A runtime branch cost
+0.4 to 0.6 ns per call and lost 31 of the 65 Cosine wrappers their inlining,
growing .text 18.4%. Compile-time selection between two named wrappers was
genuinely free on the narrow path, but its second bound of 4 * 66,051 assumed
products spread evenly across NEON's four uint32 lanes after the 32-bit
vaddq_u32 merge: the even case already lands within 1,020 of UINT32_MAX, while
the masked residual load can add 1,040,400 into one lane, so lanes wrap before
the widened reduce sees them. A correct bound would have to be derived per
kernel from its accumulator count and residual distribution. The unsigned
reduce needs none of that, because its bound is on the horizontal total, which
does not depend on how products land in lanes.
For whoever revisits it: on ARM the widening reduce is free
instruction-for-instruction. clang++ --target=aarch64-linux-gnu -O2 emits
addv/fmov w/ucvtf against uaddlv/fmov x/ucvtf, three instructions either way.
The obstacle is the lane bound, not the reduce.
Nothing comparable supports that range anyway. Lucene caps its
scalar-quantized format at 1,024 dimensions and Elasticsearch caps dense
vectors at 4,096. Faiss's QT_8bit_direct accumulates full-range bytes into
32-bit lanes with no widening and carries the same limit. Qdrant quantizes to
0..127 and its raw uint8 metric still sums into i32.
The SQ8_SQ8 kernels reuse the shared helper, on main as much as here, so they
now take its result as uint32_t. Previously the AVX-512 one assigned it to int
once the helper stopped returning int, which wrapped past 33,025, and the three
ARM ones to float, which lost exactness past 258.
Split out of #1011 because none of this depends on the SQ8 metadata contract
that PR is changing, while all of it affects code reachable today. #1011
depends on this, through the helper above.
The regression asserts the dispatched SIMD path at dimensions 33,026, 40,000
and 66,051, using all-255 against all-0 so the expected value is an exact
integer, and checks the distance is positive since going negative is the
symptom a user would have seen. It deliberately does not call the scalar
kernels, which remain undefined above 33,025. The existing UINT8 suites stop at
dimension 128, which is why this went unseen; being SIMD-versus-scalar
comparisons they would also have agreed with each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with delete
and stored the trailing norms through unaligned float casts, so measurements
taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
The uint8 kernels accumulate products or squared differences of bytes, so the
total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold that,
all of them on the plain int8/uint8 index paths that ship today.
* IP.cpp / L2.cpp: ret_t for a 1-byte element type was int, so the scalar
UINT8_InnerProduct and UINT8_L2Sqr executed signed-overflow UB from
dimension 33,026, while the comment claimed support to 2^16. The
conditional was also dead: only int8_t and uint8_t instantiate these, both
1 byte, so it always selected int. ret_t is now 64-bit for every element
type, which also covers int8 at dimension 131,072. Kept signed so the
"1 - ip" in the wrappers stays signed arithmetic and cannot underflow. The
L2 comment still carried the old byte-counting rationale and is fixed.
* UINT8_InnerProductImp returned float on NEON and SVE, accumulating exactly
in integer lanes and then discarding it, exact only to dimension 258 since
2^24 / 65025 = 258.
* AVX-512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a signed
int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned horizontal
reduce back into a signed int, so the distance went negative from the same
dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were already unsigned.
The accumulation was never the problem. The SIMD adds wrap modulo 2^32 and are
bit-exact, so the bit pattern was already correct; the top bit was being read
as a sign. An unsigned 32-bit reduce therefore costs nothing over the original
and is exact through dimension 66,051 = floor(UINT32_MAX / 65,025), twice the
old signed limit of 33,025. Verified: the AVX-512 object is byte-for-byte the
same size as before at 514,792, with the same 33 and 40 instructions for
residual 0 and 32.
Above 66,051 the choosers hand back the scalar kernel, which after the ret_t
change is exact to roughly dimension 2.8e14. One comparison at index creation,
reusing the "if (dim < 32) return ret_dist_func" idiom the choosers already
had, and it leaves every kernel untouched.
Three alternatives were tried and rejected, each on evidence:
* Widening the horizontal reduce unconditionally. Measured on an Ice Lake-SP
Xeon it costs 4 extra uops in the epilogue: +20% at dimension 32, +8-11%
across 55-200, +4-5% at 900-1024, on byte-identical loop code.
* A runtime branch selecting the width per call. Measured at +0.4 to 0.6 ns
per call, +15% at dimension 32, and it cost 31 of the 65 Cosine wrappers
their inlining, growing .text by 18.4%.
* Compile-time selection between two named wrappers, extending SIMD to a
second bound of 4 * 66,051. This one is free on the narrow path, verified:
the narrow wrappers stayed byte-identical and the out-of-line count
unchanged. It was rejected for correctness, not cost. That bound assumes
products spread evenly across the four uint32 lanes after NEON's 32-bit
vaddq_u32 merge, and the even case already lands within 1,020 of
UINT32_MAX, while the masked residual load can add up to 16 products, or
1,040,400, into specific lanes. So lanes wrap before the widened reduce
sees them, and a correct bound would have to be derived per kernel from its
accumulator count and residual distribution. The narrow reduce needs none
of that: its bound is on the horizontal total, which does not depend on how
products land in lanes.
Recorded for whoever revisits this: on ARM the widening reduce is free
instruction-for-instruction. Cross-compiling with
clang++ --target=aarch64-linux-gnu -O2 emits addv/fmov w/ucvtf against
uaddlv/fmov x/ucvtf, three instructions either way. So the obstacle to a wider
band is the lane bound, not the reduce.
Nothing comparable supports that range regardless. Lucene caps its
scalar-quantized format at 1,024 dimensions and Elasticsearch caps dense
vectors at 4,096, both keeping a 32-bit accumulator safe by contract. Faiss's
QT_8bit_direct accumulates full-range bytes into 32-bit lanes with no widening
and carries the same theoretical limit. Qdrant quantizes to 0..127, lowering
the per-element cap to 16,129, and its raw uint8 metric still sums into i32.
The SQ8_SQ8 kernels reuse this helper, on main as much as here, so they now
take its result as uint32_t. Previously the AVX-512 one assigned it to int,
which wrapped past 33,025 once the helper stopped returning int, and the three
ARM ones to float, which lost exactness past 258. Note the SQ8_SQ8 choosers
have no dimension guard, unlike the uint8 ones, so the fence belongs with SQ8
index creation in #1007; on main nothing constructs an SQ8 index.
Split out of #1011 because none of this depends on the SQ8 metadata contract
that PR is changing, while all of it affects code reachable today. #1011
depends on this, through the helper above.
The regressions use all-255 bytes, the worst case, which makes the expected
value an exact integer, at dimensions 33,026 and 40,000 for the SIMD path and
66,052 for the fallback. The fallback test asserts the returned function
pointer, not just the distance: on a host with no uint8 SIMD tier the value
comparison would pass either way, but the pointer identity would not. The
existing UINT8 suites stop at dimension 128, which is why all of this went
unseen; being SIMD-versus-scalar comparisons they would also have agreed with
each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with delete
and stored the trailing norms through unaligned float casts, so measurements
taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 17, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
The uint8 kernels accumulate products or squared differences of bytes, so the
total reaches 255 * 255 * dim = 65025 * dim. Four paths could not hold that,
all of them on the plain int8/uint8 index paths that ship today.
* IP.cpp / L2.cpp: ret_t for a 1-byte element type was int, so the scalar
UINT8_InnerProduct and UINT8_L2Sqr executed signed-overflow UB from
dimension 33,026, while the comment claimed support to 2^16. The
conditional was also dead: only int8_t and uint8_t instantiate these, both
1 byte, so it always selected int. ret_t is now 64-bit for every element
type, which also covers int8 at dimension 131,072. Kept signed so the
"1 - ip" in the wrappers stays signed arithmetic and cannot underflow. The
L2 comment still carried the old byte-counting rationale and is fixed.
* UINT8_InnerProductImp returned float on NEON and SVE, accumulating exactly
in integer lanes and then discarding it, exact only to dimension 258 since
2^24 / 65025 = 258.
* AVX-512 reduced 16 int32 lanes with _mm512_reduce_add_epi32 into a signed
int, wrapping from dimension 33,026.
* L2_AVX512F_BW_VL_VNNI_UINT8 and L2_NEON_UINT8 read an unsigned horizontal
reduce back into a signed int, so the distance went negative from the same
dimension. L2_NEON_DOTPROD_UINT8 and L2_SVE_UINT8 were already unsigned.
The accumulation was never the problem. The SIMD adds wrap modulo 2^32 and are
bit-exact, so the bit pattern was already correct; the top bit was being read
as a sign. An unsigned 32-bit reduce therefore costs nothing over the original
and is exact through dimension 66,051 = floor(UINT32_MAX / 65,025), twice the
old signed limit of 33,025. Verified: the AVX-512 object is byte-for-byte the
same size as before at 514,792, with the same 33 and 40 instructions for
residual 0 and 32.
Above 66,051 the choosers hand back the scalar kernel, which after the ret_t
change is exact to roughly dimension 2.8e14. One comparison at index creation,
reusing the "if (dim < 32) return ret_dist_func" idiom the choosers already
had, and it leaves every kernel untouched.
Three alternatives were tried and rejected, each on evidence:
* Widening the horizontal reduce unconditionally. Measured on an Ice Lake-SP
Xeon it costs 4 extra uops in the epilogue: +20% at dimension 32, +8-11%
across 55-200, +4-5% at 900-1024, on byte-identical loop code.
* A runtime branch selecting the width per call. Measured at +0.4 to 0.6 ns
per call, +15% at dimension 32, and it cost 31 of the 65 Cosine wrappers
their inlining, growing .text by 18.4%.
* Compile-time selection between two named wrappers, extending SIMD to a
second bound of 4 * 66,051. This one is free on the narrow path, verified:
the narrow wrappers stayed byte-identical and the out-of-line count
unchanged. It was rejected for correctness, not cost. That bound assumes
products spread evenly across the four uint32 lanes after NEON's 32-bit
vaddq_u32 merge, and the even case already lands within 1,020 of
UINT32_MAX, while the masked residual load can add up to 16 products, or
1,040,400, into specific lanes. So lanes wrap before the widened reduce
sees them, and a correct bound would have to be derived per kernel from its
accumulator count and residual distribution. The narrow reduce needs none
of that: its bound is on the horizontal total, which does not depend on how
products land in lanes.
Recorded for whoever revisits this: on ARM the widening reduce is free
instruction-for-instruction. Cross-compiling with
clang++ --target=aarch64-linux-gnu -O2 emits addv/fmov w/ucvtf against
uaddlv/fmov x/ucvtf, three instructions either way. So the obstacle to a wider
band is the lane bound, not the reduce.
Nothing comparable supports that range regardless. Lucene caps its
scalar-quantized format at 1,024 dimensions and Elasticsearch caps dense
vectors at 4,096, both keeping a 32-bit accumulator safe by contract. Faiss's
QT_8bit_direct accumulates full-range bytes into 32-bit lanes with no widening
and carries the same theoretical limit. Qdrant quantizes to 0..127, lowering
the per-element cap to 16,129, and its raw uint8 metric still sums into i32.
The SQ8_SQ8 kernels reuse this helper, on main as much as here, so they now
take its result as uint32_t. Previously the AVX-512 one assigned it to int,
which wrapped past 33,025 once the helper stopped returning int, and the three
ARM ones to float, which lost exactness past 258. Note the SQ8_SQ8 choosers
have no dimension guard, unlike the uint8 ones, so the fence belongs with SQ8
index creation in #1007; on main nothing constructs an SQ8 index.
Split out of #1011 because none of this depends on the SQ8 metadata contract
that PR is changing, while all of it affects code reachable today. #1011
depends on this, through the helper above.
The regressions use all-255 bytes, the worst case, which makes the expected
value an exact integer, at dimensions 33,026 and 40,000 for the SIMD path and
66,052 for the fallback. The fallback test asserts the returned function
pointer, not just the distance: on a host with no uint8 SIMD tier the value
comparison would pass either way, but the pointer identity would not. The
existing UINT8 suites stop at dimension 128, which is why all of this went
unseen; being SIMD-versus-scalar comparisons they would also have agreed with
each other wherever both wrapped.
Also fixes the uint8 spaces benchmark fixture, which paired new[] with delete
and stored the trailing norms through unaligned float casts, so measurements
taken from it can be trusted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 18, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 19, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 19, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 19, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 19, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 19, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
added a commit
that referenced
this pull request
Aug 19, 2026
…nput
QuantPreprocessor::quantize could execute undefined behaviour for input the
API accepts. Three paths, all ending at the same conversion to a byte, and
all reachable with entirely finite components.
The scale was derived in FP32, so for [-FLT_MAX, +FLT_MAX] the range
overflowed: max - min became inf, delta inf, inv_delta 0, and the
per-element product inf * 0 = NaN, whose conversion to an integer is
undefined. UBSan: "-nan is outside the range of representable values of
type 'unsigned char'". The existing diff == 0 guard covers equal values,
not overflow of the subtraction. (MOD-17528)
With WithNorm, centering is an FP32 subtraction, so finite input against a
finite mean reaches inf before any range is computed: FLT_MAX against mean
-FLT_MAX centers to 6.8e38. Widening the range does not help, because the
value is already lost upstream.
And delta is stored as FP32, so (float)(diff / 255) underflows to zero
while diff itself is nonzero, which leaves 1/delta inf and scales the
minimum element, numerator exactly zero, to 0 * inf.
find_min_max now guarantees that both endpoints are finite and ordered, and
everything else follows from that. The guarantee lives there rather than at
the call site because that is where it can be broken: the WithNorm branch
creates the inf itself, from two valid operands, and the plain branch passes
through whatever the input holds. Both endpoints get a two-sided clamp,
since for an all-+inf vector inf <= inf passes the order check and a
one-sided std::max would leave min at +inf. Clamping to the float range is
not merely defensive either: min is stored as an FP32 field, so a non-finite
endpoint could not be represented under any arithmetic.
With that established, quantize needs one delta comparison, and inv_delta
stays double. Not for precision, which needs only +/-0.5 in 255, but because
an FP32 reciprocal overflows for a subnormal delta: [0, 7e-37] gives delta
2.7e-39, whose FP64 reciprocal is finite and correctly maps the top element
to 255.
The per-element bound and rounding are written by hand. That replaces
std::round, an out-of-line libm call at this translation unit's baseline
that ran once per element: bounding first makes adding 0.5 and truncating
equivalent for non-negative values. Measured at -O3 it is 8 instructions
against 9 for std::clamp plus std::round's call.
Scope, stated precisely, because it is narrower than it might look:
* The byte conversion is defined for all finite components. That is the
goal and it is met.
* The stored min and delta are finite with delta positive. The *sums* are
not covered: they are accumulated in FP32 over the input values, so
[-FLT_MAX, +FLT_MAX] stores sum_squares as inf even though this
function's own arithmetic is now well defined. Separate problem,
separate change.
* Non-finite components are unsupported, and nothing here is tested
against them. std::minmax_element requires its comparison to induce a
strict weak ordering, and floating-point < is not one once a NaN is
present: incomparability must be transitive, yet 1.0 is incomparable
with NaN and NaN with 2.0 while 1.0 and 2.0 are comparable. Violating
that precondition is undefined behaviour inside the algorithm, before
any range exists, so no assertion about the outcome would be portable,
including a weak one about metadata finiteness. UBSan reporting nothing
does not establish otherwise. Rejecting non-finite components at the
public ingestion boundary is the actual fix and belongs in its own
change; nothing in VecSim does it today.
The order check on the endpoints stays, as an invariant guard rather than a
NaN policy: everything downstream is written assuming min <= max, and
asserting that once per vector costs less than reasoning about whether it
can be violated.
This matches how comparable systems handle it. Lucene validates vector
components and throws on NaN or infinity, and Elasticsearch rejects NaN,
infinity and magnitudes that overflow before delegating to Lucene. Faiss
guards only an exactly zero range and assumes finite input otherwise, with
the same float-to-integer concern at its final cast. Qdrant gets defined
bytes from Rust's saturating cast, which C++ does not have, and can still
store an unusable scale.
Tests: the MOD-17528 reproduction, the WithNorm centering case, and a
table-driven matrix over finite input only: constant vectors positive
negative and zero, a single element, a subnormal but representable delta, a
range that underflows and collapses, and the full FP32 range. Expected bytes
and scale metadata are asserted rather than a range check, which is vacuous
for uint8_t, and every expectation was derived by simulating the pipeline
rather than predicted. The parameterized test is named for what it checks,
ScaleMetadataAndBytesAreAsExpected, since the sums are out of scope.
Metadata meaning is unchanged: the sums are still FP32 over the input
values. Making them exact integer sums over the quantized bytes is a
storage-contract change that has to move together with every kernel that
reads them, and stays in #1011.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dor-forer
force-pushed
the
dor-forer-MOD-17526-sq8-exact-metadata
branch
from
August 19, 2026 19:07
729f621 to
810e973
Compare
dor-forer
changed the title
The stored sum and sum_squares described the input values x[i], but every kernel
term is written in terms of the reconstruction x_r[i] = min + delta * a[i]. The
two differ by the quantization error, about 0.4% of ||x||^2, which is larger than
the distance between two similar vectors. So L2 came back wrong, and negative:
for two near-duplicate vectors at dimension 128 the reconstruction distance is
2.93e-04 and the kernels returned -1.49e-01.
The existing algebra is already exact once the sums describe x_r:
IP = min1*sum2 + min2*sum1 - dim*min1*min2 + delta1*delta2*sum(a[i]*b[i])
= IP(x_r, y_r) iff sum = sum(x_r[i]) = dim*min + delta*sum(a[i])
L2 = sum_sq_x + sum_sq_y - 2*IP
= L2(x_r, y_r) iff sum_sq = sum(x_r[i]^2)
so quantize() now accumulates the quantized bytes as exact integers and derives
both sums from them in double before storing FP32:
sum = dim*min + delta*q_sum
sum_squares = dim*min^2 + 2*min*delta*q_sum + delta^2*q_sum_squares
No kernel changes. The blob layout, the slot count and the slot types are all
unchanged, and the metadata stays FP32 for every metric, so nothing downstream
has to know this happened. SUM has exactly one reader, the inner product algebra
above, which is what makes a metadata-only fix sufficient.
Note the query blob keeps sums over the input, because a query is not quantized:
the asymmetric distance is sum(x_r^2) + sum(y^2) - 2*sum(x_r*y). Storage and
query now describe different quantities, and three test assertions that compared
one against the other are updated. They only ever agreed because both used to be
the input sum, which is the same confusion this change fixes in the product code.
Tests: L2 must be non-negative and match a double-precision reference over the
reconstruction, on near-duplicate vectors across five dimensions and on both the
scalar and dispatched kernels. Near-duplicates are the case that exposes this,
since the true distance is small enough for the mismatch to dominate.
Self-distance is asserted near zero rather than exactly zero: the two sides of
sum_sq_x + sum_sq_y - 2*IP are computed by different routes and round
differently.
Left for follow-up, all pre-existing: FP32 cancellation for vectors sharing a
large offset, exact-zero self-distance, and the MAX_EXACT_DIM comment claiming a
bound the code does not have.
Verified on Graviton4: test_spaces 1549/1549 and test_components 51/51, both
suites unfiltered.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dor-forer
force-pushed
the
dor-forer-MOD-17526-sq8-exact-metadata
branch
from
August 20, 2026 07:50
ae7feb9 to
f3bf3d5
Compare
dor-forer
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ 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 f3bf3d5. Configure here.
The byte accumulators inside quantize() are local counters, not stored fields: the metadata slots remain four FP32 values and the blob layout is unchanged. But the squares counter was uint32_t, and each byte contributes up to 255^2, so the total passes UINT32_MAX just above dimension 66051. A wrapped counter feeds the double expansion that fills sum_squares, so the stored norm and every L2 distance built on it would be wrong rather than merely imprecise. It also fits the documented 2^16 limit by under 1%. The four chains and the combined total are now uint64_t, in the product code and in both test mirrors of the quantizer. The class documentation still defined x_sum and x_sum_squares as sums over the input values, which is what this branch changed. That prose is where the defect came from: it wrote the asymmetric L2 identity over x while the kernel's inner product term is over the reconstruction. Both are now written in terms of x_r, and the symmetric inner product note records that recovering the quantized sum from the stored sum is exact rather than approximate, which is what makes a metadata-only fix sufficient. Tests: The regression tolerance was 1e-3 * max(1.0, expected), which collapses to a flat 1e-3 because the distances asserted run from 7.7e-7 to 2.2e-3. At dimension 4 that is over a thousand times the value being pinned, so a kernel returning zero would pass, and the old metadata cleared the threshold by only 1.5x. The error floor is FP32 cancellation in sum_sq_x + sum_sq_y - 2*IP, which scales with the norms and not with the distance, so the bound is now 8 * FLT_EPSILON * (norm_x + norm_y). EXPECT_GE(got, 0.0f) asserted a property this change does not provide. The residual cancellation is about 1e-7 of the norm, and two identical blobs have a true distance of zero, so self distance lands just below it. The bound is now the noise floor rather than zero, and the limitation is stated where it is asserted. Added SQ8_FP32_L2_matches_reconstruction_against_float_query. The asymmetric path is how the defect stayed hidden: its tests run dimensions 1, 5, 7 and 15 against an absolute tolerance of 0.01, and 0.4% of the norm at dimension 5 is about 0.007, which fits underneath. The new test runs near duplicates out to dimension 512 against a double reference, on both the scalar and dispatched kernels. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the changes in the pull request
The stored
sumandsum_squaresdescribed the input valuesx[i], but every kernel term is written in terms of the reconstructionx_r[i] = min + delta * a[i]. The two differ by the quantization error, roughly 0.4% of||x||^2, which is larger than the distance between two similar vectors. So L2 came back wrong, and negative.Measured at dimension 128 on two near-duplicate vectors:
The existing algebra is already exact, once the sums describe
x_r:So
quantize()accumulates the quantized bytes as exact integers and derives both sums from them in double before storing FP32:sum = dim*min + delta*q_sum; sum_squares = dim*min*min + 2.0*min*delta*q_sum + delta*delta*q_sum_squares;No kernel changes. Blob layout, slot count and slot types are unchanged, and the metadata stays FP32 for every metric, so nothing downstream needs to know.
SUMhas exactly one reader, the inner product algebra above, which is what makes a metadata-only fix sufficient.Which issues this PR fixes
Main objects this PR modified
src/VecSim/spaces/computer/preprocessors.h: the derivation.tests/unit/unit_test_utils.handtests/utils/tests_utils.h: the two test mirrors of the quantizer.tests/unit/test_spaces.cpp,tests/unit/test_components.cpp.Mark if applicable
Storage and query now describe different quantities
A query is not quantized, so its metadata keeps sums over the input: the asymmetric distance is
sum(x_r^2) + sum(y^2) - 2*sum(x_r*y), which needssum(y^2)over the real query andsum(x_r^2)over the reconstruction. Three test assertions compared the query's sum against the storage reference's sum and are updated. They only ever agreed because both used to be the input sum, which is the same confusion this change fixes in the product code, encoded in the tests.Tests
SQ8_SQ8_L2_is_non_negative_and_matches_reconstructionasserts L2 is non-negative and matches a double-precision reference computed over the reconstruction, on near-duplicate vectors across dimensions 4 / 15 / 64 / 128 / 512, on both the scalar and dispatched kernels. Near-duplicates are the case that exposes this: the true distance is small enough for the 0.4% mismatch to dominate, which is why the old metadata produced a negative result rather than a slightly inaccurate one. The reference accumulates in double so the comparison is against the algebra, not against another FP32 implementation sharing the same rounding.SQ8_SQ8_L2_self_distance_is_near_zeroasserts near zero rather than exactly zero, with a tolerance scaled to||x||^2. The two sides ofsum_sq_x + sum_sq_y - 2*IPare computed by different routes and round differently, so exactness is not something this change provides and is not claimed.Verification
Graviton4, both suites unfiltered:
test_spaces1549/1549,test_components51/51.Considered and rejected
An earlier revision of this branch made the L2 metadata integer (
uint32Q_SUM/Q_SUM_SQUARES), added areconstructed_l2_sqrhelper regrouping the quadratic term soS1 + S2 - 2Qstays an exact integer, and rewrote 18 kernel files to combine in double. That buys two extra properties: exactly-zero self-distance, and protection against FP32 cancellation when two vectors share a large offset. It cost 24 files, +630/-277, a change in slot semantics per metric, and 1 to 4.8 ns per distance call.Those extra properties address the large-shared-offset case, not the defect above. This revision fixes the defect at zero per-call cost and without touching a kernel. The withdrawn work is recorded for follow-up: FP32 cancellation under a large shared offset, exactly-zero self-distance, and the
MAX_EXACT_DIMcomment describing a bound the code does not have.Note
Cursor Bugbot is generating a summary for commit d5b467e. Configure here.