diff --git a/.gitignore b/.gitignore index 8cdc92b..f3d9a46 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,5 @@ out/ *.ilk .vs/ .idea/ +__pycache__/ +*.pyc diff --git a/CHANGELOG.md b/CHANGELOG.md index 593d52d..ad5909a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,81 @@ per entry. Reconstructed from git history 2026-07-12. The format follows [Keep a Changelog](https://keepachangelog.com); this project versions by feature tier (minor = new capability, patch = fix), not strict SemVer of a public ABI. +## [3.3.0] — 2026-07-21 + +### Added +- **`PeekScratchArchive`** — reads a serialized scratch archive's header and channel + table out of a byte span and reports its geometry plus `archiveBytes`, exactly the + number of bytes a `Load` consumes, without allocating or reading the payload. It runs + the same header validation `Load` does (the two now share one validator, so they cannot + drift). A host that appends its own trailer after the archive can therefore locate and + validate that trailer *before* committing the load, rather than discovering a broken + trailer with the rows already replaced. + +### Changed +- **`ComputePrincipalComponents`'s `scratch` parameter is now `double*`** (was `float*`). + The covariance-apply accumulator now matches the mean's accumulation precision — a + float accumulator rounded once per row and degraded the power iteration with bank + size — and the caller-owned scratch buffer changed type to match. This is + source-breaking: every existing caller must change the parameter's declared type from + `float*` to `double*` **and** double the size of the buffer it allocates (`dims` + doubles, not `dims` floats). +- **`MeanNNCrossDeviceChannel`/`MaxNNCrossDeviceChannel` can now return `OutOfMemory`.** + Both now stage a pre-lift of the target's non-excluded sub-rows into the `Workspace` + argument (so each target's self-dot is computed once rather than once per source row, + see the self-dot hoist below); if that reservation fails, the call now returns + `OutOfMemory`, a status neither function could previously return. + +### Fixed +- **`ScratchBank::Load` now validates the retention region.** A retained row is by + construction the post-normalization row the quantizer consumed, so `Load` replays the + bake on each retained row and requires it to reproduce the stored row exactly — + allocation-free, mirroring the quantizer's own arithmetic. Previously a + fabricated-but-finite retained array loaded clean and handed `MeasureScratchRecall` an + invented reference to audit against, through the one API whose whole job is an honest + number. Non-finite retained values are rejected the same way (`BadFormat`). +- **Two kernel-selection rules made the segmented scan disagree with the whole-row + scan.** The float32 dispatchers `DotF32`/`L2F32` use AVX2+FMA only when the length they + are handed is a multiple of 8; `ResolveRowKernels`, which feeds the segmented scan and + `DecomposeRowScore`, wired the AVX2 kernels in directly with no such rule. Float32 + strides are multiples of 4, so at `paddedDims` 20, 36, 52, 100, 132 and similar the two + paths computed the same row with different accumulation widths and disagreed in the + last ulp. That falsified two published guarantees — that a degenerate one-segment query + equals the whole-row scan, and that decomposition contributions sum bit-exactly to the + scan's own score, "no second code path exists to drift". `ResolveRowKernels` now holds + the dispatchers, so the rule is applied once, to the length actually in hand — which + matters because the segmented scan calls the kernel per scan RANGE, and a bank whose + stride is a multiple of 8 can still present a range that is not. + **`Exactness::CrossDevice` was never affected** (int8 lengths are always multiples of + 16), so the cross-machine contract is unchanged, and no pinned golden moves: every one + of them sits at a width or quantization where the two paths already agreed. +- **`ScratchBank::Create` and `Grow` performed signed size arithmetic before bounding + their geometry.** `ArenaBytes` multiplies capacity by dims in signed `int64`; both + entry points accepted any positive `int32` pair, so a large valid-typed request + overflowed the arena computation — undefined behavior rather than a refused + allocation. Both now apply the format's own ceilings (`kMaxBankRows` rows, + `kMaxCrossDeviceDims` dims) before any size is computed, returning `InvalidArgument`. + The archive loader already applied these caps; direct construction did not, and + `Grow` bounded its request only against the current capacity. +- **Bank-inspection review follow-ups (lower-severity, from a whole-project pass).** + `graph.cpp`: `MutualFilter`/`ConnectedComponents` bound caller-supplied neighbour and + duplicate-group values to `[0, count)` before indexing the neighbour list and the + union-find scratch, so a malformed hand-built input degrades to a dropped edge rather + than an out-of-bounds access. `analytics.cpp`: the int8 off-grid channel guard from + `novelty.cpp` now also protects the channel-analytics legs (added once in the shared + `ChannelSubRange`), and the channel NN divergence pre-lifts each target's self-dot once + instead of once per source row. `pca.cpp`: `ProjectRowsOntoComponents` bounds + `componentCount` by `dims` like its sibling. Scale decode goes through + `detail::FloatBitsToDouble` on the remaining `compose.cpp`/`pca.cpp`/`analytics.cpp` + paths, so a subnormal scale decodes identically under any FTZ/DAZ mode. A comment + documents that `novelty.cpp`'s `sampleLimit` is a ceiling, not a down-sampler. +- **A Cosine channel bank with zero rows was unrepresentable.** `ValidateBank` required + the per-channel inverse sub-norm array whenever a Cosine bank carried channels, but + those norms are one per row — a bank with no rows requires none, and no scan reads the + array at zero rows. The rule now applies only when `count > 0`. The shape is not + degenerate: it is what a channel-carrying scratch bank graduates into once every row + has been removed, and rejecting it forced that graduation to drop its channels. + ## [3.2.1] — 2026-07-21 ### Fixed @@ -66,6 +141,26 @@ by feature tier (minor = new capability, patch = fix), not strict SemVer of a pu test suite) still carried 3.0.1 through both 3.1 releases; both now read 3.1.2. Found while closing an external review's release-identity finding. +## [3.1.1] — 2026-07-18 + +### Fixed +- **AVX2 float32 dot/L2 dropped a sub-8 segment remainder.** `DotF32Avx2`/`L2F32Avx2` (and + their scalar mirrors) accumulated only in whole 8-lane groups with no remainder; a + float32 segment/channel stride lies on the 4-float (16-byte) grid, so a range whose + length is congruent to 4 mod 8 had its trailing 4 elements dropped entirely — a + length-4 channel scored exactly 0 on AVX2 while SSE/portable/NEON scored it correctly. + **This changed numeric results for affected widths**: `ResolveRowKernels` wires the + AVX2 kernels in directly for the segmented and per-channel-cosine scan, so any + named-channel or segmented query over a sub-8-remainder length (e.g. a length-4 + channel) on AVX2 hardware returned a different, wrong score before this fix — a + consumer bisecting a score change across 3.1.0 to 3.1.2 should attribute the delta at + those widths to this fix, not to 3.1.0's `Relabel`. Whole-row scans were unaffected + (`DotF32`/`L2F32` route non-multiple-of-8 `paddedDims` to the SSE path). The 4-element + remainder is now added to both the intrinsics and their scalar mirrors, bit-identically. +- CI now requires the AVX2 path on the Linux job (previously accepted SSE or AVX2), so a + runner without AVX2 hardware cannot silently skip the new sub-8 remainder coverage; + Windows keeps the looser SSE-or-AVX2 check as the general x86 path. + ## [3.1.0] — 2026-07-17 ### Added @@ -89,6 +184,23 @@ by feature tier (minor = new capability, patch = fix), not strict SemVer of a pu (~5.5× cheaper), ~2.1× on float32; promote/demote adds/frees a `capacity × channelCount × 4` sub-norm arena (~3.0 MB at 100k rows / 8 channels). +## [3.0.1] — 2026-07-13 + +### Fixed +- **`version.h` still read 2.5.0 at the v3.0 tag.** Corrected to 3.0.0, with a coherence + test added so a future release cannot ship the same drift. +- **`MeanNNCrossDeviceChannel`/`MaxNNCrossDeviceChannel` rejected a zero-energy Cosine + source and skipped zero-energy targets**, contradicting the documented "a degenerate + member floors to a defined 0 in a reduction" contract already honored by + `CentroidDistanceCrossDeviceChannel`/`SpreadCrossDeviceChannel`. Both guards are + removed so a degenerate channel member floors to 0 through the shared pair score, + consistent across all four channel operators. +- **`MeasureRecallLockedChannel` aborted the whole per-channel recall audit** when a + sampled row's channel sub-vector was zero-energy — a valid row whose per-channel + self-query is `ZeroNormQuery`. It now excludes that sample, refills the reservoir from + the remaining rows, and reports the sample count actually scored rather than the + pre-set target. + ## [3.0.0] — 2026-07-12 ### Added diff --git a/docs/API.md b/docs/API.md index 892199f..b8fe014 100644 --- a/docs/API.md +++ b/docs/API.md @@ -413,6 +413,9 @@ payloads carry no per-row sub-norm). Scratch sizing matches the whole-vector operators. A channel outside `[0, channelCount)`, or a bank with no channel table, is `InvalidArgument`; a degenerate zero-sub-norm channel member floors to a defined `0` in a reduction (a single per-channel query on a zero-norm sub-vector is still `ZeroNormQuery`). +`MeanNNCrossDeviceChannel`/`MaxNNCrossDeviceChannel` additionally pre-lift the target's +non-excluded sub-rows into `ws` before scoring, so both can also return `OutOfMemory` if +that reservation fails (v3.3). "This mind's identity is drifting but its appearance is stable" is a `CentroidDistanceCross DeviceChannel` over the identity channel versus the appearance channel across checkpoints. Determinism per channel: [DETERMINISM.md §2e](DETERMINISM.md). @@ -422,7 +425,7 @@ Determinism per channel: [DETERMINISM.md §2e](DETERMINISM.md). ```cpp Status ComputePrincipalComponents(const BankView&, int32_t componentCount, int32_t iterationsPerComponent, float* outMean, - float* outComponents, float* scratch); + float* outComponents, double* scratch); // dims doubles Status ProjectRowsOntoComponents (const BankView&, const float* mean, const float* components, int32_t componentCount, float* outCoords); @@ -605,6 +608,13 @@ struct ScratchRecallReport { // v2.3 // mathematically valid, statistically uninformative }; struct ScratchArchive { bool(*write)(void*, const void*, size_t); bool(*read)(void*, void*, size_t); void* user; }; +struct ScratchArchiveInfo { // v3.3.0 + int32_t capacity, count, dims, paddedDims; + Metric metric; Quantization quant; bool retainFloats; + int32_t channelCount; ChannelInfo channels[kMaxChannels]; // 0 / unused for single-space + int64_t archiveBytes; // exactly what a Load consumes +}; +Status PeekScratchArchive(const void* data, int64_t bytes, ScratchArchiveInfo* outInfo); // v3.3.0 ``` A snapshot IS a `BankView` — every query entry point works on it unchanged. @@ -620,6 +630,26 @@ allocation: everything lives in the arena from `Create` (retention included — `ArenaBytes` simply grows). Archive format: [FORMAT.md](FORMAT.md) section 3; concurrency guarantees: [DETERMINISM.md](DETERMINISM.md) section 2b. +**Geometry ceilings (v3.3.0):** every `Create` overload and `Grow` bound +`capacity` at `kMaxBankRows` and `dims` at `kMaxCrossDeviceDims` *before* the +arena size is computed, returning `InvalidArgument` above either. The arena math +multiplies capacity by dims in signed `int64`, so an unbounded `int32` pair is +overflow rather than a refused allocation. These are the immutable format's own +caps, already applied by the archive loader — a geometry `Create` refuses is one +no archive could have carried. + +**Peeking an archive (v3.3.0):** `PeekScratchArchive` reads a serialized +archive's header (and channel table) out of a contiguous byte span and reports +its geometry plus `archiveBytes` — exactly the number of bytes a `Load` will +consume — without allocating, reading the payload, or touching a bank. It +applies the same header rules `Load` does, so whatever a peek rejects a load +rejects, and a span shorter than the archive it declares is `BadFormat`; a +*longer* span is not, because whatever follows belongs to the caller. That is +the point: a host appending its own trailer after the archive (channel names, +say) uses `archiveBytes` to find and validate that trailer *before* committing +the load, instead of discovering a broken trailer with the rows already +replaced. Reading the header twice is free; unwinding a load is not. + **Recall audit (v2.3):** the import-time recall-honesty pattern, extended to the mutable half. `Create(..., retainFloats = true)` — opt-in, never the default — retains the post-normalization float row the quantizer consumed (on Cosine diff --git a/include/superfaiss/analytics.h b/include/superfaiss/analytics.h index 40bd034..7040bb0 100644 --- a/include/superfaiss/analytics.h +++ b/include/superfaiss/analytics.h @@ -109,8 +109,13 @@ Status SpreadCrossDevice( // inherits it once Tier 1 put the channel table on the snapshot. A bank with no channel // table, or `channel` outside [0, channelCount), is InvalidArgument; a zero-sub-norm // channel member floors to a defined 0 in a reduction, while a single -// per-channel query on a zero-norm sub-vector still rejects (ZeroNormQuery). The scratch +// per-channel query on a zero-norm sub-vector still rejects (ZeroNormQuery). The // scratch-buffer sizing matches the whole-vector operators. +// +// MeanNNCrossDeviceChannel/MaxNNCrossDeviceChannel additionally stage a pre-lift of the +// target's non-excluded sub-rows in `ws` (each target's self-dot is computed once, not +// once per source row), so both can return OutOfMemory if that reservation fails, in +// addition to the InvalidArgument cases above. Status CentroidDistanceCrossDeviceChannel( const BankView& bankA, const int32_t* rowIndicesA, int32_t rowCountA, const int32_t* weightsA, const uint32_t* excludeBitsA, diff --git a/include/superfaiss/pca.h b/include/superfaiss/pca.h index 9ab8386..9d482c2 100644 --- a/include/superfaiss/pca.h +++ b/include/superfaiss/pca.h @@ -15,20 +15,24 @@ namespace superfaiss // Buffers (caller-provided): // outMean dims floats — the bank mean (int8 rows dequantized). // outComponents componentCount x dims floats — unit-norm, mutually orthogonal. -// scratch dims floats. -// A degenerate direction (zero variance after deflation) yields a zero component -// vector rather than an error; callers render fewer axes. +// scratch dims doubles — the covariance-apply accumulator. Double so it +// matches the mean's accumulation precision; a float accumulator +// rounds once per row and degrades the power iteration with bank size. +// componentCount must be in [1, dims]. A degenerate direction (zero variance after +// deflation) yields a zero component vector rather than an error; callers render fewer axes. Status ComputePrincipalComponents( const BankView& bank, int32_t componentCount, int32_t iterationsPerComponent, float* outMean, float* outComponents, - float* scratch); + double* scratch); // Projects every row onto the components: outCoords is count x componentCount, // row-major — coordinate j of row r is (x_r - mean) . component_j. Rows are -// dequantized exactly as the kernels see them. +// dequantized exactly as the kernels see them. componentCount must be in [1, dims]: +// each of the componentCount vectors in `components` is a dims-length row, so an +// over-large count would read past the buffer (matches ComputePrincipalComponents). Status ProjectRowsOntoComponents( const BankView& bank, const float* mean, diff --git a/include/superfaiss/scratch.h b/include/superfaiss/scratch.h index a37aa63..66ca064 100644 --- a/include/superfaiss/scratch.h +++ b/include/superfaiss/scratch.h @@ -59,6 +59,37 @@ struct ScratchRecallReport bool informative = false; // false when liveRows < kRecallInformativeRows }; +// What a serialized scratch archive declares about itself, read from its header +// without loading it (PeekScratchArchive below). +struct ScratchArchiveInfo +{ + int32_t capacity = 0; + int32_t count = 0; + int32_t dims = 0; + int32_t paddedDims = 0; + Metric metric = Metric::Dot; + Quantization quant = Quantization::Float32; + bool retainFloats = false; + // The archive's channel table, or count 0 for a single-space archive. + int32_t channelCount = 0; + ChannelInfo channels[kMaxChannels] = {}; + // Exactly the number of bytes ScratchBank::Load consumes for this archive. + int64_t archiveBytes = 0; +}; + +// Reads a scratch archive's header (and channel table) from a contiguous byte span and +// reports its geometry plus the exact byte length a Load will consume — without +// allocating an arena, reading the payload, or touching any bank. Applies the same +// header rules Load does, so a header Load would reject is rejected here with the same +// status; a span shorter than the archive it declares is BadFormat. +// +// This exists for hosts that append their own trailer after the archive (the plugin's +// channel-name frame is one): archiveBytes locates that trailer so it can be validated +// BEFORE anything is committed. Without it a host must load first and discover a broken +// trailer with the rows already replaced, which is degrade-then-notice rather than +// reject-over-degrade. Reading the header twice is free; unwinding a load is not. +Status PeekScratchArchive(const void* data, int64_t bytes, ScratchArchiveInfo* outInfo); + class ScratchBank { public: @@ -78,6 +109,13 @@ class ScratchBank // Allocates the arena (rows + scales + tombstones + staging) in ONE allocation // through the seam — zero steady-state allocation after Create. Capacity is the // caller's memory budget made explicit. Not valid to call on a created bank. + // + // Geometry ceilings (every Create overload, and Grow): capacity <= kMaxBankRows and + // dims <= kMaxCrossDeviceDims, enforced BEFORE the arena size is computed. The arena + // math multiplies capacity by dims in signed int64, so an unbounded pair is overflow + // — undefined behavior rather than a refused allocation. Over-cap geometry is + // InvalidArgument. These are the immutable format's own caps, so a bank the + // constructor refuses is one no archive could carry either. Status Create( int32_t capacity, int32_t dims, diff --git a/include/superfaiss/version.h b/include/superfaiss/version.h index eb47d97..0f25cdf 100644 --- a/include/superfaiss/version.h +++ b/include/superfaiss/version.h @@ -3,8 +3,8 @@ #include #define SUPERFAISS_VERSION_MAJOR 3 -#define SUPERFAISS_VERSION_MINOR 2 -#define SUPERFAISS_VERSION_PATCH 1 +#define SUPERFAISS_VERSION_MINOR 3 +#define SUPERFAISS_VERSION_PATCH 0 namespace superfaiss { diff --git a/src/analytics.cpp b/src/analytics.cpp index 95159a5..6b34656 100644 --- a/src/analytics.cpp +++ b/src/analytics.cpp @@ -398,8 +398,25 @@ inline bool ChannelSubRange(const BankView& bank, int32_t channel, int32_t* outO { return false; } - *outOffset = bank.channels[channel].offset; - *outLength = bank.channels[channel].length; + const int32_t offset = bank.channels[channel].offset; + const int32_t length = bank.channels[channel].length; + // Every channel leg below scores this sub-range through detail::DotI8I8, whose SIMD + // paths assume a length on the int8 grid (kAlignment / ElementSize(Int8) == 16) and + // carry no scalar remainder tail — an off-grid range would read SIMD-padded slop. A + // validated bank's channel table is grid-aligned by construction (validate.cpp), so + // this never fires on production data; it closes the gap for a hand-built BankView, + // exactly as novelty.cpp does for its own DotI8I8 channel path. Resolved here, at the + // single point all three legs read the range, rather than copied into each. + if (bank.quant == Quantization::Int8) + { + const int32_t grid = kAlignment / ElementSize(Quantization::Int8); + if (offset % grid != 0 || length % grid != 0) + { + return false; + } + } + *outOffset = offset; + *outLength = length; return true; } @@ -453,12 +470,12 @@ Status MakeChannelCentroid(const BankView& bank, const int32_t* rowIndices, int3 Status NNDivergenceChannel( const BankView& source, const uint32_t* sourceExcludeBits, const BankView& target, const uint32_t* targetExcludeBits, int32_t channel, Reduce reduce, - XdQuery* queryScratch, float* outValue) + XdQuery* queryScratch, Workspace& ws, float* outValue) { - // Unlike the whole-vector NNDivergence (which routes through QueryXdBatch), the channel - // path scores each nearest in place, so it needs only the source-lift buffer - // (queryScratch) — the Hit/count/Workspace scratch the public signature carries for - // parity with the whole-vector twin is unused here and not required non-null. + // The channel path scores each nearest in place rather than through QueryXdBatch, so it + // needs the source-lift buffer (queryScratch) plus the Workspace, which stages the + // target lift (below). The Hit/count scratch the public signature carries for parity + // with the whole-vector twin is unused here. if (outValue == nullptr || queryScratch == nullptr || !IsInt8CrossDevice(source) || !IsInt8CrossDevice(target) || source.paddedDims != target.paddedDims) @@ -520,6 +537,31 @@ Status NNDivergenceChannel( // ZeroNormQuery rejection is for a SINGLE per-channel query, not a reduction over a pool // (a valid row can have a whole-row-normalized image with one channel exactly zero). + // Pre-lift the non-excluded target sub-rows ONCE: each target's self-dot + // (DotI8I8(timg, timg, length)) is independent of the source, so recomputing it inside + // the inner loop scored it m times per target. Stage the target queries in the + // Workspace's XdQuery scratch (warm-reusable, zero steady-state allocation) so the + // inner loop reads a prebuilt query. The int8 image pointers alias the target bank + // directly — targets are not requantized — so only scale and self-dot are computed. + if (!ws.ReserveXdQuery(target.paddedDims, target.count)) + { + return Status::OutOfMemory; + } + XdQuery* targetQueries = ws.XdSlots(); + int32_t tCount = 0; + for (int32_t r = 0; r < target.count; ++r) + { + if (IsExcluded(targetExcludeBits, r)) + { + continue; + } + const int8_t* timg = tgtRows + static_cast(r) * tpd + tOff; + targetQueries[tCount].q8 = timg; + targetQueries[tCount].scale = detail::FloatBitsToDouble(target.scales[r]); + targetQueries[tCount].sqSum = detail::DotI8I8(timg, timg, length); + ++tCount; + } + double acc = 0.0; double best = 0.0; bool have = false; @@ -529,19 +571,13 @@ Status NNDivergenceChannel( const XdQuery& q = queryScratch[i]; double nn = 0.0; bool haveNn = false; - for (int32_t r = 0; r < target.count; ++r) + for (int32_t t = 0; t < tCount; ++t) { - if (IsExcluded(targetExcludeBits, r)) - { - continue; - } - const int8_t* timg = tgtRows + static_cast(r) * tpd + tOff; - const XdQuery tq{timg, detail::FloatBitsToDouble(target.scales[r]), - detail::DotI8I8(timg, timg, length)}; // A zero sub-norm target is NOT skipped: XdChannelPairScore floors a degenerate // Cosine member to a defined 0 (C-5), so a zero-energy target scores distance 0 // and is the nearest -- consistent with Centroid/Spread and never NaN. - const double d = static_cast(XdChannelPairScore(q, tq, length, metric)); + const double d = + static_cast(XdChannelPairScore(q, targetQueries[t], length, metric)); const bool better = !haveNn || (metric == Metric::Dot ? d > nn : d < nn); if (better) { @@ -621,21 +657,21 @@ Status CentroidDistanceCrossDeviceChannel( Status MeanNNCrossDeviceChannel( const BankView& source, const uint32_t* sourceExcludeBits, const BankView& target, const uint32_t* targetExcludeBits, int32_t channel, - XdQuery* queryScratch, Hit*, int32_t*, Workspace&, + XdQuery* queryScratch, Hit*, int32_t*, Workspace& ws, float* outValue) { return NNDivergenceChannel(source, sourceExcludeBits, target, targetExcludeBits, channel, - Reduce::Mean, queryScratch, outValue); + Reduce::Mean, queryScratch, ws, outValue); } Status MaxNNCrossDeviceChannel( const BankView& source, const uint32_t* sourceExcludeBits, const BankView& target, const uint32_t* targetExcludeBits, int32_t channel, - XdQuery* queryScratch, Hit*, int32_t*, Workspace&, + XdQuery* queryScratch, Hit*, int32_t*, Workspace& ws, float* outValue) { return NNDivergenceChannel(source, sourceExcludeBits, target, targetExcludeBits, channel, - Reduce::Max, queryScratch, outValue); + Reduce::Max, queryScratch, ws, outValue); } Status SpreadCrossDeviceChannel( @@ -765,7 +801,10 @@ Status ProjectionReport(const BankView& bank, const float* paddedDirection, { acc += static_cast(row[i]) * static_cast(paddedDirection[i]); } - acc *= static_cast(bank.scales[r]); + // Decode the stored scale through the DAZ-safe path, as every other scale read + // in this file does, so a subnormal scale decodes identically whether or not + // the calling thread has denormals-are-zero enabled. + acc *= detail::FloatBitsToDouble(bank.scales[r]); } else { diff --git a/src/compose.cpp b/src/compose.cpp index f620e68..7e572df 100644 --- a/src/compose.cpp +++ b/src/compose.cpp @@ -89,7 +89,9 @@ Status MakeCentroid( { const int8_t* r = static_cast(bank.rows) + static_cast(row) * bank.paddedDims; - const double scale = bank.scales[row]; + // DAZ-safe scale decode, consistent with the other scale reads in this file + // (a subnormal scale decodes the same regardless of the thread's FTZ/DAZ mode). + const double scale = detail::FloatBitsToDouble(bank.scales[row]); for (int32_t j = 0; j < width; ++j) { acc[j] += static_cast(r[base + j]) * scale; diff --git a/src/graph.cpp b/src/graph.cpp index f313714..7cbbc55 100644 --- a/src/graph.cpp +++ b/src/graph.cpp @@ -168,7 +168,12 @@ Status MutualFilter(int32_t count, int32_t k, const int32_t* neighbors, uint8_t* { const int64_t slot = static_cast(i) * k + t; const int32_t j = neighbors[slot]; - outMutualFlags[slot] = (j >= 0 && inList(j, i)) ? uint8_t{1} : uint8_t{0}; + // `j` is a caller-supplied neighbour value and is used to index the + // `neighbors` array (inList reads neighbors + j*k). Bound it to [0, count): + // -1 is the "no neighbour" sentinel, and any other out-of-range value is a + // malformed input that must degrade to "no edge", never an out-of-bounds read. + outMutualFlags[slot] = + (j >= 0 && j < count && inList(j, i)) ? uint8_t{1} : uint8_t{0}; } } return Status::Ok; @@ -285,9 +290,15 @@ Status ConnectedComponents( { for (int32_t r = 0; r < count; ++r) { - if (duplicateGroups[r] != r) + // A group representative indexes the union-find scratch through unite/find. + // Bound it to [0, count): a well-formed table from BuildDuplicateGroups always + // holds an in-range representative, but this entry point takes the table from + // the caller, and an out-of-range value must be ignored rather than walked into + // the parent array as an out-of-bounds access. + const int32_t g = duplicateGroups[r]; + if (g >= 0 && g < count && g != r) { - unite(r, duplicateGroups[r]); + unite(r, g); } } } @@ -302,7 +313,10 @@ Status ConnectedComponents( if (mutualFlags[slot]) { const int32_t j = neighbors[slot]; - if (j >= 0) + // unite(i, j) walks parent[j]; bound j to [0, count) so a malformed + // neighbour value is a dropped edge, not an out-of-bounds write into the + // union-find scratch. -1 is the standing "no neighbour" sentinel. + if (j >= 0 && j < count) { unite(i, j); } diff --git a/src/kernels.cpp b/src/kernels.cpp index 8f66e40..d35bb8e 100644 --- a/src/kernels.cpp +++ b/src/kernels.cpp @@ -356,10 +356,18 @@ static float L2I8Sse(const int8_t* row, float scale, const float* query, int32_t return (SumLanes(acc0) + SumLanes(acc1)) + (SumLanes(acc2) + SumLanes(acc3)); } -// Dispatchers: AVX2+FMA when the CPU has it and the stride fits 8-lane blocks +// Dispatchers: AVX2+FMA when the CPU has it and the LENGTH fits 8-lane blocks // (float32 strides are multiples of 4, not always 8; int8 strides are always multiples -// of 16). The choice is a pure function of device and bank shape, so per-device -// determinism is preserved. +// of 16). The choice is a pure function of device and the length handed in, so +// per-device determinism is preserved. +// +// These dispatchers are the only place the choice is made. The segmented scan reaches +// them through ResolveRowKernels' function-pointer table and calls them once per scan +// range, so the deciding length is the RANGE's, not the bank's — which is why the rule +// lives here, keyed on the argument, rather than being resolved once from bank shape. +// Wiring an AVX2 kernel into that table directly creates a second selection rule, and +// the whole-row and segmented paths then compute different last-ulp results on the same +// row; see TestSegmentedEqualsWholeRowAcrossWidths. float DotF32(const float* row, const float* query, int32_t paddedDims) { @@ -908,18 +916,33 @@ namespace inline FRowKernels ResolveRowKernels() { FRowKernels k; + // The float32 slots hold the DISPATCHERS, never the AVX2 kernels directly. Those + // dispatchers apply the `% 8 == 0` rule to the length they are actually handed, + // and float32 strides are multiples of 4, not 8 — so wiring the AVX2 kernel in + // here made this a second kernel-selection rule, and the segmented scan computed + // an 8-lane FMA where the whole-row scan computed a 4-lane mul-add. The results + // differ in the last ulp, which falsified two published guarantees: that a + // degenerate one-segment query equals the whole-row scan, and that decomposition + // contributions sum bit-exactly to the score the scan produced. + // + // Note the rule keys on the RANGE length, not on paddedDims: this scan calls the + // kernel once per scan range, so a bank whose stride is a multiple of 8 can still + // present a range that is not. One rule, evaluated on the length in hand, is what + // makes the two paths one path. + // + // int8 needs no such care: channel and segment ranges sit on the 16-byte element + // grid, so an int8 length is always a multiple of 16, and its dispatcher carries + // no stride condition to diverge over. + k.dotF32 = detail::DotF32; + k.l2F32 = detail::L2F32; #if defined(SUPERFAISS_SIMD_SSE) if (detail::IsAvx2()) { - k.dotF32 = detail::DotF32Avx2; - k.l2F32 = detail::L2F32Avx2; k.dotI8 = detail::DotI8Avx2; k.l2I8 = detail::L2I8Avx2; return k; } #endif - k.dotF32 = detail::DotF32; - k.l2F32 = detail::L2F32; k.dotI8 = detail::DotI8; k.l2I8 = detail::L2I8; return k; diff --git a/src/novelty.cpp b/src/novelty.cpp index e828199..ca34ba3 100644 --- a/src/novelty.cpp +++ b/src/novelty.cpp @@ -238,6 +238,11 @@ Status CalibrateNoveltyBaseline( const BankView& bank, int32_t k, int32_t sampleLimit, float* outSortedDistances, int32_t* outCount, Workspace& workspace) { + // `sampleLimit` is a CEILING the bank must fit under, not a cap this function samples + // down to: `bank.count > sampleLimit` is a defined rejection (the header states this). + // A caller wanting a baseline over a subset pre-samples into a smaller BankView and + // passes that — which is exactly what the inspector does — rather than expecting this + // to draw a sample. Named `sampleLimit` because it bounds the sample the CALLER built. if (bank.metric == Metric::Dot || k < 1 || bank.count < 1 || k >= bank.count || sampleLimit < 1 || bank.count > sampleLimit || bank.rows == nullptr || outSortedDistances == nullptr || outCount == nullptr) { diff --git a/src/pca.cpp b/src/pca.cpp index 625677b..a552226 100644 --- a/src/pca.cpp +++ b/src/pca.cpp @@ -1,5 +1,7 @@ #include "superfaiss/pca.h" +#include "superfaiss/kernels.h" // detail::FloatBitsToDouble (DAZ-safe scale decode) + #include #include @@ -15,7 +17,9 @@ namespace { const int8_t* row = static_cast(bank.rows) + static_cast(r) * bank.paddedDims; - return static_cast(row[j]) * bank.scales[r]; + // DAZ-safe scale decode (a subnormal scale decodes identically under FTZ/DAZ), + // matching the reductions in analytics.cpp/compose.cpp over the same field. + return static_cast(row[j]) * detail::FloatBitsToDouble(bank.scales[r]); } const float* row = static_cast(bank.rows) + static_cast(r) * bank.paddedDims; @@ -29,7 +33,7 @@ Status ComputePrincipalComponents( int32_t iterationsPerComponent, float* outMean, float* outComponents, - float* scratch) + double* scratch) { if (outMean == nullptr || outComponents == nullptr || scratch == nullptr || componentCount <= 0 || componentCount > bank.dims || @@ -80,10 +84,12 @@ Status ComputePrincipalComponents( } } - // scratch <- Cov * v, serial in row order. + // scratch <- Cov * v, serial in row order, accumulated in DOUBLE so the + // operator is applied at the same precision as the mean above (a float + // accumulator rounds once per row and loses accuracy as count grows). for (int32_t j = 0; j < dims; ++j) { - scratch[j] = 0.0f; + scratch[j] = 0.0; } for (int32_t r = 0; r < count; ++r) { @@ -94,15 +100,14 @@ Status ComputePrincipalComponents( } for (int32_t j = 0; j < dims; ++j) { - scratch[j] = static_cast( - scratch[j] + dot * (RowElem(bank, r, j) - outMean[j])); + scratch[j] += dot * (RowElem(bank, r, j) - outMean[j]); } } double norm = 0.0; for (int32_t j = 0; j < dims; ++j) { - norm += static_cast(scratch[j]) * scratch[j]; + norm += scratch[j] * scratch[j]; } if (norm == 0.0) { @@ -160,8 +165,12 @@ Status ProjectRowsOntoComponents( float* outCoords) { if (mean == nullptr || components == nullptr || outCoords == nullptr || - componentCount <= 0) + componentCount <= 0 || componentCount > bank.dims) { + // componentCount is bounded by dims exactly as ComputePrincipalComponents bounds + // it: `components` holds componentCount rows of `dims` each, and an over-large + // count would index past the buffer. The two functions are called in sequence and + // must agree on the argument. return Status::InvalidArgument; } const int32_t dims = bank.dims; diff --git a/src/scratch.cpp b/src/scratch.cpp index e3d53d7..2f5a319 100644 --- a/src/scratch.cpp +++ b/src/scratch.cpp @@ -125,8 +125,10 @@ namespace constexpr uint8_t kScratchFlagChannels = 0x02; // reserved[0] bit 1 // Archive geometry ceiling (review M2): the largest row capacity a load will // entertain. With paddedDims capped at kMaxCrossDeviceDims, every ArenaBytes - // term stays below 2^49 — far from int64 overflow. - constexpr int32_t kMaxScratchArchiveRows = 1 << 28; + // term stays below 2^49 — far from int64 overflow. The same ceiling Create and + // Grow apply to a directly-supplied capacity: one number, so a geometry an + // archive cannot carry is also one the constructor refuses to build. + constexpr int32_t kMaxScratchArchiveRows = kMaxBankRows; struct ScratchHeader { @@ -141,8 +143,255 @@ namespace uint8_t reserved[6] = {}; }; static_assert(sizeof(ScratchHeader) == 32, "scratch header layout is the format"); + + // Validates a loaded retention region against the rows it claims to be the reference + // for. The retained row is BY CONSTRUCTION the post-normalization row the quantizer + // consumed (see AppendValidated), so replaying the bake on it must reproduce the stored + // row exactly — same inputs, same deterministic arithmetic, same bytes. That makes the + // retention region DERIVED-CHECKED rather than trusted: a fabricated-but-finite array + // would otherwise load clean and hand MeasureScratchRecall an invented reference to + // audit against, through the one API whose entire job is an honest number. + // + // Replays the quantizer inline rather than into a scratch buffer, so it needs no + // allocation; the arithmetic mirrors QuantizeRowsInt8/PadRowsFloat32 exactly. + bool RetainedMatchesRows(const ScratchBank& bank, const float* retained, const void* rows, + const float* scales, int32_t count, int32_t dims, int32_t paddedDims, Quantization quant) + { + (void)bank; + for (int32_t r = 0; r < count; ++r) + { + const float* src = retained + static_cast(r) * dims; + for (int32_t i = 0; i < dims; ++i) + { + if (!std::isfinite(src[i])) + { + return false; + } + } + if (quant == Quantization::Float32) + { + const float* dst = static_cast(rows) + + static_cast(r) * paddedDims; + for (int32_t i = 0; i < dims; ++i) + { + if (dst[i] != src[i]) + { + return false; + } + } + for (int32_t i = dims; i < paddedDims; ++i) + { + if (dst[i] != 0.0f) + { + return false; + } + } + continue; + } + + const int8_t* dst = static_cast(rows) + + static_cast(r) * paddedDims; + float maxAbs = 0.0f; + for (int32_t i = 0; i < dims; ++i) + { + const float a = std::fabs(src[i]); + if (a > maxAbs) + { + maxAbs = a; + } + } + if (maxAbs == 0.0f) + { + if (scales[r] != 0.0f) + { + return false; + } + for (int32_t i = 0; i < paddedDims; ++i) + { + if (dst[i] != 0) + { + return false; + } + } + continue; + } + if (scales[r] != maxAbs / 127.0f) + { + return false; + } + const float inv = 127.0f / maxAbs; + for (int32_t i = 0; i < dims; ++i) + { + float q = std::nearbyint(src[i] * inv); + if (q > 127.0f) + { + q = 127.0f; + } + if (q < -127.0f) + { + q = -127.0f; + } + if (dst[i] != static_cast(q)) + { + return false; + } + } + for (int32_t i = dims; i < paddedDims; ++i) + { + if (dst[i] != 0) + { + return false; + } + } + } + return true; + } + + // The header rules, shared by Load and PeekScratchArchive so the two can never + // disagree about what a valid archive header is — one validator, one truth. Pure on + // the header bytes: it allocates nothing and reads no payload. + Status ValidateScratchHeader( + const ScratchHeader& header, bool* outHasChannels, bool* outRetain) + { + // Accepts {1, 2, 3}; version 0 or anything newer is the standing old-reader/new-data + // hard-reject (a future v4 archive rejects here). Legacy 1/2 encode retention as the + // version integer; version 3 makes the flags byte authoritative. + if (header.magic != kScratchMagic || + header.version < 1u || header.version > kScratchVersionChannels) + { + return Status::BadFormat; + } + // Version 3: retention and channel presence come from the reserved[0] flags byte, and + // bits 2-7 are reserved — masked off, tolerated (never rejected, never mis-read as + // retention or channels; forward tolerance by design). Legacy 1/2 read retention from + // the version integer, exactly as shipped, and carry no channel table. + const uint8_t flags = header.reserved[kScratchFlagsByteIndex]; + const bool hasChannels = header.version == kScratchVersionChannels && + (flags & kScratchFlagChannels) != 0; + const bool wantRetain = header.version == kScratchVersionChannels + ? (flags & kScratchFlagRetention) != 0 + : header.version == kScratchVersionRetain; + // Version 3 is emitted ONLY for a channel-carrying bank (channels trigger the presence- + // flags format; a retention-only bank stays legacy v2). A v3 blob without the channels + // flag is therefore something the writer never produces — reject it as corrupt rather + // than adopt it as a plain bank. This also catches a legacy blob whose version integer + // was hand-bumped to 3 (its reserved bytes are zero, so the channels flag is clear). + if (header.version == kScratchVersionChannels && !hasChannels) + { + return Status::BadFormat; + } + const bool metricOk = header.metric <= static_cast(Metric::L2); + const bool quantOk = header.quant <= static_cast(Quantization::Int8); + if (!metricOk || !quantOk || header.capacity <= 0 || header.dims <= 0 || + header.count < 0 || header.count > header.capacity || + header.paddedDims != + PaddedDims(header.dims, static_cast(header.quant))) + { + return Status::BadFormat; + } + // The archive is an untrusted medium (review M2, the T-062 idiom): bound the + // geometry BEFORE any byte-size arithmetic — the arena math multiplies the + // caller-controlled capacity and dims, and an unbounded pair is signed int64 + // overflow (UB), not merely a failed allocation. paddedDims is capped at the + // widest bank the library proves (kMaxCrossDeviceDims); capacity at 2^28 rows + // keeps every ArenaBytes term below 2^49. Absurd geometry is a format defect — + // a hard BadFormat, never an allocator outcome. + if (header.paddedDims > kMaxCrossDeviceDims || + header.capacity > kMaxScratchArchiveRows) + { + return Status::BadFormat; + } + *outHasChannels = hasChannels; + *outRetain = wantRetain; + return Status::Ok; + } } // namespace +Status PeekScratchArchive(const void* data, int64_t bytes, ScratchArchiveInfo* outInfo) +{ + if (data == nullptr || outInfo == nullptr || bytes < 0) + { + return Status::InvalidArgument; + } + *outInfo = ScratchArchiveInfo{}; + if (bytes < static_cast(sizeof(ScratchHeader))) + { + return Status::BadFormat; + } + const uint8_t* cursor = static_cast(data); + ScratchHeader header; + std::memcpy(&header, cursor, sizeof(header)); + bool hasChannels = false; + bool retain = false; + const Status headerStatus = ValidateScratchHeader(header, &hasChannels, &retain); + if (headerStatus != Status::Ok) + { + return headerStatus; + } + int64_t consumed = static_cast(sizeof(ScratchHeader)); + + // The channel table, written immediately after the header on a v3 archive. Its COUNT is + // bounded here because it sizes the read below; the ranges themselves are validated where + // they are adopted (the channel Create inside Load), so a peek and a load agree on what a + // well-formed archive is without duplicating the geometry rules. + if (hasChannels) + { + if (bytes - consumed < static_cast(sizeof(int32_t))) + { + return Status::BadFormat; + } + int32_t channelCount = 0; + std::memcpy(&channelCount, cursor + consumed, sizeof(channelCount)); + consumed += static_cast(sizeof(channelCount)); + if (channelCount <= 0 || channelCount > kMaxChannels) + { + return Status::BadFormat; + } + const int64_t tableBytes = + static_cast(channelCount) * static_cast(sizeof(ChannelInfo)); + if (bytes - consumed < tableBytes) + { + return Status::BadFormat; + } + std::memcpy(outInfo->channels, cursor + consumed, static_cast(tableBytes)); + consumed += tableBytes; + outInfo->channelCount = channelCount; + } + + // The payload's byte length, in the order Save writes it. Every term is bounded by the + // header caps checked above, so the int64 arithmetic cannot overflow. + const Quantization quant = static_cast(header.quant); + consumed += static_cast(header.count) * header.paddedDims * ElementSize(quant); + if (quant == Quantization::Int8) + { + consumed += static_cast(header.count) * static_cast(sizeof(float)); + } + consumed += static_cast(ScratchBank::TombstoneWords(header.count)) * + static_cast(sizeof(uint32_t)); + if (retain) + { + consumed += static_cast(header.count) * header.dims * + static_cast(sizeof(float)); + } + // A span that does not cover the archive it declares is truncated, which is a format + // defect. A span that is LONGER is not: whatever follows belongs to the caller (a host + // trailer), and reporting archiveBytes is precisely how the caller finds it. + if (bytes < consumed) + { + return Status::BadFormat; + } + + outInfo->capacity = header.capacity; + outInfo->count = header.count; + outInfo->dims = header.dims; + outInfo->paddedDims = header.paddedDims; + outInfo->metric = static_cast(header.metric); + outInfo->quant = quant; + outInfo->retainFloats = retain; + outInfo->archiveBytes = consumed; + return Status::Ok; +} + ScratchBank::~ScratchBank() { Destroy(); @@ -245,6 +494,15 @@ Status ScratchBank::Create( { return Status::InvalidArgument; } + // Geometry ceilings BEFORE any size arithmetic (the Load idiom, now on the public + // construction path): ArenaBytes multiplies capacity by dims in signed int64, and an + // uncapped int32 pair overflows it — undefined behavior, not a failed allocation. The + // caps are the format's own (kMaxBankRows rows, kMaxCrossDeviceDims dims), so a bank + // this refuses to build is one no archive could have carried either. + if (capacity > kMaxScratchArchiveRows || dims > kMaxCrossDeviceDims) + { + return Status::InvalidArgument; + } const int32_t paddedDims = PaddedDims(dims, quant); const int64_t bytes = ArenaBytes(capacity, dims, paddedDims, quant, retainFloats, 0); @@ -295,6 +553,12 @@ Status ScratchBank::Create( { return Status::InvalidArgument; } + // The same geometry ceilings the retention overload applies, for the same reason: + // bound capacity and dims before ArenaBytes multiplies them. + if (capacity > kMaxScratchArchiveRows || dims > kMaxCrossDeviceDims) + { + return Status::InvalidArgument; + } const int32_t paddedDims = PaddedDims(dims, quant); @@ -595,8 +859,11 @@ Status ScratchBank::Grow(int32_t newCapacity) return Status::InvalidArgument; } WriterGuard guard(WriterBusy_); - if (newCapacity <= Capacity_) + if (newCapacity <= Capacity_ || newCapacity > kMaxScratchArchiveRows) { + // The row ceiling applies to a grown capacity exactly as it does to a created + // one: ArenaBytes below multiplies newCapacity by the row stride in signed + // int64, so an uncapped request is overflow, not an allocation failure. return Status::InvalidArgument; } const int32_t subNormPerRow = (Metric_ == Metric::Cosine && ChannelCount_ > 0) @@ -1110,53 +1377,15 @@ Status ScratchBank::Load(const ScratchArchive& archive, const Allocator& allocat { return Status::BadFormat; } - // Accepts {1, 2, 3}; version 0 or anything newer is the standing old-reader/new-data - // hard-reject (a future v4 archive rejects here). Legacy 1/2 encode retention as the - // version integer; version 3 makes the flags byte authoritative. - if (header.magic != kScratchMagic || - header.version < 1u || header.version > kScratchVersionChannels) + // Version acceptance, the presence-flags byte, and the geometry ceilings all live in + // the shared header validator PeekScratchArchive runs, so a peek and a load can never + // disagree about which archives are well-formed. + bool hasChannels = false; + bool wantRetain = false; + const Status headerStatus = ValidateScratchHeader(header, &hasChannels, &wantRetain); + if (headerStatus != Status::Ok) { - return Status::BadFormat; - } - // Version 3: retention and channel presence come from the reserved[0] flags byte, and - // bits 2-7 are reserved — masked off, tolerated (never rejected, never mis-read as - // retention or channels; forward tolerance by design). Legacy 1/2 read retention from - // the version integer, exactly as shipped, and carry no channel table. - const uint8_t flags = header.reserved[kScratchFlagsByteIndex]; - const bool hasChannels = header.version == kScratchVersionChannels && - (flags & kScratchFlagChannels) != 0; - const bool wantRetain = header.version == kScratchVersionChannels - ? (flags & kScratchFlagRetention) != 0 - : header.version == kScratchVersionRetain; - // Version 3 is emitted ONLY for a channel-carrying bank (channels trigger the presence- - // flags format; a retention-only bank stays legacy v2). A v3 blob without the channels - // flag is therefore something the writer never produces — reject it as corrupt rather - // than adopt it as a plain bank. This also catches a legacy blob whose version integer - // was hand-bumped to 3 (its reserved bytes are zero, so the channels flag is clear). - if (header.version == kScratchVersionChannels && !hasChannels) - { - return Status::BadFormat; - } - const bool metricOk = header.metric <= static_cast(Metric::L2); - const bool quantOk = header.quant <= static_cast(Quantization::Int8); - if (!metricOk || !quantOk || header.capacity <= 0 || header.dims <= 0 || - header.count < 0 || header.count > header.capacity || - header.paddedDims != - PaddedDims(header.dims, static_cast(header.quant))) - { - return Status::BadFormat; - } - // The archive is an untrusted medium (review M2, the T-062 idiom): bound the - // geometry BEFORE any byte-size arithmetic — the arena math multiplies the - // caller-controlled capacity and dims, and an unbounded pair is signed int64 - // overflow (UB), not merely a failed allocation. paddedDims is capped at the - // widest bank the library proves (kMaxCrossDeviceDims); capacity at 2^28 rows - // keeps every ArenaBytes term below 2^49. Absurd geometry is a format defect — - // a hard BadFormat, never an allocator outcome. - if (header.paddedDims > kMaxCrossDeviceDims || - header.capacity > kMaxScratchArchiveRows) - { - return Status::BadFormat; + return headerStatus; } // The channel table (v3), read here — before the arena is created — so the channel @@ -1266,6 +1495,18 @@ Status ScratchBank::Load(const ScratchArchive& archive, const Allocator& allocat return content; } + // The retention region is validated too, against the rows just proven good: replaying + // the bake on each retained row must reproduce the stored row byte for byte. Without + // this the archive's "not trusted" claim held for the rows and not for the audit + // reference beside them, so a fabricated retained array loaded clean and + // MeasureScratchRecall reported a number derived from it. + if (wantRetain && header.count > 0 && + !RetainedMatchesRows(incoming, incoming.Retained_, incoming.Rows_, incoming.Scales_, + header.count, header.dims, header.paddedDims, incoming.Quant_)) + { + return Status::BadFormat; + } + // Re-derive the per-channel sub-norm arena on Load: recompute it from the // loaded rows + channel table rather than trust a serialized copy, so a desynced arena // cannot load as authoritative. The channel Create above sized the region into diff --git a/src/validate.cpp b/src/validate.cpp index 1bfdb78..3df8461 100644 --- a/src/validate.cpp +++ b/src/validate.cpp @@ -77,8 +77,13 @@ Status ValidateBank(const BankView& bank) } prevEnd = channel.offset + channel.length; } - // Per-channel cosine requires the baked inverse sub-norms. - if (bank.metric == Metric::Cosine && bank.channelInvNorms == nullptr) + // Per-channel cosine requires the baked inverse sub-norms — one per row, so a bank + // with no rows requires none. Demanding the array unconditionally made a zero-row + // Cosine channel bank unrepresentable, which is not a degenerate case: it is what + // a channel-carrying scratch bank graduates into when every row has been removed, + // and an emptied channel bank is still a channel bank. No scan reads the array at + // zero rows, so there is nothing to protect against here. + if (bank.metric == Metric::Cosine && bank.count > 0 && bank.channelInvNorms == nullptr) { return Status::BadFormat; } diff --git a/tests/test_main.cpp b/tests/test_main.cpp index ffa0e03..fbe6db5 100644 --- a/tests/test_main.cpp +++ b/tests/test_main.cpp @@ -15,6 +15,7 @@ #include "xd_fixtures.h" #include +#include #include #include #include @@ -330,6 +331,31 @@ static void PadQuery(const std::vector& q, int32_t pd, float* out) std::memcpy(out, q.data(), q.size() * sizeof(float)); } +// Bounded-wait for a ScratchBank's published count to advance past `appended` -- the +// deterministic form of "the writer has published at least one fresh row" that a storm +// test's reader needs before its measured loop starts. Count() is an atomic acquire load +// (safe to poll unpinned, from any thread), so this returns as soon as the writer's first +// successful Append lands under real scheduling instead of on a fixed iteration budget. +// Bounded by `timeoutMs`: a genuine regression in the append/publish path (the writer never +// actually advancing the count) still fails the test loudly here, rather than leaving the +// caller's own liveness assertion to trip only when the reader happens to outrun the writer, +// or leaving the wait to hang forever. +static bool WaitForPublishPastCount(const ScratchBank& bank, int32_t appended, + int32_t timeoutMs = 5000) +{ + const auto deadline = + std::chrono::steady_clock::now() + std::chrono::milliseconds(timeoutMs); + while (bank.Count() <= appended) + { + if (std::chrono::steady_clock::now() >= deadline) + { + return false; + } + std::this_thread::yield(); + } + return true; +} + // --------------------------------------------------------------------------- // T1 — known geometry (plan A6): hand-built vectors with hand-computed neighbors. @@ -1100,11 +1126,15 @@ static void TestSimdEqualsScalar() // Where the defect actually bites (established while authoring this test — the coord's // original "length-4 scores 0 on the whole-row path" framing was imprecise): // * The WHOLE-ROW dispatch DotF32()/L2F32() (src/kernels.cpp) selects the AVX2 -// intrinsic ONLY when paddedDims % 8 == 0 (else DotF32Sse, which handles the tail). -// PaddedDims is always a multiple of 8 for f32 banks big enough to matter, so a -// length ≡ 4 mod 8 never reaches the intrinsic through that dispatch — the whole- -// row path was NOT wrong in production. That is why the correctness check in Part A -// runs against the scalar MIRROR directly, not through DotF32(). +// intrinsic ONLY when the length it is handed is a multiple of 8 (else DotF32Sse, +// which handles the tail), so a length ≡ 4 mod 8 never reaches the intrinsic through +// that dispatch. That is why the correctness check in Part A runs against the scalar +// MIRROR directly, not through DotF32(). +// (This bullet used to add "PaddedDims is always a multiple of 8 for f32 banks big +// enough to matter". That is FALSE — f32 strides are multiples of 4, so dims 20, 36, +// 52, 100 and 132 all produce odd strides — and the belief is what let the two scan +// paths diverge unnoticed at those widths. TestSegmentedEqualsWholeRowAcrossWidths +// now sweeps them and compares the two paths to each other.) // * The defect bites the SEGMENTED / per-channel-cosine scan: ResolveRowKernels() // (src/kernels.cpp) wires k.dotF32 = DotF32Avx2 RAW (no % 8 guard), and the non- // foldable per-channel-cosine path (ScoreChunkSegmented) calls it over each @@ -2027,7 +2057,7 @@ static void TestPca() std::vector mean(static_cast(dims)); std::vector comps(static_cast(2) * dims); - std::vector scratch(static_cast(dims)); + std::vector scratch(static_cast(dims)); CHECK(ComputePrincipalComponents(view, 2, 48, mean.data(), comps.data(), scratch.data()) == Status::Ok); @@ -2062,6 +2092,12 @@ static void TestPca() std::vector coords(static_cast(count) * 2); CHECK(ProjectRowsOntoComponents(view, mean.data(), comps.data(), 2, coords.data()) == Status::Ok); + // componentCount is bounded by dims on BOTH entry points (a count above dims would + // index past `components`), the same rejection ComputePrincipalComponents applies. + CHECK(ProjectRowsOntoComponents(view, mean.data(), comps.data(), dims + 1, + coords.data()) == Status::InvalidArgument); + CHECK(ProjectRowsOntoComponents(view, mean.data(), comps.data(), 0, + coords.data()) == Status::InvalidArgument); { const int32_t r = 7; double expect = 0.0; @@ -2105,7 +2141,7 @@ static void TestPca() view.metric = Metric::Dot; std::vector mean(static_cast(d)); std::vector comps(static_cast(d)); - std::vector scratch(static_cast(d)); + std::vector scratch(static_cast(d)); CHECK(ComputePrincipalComponents(view, 1, 16, mean.data(), comps.data(), scratch.data()) == Status::Ok); double norm = 0.0; @@ -2131,6 +2167,107 @@ static void TestPca() // B — repeat determinism, external chunk-fusion merge bit-identity, // validation rejections including the query-side zero-norm segment law. +// The path-equality contract, swept across the widths that can break it. +// +// TestSegmentedScan's A1 already asserts "degenerate one-segment query == whole-row +// scan, bit-identical" — but only at dims 32, where the float32 stride is a multiple of +// 8. That is precisely where the two paths cannot disagree, because the float32 kernel +// dispatchers select AVX2 only at `length % 8 == 0` and both paths then choose it. The +// mechanism was covered; the width was not, and 12/32/64/128/256/768 all miss it. +// +// Float32 strides are multiples of 4, so dims 20, 36, 52, 100 and 132 produce strides +// that are NOT multiples of 8. At those widths a second selection rule anywhere in the +// scan shows up as a last-ulp disagreement between the two paths — which is what +// DETERMINISM.md's "no second code path exists to drift" forbids, and what the plugin's +// "contributions sum EXACTLY to the hit's score" is built on. +// +// This is the cross-PATH check. T11's sweep compares each kernel against its own scalar +// mirror, which stays green while the two paths pick different kernels, because each is +// individually correct. Comparing a path to a mirror is not comparing a path to a path. +static void TestSegmentedEqualsWholeRowAcrossWidths() +{ + Rng rng(0x9D71A55ull); + const int32_t count = 120; + const int32_t k = 8; + // Chosen so the float32 stride is NOT a multiple of 8 (20, 36, 52, 100, 132) with + // multiple-of-8 controls (32, 64) beside them. + const int32_t dimsSet[] = {20, 32, 36, 52, 64, 100, 132}; + + for (int32_t dims : dimsSet) + { + for (Quantization quant : {Quantization::Float32, Quantization::Int8}) + { + if (dims % (kAlignment / ElementSize(quant)) != 0) + { + continue; // not on this quantization's element grid; not a legal segment + } + for (Metric metric : {Metric::Dot, Metric::Cosine, Metric::L2}) + { + TestBank bank(rng, count, dims, quant, metric); + const int32_t pd = bank.view.paddedDims; + AlignedBuf q(static_cast(pd) * sizeof(float)); + std::vector qv(static_cast(dims)); + for (auto& x : qv) + { + x = rng.NextFloat(); + } + PadQuery(qv, pd, q.F32()); + + QueryParams plain; + plain.k = k; + Workspace ws1; + std::vector whole(static_cast(k)); + int32_t wholeCount = 0; + CHECK(Query(bank.view, q.F32(), plain, ws1, whole.data(), &wholeCount) == + Status::Ok); + + const QuerySegment degenerate[1] = {{0, pd, 1.0f}}; + QueryParams seg = plain; + seg.segments = degenerate; + seg.segmentCount = 1; + Workspace ws2; + std::vector segmented(static_cast(k)); + int32_t segCount = 0; + CHECK(Query(bank.view, q.F32(), seg, ws2, segmented.data(), &segCount) == + Status::Ok); + + CHECK_MSG(segCount == wholeCount, + "dims=%d pd=%d quant=%d metric=%d: hit counts differ (%d vs %d)", + dims, pd, static_cast(quant), static_cast(metric), + wholeCount, segCount); + for (int32_t i = 0; i < wholeCount && i < segCount; ++i) + { + CHECK_MSG(segmented[i].index == whole[i].index && + segmented[i].score == whole[i].score, + "dims=%d pd=%d quant=%d metric=%d hit %d: whole-row (%d, %.9g) vs " + "one-segment (%d, %.9g) — the two scan paths disagree", + dims, pd, static_cast(quant), static_cast(metric), i, + whole[i].index, static_cast(whole[i].score), + segmented[i].index, static_cast(segmented[i].score)); + } + + // The decomposition surface rides the same kernel table, and its published + // claim is that a contribution IS the score. With one whole-row segment + // there is exactly one contribution and no summation order to argue about, + // so this is the claim stated at its sharpest. + if (wholeCount > 0) + { + float contribution = 0.0f; + const float total = DecomposeRowScore(bank.view, q.F32(), + whole[0].index, degenerate, 1, &contribution); + CHECK_MSG(total == whole[0].score, + "dims=%d pd=%d quant=%d metric=%d: decomposed total %.9g != scan " + "score %.9g for row %d", + dims, pd, static_cast(quant), static_cast(metric), + static_cast(total), + static_cast(whole[0].score), whole[0].index); + CHECK(contribution == total); + } + } + } + } +} + static void TestSegmentedScan() { Rng rng(0x5E63E17ull); @@ -2722,6 +2859,339 @@ namespace }; } // namespace +// The public construction paths bound their geometry BEFORE the arena size arithmetic +// runs on it. ArenaBytes multiplies capacity by dims in signed int64, so an uncapped +// int32 pair is overflow — undefined behavior, not a refused allocation. The archive +// loader already applied these ceilings; Create and Grow did not. +static void TestScratchGeometryCeilings() +{ + // Over-cap capacity: an argument defect, and nothing is allocated — the bank stays + // uncreated, so a subsequent valid Create on the same object still succeeds. + { + ScratchBank bank; + CHECK(bank.Create(kMaxBankRows + 1, 64, Metric::Dot, Quantization::Float32) == + Status::InvalidArgument); + CHECK(!bank.IsCreated()); + CHECK(bank.Create(16, 64, Metric::Dot, Quantization::Float32) == Status::Ok); + } + // Over-cap dims, on the retention overload. + { + ScratchBank bank; + CHECK(bank.Create(16, kMaxCrossDeviceDims + 1, Metric::Dot, Quantization::Float32) == + Status::InvalidArgument); + CHECK(!bank.IsCreated()); + } + // The channel overload carries its own preamble, so it needs its own proof. A fresh + // bank per case: on a shared object the second call would be refused for being + // already-created, and the cap would read proven while never having run. + { + const ChannelInfo channels[1] = {{0, 16}}; + ScratchBank overRows; + CHECK(overRows.Create(kMaxBankRows + 1, 64, Metric::Cosine, Quantization::Float32, + channels, 1) == Status::InvalidArgument); + CHECK(!overRows.IsCreated()); + ScratchBank overDims; + CHECK(overDims.Create(16, kMaxCrossDeviceDims + 1, Metric::Cosine, + Quantization::Float32, channels, 1) == Status::InvalidArgument); + CHECK(!overDims.IsCreated()); + } + // The pair that overflows the arena arithmetic outright. Rejected on geometry, so the + // multiply never runs on it. + { + ScratchBank bank; + CHECK(bank.Create(INT32_MAX, INT32_MAX, Metric::Dot, Quantization::Float32) == + Status::InvalidArgument); + CHECK(!bank.IsCreated()); + } + // Grow takes the same ceiling, and a refused Grow leaves the bank exactly as it was. + { + ScratchBank bank; + CHECK(bank.Create(16, 64, Metric::Dot, Quantization::Float32) == Status::Ok); + CHECK(bank.Grow(kMaxBankRows + 1) == Status::InvalidArgument); + CHECK(bank.Grow(INT32_MAX) == Status::InvalidArgument); + CHECK(bank.Capacity() == 16); + CHECK(bank.Grow(32) == Status::Ok); + CHECK(bank.Capacity() == 32); + } +} + +// A Cosine channel bank with zero rows is representable: the inverse sub-norms are one +// per row, so no rows require none. This is the shape a channel-carrying scratch bank +// graduates into once every row has been removed — an emptied channel bank is still a +// channel bank, and rejecting it forced the graduation to drop its channels instead. +static void TestEmptyCosineChannelBankValidates() +{ + const ChannelInfo channels[2] = {{0, 32}, {32, 32}}; + for (Quantization quant : {Quantization::Float32, Quantization::Int8}) + { + BankView empty; + empty.rows = nullptr; + empty.scales = nullptr; + empty.count = 0; + empty.dims = 64; + empty.paddedDims = PaddedDims(64, quant); + empty.quant = quant; + empty.metric = Metric::Cosine; + empty.channels = channels; + empty.channelCount = 2; + empty.channelInvNorms = nullptr; + CHECK_MSG(ValidateBank(empty) == Status::Ok, + "zero-row Cosine channel bank rejected (quant %d)", static_cast(quant)); + CHECK(ValidateBankData(empty, nullptr) == Status::Ok); + } + // With rows present the array is still required — the relaxation is about count, not + // about the rule. + { + Rng rng(0x4E5701ull); + const int32_t dims = 64, count = 8; + TestBank bank(rng, count, dims, Quantization::Int8, Metric::Cosine); + BankView view = bank.view; + view.channels = channels; + view.channelCount = 2; + view.channelInvNorms = nullptr; + CHECK_MSG(ValidateBank(view) == Status::BadFormat, + "a populated Cosine channel bank must still require its sub-norms"); + } +} + +// The retention region is part of what "the archive is not trusted" covers. A retained +// row is by construction the post-normalization row the quantizer consumed, so replaying +// the bake on it must reproduce the stored row exactly; a fabricated-but-finite retained +// array would otherwise load clean and hand MeasureScratchRecall an invented reference to +// audit the quantized rows against — through the one API whose whole job is honest. +static void TestScratchLoadValidatesRetainedRegion() +{ + Rng rng(0x2E7A15ull); + const int32_t dims = 32, count = 24; + + for (Quantization quant : {Quantization::Float32, Quantization::Int8}) + { + ScratchBank bank; + CHECK(bank.Create(count, dims, Metric::Cosine, quant, /*retainFloats=*/true) == + Status::Ok); + std::vector row(static_cast(dims)); + for (int32_t r = 0; r < count; ++r) + { + for (auto& v : row) + { + v = rng.NextFloat(); + } + CHECK(bank.Append(row.data(), dims, nullptr) == Status::Ok); + } + MemArchive good; + CHECK(bank.Save(good.Writer()) == Status::Ok); + + // The clean blob loads, so the corruptions below fail for their own reason. + { + MemArchive a; + a.bytes = good.bytes; + ScratchBank target; + CHECK(target.Load(a.Reader()) == Status::Ok); + CHECK(target.RetainsFloats()); + } + + // The retained region is the LAST thing Save writes, so the trailing + // count * dims floats are it. Perturb one value: still finite, still plausible, + // but no longer the row the quantizer consumed. + const size_t retainedBytes = + static_cast(count) * dims * sizeof(float); + CHECK(good.bytes.size() > retainedBytes); + { + MemArchive a; + a.bytes = good.bytes; + float* retained = reinterpret_cast(a.bytes.data() + + (a.bytes.size() - retainedBytes)); + retained[0] = retained[0] + 0.25f; // finite, wrong + ScratchBank target; + CHECK_MSG(target.Load(a.Reader()) == Status::BadFormat, + "a fabricated retained row must be rejected (quant %d)", + static_cast(quant)); + } + // A non-finite retained value is rejected too. + { + MemArchive a; + a.bytes = good.bytes; + float* retained = reinterpret_cast(a.bytes.data() + + (a.bytes.size() - retainedBytes)); + retained[3] = std::numeric_limits::quiet_NaN(); + ScratchBank target; + CHECK(target.Load(a.Reader()) == Status::BadFormat); + } + } +} + +// PeekScratchArchive reports a serialized archive's geometry and — the reason it exists — +// the exact byte length a Load consumes, so a host that appends its own trailer can find +// and validate that trailer BEFORE committing the load. +static void TestPeekScratchArchive() +{ + Rng rng(0x9E5A11ull); + const int32_t dims = 64; + const ChannelInfo channels[2] = {{0, 32}, {32, 32}}; + + struct Shape + { + const char* name; + bool hasChannels; + bool retain; + Quantization quant; + Metric metric; + }; + const Shape shapes[] = { + {"legacy f32 dot", false, false, Quantization::Float32, Metric::Dot}, + {"legacy int8 cosine", false, false, Quantization::Int8, Metric::Cosine}, + {"retention int8 cosine", false, true, Quantization::Int8, Metric::Cosine}, + {"channels f32 cosine", true, false, Quantization::Float32, Metric::Cosine}, + {"channels+retention int8 cosine", true, true, Quantization::Int8, Metric::Cosine}, + {"channels int8 dot", true, false, Quantization::Int8, Metric::Dot}, + }; + for (const Shape& shape : shapes) + { + ScratchBank bank; + const Status created = shape.hasChannels + ? bank.Create(32, dims, shape.metric, shape.quant, channels, 2, shape.retain) + : bank.Create(32, dims, shape.metric, shape.quant, shape.retain); + CHECK_MSG(created == Status::Ok, "%s: create failed (%d)", shape.name, + static_cast(created)); + if (created != Status::Ok) + { + continue; + } + std::vector row(static_cast(dims)); + for (int32_t r = 0; r < 20; ++r) + { + for (auto& v : row) + { + v = rng.NextFloat(); + } + CHECK(bank.Append(row.data(), dims, nullptr) == Status::Ok); + } + CHECK(bank.Remove(4) == Status::Ok); // tombstone words are part of the length + + MemArchive archive; + CHECK(bank.Save(archive.Writer()) == Status::Ok); + + ScratchArchiveInfo info; + const Status peeked = PeekScratchArchive( + archive.bytes.data(), static_cast(archive.bytes.size()), &info); + CHECK_MSG(peeked == Status::Ok, "%s: peek failed (%d)", shape.name, + static_cast(peeked)); + if (peeked != Status::Ok) + { + continue; + } + CHECK_MSG(info.archiveBytes == static_cast(archive.bytes.size()), + "%s: archiveBytes %lld, blob %lld", shape.name, + static_cast(info.archiveBytes), + static_cast(archive.bytes.size())); + CHECK(info.count == bank.Count()); + CHECK(info.capacity == bank.Capacity()); + CHECK(info.dims == dims); + CHECK(info.paddedDims == bank.GetPaddedDims()); + CHECK(info.metric == shape.metric); + CHECK(info.quant == shape.quant); + CHECK_MSG(info.retainFloats == shape.retain, "%s: retention flag lost", shape.name); + CHECK_MSG(info.channelCount == (shape.hasChannels ? 2 : 0), + "%s: channelCount %d", shape.name, info.channelCount); + for (int32_t c = 0; c < info.channelCount; ++c) + { + CHECK(info.channels[c].offset == channels[c].offset && + info.channels[c].length == channels[c].length); + } + + // A span that does not cover the archive it declares is truncated — a format + // defect, never a short read to paper over. + ScratchArchiveInfo truncated; + CHECK(PeekScratchArchive(archive.bytes.data(), + static_cast(archive.bytes.size()) - 1, &truncated) == + Status::BadFormat); + CHECK(PeekScratchArchive(archive.bytes.data(), 8, &truncated) == Status::BadFormat); + CHECK(PeekScratchArchive(archive.bytes.data(), 0, &truncated) == Status::BadFormat); + + // Trailing bytes belong to the caller: the peek succeeds and points at them. + std::vector withTrailer = archive.bytes; + withTrailer.push_back(0xABu); + withTrailer.push_back(0xCDu); + ScratchArchiveInfo trailed; + CHECK(PeekScratchArchive(withTrailer.data(), + static_cast(withTrailer.size()), &trailed) == Status::Ok); + CHECK_MSG(trailed.archiveBytes == static_cast(archive.bytes.size()), + "%s: trailer moved the reported archive length", shape.name); + } + + // The header rules are the loader's own: whatever a peek rejects, a Load rejects. + { + ScratchBank bank; + CHECK(bank.Create(8, 32, Metric::Dot, Quantization::Float32) == Status::Ok); + MemArchive archive; + CHECK(bank.Save(archive.Writer()) == Status::Ok); + + std::vector badMagic = archive.bytes; + badMagic[0] = static_cast(badMagic[0] ^ 0xFFu); + std::vector badVersion = archive.bytes; + badVersion[4] = 4u; // a future version: the standing old-reader/new-data reject + std::vector zeroVersion = archive.bytes; + zeroVersion[4] = 0u; + std::vector forgedV3 = archive.bytes; + forgedV3[4] = 3u; // v3 without the channels flag is a shape the writer never emits + + const std::vector* bad[] = {&badMagic, &badVersion, &zeroVersion, &forgedV3}; + for (const std::vector* blob : bad) + { + ScratchArchiveInfo info; + CHECK(PeekScratchArchive(blob->data(), static_cast(blob->size()), &info) == + Status::BadFormat); + MemArchive corrupt; + corrupt.bytes = *blob; + ScratchBank target; + CHECK(target.Load(corrupt.Reader()) == Status::BadFormat); + } + + ScratchArchiveInfo info; + CHECK(PeekScratchArchive(nullptr, 32, &info) == Status::InvalidArgument); + CHECK(PeekScratchArchive(archive.bytes.data(), + static_cast(archive.bytes.size()), nullptr) == + Status::InvalidArgument); + } + + // The header claims the peek allocates nothing — proven, not asserted. Everything the + // tracked scope touches is built before it opens. + { + const ChannelInfo table[2] = {{0, 32}, {32, 32}}; + ScratchBank bank; + CHECK(bank.Create(32, dims, Metric::Cosine, Quantization::Int8, table, 2, + /*retainFloats=*/true) == Status::Ok); + std::vector row(static_cast(dims)); + for (int32_t r = 0; r < 12; ++r) + { + for (auto& v : row) + { + v = rng.NextFloat(); + } + CHECK(bank.Append(row.data(), dims, nullptr) == Status::Ok); + } + MemArchive archive; + CHECK(bank.Save(archive.Writer()) == Status::Ok); + const uint8_t* blob = archive.bytes.data(); + const int64_t blobBytes = static_cast(archive.bytes.size()); + ScratchArchiveInfo info; + CHECK(PeekScratchArchive(blob, blobBytes, &info) == Status::Ok); // warm + + const uint64_t allocsBefore = AllocationCount(); + { + ScopedRawNewTracking rawTracking; + for (int32_t i = 0; i < 20; ++i) + { + CHECK(PeekScratchArchive(blob, blobBytes, &info) == Status::Ok); + } + CHECK_MSG(rawTracking.Count() == 0, + "PeekScratchArchive allocated %llu time(s) outside the seam", + static_cast(rawTracking.Count())); + } + CHECK(AllocationCount() == allocsBefore); + } +} + static void TestScratchBanks() { Rng rng(0x5C247C4Bull); @@ -11574,6 +12044,16 @@ static void TestChannelQueryOnlyStorm() } }); + // Deterministic liveness gate: do not begin the measured loop until the writer has + // actually published a row past `appended`. An unsynchronized start races the reader's + // 300 fast iterations against OS scheduling of the writer thread -- under concurrent + // machine load the reader can finish first and the storm never exercises a concurrent + // publish at all. Bounded so a genuine append/publish regression fails loudly here + // instead of leaving the loop to run vacuously. + CHECK_MSG(WaitForPublishPastCount(fx.bank, appended), + "writer never published a row past appended before the reader began measuring " + "(count stayed at %d)", fx.bank.Count()); + std::vector tombs(ScratchBank::TombstoneWords(capacity), 0u); std::vector recomputed(static_cast(capacity) * cc); int32_t maxCountSeen = 0; @@ -11594,7 +12074,11 @@ static void TestChannelQueryOnlyStorm() iter, C); // The channel query over the fresh snapshot succeeds with finite scores (a torn / - // stale sub-norm would surface here as a non-finite or wrong score). + // stale sub-norm would surface here as a non-finite or wrong score). One CHECK per + // iteration covering every returned hit -- the hit count `n` itself moves with + // scheduling (how far the writer has appended by this iteration), so a per-hit CHECK + // would make the suite's total check count scheduling-dependent too. Every score is + // still examined; only the number of CHECK calls is now fixed at one per iteration. Workspace ws; Hit hits[10]; int32_t n = 0; @@ -11604,15 +12088,23 @@ static void TestChannelQueryOnlyStorm() params.segmentCount = 1; params.excludeBits = tombs.data(); CHECK(Query(snap, qbuf.F32(), params, ws, hits, &n) == Status::Ok); + bool allFinite = true; for (int32_t i = 0; i < n; ++i) { - CHECK(std::isfinite(hits[i].score)); + allFinite = allFinite && std::isfinite(hits[i].score); } + CHECK_MSG(allFinite, "channel query returned a non-finite score (iter %d, n %d)", + iter, n); } stop.store(true, std::memory_order_relaxed); writer.join(); // The reader must have observed the count ADVANCE past the initial rows, or the storm - // never exercised a concurrent publish (the P-1 defect this rewrite fixes). + // never exercised a concurrent publish (the P-1 defect this rewrite fixes). NOT implied + // by the wait above: the gate polls bank.Count() directly, while maxCountSeen is + // accumulated from snap.count via bank.Snapshot() -- a different read path. This is the + // check that catches a regression in Snapshot's own acquire ordering (Snapshot returning + // a stale count even though Count() itself has advanced); it is a live guard on a + // distinct path from the gate and must not be removed as redundant with it. CHECK_MSG(maxCountSeen > appended, "reader never observed a freshly-published row (count stayed at %d)", maxCountSeen); } @@ -11671,6 +12163,14 @@ static void TestChannelScopedReductionStorm() } }); + // Deterministic liveness gate: do not begin the measured loop until the writer has + // actually published a row past `appended` -- see WaitForPublishPastCount. Without it, + // the reader's 200 fast iterations can complete before the OS schedules the writer at + // all under concurrent load, and the storm never exercises a concurrent publish. + CHECK_MSG(WaitForPublishPastCount(fx.bank, appended), + "writer never published a row past appended before the reader began measuring " + "(count stayed at %d)", fx.bank.Count()); + std::vector tombs(ScratchBank::TombstoneWords(capacity), 0u); std::vector recomputed(static_cast(capacity) * cc); int32_t maxCountSeen = 0; @@ -11707,6 +12207,11 @@ static void TestChannelScopedReductionStorm() } stop.store(true, std::memory_order_relaxed); writer.join(); + // NOT implied by the wait above: the gate polls bank.Count() directly, while + // maxCountSeen is accumulated from snap.count via bank.Snapshot() -- a different read + // path. This is the check that catches a regression in Snapshot's own acquire ordering + // (Snapshot returning a stale count even though Count() itself has advanced); it is a + // live guard on a distinct path from the gate and must not be removed as redundant. CHECK_MSG(maxCountSeen > appended, "reader never observed a freshly-published row (count stayed at %d)", maxCountSeen); } @@ -12502,17 +13007,17 @@ static void TestPerChannelRecallOracle() } } -// [hole] Version/header coherence. version.h must declare v3.0.0 for the v3.0 release. RED now -// (it reads 2/5/0); the header is bumped separately. Kept as a standalone assertion so the release-version -// truth has a test, not just a header edit. +// Version/header coherence: version.h declares the release this tree IS. Kept as a +// standalone assertion so the release-version truth has a test, not just a header edit — +// the CHANGELOG's newest entry and these three numbers move together or the suite says so. static void TestVersionHeaderCoherence() { CHECK_MSG(SUPERFAISS_VERSION_MAJOR == 3, - "SUPERFAISS_VERSION_MAJOR should be 3 for v3.2.1, got %d", SUPERFAISS_VERSION_MAJOR); - CHECK_MSG(SUPERFAISS_VERSION_MINOR == 2, - "SUPERFAISS_VERSION_MINOR should be 2 for v3.2.1, got %d", SUPERFAISS_VERSION_MINOR); - CHECK_MSG(SUPERFAISS_VERSION_PATCH == 1, - "SUPERFAISS_VERSION_PATCH should be 1 for v3.2.1, got %d", SUPERFAISS_VERSION_PATCH); + "SUPERFAISS_VERSION_MAJOR should be 3 for v3.3.0, got %d", SUPERFAISS_VERSION_MAJOR); + CHECK_MSG(SUPERFAISS_VERSION_MINOR == 3, + "SUPERFAISS_VERSION_MINOR should be 3 for v3.3.0, got %d", SUPERFAISS_VERSION_MINOR); + CHECK_MSG(SUPERFAISS_VERSION_PATCH == 0, + "SUPERFAISS_VERSION_PATCH should be 0 for v3.3.0, got %d", SUPERFAISS_VERSION_PATCH); } // =========================================================================== @@ -13186,6 +13691,16 @@ static void TestRelabelExclusiveDrainStorm() } }); + // Deterministic liveness gate: do not begin the measured loop until the writer has + // actually published a row past `appended` -- see WaitForPublishPastCount. Without it, + // the reader's 300 fast pin/snapshot iterations can complete before the OS schedules the + // writer thread at all under concurrent load, and the storm never exercises a concurrent + // publish (or, since the writer's very first iteration is a relabel, a concurrent + // relabel either). + CHECK_MSG(WaitForPublishPastCount(bank, appended), + "writer never published a row past appended before the reader began measuring " + "(count stayed at %d)", bank.Count()); + std::vector tombs(ScratchBank::TombstoneWords(capacity), 0u); int32_t maxCountSeen = 0; for (int32_t iter = 0; iter < 300; ++iter) @@ -13213,6 +13728,10 @@ static void TestRelabelExclusiveDrainStorm() stop.store(true, std::memory_order_relaxed); writer.join(); + // The writer's own iteration order puts a relabel attempt at iter 0, before its first + // append attempt at iter 1 -- so the wait above (which requires at least one successful + // append) already implies at least one completed relabel by construction, and this check + // is deterministic as a consequence rather than independently scheduling-dependent. CHECK_MSG(relabelCount.load(std::memory_order_relaxed) > 0, "relabel storm: the writer never completed a relabel"); BankView finalSnap; @@ -13230,6 +13749,11 @@ static void TestRelabelExclusiveDrainStorm() CHECK_MSG(matchedSomeTarget, "relabel storm: the final channel table never left the fixture's initial (1-channel) " "table -- Relabel never mutated the bank under concurrent load"); + // NOT implied by the wait above: the gate polls bank.Count() directly, while + // maxCountSeen is accumulated from snap.count via bank.Snapshot() -- a different read + // path. This is the check that catches a regression in Snapshot's own acquire ordering + // (Snapshot returning a stale count even though Count() itself has advanced); it is a + // live guard on a distinct path from the gate and must not be removed as redundant. CHECK_MSG(maxCountSeen > appended, "relabel storm: reader never observed a freshly-published row (count stayed at %d)", maxCountSeen); @@ -15291,6 +15815,107 @@ static void TestM1TrustBoundaries() } } +// A caller-supplied neighbour value is used to index the neighbours array (MutualFilter) +// and the union-find scratch (ConnectedComponents). Values are validated for the -1 +// "no neighbour" sentinel; a value >= count is a malformed input that must degrade to a +// dropped edge, never an out-of-bounds read or write. BuildKnnNeighbors always produces +// in-range values, so this is reachable only through a hand-built neighbour list — the +// same trust posture the other core entry points hold on the indices they are handed. +static void TestGraphOutOfRangeNeighbours() +{ + Rng rng(0x60B5); + const int32_t dims = 8, count = 6, k = 2; + std::vector src; + for (int32_t i = 0; i < count * dims; ++i) src.push_back(rng.NextFloat()); + GBank b(src, count, dims, Quantization::Float32, Metric::L2); + + // A neighbour list holding out-of-range values: count (one past the end), a large + // positive, and INT32_MAX, mixed with the -1 sentinel. Every slot is poisoned; none + // may be dereferenced as an index. + std::vector neighbours(static_cast(count) * k); + for (int32_t i = 0; i < count; ++i) + { + neighbours[static_cast(i) * k + 0] = count; // exactly one past the end + neighbours[static_cast(i) * k + 1] = (i % 2) ? INT32_MAX : -1; + } + + std::vector flags(static_cast(count) * k, 0xEE); + CHECK(MutualFilter(count, k, neighbours.data(), flags.data()) == Status::Ok); + // No out-of-range value can be its own mutual: every flag is 0, and nothing crashed. + for (uint8_t f : flags) + { + CHECK(f == 0); + } + + // Force the union-find to CONFRONT the out-of-range neighbour: hand it a flag array + // with every slot marked mutual, so ConnectedComponents must act on each neighbour + // value, and a duplicate-group table whose every representative is INT32_MAX. With the + // < count guard both are dropped and each row is its own component. Without it, find() + // indexes parent[] with these values — undefined behaviour. This cell asserts the + // GUARDED result holds on poisoned input; it is not a crash reproduction, because an + // out-of-bounds access in a non-instrumented build is UB that neither reliably crashes + // nor changes this observable, so it cannot serve as the guard's oracle here. The + // address-sanitizer job in core CI is where the unguarded access is caught as a fault. + std::vector allMutual(static_cast(count) * k, 1); + std::vector badGroups(static_cast(count)); + for (int32_t i = 0; i < count; ++i) + { + badGroups[static_cast(i)] = INT32_MAX; // every representative wildly out of range + } + std::vector ids(static_cast(count), -999), uf(static_cast(count), 0); + CHECK(ConnectedComponents(count, k, neighbours.data(), allMutual.data(), badGroups.data(), + ids.data(), uf.data()) == Status::Ok); + // Every edge dropped (poisoned neighbours and poisoned groups alike), so each row is + // its own component — a defined, in-range result, no corruption, no crash. + for (int32_t r = 0; r < count; ++r) + { + CHECK(ids[r] == r); + } +} + +// The int8 channel-analytics legs score their sub-range through detail::DotI8I8, whose +// SIMD paths assume a length on the 16-byte int8 grid and carry no scalar tail. A +// validated bank's channel table is grid-aligned, so the archive loader rejects an +// off-grid table before it can be scored (covered elsewhere); this closes the same gap +// for a HAND-BUILT BankView that never went through validation, so the analytics +// functions themselves refuse rather than read SIMD-padded slop. +static void TestChannelAnalyticsOffGridRejected() +{ + Rng rng(0x0FF6); + const int32_t dims = 32, count = 8; // int8 grid is 16; 8 is deliberately off it + std::vector src; + for (int32_t i = 0; i < count * dims; ++i) src.push_back(rng.NextFloat()); + GBank b(src, count, dims, Quantization::Int8, Metric::Cosine); + + // A hand-built channel table with an off-grid range (offset 0, length 8) — a table a + // validated bank could never carry. + const ChannelInfo offGrid[1] = {{0, 8}}; + BankView view = b.view; + view.channels = offGrid; + view.channelCount = 1; + + Workspace ws; + std::vector qbuf(static_cast(count)); + std::vector hbuf(static_cast(count)); + std::vector nbuf(static_cast(count)); + float out = -1.0f; + CHECK(MeanNNCrossDeviceChannel(view, nullptr, view, nullptr, 0, qbuf.data(), + hbuf.data(), nbuf.data(), ws, &out) == Status::InvalidArgument); + CHECK(MaxNNCrossDeviceChannel(view, nullptr, view, nullptr, 0, qbuf.data(), + hbuf.data(), nbuf.data(), ws, &out) == Status::InvalidArgument); + + std::vector cA(static_cast(view.paddedDims)); + std::vector cB(static_cast(view.paddedDims)); + const int32_t idx[1] = {0}; + CHECK(CentroidDistanceCrossDeviceChannel(view, idx, 1, nullptr, nullptr, view, idx, 1, + nullptr, nullptr, Metric::Cosine, 0, cA.data(), cB.data(), &out) == + Status::InvalidArgument); + + std::vector cScratch(static_cast(view.paddedDims)); + CHECK(SpreadCrossDeviceChannel(view, idx, 1, nullptr, Reduce::Mean, 0, cScratch.data(), + &out) == Status::InvalidArgument); +} + // M1 / dim 2 (byte-confirm) + dim 7 (int8 same-decode-different-scale NOT unioned) — // grouping is FULL BYTE equality, not decode equality: a scalar-multiple pair that // decodes to the same direction but stores different int8 bytes/scale is a near- @@ -17864,6 +18489,7 @@ namespace {"scratch.h", "ScratchBank::GetChannels", AllocBinding::Trivial, "TestAllocFlatScratchAccessors", 1}, {"scratch.h", "ScratchBank::RecallReportStale", AllocBinding::Trivial, "TestAllocFlatScratchAccessors", 1}, + {"scratch.h", "PeekScratchArchive", AllocBinding::Binds, "TestPeekScratchArchive", 1}, // --- topk.h (6 entry points, all Trivial) --- {"topk.h", "Better", AllocBinding::Trivial, "TestAllocFlatHeaderOnlyMath", 1}, @@ -17899,7 +18525,7 @@ namespace {"alloc.h", 26}, {"analytics.h", 10}, {"bake.h", 5}, {"compose.h", 4}, {"graph.h", 4}, {"inspector_common.h", 1}, {"kernels.h", 36}, {"matching.h", 1}, {"novelty.h", 4}, {"pca.h", 2}, {"query.h", 5}, - {"scratch.h", 40}, {"superfaiss.h", 0}, {"topk.h", 6}, {"types.h", 8}, + {"scratch.h", 41}, {"superfaiss.h", 0}, {"topk.h", 6}, {"types.h", 8}, {"validate.h", 5}, {"version.h", 0}, }; @@ -18716,7 +19342,7 @@ static void TestAllocFlatBakeAndPca() const int32_t componentCount = 2, iterations = 4; std::vector mean(static_cast(cosineBank.view.dims)); std::vector components(static_cast(componentCount) * cosineBank.view.dims); - std::vector scratch(static_cast(cosineBank.view.dims)); + std::vector scratch(static_cast(cosineBank.view.dims)); std::vector coords(static_cast(count) * componentCount); auto driveOnce = [&]() { @@ -19254,6 +19880,7 @@ int main() TestIntersect(); TestPca(); TestSegmentedScan(); + TestSegmentedEqualsWholeRowAcrossWidths(); TestPerChannelCosine(); TestScratchBanks(); TestScratchChannelCreateRejections(); @@ -19363,6 +19990,8 @@ int main() // V3.2 red suite: Bank Inspector I — module M1 graph.h. TestM1TrustBoundaries(); + TestGraphOutOfRangeNeighbours(); + TestChannelAnalyticsOffGridRejected(); TestM1DuplicateGroupingByteConfirm(); TestM1EdgesExactAcrossMetrics(); TestM1DuplicateUnionConstructionCounts(); @@ -19394,6 +20023,13 @@ int main() TestS1FlatAllocationKthNeighborDistanceProbe(); TestS1FlatAllocationMutualNearestMatches(); + // V3.3.0: public-path geometry ceilings, and the archive peek the host trailer + // validation is built on. + TestScratchGeometryCeilings(); + TestScratchLoadValidatesRetainedRegion(); + TestEmptyCosineChannelBankValidates(); + TestPeekScratchArchive(); + // Coverage audit §7 -- the structural registry guard. Runs last so every // cell it references above has already executed at least once. TestAllocationCellRegistryComplete();