sigker: delegate randomized-signature encode to the ndarray SIMD sweep - #1166
Conversation
Closes the follow-up ndarray PR #294 explicitly deferred ("Wiring sigker to actually call this is out of scope"). Until now the primitive was dead code: ndarray shipped and benchmarked the SIMD recurrence, but RandomizedSignatureBuilder::encode still ran its own row-major scalar loop, so the measured 2.0x-3.8x speedup was unrealised. encode now delegates to ndarray::hpc::randomized_signature::randomized_signature_sweep. The buffer layout was already identical (matrices[i*k*k + row*k + col], biases concatenated) and so is the |dx_i| < 1e-15 skip, so the delegation carries the numerics unchanged. This mirrors the precedent kernel.rs:35 already set for PR #293's signature_pde_sweep. BEHAVIOUR CHANGE, deliberate and documented. Both of sigker's stricter caller-facing asserts (non-empty path; path[0].len() == path_dim) are KEPT ahead of the delegation — the ndarray primitive accepts an empty path and returns the zero state, so dropping them would have been a silent contract change. But the primitive additionally asserts that EVERY path point matches path[0]'s dimension. sigker previously checked only path[0], so a ragged path used to produce a silently truncated wrong signature and now panics. That is a strict improvement; it is recorded in a # Panics section and pinned by a test. Tests (both new, both discriminating): - encode_matches_scalar_reference_within_tolerance — parity against an inlined scalar oracle transcribed from the pre-delegation loop, at 1e-9 relative tolerance rather than bit-equality (the SIMD GEMV reduces eight partial sums and fuses products, and reduce_sum order differs per backend). Catches drift in the recurrence, the skip epsilon, the activation, or the buffer layout. - ragged_path_now_panics — the exact input that did NOT panic before this change and produced a wrong signature instead. Board hygiene in the same commit, per the Mandatory Board-Hygiene Rule: - EPIPHANIES: E-A-SKETCH-THAT-MISSED-TWICE-WILL-MISS-A-THIRD-TIME-1. The contract doc's W1.5 lane-type sketches are 3-for-3 wrong (#6 and #7 sketched f32/F32x16 against an f64 consumer; #8 sketches I16x16 where log_signature.rs is Vec<f64> throughout). The entry also records that W1.5-#8 is NOT a well-shaped SIMD primitive at all — its real cost centres are recursive sparse bracket expansion and data-dependent scatter-adds, not a dense kernel — and that the "7-13x compression" headline is asymptotic, per the crate's own named falsifier test. - TECH_DEBT: TD-SIGKER-CLIPPY-RED-ON-BASE-1. sigker fails clippy --all-targets -D warnings at signature.rs:133, and did so BEFORE this change (verified by stashing and re-running against base). Not fixed here to avoid widening a wiring PR; the 3-line patch is in the entry. Root cause is that sigker is workspace-excluded, so root-level clippy never reaches it. Verified: cargo test --manifest-path crates/sigker/Cargo.toml — 62 passed, 0 failed; cargo fmt --check clean; clippy clean on every site this diff touches (only the pre-existing signature.rs:133 remains, tracked above). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WkNBjHc2e3zuyz9i8qJEv
|
Warning Review limit reachedNext included review available in 5 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 87 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (4)
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_377fbce7-e47f-4562-8c47-be08d0de482d) |
sigker's 62 tests have never run in CI. `rust-test.yml` enumerates workspace-excluded crates one scoped step at a time — deepnsm, deepnsm-v2, supervisor, causal-edge, bgz-tensor, ogar, weather-poc, jc — and sigker was simply never added to that list. That matters most right now: BOTH of the crate's W1.5 consumer wirings, which are its whole current purpose, were invisible to CI. kernel.rs's signature_pde_sweep delegation (ndarray PR #293) has been unguarded since it landed, and randomized.rs's randomized_signature_sweep delegation is added in the preceding commit of this same PR. A parity test against a scalar oracle that CI never executes is not a gate. This is the same "blind gate" the workflow's own comments say this repo has closed "one crate at a time" for five other excluded crates. One scoped step arms it. Deliberately TESTS ONLY, following the causal-edge precedent immediately above it in the same file: a `clippy -D warnings` step would be red on arrival, because sigker carries one pre-existing finding at signature.rs:133 (needs_range_loop) that reproduces identically on the base commit and is not in the randomized.rs this PR touches. Gating it here would fail this PR for a defect it did not introduce. It stays recorded in TECH_DEBT TD-SIGKER-CLIPPY-RED-ON-BASE-1, with its patch, and that entry is extended here to cover the test half of the same exclusion blind spot. Verified: YAML parses; cargo test --manifest-path crates/sigker/Cargo.toml — 62 passed, 0 failed; cargo fmt --check clean; supersession index regenerated after the board write and confirmed current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WkNBjHc2e3zuyz9i8qJEv
…path-validation-170zcy
…he lint name Merging origin/main brought in the new `citation-decay` gate (#1168/#1170), which fired on exactly one new citation — mine: TECH_DEBT.md -> crates/sigker/src/signature.rs:133 anchor=symbol:"needs_range_loop" (anchor absent within +-3 lines) The gate is right and the finding is a real defect, not a false positive. `needs_range_loop` is clippy's LINT NAME; it appears nowhere in the cited source. The gate extracts the backticked token nearest the citation as the anchor, so the lint name won that race and then could not be found at the line it supposedly addressed. A citation whose anchor is absent from its target is unverifiable by construction — which is the whole point of the gate. Re-anchored on `for flat in 0..len`, which is the actual text at signature.rs:133, and the lint name is now named in prose instead. The entry also records why, so the next reader does not "fix" it back. Local gate runs after the merge, all green: - citation_decay.py --self-test: passed (both halves) - citation_decay.py --since <base>: 0 new decays (148 pre-existing backlog, unchanged and not failing) - append_only_gate.py --self-test: passed, 7 cases - append_only_gate.py origin/main: no protected file shrank, 9 checked (TECH_DEBT 4161 -> 4212, +51) - supersession_index.py: regenerated after the merge, byte-identical - plan_dids.py: no added plans, nothing to check - cargo test --manifest-path crates/sigker/Cargo.toml: 62 passed, 0 failed - cargo fmt --check: clean Also merges origin/main (f30e300..ac9148f) to clear the merge conflict that made this PR un-mergeable. The merge itself was clean; EPIPHANIES.md auto-merged, both sides having prepended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016WkNBjHc2e3zuyz9i8qJEv
Summary
Closes the follow-up ndarray PR #294 explicitly deferred ("Wiring sigker to actually call this is out of scope").
Until now the primitive was dead code: ndarray shipped, tested and benchmarked the SIMD recurrence, but
RandomizedSignatureBuilder::encodestill ran its own row-major scalar loop — so the measured 2.0×–3.8× speedup was unrealised.encodenow delegates tondarray::hpc::randomized_signature::randomized_signature_sweep, mirroring the precedentkernel.rs:35already set for PR #293'ssignature_pde_sweep.The buffer layout was already identical (
matrices[i*k*k + row*k + col], biases concatenated) and so is the|Δx_i| < 1e-15skip, so the delegation carries sigker's numerics unchanged.Behaviour change — deliberate, documented, tested
Both of sigker's stricter caller-facing asserts are kept ahead of the delegation:
The ndarray primitive accepts an empty path and returns the zero state, so dropping these would have been a silent contract change.
But the primitive additionally asserts that every path point matches
path[0]'s dimension. sigker previously checked onlypath[0], so a ragged path used to produce a silently truncated wrong signature and now panics. That is a strict improvement, recorded in a# Panicssection and pinned by a test — not hidden.Tests
Both new, both discriminating:
encode_matches_scalar_reference_within_tolerance— parity against an inlined scalar oracle transcribed from the pre-delegation loop, at1e-9relative tolerance rather than bit-equality. The SIMD GEMV reduces eight partial sums and fuses products, andreduce_sumorder differs per backend, so bit-equality is the wrong contract by construction. Catches drift in the recurrence, the skip epsilon, the activation, or the buffer layout.ragged_path_now_panics— the exact input that did not panic before this change and produced a wrong signature instead.Board hygiene (same commit, per the Mandatory Board-Hygiene Rule)
EPIPHANIES—E-A-SKETCH-THAT-MISSED-TWICE-WILL-MISS-A-THIRD-TIME-1. ndarray's W1.5 lane-type sketches are 3-for-3 wrong:&[F32x16]f64F64x8F32x16statef64/Vec<f64>F64x8I16x16stateVec<f64>(log_signature.rs:272)The entry also records a stronger finding: W1.5-#8 is not a well-shaped SIMD primitive at all. "Pack/unpack" names no operation that exists; the real cost centres are
bracket_expansion(:227-256, recursive sparse outer-product with sort/coalesce) and a data-dependent scatter-add peel (:368-386) — the opposite of a dense fixed-lane kernel. The basis is generated on the fly by Duval's algorithm (:164,:346) with no table to consume. The actual bottleneck the module names itself (:100) is the Magnus expansion intensor_multiply, a different primitive. And the doc's "7-13× compression" headline is asymptotic — the crate's own named falsifiercompression_at_shallow_depth_is_far_below_the_headline(:744) pins ~2.1× at d=4,N=2.TECH_DEBT—TD-SIGKER-CLIPPY-RED-ON-BASE-1. sigker failsclippy --all-targets -D warningsatsignature.rs:133, and did so before this change (verified by stashing the tree and re-running against base). Not fixed here — that would widen a wiring PR — but the 3-line patch is in the entry. Root cause worth noting: sigker is workspace-excluded, so root-levelcargo clippynever reaches it. The other excluded crates likely share the blind spot.Test plan
cargo test --manifest-path crates/sigker/Cargo.toml— 62 passed, 0 failedcargo fmt --manifest-path crates/sigker/Cargo.toml -- --check— cleancargo clippy --manifest-path crates/sigker/Cargo.toml --all-targets -- -D warnings— clean on every site this diff touches; only the pre-existingsignature.rs:133remains, tracked aboveNot changed
examples/randomized_signature_demo.rsandcubature_vs_randomized.rsonly callencode/newwith no internal-field access, so they need no edit.🤖 Generated with Claude Code
https://claude.ai/code/session_016WkNBjHc2e3zuyz9i8qJEv
Generated by Claude Code