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

fix: force the RLS weight-channel stride to 0 for broadcast reads - #48

Merged
balbasty merged 1 commit into
mainfrom
claude/62-rls-broadcast-stride
Aug 2, 2026
Merged

fix: force the RLS weight-channel stride to 0 for broadcast reads#48
balbasty merged 1 commit into
mainfrom
claude/62-rls-broadcast-stride

Conversation

@balbasty

Copy link
Copy Markdown
Collaborator

Summary

field_matvec_rls/field_diag_rls/field_relax_rls (the wc == 1
"single weight shared across all nc field channels" RLS path, for
absolute/membrane/bending) computed the weight tensor's
per-channel stride (wsc) from its real, contiguous last-dim stride
and then indexed wgt[wsc*c] inside a loop over the field'snc
channels. That's the right thing to do for the genuine per-channel
(JRLS, wc == nc) path, but wrong for RLS: the weight tensor
genuinely has only one element in that dimension, so for c > 0 the
read walks off the wc=1 buffer into the next spatial voxel's
weight (or out of bounds at the very last voxel, landing on
zero-initialized memory and reading -0).

Since these nine functions (matvec/diag/relax_ ×
absolute/membrane/bending) are only ever dispatched to when
wc == 1, the fix forces wsc = 0 unconditionally instead of
deriving it from stride_wgt — exactly the broadcast semantics the
RLS path is supposed to have. wgt[0*c] == wgt[0] for every c, so
this needs no change to the kernels-layer math.

Root cause, precisely

Confirmed with a minimal, direct field_matvec_rls harness at
C=2, wc=1, order=1 (absolute-only, no neighbour terms, so the bad
read is maximally localized) with a non-constant weight map (the
existing regression test uses an all-ones map, which masks the bug
almost everywhere by coincidence): channel 1 read the next spatial
voxel's
weight instead of its own, at every interior voxel, and 0
(out of bounds) at the last voxel — exactly the wgt[wsc*c]
stride-indexing bug described above. This is a distinct bug from
kernels#40 (which only touched make_kernel_bending_rls, i.e.
order=3) — it explains the order=1/order=2 failures kernels#40
never could have.

flow_matvec_rls/diag_rls/relax_rls are not affected: flow's
wgt is architecturally always wc=1 (a CHECK_SAME(wgt.shape[...] == 1) at the cpu-lib API boundary — there is no flow JRLS variant),
and the flow kernels never index by a per-component weight stride in
the first place, so there's no equivalent bug to trigger.

A second, separate bug found while tracing this (not fixed here)

The _jrls (wc == nc, genuine per-channel-weight) counterparts
never thread a per-channel stride through to the kernels at all —
they read a single weight value once and apply it to every field
channel, silently ignoring channels 1..nc-1. That's real but
untested (the existing tests either use identical weights across
channels or only check symmetry/self-consistency, both blind to it),
needs kernel-level changes rather than a stride fix, and is out of
scope for this PR — will be filed as a follow-up issue.

Verified

make -C fastfields-cpu-lib test CXX=clang++

All 12 suites pass, 0 failures (was 29/8440 failing in
test_reg_field before this fix):

running ./build/test_reg_field
reg_field module CPU tests
checks: 8440, failures: 0
PASSED
running ./build/test_reg_flow
reg_flow module CPU tests
checks: 11415, failures: 0
PASSED

(plus distance/distance_mesh/distance_spline/posdef/pushpull/
pushpull_backward/reg_op/resize/restrict/splinc, all green.)

part of fastfields/fastfields-cpu-lib#62

Workstream: claude-jitfields-to-fastfields

🤖 Generated with Claude Code


Generated by Claude Code

field_matvec_rls/diag_rls/relax_rls (the wc=1 "single weight shared
across all channels" path) computed wsc from the wgt tensor's real
(contiguous) last-dim stride and then indexed wgt[wsc*c] per field
channel. That's correct when wc == nc (the JRLS, per-channel-weight
path) but wrong when wc == 1: the tensor genuinely has only one
element in that dimension, so for c > 0 the read walks off the wc=1
buffer into the next spatial voxel's weight (or out of bounds at the
very last voxel, reading -0 from adjacent padding).
Since these nine functions (matvec/diag/relax x absolute/membrane/
bending) are only ever dispatched to when wc == 1, force wsc to 0
unconditionally instead of deriving it from stride_wgt -- this is
exactly the broadcast semantics the RLS path is supposed to have,
and it requires no change to the kernels-layer math at all (wgt[0*c]
is correct for every c).
Root-caused with a minimal C=2, wc=1, order=1 (absolute-only) direct
harness against field_matvec_rls: channel 1 was reading the next
spatial voxel's weight value instead of its own, confirming the
stride-indexing bug precisely (not a magnitude/scale error, and
unrelated to kernels#40's bending-kernel-table fix).
flow_matvec_rls/diag_rls/relax_rls are not affected: flow's wgt is
architecturally always wc=1 (enforced by a CHECK_SAME at the
cpu-lib API boundary) and its kernels never index by a per-component
weight stride, so there is no equivalent broadcast bug there.
A second, distinct issue was found while tracing this: the
"_jrls" (wc == nc, genuine per-channel-weight) functions never
thread a per-channel stride through to the kernels at all -- they
read a single weight value once and apply it to every field
channel, silently ignoring channels 1..nc-1. That bug is untouched
by this commit (it needs kernel-level changes, not just a stride
fix) and is filed separately.
part of fastfields-cpu-lib#62
@balbastybalbasty added bug Something isn't working claude-jitfields-to-fastfields labels Aug 2, 2026 — with Claude
@balbasty
balbasty merged commit 1bfacc5 into mainAug 2, 2026
2 of 3 checks passed
@balbasty
balbasty deleted the claude/62-rls-broadcast-stride branch August 2, 2026 08:27
@balbastyClaude

Copy link
Copy Markdown
CollaboratorAuthor

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

Root cause confirmed precise and narrow: the 9 RLS driver functions (matvec/diag/relax_ × absolute/membrane/bending) computed the weight channel-stride as stride_wgt[nall] — correct for JRLS (wc == nc) but wrong for RLS, which is the only mode these functions are ever called in (wc == 1, one weight broadcast across all field channels). For c > 0 this walked off the 1-element weight buffer into the next voxel's weight (or OOB at the last voxel, landing on zeroed memory — explains the exact -0 the issue's order=1 failure showed). Fix forces wsc = 0, which is exactly correct broadcast semantics.

Verified independently, not just trusted: real kernel-level harness at C=2/wc=1/order=1 confirmed channel 1 was reading the neighboring voxel's weight before the fix; test-via-cpu-lib CI green.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

balbasty added a commit to fastfields/fastfields-cuda-impl that referenced this pull request Aug 2, 2026
Structural mirror of the fastfields-cpu-impl fix for the same bug:
matvec/diag/relax_ for absolute/membrane/bending's RLS (wc=1,
single weight shared across all `nc` field channels) path computed
wsc from the wgt tensor's real stride and indexed wgt[wsc*c] per
field channel -- correct for the JRLS (wc == nc) path, but wrong
for RLS, where the tensor genuinely has one element in that
dimension and c > 0 walks off it into the next voxel's weight.
These nine host launchers are only ever invoked at wc == 1, so
force wsc = 0 unconditionally instead of deriving it from
stride_wgt -- no kernels-layer change needed. CUDA has no GPU in CI
(compile+link only), so this could not be runtime-verified here;
it is a mechanical, line-for-line mirror of the fastfields-cpu-impl
fix (fastfields/fastfields-cpu-impl#48), which was verified against
a full `make test` run (test_reg_field: 8440 checks, 0 failures,
was 29 failing before the fix).
part of fastfields-cpu-lib#62
Co-authored-by: Claude <noreply@anthropic.com>
balbasty pushed a commit that referenced this pull request Aug 3, 2026
Mechanical formatting only (alignment, template-bracket spacing, line
wrapping) -- no semantic change. The pre-#48 code this revert restores
predates the clang-format lint gate and wasn't clean against it.
balbasty added a commit that referenced this pull request Aug 3, 2026
…uperseded by the field_rls_is_jrls predicate fix (#59)
* Revert "fix: thread a real per-channel weight stride through the field _jrls drivers (#53)"
This reverts commit 24254d4.
* Revert "fix: force the RLS weight-channel stride to 0 for broadcast reads (#48)"
This reverts commit 1bfacc5.
* chore: bump kernels submodule pin to the reverted _jrls per-channel stride
part of fastfields-cpu-lib#65
* style: run clang-format on reg_field.h after the RLS/JRLS revert
Mechanical formatting only (alignment, template-bracket spacing, line
wrapping) -- no semantic change. The pre-#48 code this revert restores
predates the clang-format lint gate and wasn't clean against it.
---------
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.

2 participants

@balbasty@claude