feat(diskann): optional lattice-embed backend for f32 distance kernels, plus a native cross-dimension parity test - #766
Open
ohdearquant wants to merge 3 commits into
Open
Conversation
…d a feature Adds an off-by-default `lattice-simd` backend for `l2_squared` and `inner_product`, ahead of the existing wasm32 SIMD128, SimSIMD and scalar arms. The arms stay mutually exclusive and exhaustive, so exactly one compiles for any feature/target combination and default builds are unchanged. Also adds `backend_matches_scalar_reference`, a cross-dimension parity test against naive scalar references. The existing cross-dimension tests are gated on wasm32 + simd128, so no native backend was checked above dim 3. The new test is backend-independent and covers the SimSIMD, scalar and lattice paths alike.
ohdearquant
marked this pull request as ready for review
August 3, 2026 18:08
The cross-dimension parity test checks numerical equivalence against a naive scalar oracle, which a lattice-simd build that silently fell back to the scalar/native kernel would still pass. Add a backend-selection seam (atomics set only from inside the lattice-simd dispatch arms, alongside the real kernel call) and a lattice-simd-gated test that fails if either arm's lattice call is replaced by a fallback route. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Move the lattice-embed dispatch calls for l2_squared and inner_product into dedicated wrapper functions (l2_lattice, inner_lattice) that are the sole callers of the lattice-embed kernels. The test witness that confirms backend selection is now set inside these wrappers, after the real kernel call returns, so a dispatch arm that swaps the wrapper call for a scalar/native fallback also loses the witness. Switch the witness flags from process-global atomics to thread-locals so a concurrently running sibling test cannot set them between this test's reset and its assert. Also correct the lattice-simd feature comment (the other backends' helper functions still compile; only the dispatch arm changes) and the lattice-embed dependency comment (blake3, a transitive dependency, still needs a C toolchain on AArch64).
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.
What this is, stated plainly
A fourth backend for
ruvector-diskann's two f32 distance kernels, behind an off-by-defaultlattice-simdfeature.This one is a substitution, not a gap-fill, and the difference is worth being clear about.
The wasm argument that motivates the same feature in
ruvector-coredoes not transfer here:ruvector-diskannalready has hand-writtensimd128kernels for both metrics, so wasm is coveredand lattice is not filling a hole. On native, SimSIMD already vectorizes. This is one SIMD
implementation swapped for another, and it should be judged on what the swap buys rather than on a
speed claim.
What the swap buys
A pure-Rust dependency in place of a C one.
simsimdbuilds its kernels throughbuild.rs->cc::Build->c/lib.c, so enablingsimdhere needs a working C toolchain at build time.lattice-embed's kernels are Ruststd::archintrinsics with runtime dispatch and no buildscript of their own. One honesty note on the toolchain claim: this does not make the enabled
build C-free, because
lattice-embed's mandatoryblake3dependency compiles a C NEON sourcethrough its build script on aarch64 targets, so cross-compiling with
lattice-simdenabledstill needs a target C compiler there. The swap removes the C dependency from the distance
kernels themselves, not from the feature's whole dependency tree.
One dependency entry rather than a target-gated one.
simsimdis declared under[target.'cfg(not(target_arch = "wasm32"))'.dependencies]because it does not build for wasm32.lattice-embeddoes, so it is a plain dependency.One kernel set across the workspace. If
ruvector-coreroutes through lattice, leavingdiskannon a different SIMD library means two implementations of the same arithmetic, withindependent rounding behaviour, inside one search path.
On performance claims. When this was first opened, no measurement was attached: at the time
the host could not certify A/B deltas in the plausible range (an A/A control on byte-identical
source produced differences up to 9.97%). The measurement section added later, below, supersedes
this position and states its own conditions and limits.
The change
l2_squaredandinner_producteach gain alattice-simdarm ahead of the existing three. Thearms stay mutually exclusive and exhaustive, so exactly one route is selected for any
feature/target combination (the other backends' helper functions still compile; only the
dispatch arm actually called changes):
lattice-simdlattice_embed::simdsimd128wasm_simd128_*simdDefault builds (
default = []) are untouched and use the scalar path;simd(SimSIMD)is opt-in.
SpatialSimilarity::inneris a plain alias fordot— it forwards directly, with no1 - xtransform — so the negation both paths apply is over the same raw dot product. I checked that at
the SimSIMD source rather than inferring it from the method name, because had the two differed by
a constant, the scalar fallback sitting beside it would already disagree with the SimSIMD path
today.
scalar_inner_productbecomes unreachable in alattice-simdbuild. It is annotated#[cfg_attr(feature = "lattice-simd", allow(dead_code))]rather than deleted or madepub: it isstill the fallback the
simdbackend calls when SimSIMD returnsNone. (The parity test'soracle is a separate local
naive_inner_product, not this function.)A test gap this turned up, independent of lattice
The cross-dimension parity tests in this file are gated on
#[cfg(all(test, target_arch = "wasm32", target_feature = "simd128"))]. They are good tests: a14-dimension grid straddling lane widths and remainders, with a documented tolerance. They only
ever run for wasm32.
The native backends' fixed-value checks all sat at dim 3 — below every lane width in play, so
they exercise a remainder loop and nothing else. (An identical-vectors check did run at dim 128,
but identical inputs cannot catch a wrong-value bug; no prior non-identical, reference-checked
native test exercised dimensions above 3.)
I did not want to assert the consequence of that without measuring it, so I injected a
dropped-remainder bug — truncating the distance computation to the first 3 lanes, which is exactly
correct at dim <= 3 and wrong above it — and ran the pre-existing suite against it:
The load-bearing result there is the pass/fail column, not the clock: the suite does not catch the
bug. On the wall times, one scope note — those two runs were sequential on a machine I did not
hold exclusively, so treat them as an order-of-magnitude observation rather than a controlled
timing measurement. The ratio is far too large to be contention, but I would rather bound the
claim than have it read as a benchmark.
That collapse is still the interesting part: those tests build indexes and drive the
wide-dimension distance path constantly, they simply never compare a value against a reference,
so an incorrect result above dim 3 has nothing asserting against it.
backend_matches_scalar_referencecloses that, and fails on that same injected bug. It isbackend-independent by construction, so it covers the SimSIMD path shipping today, the scalar
path, and the lattice path alike, and it keeps its value in this file whether or not you take the
rest of this PR. Its references are naive single-pass loops rather than the crate's own
scalar_*functions, which are themselves 4-accumulator implementations — using those as the reference would
let a shared reduction-order bug pass.
Verification
test --features lattice-simd --libtest --features simd --lib(SimSIMD, opt-in)test --lib(scalar, the default)clippy --features lattice-simd --all-targets -- -D warningsclippy --all-targets -- -D warningsThe scalar-oracle parity test runs in every configuration (named in each arm's output), which is
what shows it is backend-independent rather than compiled into one configuration; the
lattice-simdarm's one extra test is the cfg-gated routing witness.Mutation-checked per adapter, not per patch, since mutating a patch as a unit only certifies
its best line:
lattice-simdarmsquared_euclidean_distance->euclidean_distanceEach mutation leaving the default arm green is what proves it stayed inside its own
cfgblockinstead of breaking the crate outright.
Two consequences worth stating
This resolves a second copy of
lattice-embedinto the workspace.ruvector-corepins0.6.1; this pins0.7.1, which is the version the other in-flight lattice PRs move to.^0.6and
^0.7cannot unify, so until those land, a build enabling bothruvector-core/lattice-embeddingsand
ruvector-diskann/lattice-simdcompileslattice-embedtwice. Verified withcargo treerather than read off the lockfile. Whichever of these PRs lands second will want a lockfile
rebase.
Debug-profile test time differed noticeably between arms (the lattice arm ran longer than the
SimSIMD arm). I am deliberately not putting numbers on that: the arms ran sequentially on a shared
machine under varying load, it is a debug profile, and I have not controlled it. Flagging it only
so it is not a surprise, and it is worth a release-profile check before adoption.
Cost
lattice-embedrequires Rust >= 1.93. Enabling this feature raises the effective MSRV for whoeverturns it on. The default build is unaffected and stays on the workspace MSRV. That is the real
price and it is why the feature is opt-in.
Measurement
When this section was first written the crate had no benchmark to run, and the
position taken was that the harness should be its own reviewable change. That
harness is now #783; the numbers below come from cherry-picking #783's commit
onto this PR's head locally (clean pick). If #783 lands first, this branch
measures as-is.
Apple silicon Mac mini, macOS, aarch64, rustc 1.93.0,
cargo bench -p ruvector-diskann --bench distance, Criterion--measurement-time 10. Samecommit both phases, only the feature flag differs. This crate's
defaultisempty, so off is the scalar path. CPU idle sampled every 20s inside each
measured phase: off minimum 85%, on minimum 78% — both phases quiet.
l2_squared/384l2_squared/1536inner_product/384inner_product/1536scalar_l2_squared/1536pq_asymmetric_distance/1536(128/768 dims behave the same as their neighbours; all p = 0.00; seeded-random
inputs.)
The last two rows are the table's own controls, and they are why the first
four are believable.
scalar_l2_squaredis deliberately not routed by thisfeature and moves only ~3%;
pq_asymmetric_distanceis untouched and movesunder 1%. If the harness or the machine had shifted between phases, those two
rows are where it would show.
Kernel-level numbers only: nothing here measures DiskANN search end to end,
and the sibling PRs show the kernel-to-end-to-end ratio varies widely by
crate. The feature stays off by default.
Note on this PR's CI.
Tests (core-and-rest)is red on every branch in this repository,including
main: the job is cancelled at its 240-minute cap while still compiling and neverreaches the test phase. #786 restores the exclusion list that the shard's
packages:value losesto a shell comment, #784 unblocks the
ruvector-filtertest target that the compiler cannotfinish, and #787 fixes a deadlock waiting behind both. That failure is not caused by this branch.