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

reg: restore the in-place diag/kernel accumulate primitives (lost from jitfields) - #33

Merged
balbasty merged 2 commits into
mainfrom
claude/reg-accumulate-inplace-restore
Aug 1, 2026
Merged

reg: restore the in-place diag/kernel accumulate primitives (lost from jitfields)#33
balbasty merged 2 commits into
mainfrom
claude/reg-accumulate-inplace-restore

Conversation

@balbasty

Copy link
Copy Markdown
Contributor

Changes (fastfields-cuda-lib)

Structural mirror of fastfields-cpu-lib#50 — that repo is the correctness
gate
(no GPU in CI), and this layer is verified by compile+link only.

  • _field_diag / _field_kernel / _flow_diag / _flow_kernel are now
    templated on char op (they hardcoded '='); the impl layer already took the
    op, so this only threads it through.
  • New ADD_DG_DT / SUB_DG_DT / ADD_KN_DT / SUB_KN_DT dispatch macros,
    mirroring the existing ADD_MV_DT / SUB_MV_DT.
  • New public entry points: {field,flow}_{diag,kernel}_{add_,sub_} (8).
  • Renamed the four task-#53 symbols {field,flow}_matvec_{add,sub} ->
    ..._{add_,sub_} (see Naming below).

Stream plumbing (_reg_stream) and the Lamé matrix-stencil nbatch handling
(ntrail = 2 when shears/div are active) are carried through unchanged.

Verification status — please let CI be the gate here

Honest status at time of opening: the local nvcc build had compiled the new
code for ~32 min at the Makefile default -O3 and a further ~10 min at CI's
-O1 with zero compile errors, but had not yet reached the link step —
distance.o done, reg_field.o still in cicc. nvcc on these heavily
templated TUs is simply slow here (this repo's own notes put pushpull at ~40 min
on its own). The earlier EXIT=2 in my logs was nvcc: Terminated — my own
pkill when switching to CI's flags — not a compile failure.

What is established:

  • The new instantiations were observed compiling (they appear by name in
    nvcc's diagnostic traces, e.g.
    _field_kernel<ndim,op,scalar_t,offset_t,BOUND...> at reg_field.cpp(1007)),
    with no diagnostics beyond the pre-existing
    warning #186-D: pointless comparison of unsigned integer with zero.
  • The declarations are consistent with their consumers: fastfields-lib
    compiles and links against these headers with FF_CUDA:: dispatch, and its
    make all is EXIT=0 with all 12 symbols exported.
  • The logic is identical to the CPU implementation, which is green
    (make test: 11 binaries, 0 failures — reg_field 7426 checks, reg_flow
    11415, reg_op 186).

CI here runs make -C . CXX=clang++ CXXFLAGS="-std=c++14 -O1", which is the
real gate for this repo. I'll post a follow-up comment with the local link
result once it completes.

Background (shared by this PR series)

What jitfields actually does (verified, not assumed)

jitfields/csrc/lib/regularisers/{field,flow}/utils.h:

template <char op, typenamescalar_t, typenamereduce_t = scalar_t>
structOp { staticconstexpr FuncType f = set; };
template <...> structOp<'+', ...> { staticconstexpr FuncType f = iadd; };
template <...> structOp<'-', ...> { staticconstexpr FuncType f = isub; };

Every matvec_* / kernel_* / diag_* entry point takes that char op and
writes through out, so '+'/'-' are read-modify-write on the caller's
buffer
. There is no separate "return a fresh tensor" C entry point. In
jitfields' Python layer the two spellings share that one kernel —
field_matvec_add does out = inp.clone() then calls the same C function
with 'add'.

Scope check:grep -rl "char op" over all of jitfields/csrc matches
onlyreg_field / reg_flow (and their regularisers/*/utils.h), so
field+flow × matvec/kernel/diag is complete parity.

What was actually missing in fastfields

layerstate before
kernelsOp<op> intact
cpu-impl / cuda-implchar op on matvec/kernel/diag — intact
cpu-lib / cuda-libonly {field,flow}_matvec_{add,sub} (task #53); _field_diag/_field_kernel hardcoded '='
lib (hub)nothing — no accumulate entry point at all
bind-py / dlpacknothing
numpy / torch / cupyfaked it in Python: inp + field_matvec(...)

So the earlier claim "no accumulate-into-out kernel exists below" was observing
a real gap — but the gap was a missing surface, not a missing capability.

Naming: trailing underscore

In-place only, so the name says so — matching ff::'s existing marker for
accumulate-into-out (sym_addmatvec_, sym_submatvec_) and the
field_precond / field_precond_ pairing. This renames the four task-#53
symbols; the project is unreleased, and it removes a real collision, since
fastfields.{numpy,torch,cupy}.field_matvec_add is the out-of-place spelling.

The one deliberate deviation from "mirror #53 exactly" — a one-line sed to
revert if you'd rather keep the un-suffixed spelling.

Divergence from jitfields (deliberate)

jitfields.field_kernel_add calls impl.flow_kernel — an upstream copy-paste
bug (its docstring even reads "See flow_kernel"). fastfields routes
field_kernel_add_ to the field kernel. Not reproduced.

PR series

  1. fastfields-cpu-libreg: restore the in-place diag/kernel accumulate primitives (lost from jitfields) fastfields-cpu-lib#50 — the correctness gate (all CPU tests green)
  2. fastfields-cuda-libthis PR
  3. fastfields-libreg: expose the 12 in-place accumulate entry points at the hub + bump pins fastfields-lib#47 — the 12 hub entry points
  4. fastfields-dlpackreg: bind the 12 in-place accumulate primitives + bump lib pin fastfields-dlpack#20 — nanobind + dlpack re-exports
  5. fastfields-numpyreg: route accumulate wrappers through the fused in-place primitive fastfields-numpy#19 / fastfields-torchreg: implement the accumulate ops as autograd-safe in-place Functions (supersedes #19) fastfields-torch#21 (supersedes refactor(posdef,splinc): teeny dispatch + stream; add to MODULES #19) / fastfields-cupyreg: route accumulate wrappers through the fused in-place primitive fastfields-cupy#14
  6. fastfieldscontract: replace the blanket torch in-place ban with a per-op rule (supersedes #21) fastfields#22 — API_CONTRACT + conformance (supersedes reg_flow: add flow_kernel dispatch (forwards CUDA stream) #21)

fastfields#20 is re-pointed at this series.

🤖 Generated with Claude Code

Workstream: claude-jitfields-to-fastfields


Generated by Claude Code

Structural mirror of the fastfields-cpu-lib change (that repo is the
correctness gate; there is no GPU in CI, so this layer is verified by
compile+link only).
Adds the {field,flow}_{diag,kernel}_{add_,sub_} in-place entry points, which
were lost in the port from jitfields -- there, every regulariser entry point is
templated on `char op` (Op<'='> = set, '+' = iadd, '-' = isub) and writes
*through* `out`, so add/sub are in-place only at the C level.
_{field,flow}_{diag,kernel} hardcoded '=' and are now op-templated, with
ADD_/SUB_ dispatch macros mirroring the existing ADD_MV_DT / SUB_MV_DT. Stream
plumbing (_reg_stream) and the Lame matrix-stencil nbatch handling carry
through unchanged.
Also renames the four symbols added by task #53,
{field,flow}_matvec_{add,sub} -> ..._{add_,sub_}: they are in-place only, and
ff:: already marks accumulate-into-out with a trailing underscore
(sym_addmatvec_, sym_submatvec_, and the field_precond / field_precond_ pair).
Rebased onto main (which had since gained the BoundVec/FF_BOUND_* runtime
boundary-condition plumbing) to resolve real content conflicts in
reg_field.cpp / reg_flow.cpp: every impl wrapper this touches now threads both
`bvec` (from main) and the templated `op` (from this change).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
@balbasty
balbastyforce-pushed the claude/reg-accumulate-inplace-restore branch from 4c12e26 to bab1a52CompareAugust 1, 2026 11:44
…_/...)
Structural mirror of the fastfields-cpu-lib rename. Repo-owner decision: the
C++ and Python surfaces should be consistent with each other and with this
codebase's own existing convention (posdef's sym_addmatvec_/sym_submatvec_),
even where that diverges from jitfields' own (noun-first) naming.
{field,flow}_matvec_add_ -> {field,flow}_addmatvec_
{field,flow}_matvec_sub_ -> {field,flow}_submatvec_
{field,flow}_diag_add_ -> {field,flow}_adddiag_
{field,flow}_diag_sub_ -> {field,flow}_subdiag_
{field,flow}_kernel_add_ -> {field,flow}_addkernel_
{field,flow}_kernel_sub_ -> {field,flow}_subkernel_
Internal dispatch macro names unchanged; only the public ff:: symbols move.
Verified with a full nvcc build+link (compile+link only, no GPU in CI, per
this repo's standing convention): EXIT=0, 0 errors, all 12 renamed symbols
exported from libfastfields-cuda.so.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
@balbasty
balbasty merged commit 3481ec4 into mainAug 1, 2026
1 check passed
@balbasty
balbasty deleted the claude/reg-accumulate-inplace-restore branch August 1, 2026 16:50
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@balbasty@claude