Repository navigation
fix: reject non-finite input, isolate FastScan estimator helpers, and harden release gates - #195
Merged
Merged
Conversation
…r count Graph construction can leave centroids unreachable, so points in those clusters are never returned and an nprobe close to the cluster count raises instead of probing every cluster. State the intended regime and point at flat and flat_rabitq for exhaustive probing.
The batch correction in estimator_kernels.hpp and qg_estimator.hpp instantiated Eigen assignment loops from the generic, AVX2 and AVX-512 translation units. Below -O3 those helpers survive as identically named weak symbols with different instruction sets, and only the static-archive member order kept the generic copy in front of the ISA copies. Write the 32-lane corrections as plain loops so each backend vectorizes them with its own flags and no ISA source emits a shared Eigen helper. check-isa-symbols.sh now passes at -O1 and -O2 -g as well as -O3; search outputs on identical saved indexes are bit-identical before and after.
Each wheel gate was skipped whenever its build job was skipped, and the build job is skipped when the prepare job fails. A skipped required check satisfies branch protection, so a failed eligibility check waived the wheel builds, the installed-wheel suite and the typing checks on a pull request. Run the gates with always() and decide from the prepare, changes and build results, matching the other aggregator jobs; the contract test now exercises every input combination of the shared verdict script.
… point IVF, HNSW and SymphonyQG accepted NaN, infinite and arbitrarily large coordinates in data, centroids and queries. The core quantizer then converted NaN to int (undefined behaviour, reported by UBSan's float-cast-overflow check), stored NaN factors, and in one-bit high-accuracy IVF search returned a poisoned point first with a NaN distance even after remove(). The stable core API and the clustering APIs already rejected such input. Add utils/input_checks.hpp with a finiteness scan and the clustering coordinate bound, call it at construct, add and search entry points before any state changes, and reject non-finite queries in Lut. Enable float-cast-overflow and float-divide-by-zero in the sanitizer build, since -fsanitize=undefined does not include them. Document the contract in the usage guide and the Python docstrings, and cover each entry point with C++ and Python regression tests.
lut.hpp includes utils/input_checks.hpp, so installed consumers of the FastScan headers need it too.
…orts The installed-consumer options check hard-coded -fsanitize=address,undefined; the sanitizer build now also enables float-cast-overflow and float-divide-by-zero.
This was referenced Oct 31, 2026
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
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.
Three fixes from a component audit of the library, plus one documentation note. The commits are independent and can be reviewed separately.
Reject non-finite and oversized coordinates (
fix:)IVF, HNSW and SymphonyQG accepted NaN, infinite and arbitrarily large coordinates in data, centroids and queries. The quantizer then converted NaN to
int(UB, reported by UBSanfloat-cast-overflow), stored NaN factors, and one-bit high-accuracy IVF search returned the poisoned point first with a NaN distance, even afterremove(). The stable core API and the clustering APIs already rejected the same input.utils/input_checks.hpp: finiteness scan (same integer reduction the v1 API used, which now calls it) and the clustering coordinate boundsqrt(sqrt(FLT_MAX) / (64 * dim));static inlinebecause ISA-flagged sources include it.construct,add,searchand batch search after the existing shape checks and before any state change;Lutrejects non-finite rotated queries (coversBatchQuery<double>).RABITQ_ENABLE_SANITIZERSnow addsfloat-cast-overflow,float-divide-by-zero; neither is part of-fsanitize=undefined, so the existing sanitizer job could not see this class of bug.usage.mdand the three index docstrings (.pyikept in sync); C++ tests per entry point and Python tests for build/add/search.QGAssignerTest.PropagatesSearchFailureFromWorkersinjected its worker failure with 1e19 coordinates; the per-query entry the workers call now rejects them withinvalid_argument, so the test asserts that type and message. The failure still originates inside the parallel region.FastScan estimator Eigen isolation (
fix(fastscan):)estimator_kernels.hppandqg_estimator.hppinstantiated Eigen assignment loops from the generic, AVX2 and AVX-512 translation units. Below-O3those survive as identically named weak symbols with zmm/ymm/SSE bodies, and only the static-archive member order kept the generic copy in front (the-O1sanitizer job compiles exactly that configuration). The 32-lane corrections are now plain loops that each backend vectorizes with its own flags (verified: xmm/ymm/zmm arithmetic in the respective objects at-O3).check-isa-symbols.shpasses at-O1and-O2 -gas well as-O3;development.mdupdated accordingly.Release wheel gates (
ci:)Each wheel gate was skipped whenever its build was skipped, and the build is skipped when
preparefails; a skipped required check satisfies branch protection, so a failed eligibility check on a PR waived the wheel builds, the installed-wheel suite and the typing checks. The gates now run withalways()and decide from theprepare,changesand build results, like the other aggregator jobs. The contract test runs the shared verdict script over all 900 input combinations and fails on the previous workflow.Reviewer note: on PRs without wheel-related changes the four wheel checks now show green instead of skipped.
Docs (
docs(ivf):)The
hnswinitializer suitsnprobefar below the cluster count: graph construction can leave centroids unreachable, so points in those clusters are not returned at anynprobe, and annprobenear the cluster count raises instead of probing every cluster.Verification
check-format.sh,check-python.sh, ruff on.github/scripts,git diff --check.ctest --no-tests=error434/434;check-isa-symbols.shat-O3,-O1,-O2 -g.pointer-overflowdiagnostic suppressed; unrelated).check-tidy.sh build-tidy(Clang 23).pytest tests/python837 passed, 2 skipped (wheel-only); editor-support, mypy and pyright tests.mkdocs build --strict;.github/scriptsunittest suite 60/60.Not verified locally: hosted-runner behaviour of the gate change (its own CI run on this PR is the check), MSVC, NEON.