Skip to content

sigker: delegate randomized-signature encode to the ndarray SIMD sweep - #1166

Merged
AdaWorldAPI merged 4 commits into
mainfrom
claude/pr-294-ragged-path-validation-170zcy
Sep 4, 2026
Merged

sigker: delegate randomized-signature encode to the ndarray SIMD sweep#1166
AdaWorldAPI merged 4 commits into
mainfrom
claude/pr-294-ragged-path-validation-170zcy

Conversation

@AdaWorldAPI

Copy link
Copy Markdown
Owner

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::encode still ran its own row-major scalar loop — so the measured 2.0×–3.8× speedup was unrealised. encode now delegates to ndarray::hpc::randomized_signature::randomized_signature_sweep, mirroring the precedent kernel.rs:35 already set for PR #293's signature_pde_sweep.

The buffer layout was already identical (matrices[i*k*k + row*k + col], biases concatenated) and so is the |Δx_i| < 1e-15 skip, 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:

assert!(!path.is_empty(), "path must have ≥1 point");
assert_eq!(path[0].len(), self.path_dim, "path point dim mismatch");

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 only path[0], so a ragged path used to produce a silently truncated wrong signature and now panics. That is a strict improvement, recorded in a # Panics section 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, 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, 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)

EPIPHANIESE-A-SKETCH-THAT-MISSED-TWICE-WILL-MISS-A-THIRD-TIME-1. ndarray's W1.5 lane-type sketches are 3-for-3 wrong:

item doc sketch real consumer shipped as
#6 signature_pde &[F32x16] f64 F64x8
#7 randomized F32x16 state f64/Vec<f64> F64x8
#8 lyndon-pack I16x16 state Vec<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 in tensor_multiply, a different primitive. And the doc's "7-13× compression" headline is asymptotic — the crate's own named falsifier compression_at_shallow_depth_is_far_below_the_headline (:744) pins ~2.1× at d=4,N=2.

TECH_DEBTTD-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 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-level cargo clippy never reaches it. The other excluded crates likely share the blind spot.

Test plan

  • cargo test --manifest-path crates/sigker/Cargo.toml62 passed, 0 failed
  • cargo fmt --manifest-path crates/sigker/Cargo.toml -- --check — clean
  • cargo clippy --manifest-path crates/sigker/Cargo.toml --all-targets -- -D warnings — clean on every site this diff touches; only the pre-existing signature.rs:133 remains, tracked above
  • Board files verified append-only (insertions only, zero deletions)

Not changed

examples/randomized_signature_demo.rs and cubature_vs_randomized.rs only call encode / new with no internal-field access, so they need no edit.

🤖 Generated with Claude Code

https://claude.ai/code/session_016WkNBjHc2e3zuyz9i8qJEv


Generated by Claude Code

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
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 5 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: d811dad2-6e17-47ae-8c91-52fb07445628

📥 Commits

Reviewing files that changed from the base of the PR and between ac9148f and 47c9c3d.

📒 Files selected for processing (4)
  • .claude/board/EPIPHANIES.md
  • .claude/board/TECH_DEBT.md
  • .github/workflows/rust-test.yml
  • crates/sigker/src/randomized.rs

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Sep 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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
…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
@AdaWorldAPI
AdaWorldAPI marked this pull request as ready for review September 4, 2026 03:57
@AdaWorldAPI
AdaWorldAPI merged commit 1c696b0 into main Sep 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants