Skip to content
This repository was archived by the owner on Aug 20, 2026. It is now read-only.

fix: bump impl submodule pin to the RLS broadcast-stride fix - #64

Merged
balbasty merged 1 commit into
mainfrom
claude/62-bump-impl-pin
Aug 2, 2026
Merged

fix: bump impl submodule pin to the RLS broadcast-stride fix#64
balbasty merged 1 commit into
mainfrom
claude/62-bump-impl-pin

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

Summary

Bumps the impl submodule pin to fastfields-cpu-impl#48, which
fixes the bug reported in #62.

Root cause (precise): field_matvec_rls/field_diag_rls/
field_relax_rls's wc == 1 broadcast path (single weight shared
across all nc field channels, for absolute/membrane/bending)
computed the weight tensor's per-channel stride (wsc) from its
real, contiguous last-dim stride, then indexed wgt[wsc*c] per field
channel inside the cpu-impl loop. That indexing is correct for the
genuine per-channel (JRLS, wc == nc) path, but wrong for RLS: since
the weight tensor genuinely has only one element in that dimension,
reading wgt[wsc*c] for c > 0 walks off the wc=1 buffer into the
next spatial voxel's weight value (or out of bounds at the very
last voxel, landing on zero-initialized memory — the -0 the issue's
order=1 failure reported). This explains all three failing orders
(order=1 absolute-only, order=2 membrane, order=3 bending) —
it's independent of, and distinct from, kernels#40 (which only ever
touched the order=3 bending kernel table).

Fixed by forcing wsc = 0 for the nine RLS functions (matvec/
diag/relax_ × absolute/membrane/bending), since they are
only ever dispatched to when wc == 1 — see
fastfields-cpu-impl#48 for the full writeup, the root-causing
harness, and confirmation that flow_matvec_rls and friends are
not affected (flow's weight map is architecturally always wc=1,
enforced at the cpu-lib API boundary, and its kernels never index by
a per-component stride).

A second, distinct bug was found while tracing this (the _jrls
functions silently ignore per-channel weight variation) — it's
real but untested by the current suite, needs kernel-level changes,
and is out of scope here; tracked as a follow-up issue.

Verified

make -C . test CXX=clang++
running ./build/test_distance checks: 2352, failures: 0 PASSED
running ./build/test_distance_mesh checks: 4622, failures: 0 PASSED
running ./build/test_distance_spline checks: 704, failures: 0 PASSED
running ./build/test_posdef checks: 4012, failures: 0 PASSED
running ./build/test_pushpull checks: 308, failures: 0 PASSED
running ./build/test_pushpull_backward checks: 6381, failures: 0 PASSED
running ./build/test_reg_field checks: 8440, failures: 0 PASSED (was 29 failing)
running ./build/test_reg_flow checks: 11415, failures: 0 PASSED
running ./build/test_reg_op checks: 186, failures: 0 PASSED
running ./build/test_resize checks: 630, failures: 0 PASSED
running ./build/test_restrict checks: 65, failures: 0 PASSED
running ./build/test_splinc checks: 4577, failures: 0 PASSED

All 12 suites green, 0 failures across 43,692 checks.

Closes#62

Workstream: claude-jitfields-to-fastfields

🤖 Generated with Claude Code


Generated by Claude Code

Pulls in fastfields-cpu-impl#48, which forces the RLS weight-channel
stride (wsc) to 0 in matvec/diag/relax_rls for absolute/membrane/
bending instead of deriving it from the wgt tensor's real (size-1)
last-dim stride -- the latter let wgt[wsc*c] walk off the wc=1
buffer into the next spatial voxel's weight for field channels c>0,
which is what run_2d_matvec_rls_unit_weight (added in #59, the
regression test for kernels#40) was catching as 29 mismatched
elements in test_reg_field.
Verified: `make -C . test CXX=clang++` -- all 12 suites pass, 0
failures. test_reg_field: 8440 checks, 0 failures (was 29 failing).
test_reg_flow: 11415 checks, 0 failures (flow's RLS path was never
affected -- see fastfields-cpu-impl#48 for why).
Closes#62
@balbastybalbasty added bug Something isn't working claude-jitfields-to-fastfields labels Aug 2, 2026 — with Claude
@balbasty
balbasty merged commit 6b9cfec into mainAug 2, 2026
5 checks passed
@balbasty
balbasty deleted the claude/62-bump-impl-pin branch August 2, 2026 08:27
@balbastyClaude

Copy link
Copy Markdown
ContributorAuthor

Review summary (triage, jitfields-to-fastfields): merged as squash. Closes #62.

impl pin bumped to pick up cpu-impl#48's fix. CI genuinely green across all 3 bound-policy matrix legs (bounds=static/dynamic/cuda-default), not just claimed — confirms main is no longer red: test_reg_field now passes all 8440 checks (was 29 failing), test_reg_flow unaffected (11415/11415, confirming flow was never broken — flow's RLS is architecturally always wc=1 enforced by CHECK_SAME, so no equivalent broadcast bug there).

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

balbasty added a commit that referenced this pull request Aug 2, 2026
…ib#15) (#68)
Adds the three pieces of fastfields-lib#15's automation to this repo:
* submodule-staleness.yaml -- calls fastfields/.github's reusable
cpp-submodule-staleness.yml on every push/PR, reporting how far the
`impl` pin is behind fastfields-cpu-impl's main as a non-blocking
::warning:: + PR comment. Verified against this repo's OWN current state:
the `impl` pin right now (f51a9b9, set by #64) references a
fastfields-cpu-impl PR-branch commit that was later squash-merged as
1bfacc5 -- so it shows up as "diverged" (not an ancestor of main), not
merely "behind". This is exactly the class of pin fastfields-lib#15's
incident writeup describes: a pin captured off an unmerged branch before
the real merge landed.
* submodule-bump.yaml -- calls the reusable cpp-submodule-bump.yml, either
by hand (workflow_dispatch) or automatically via a repository_dispatch
ping from fastfields-cpu-impl's notify-parent.yaml (added separately).
* notify-parent.yaml -- the producer-side half: pings fastfields-lib (which
pins this repo as `cpu`) on push to main.
Needs fastfields/.github#6 merged first for the `uses:
fastfields/.github/...@main` references to resolve. SUBMODULE_DISPATCH_TOKEN
still needs to be added as a secret on this repo (contents:write on
fastfields-lib) for the automatic dispatch to fire; see notify-parent.yaml.
No submodule pin values touched.
part of fastfields-lib#15, fastfields-lib#10
_Workstream: claude-jitfields-to-fastfields_
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugSomething isn't workingclaude-jitfields-to-fastfields

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: field_matvec_rls wrong under shared-weight broadcast (wc=1, C=2) — main's Test CI red since #59

2 participants

@balbasty@claude