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 - #27

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#27
balbasty merged 1 commit into
mainfrom
claude/62-rls-broadcast-stride

Conversation

@balbasty

@balbastybalbasty commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Structural mirror of fastfields-cpu-impl#48 (same bug, same fix, in
the CUDA host launchers for the same nine functions:
matvec/diag/relax_ × absolute/membrane/bending, all in
the wc == 1 RLS/broadcast path).

matvec_absolute_rls etc. computed the weight tensor's per-channel
stride (wsc) from its real, contiguous last-dim stride and indexed
wgt[wsc*c] per field channel — correct for the genuine per-channel
(JRLS, wc == nc) path, but wrong for RLS: the weight tensor has
only one element in that dimension, so for c > 0 the read walks off
the wc=1 buffer into the next voxel's weight.

Since these launchers are only ever invoked at wc == 1, the fix
forces wsc = 0 unconditionally instead of deriving it from
stride_wgt (grep -n "offset_t wsc = stride_wgt\[nall\];" reg_field.h
found the same 9 call sites as fastfields-cpu-impl, in the same 9
functions, confirmed by name before editing).

flow's RLS path is not affected by the analogous bug — see
fastfields-cpu-impl#48's description for why.

Verified

Compile+link gate (this repo has no GPU in CI, per this org's
CONTRIBUTING.md — compile+link is the real gate here):

nvcc -std=c++14 -O1 -x cu -DFF_STATIC_BOUNDS=0 -DFF_STATIC_BOUND_DCT2=1 \
-DFF_STATIC_BOUND_DST2=1 -c reg_field_rls.cpp -I. -o reg_field_rls.o

(matching the BOUNDFLAGS this repo's own fastfields-cuda-lib
Makefile uses by default — my first attempt omitted BOUNDFLAGS and
defaulted to FF_STATIC_BOUNDS=1, the heaviest all-8-bound-policies
static explosion, which is consistent with why this repo's own CI
needed -O1 + a 120-minute timeout for these modules; with the
correct default flags the compile finishes in a few minutes.)

Clean compile, zero errors/warnings, valid ELF relocatable object
produced (impl/kernels symlinked to this branch's checkout).

The change itself is a line-for-line mechanical mirror of
fastfields-cpu-impl#48, which was fully verified on the CPU side
(make -C fastfields-cpu-lib test CXX=clang++: all 12 suites green,
test_reg_field 8440/8440 checks passing, was 29 failing).

part of fastfields-cpu-lib#62

Workstream: claude-jitfields-to-fastfields

🤖 Generated with Claude Code

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
@balbasty
balbasty marked this pull request as ready for review August 2, 2026 08:26
@balbasty
balbasty merged commit 6caff45 into mainAug 2, 2026
3 checks passed
@balbasty
balbasty deleted the claude/62-rls-broadcast-stride branch August 2, 2026 08:47
@balbastyClaude

Copy link
Copy Markdown
ContributorAuthor

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

Same fix pattern as fastfields-cpu-impl#48 (wsc = 0 in the 9 RLS driver functions, found via grep for the identical structure in the CUDA impl), plus the stream-widening/forwarding work from #25. build-via-cuda-lib (real nvcc 12.0 compile) green — took ~50 min, consistent with this repo's documented nvcc build times for reg_field.

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

balbasty added a commit that referenced this pull request Aug 3, 2026
…eads (#27)" (#33)
This reverts commit 6caff45.
Co-authored-by: Claude <noreply@anthropic.com>
balbasty added a commit that referenced this pull request Aug 9, 2026
…S fixes (#41)
* deps: bump kernels pin to main -- the CUDA path is missing two RLS/JRLS fixes
The kernels pin here was last moved at d0f1605 and has since fallen nine
commits behind kernels' main. Two of those are field/flow regulariser
correctness fixes that the CPU path has had for a while and the CUDA path
still does not:
fastfields-kernels#64 make_kernel_bending_rls scaled the *whole*
coefficient table by 0.25, which runs the membrane
penalty at half strength whenever the bending-order
RLS/JRLS kernel is built (bending != 0 alongside
membrane != 0). The pinned tree here still has the
pre-fix blanket rescale:
make_kernel_bending(kernel, ...);
for (int k=0; k<...; ++k) {
if (k % 6 == 0) continue;
kernel[k] *= 0.25;
}
Upstream measured max |matvec_bending -
matvec_bending_rls(w=1)| of 1.8e+00 (2D) and
1.7e+00 (3D) before the fix, ~1e-14 after.
fastfields-kernels#52 guards the weight-map neighbour reads in the
RLS/JRLS kernels (out-of-bounds read at some
boundaries). Touches regularisers/field/ and
regularisers/flow/.
The rest of the range is CI config, a bounds docstring correction, and
kernels#70 with its revert kernels#73 -- which cancel out, so this bump does
not move the _jrls signatures at all (this repo never pinned #70, and never
had a #53-equivalent to revert; cuda-impl#33 already reverted the
#27/#48-style wsc forcing).
Verified locally with nvcc 12.0 that this repo's regulariser launchers still
compile against the new pin:
nvcc -std=c++11 -x cu -I. -Ikernels -c <<< '#include "reg_field.h"
#include "reg_flow.h"'
-> exit 0
Found during an independent re-review of the fastfields-cpu-lib#65 fix chain.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
* fix: correct codespell-flagged typo in distance_mesh.h comment
"Re-declaring" trips codespell's re-declaring -> redeclaring rule. Pre-existing
on main (predates this PR's kernels pin bump; unrelated to it) and was failing
the Lint job's codespell check on every push, including this PR's. Comment
meaning is unchanged.
Verified clean with `codespell .` locally (codespell==2.4.3, matching CI).
---------
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude