Uh oh!
There was an error while loading. Please reload this page.
This repository was archived by the owner on Aug 20, 2026. It is now read-only.
pushpull: thread BoundVec/SplineVec runtime carriers + drop extrapolate template arg - #21
Merged
Merged
Conversation
…plate arg
Two independent changes to the combinatorial surface, both needed before
pushpull can be wired into cuda-lib's CI without an OOM:
1. `extrapolate` was a *compile-time* template parameter on every CUGLOB
kernel here (dispatched from the runtime value via FF_PP_EX), a 3x
multiplier on every pushpull kernel instantiation. fastfields-cpu-impl
never paid this cost -- its `infov_dyn` treats extrapolate as a plain
runtime int, since it only gates a cheap bounds test independent of
spline/bound. Port `infov_dyn` here (CUDEV, so it also works on
device) and drop `extrapolate` from every kernel's and launcher's
template argument list; FF_PP_EX and its dispatch are gone entirely,
FF_PP_DISPATCH now only branches on nbatch.
2. Every CUGLOB kernel and CUHOST launcher now takes `bound::BoundVec` +
`spline::SplineVec` (by value, mirroring the existing reg_field.cpp/
reg_flow.cpp convention for BoundVec), materialises them into local
bound_t[3]/spline_t[3] arrays, and forwards those as the trailing
arguments to PushPull<...>::{pull,push,count,grad,hess,*_backward},
which the kernels layer already accepts for whichever axis is
compiled as bound::type::Dynamic / spline::type::Dynamic.
`abs` stays a compile-time template bool, matching cpu-impl -- it gates
a real per-voxel branch (fabs on the gradient/hessian), not a candidate
for the Dynamic treatment.
Verified via nvcc instantiation smoke tests (function-pointer
instantiation of every op, 1D/2D/3D, under both a fully static
(Cubic/DCT2) and a fully Dynamic (spline::type::Dynamic/bound::type::
Dynamic) parameterisation) -- no errors, only pre-existing unused-
variable warnings already present in the kernels layer.
Part of fastfields-cuda-lib#30.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z…commits Part of fastfields-cuda-lib#30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
The prior commit (52b06af) intended to bump the `kernels` submodule pin to the spline::type::Dynamic + pushpull dyn<> commit via the safe procedure (`git update-index --add --cacheinfo 160000,<SHA>,kernels`), but the final `git commit -- kernels` step re-staged the path from the working tree before committing -- and the working tree holds a *symlink* (this dev tree's local convention, see CLAUDE.md), not the submodule gitlink. That silently replaced the staged 160000 gitlink with a 120000 symlink blob, corrupting the pin. `git ls-tree` on that commit confirmed `120000 blob ... kernels` instead of `160000 commit ...`. Re-applies the same procedure and commits with a bare `git commit` (no trailing pathspec) so the already-staged gitlink is used as-is. Now points at the kernels commit with the two pushpull correctness fixes (0174be7 -- see the previous non-gitlink commit here for detail). `git diff --cached -- kernels` before this commit showed a clean `160000 <old> -> 160000 <new>` transition once the corrupted HEAD is accounted for; verified again after committing via `git ls-tree HEAD -- kernels`. Part of fastfields-cuda-lib#30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
…with defaults Generalizes the fastfields-cpu-impl#38 review feedback to its CUDA-side counterpart: cuda-impl's CUHOST launchers (pull/push/count/grad -- the external-facing API cuda-lib calls, mirroring cpu-impl's role) had the exact same `bnd`/`spl` leading-parameter pattern cpu-impl's launchers did. Same fix: moved to the end (before the existing `stream = 0` parameter), given defaults (`bound::BoundVec()` / `spline::SplineVec()`), so a fully- static instantiation can omit them entirely, matching the "isotropic convenience" the template parameter list's `IY=IX, BY=BX, IZ=IY, BZ=BY` defaults already provide. The internal `<<<>>>` launch of the CUGLOB device kernel is unaffected -- it refers to the local `bnd`/`spl` variables by name, and the device kernels' own signatures (always called with every argument explicit, never omitted) are intentionally left alone; reordering them would be internal churn with no external caller ever benefiting from the defaults. Verified via nvcc instantiation (no GPU needed for this): a fully-static `pull<...>(nbatch, extrapolate, out, ...)` with bnd/spl/stream all omitted, an explicit Dynamic call with bnd/spl supplied as trailing args (stream still defaulted), and a fully-explicit call including stream -- all four launchers (pull/push/count/grad), clean compile. Part of fastfields-cuda-lib#30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
…commits Part of fastfields-cuda-lib#30. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
…merge) kernels#44 merged as 15295c2 (via merge commit, so the SHA differs from the PR branch head this was previously pinned to). Re-point to the actual main tip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workstream: claude-jitfields-to-fastfields
Part of fastfields/fastfields-cuda-lib#30 (wire
pushpullinto cuda-lib's CI). This is the cuda-impl layer piece.Two independent changes to the combinatorial surface
Both needed before
pushpullcan be wired into cuda-lib's CI without an OOM/multi-hour compile.1.
extrapolatewas a compile-time template parameter on everyCUGLOBkernel here (dispatched from the runtime value via theFF_PP_EXmacro) — a gratuitous 3x multiplier on every pushpull kernel instantiation, for a check that's a cheap runtime branch.fastfields-cpu-implnever paid this cost: itsinfov_dyntreatsextrapolateas a plain runtimeint, since it only gates a bounds test independent of spline/bound. Portedinfov_dynhere (CUDEV, works on device) and droppedextrapolatefrom every kernel's and launcher's template argument list;FF_PP_EXand its dispatch are gone entirely,FF_PP_DISPATCHnow only branches onnbatch.2. Every
CUGLOBkernel andCUHOSTlauncher now takesbound::BoundVec+spline::SplineVec(by value, mirroring the existingreg_field.cpp/reg_flow.cppconvention forBoundVec), materialises them into localbound_t[3]/spline_t[3]arrays, and forwards those as the trailing arguments toPushPull<...>::{pull,push,count,grad,hess,*_backward}, which the kernels layer (fastfields/fastfields-kernels#44) already accepts for whichever axis is compiled asbound::type::Dynamic/spline::type::Dynamic.absstays a compile-time templatebool, matchingcpu-impl— it gates a real per-voxel branch (fabson the gradient/hessian), not a Dynamic candidate.Verification
No GPU in this sandbox (as usual for this repo), so verified via
nvccinstantiation smoke tests: function-pointer instantiation of every op (pull/push/count/grad/hess/pull_backward/push_backward/count_backward/grad_backward), 1D/2D/3D, under both a fully static (Cubic/DCT2) and a fully Dynamic (spline::type::Dynamic/bound::type::Dynamic) parameterisation — clean compile, only pre-existing unused-variable warnings already present in the kernels layer (unrelated to this change).Depends on fastfields/fastfields-kernels#44 (bumped the
kernelspin to it).🤖 Generated with Claude Code
Generated by Claude Code