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

fix: thread a real per-channel weight stride through the field _jrls drivers (CUDA) - #32

Closed
balbasty wants to merge 1 commit into
mainfrom
claude/jrls-per-channel-weight-fix-65
Closed

fix: thread a real per-channel weight stride through the field _jrls drivers (CUDA)#32
balbasty wants to merge 1 commit into
mainfrom
claude/jrls-per-channel-weight-fix-65

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

Summary

Structural mirror of the fastfields-cpu-impl driver fix for the same bug
(fastfields-cpu-impl#53). The field regulariser's _jrls CUDA driver
layer (matvec/diag/relax x absolute/membrane/bending, 9 functions) never
derived a per-channel weight stride at all: the CUGLOB kernels' local
stride_wgt stack copy was only nall long (one short of the
channel-stride entry at index nall), and the CUHOST launchers only
uploaded nall elements to the device -- so there was no wsc to compute
in the first place, and every channel implicitly read wgt[wgt_offset]
(channel 0's weight) via the same base pointer.

Fix, mirroring the RLS (wc=1) drivers' already-correct plumbing
(fastfields-cuda-impl#27) but going the other way:

  • CUGLOB kernels: stride_wgt[nall] -> stride_wgt[nall+1], and compute
    wsc = stride_wgt[nall] -- the weight tensor's genuine last-dim stride
    (RLS forces this to 0 to broadcast instead).
  • CUHOST launchers: copyToDevice(stride_wgt, nall) ->
    copyToDevice(stride_wgt, nall + 1), so the device-side array actually
    holds the element wsc reads.
  • wsc threaded into every matvec_*_jrls/diag_*_jrls kernel call
    (including the two calls each relax_*_jrls_ makes internally).

Also bumps the kernels submodule pin to fastfields-kernels#70 (merged),
which gives the underlying _jrls kernel functions themselves the
matching wsc parameter this driver layer now passes through -- the two
changes must land together, since neither compiles against the other's
opposite side.

Status

No GPU in CI -- validated by compile+link only. Opening as draft
while a full make -C ../fastfields-cuda-lib CXX=clang++ (real nvcc, no
GPU) run finishes; will mark ready and merge once confirmed. The CPU-side
fix (fastfields-cpu-lib#65, fastfields-cpu-impl#53, fastfields-kernels#70)
is the one with a real ground-truth test run.

part of fastfields-cpu-lib#65

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com


Generated by Claude Code

…drivers (CUDA)
Structural mirror of the fastfields-cpu-impl driver fix for the same bug
(fastfields-cpu-impl#53). The field regulariser's `_jrls` CUDA driver
layer (matvec/diag/relax x absolute/membrane/bending, 9 functions) never
derived a per-channel weight stride at all: the CUGLOB kernels' local
`stride_wgt` stack copy was only `nall` long (one short of the
channel-stride entry at index `nall`), and the CUHOST launchers only
uploaded `nall` elements to the device -- so there was no `wsc` to
compute in the first place, and every channel implicitly read
`wgt[wgt_offset]` (channel 0's weight) via the same base pointer.
Fix, mirroring the RLS (wc=1) drivers' already-correct plumbing
(fastfields-cuda-impl#27) but going the other way:
* CUGLOB kernels: `stride_wgt[nall]` -> `stride_wgt[nall+1]`, and
compute `wsc = stride_wgt[nall]` -- the weight tensor's genuine
last-dim stride (RLS forces this to 0 to broadcast instead).
* CUHOST launchers: `copyToDevice(stride_wgt, nall)` ->
`copyToDevice(stride_wgt, nall + 1)`, so the device-side array
actually holds the element `wsc` reads.
* `wsc` threaded into every matvec_*_jrls / diag_*_jrls kernel call
(including the two calls each relax_*_jrls_ makes internally).
Also bumps the kernels pin to fastfields-kernels#70, which gives the
underlying `_jrls` kernel functions themselves (matvec/diag x
absolute/membrane/bending, kernels/regularisers/field/{1,2,3}d.h) the
matching `wsc` parameter this driver layer now passes through -- the two
changes must land together, since neither compiles against the other's
opposite (old kernels + new drivers, or new kernels + old drivers) side.
No GPU in CI, so this is validated by compile+link only
(`make -C ../fastfields-cuda-lib CXX=clang++`, real nvcc, no GPU
present); the CPU-side fix (fastfields-cpu-lib#65, cpu-impl#53,
kernels#70) is the one with a real ground-truth test run.
part of fastfields-cpu-lib#65
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
@github-actions

Copy link
Copy Markdown

Submodule staleness (non-blocking -- fastfields-lib#15)

submodulepinned committracksstatusbehind by
kernels4333650fastfields/fastfields-kernels@mainup to date0

@balbastyClaude

Copy link
Copy Markdown
ContributorAuthor

Closing as superseded by #33.

A follow-up audit found that field_rls_is_jrls() (in fastfields-cpu-lib/fastfields-cuda-lib) had RLS/JRLS backwards — the opposite of the original jitfields/nitorch semantics (RLS = per-channel wc == nc, JRLS = shared/broadcast wc == 1). This PR's per-channel-stride threading into the _jrls drivers was the right shape of fix applied to the wrong (mislabelled) case, mirroring the same wrong-direction consequence of fastfields-kernels#70/fastfields-cpu-impl#53 on the CPU side.

#33 instead reverts fastfields-cuda-impl#27's wrong-direction wsc=0 force, restoring the genuine per-channel stride in the _rls drivers (no CUDA-side mirror of #70/#53 had been merged into this repo's main beyond this draft, so no kernels-pin bump was needed here). The real fix — flipping field_rls_is_jrls() — lands in fastfields-cpu-lib#73 / a matching fastfields-cuda-lib predicate flip, both closing fastfields-cpu-lib#65.

Validated by compile+link via fastfields-cuda-lib's CI (build-via-cuda-lib, real nvcc, no GPU) — green.


Generated by Claude Code

@balbastybalbasty closed this Aug 3, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude