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: fix three zero-stub bugs in the linear grad_backward kernels - #45
Merged
Conversation
`grad_backward` (the adjoint of the spatial-gradient sampler `grad`) had hard-coded zeros in the Linear specialisations, inherited verbatim from jitfields. Exposing the backward ops through the public ABI put them under a finite-difference oracle for the first time (fastfields-cpu-lib/tests/test_pushpull_backward.cpp) and all three showed up as hard failures: * **1D Linear** returned a zero gradient wrt `inp`. `grad` is linear in `inp` with the location-independent weight pair (negate, +1), so the adjoint is that same pair scattered back -- definitely not zero. (Its zero gradient wrt `loc` *is* correct: a linear basis has a piecewise constant derivative, whose own derivative vanishes.) * **2D and 3D Linear** returned a zero gradient wrt `grid`. Only the *pure* second derivatives d2/dx2, d2/dy2, d2/dz2 vanish for a linear basis; the *mixed* ones do not, and they are exactly what dL/d(loc) is made of here. The general-order kernels already get this right (they accumulate accxy/accxz/accyz from `hindex`); the Linear fast paths just dropped the cross terms. Restored with the same convention, so Linear now agrees with what the general-order path would compute. Reading `inp` from the 2D/3D kernels means their `stride_inp` argument is now live (3D's was even marked `/* unused */`); the input offsets are built before ix/iy/iz are scaled in place by the *output* strides, since the two tensors need not share a layout. Verified: 6381/6381 finite-difference checks pass across ndim 1/2/3, orders 0-3, bounds DCT2/DST2, with and without a batch dimension. Before the fix, the Linear cases failed 160 of them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016AjQcY78NgbagPSbPJRr6Z
This was referenced Jul 31, 2026
balbasty
commented
Jul 31, 2026
CollaboratorAuthor
good to go |
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.
Part of the stack for fastfields-torch#16 (backpropagate through the pushpull sampling
grid).What
grad_backward(the adjoint of the spatial-gradient samplergrad) had hard-coded zeros in the Linear specialisations, inherited verbatim from jitfields. Exposing the backward ops through the public ABI put them under a finite-difference oracle for the first time, and all three showed up as hard failures.d/d(inp) = 0gradis linear ininpwith the location-independent weight pair(negate, +1); the adjoint is that same pair scattered back. (Its zerod/d(loc)is correct — a linear basis has a piecewise-constant derivative, whose own derivative vanishes.)d/d(grid) = 0dL/d(loc)is made of here.d/d(grid) = 0The general-order kernels already get this right — they accumulate
accxy/accxz/accyzfromhindex. The Linear fast paths just dropped the cross terms. Restored with the same convention, so Linear now agrees with what the general-order path would compute for the same configuration.Reading
inpfrom the 2D/3D kernels makes theirstride_inpargument live (3D's was even marked/* unused */). Input offsets are built beforeix/iy/izare scaled in place by the output strides, since the two tensors need not share a layout.Verification
6381/6381 finite-difference checks pass — ndim 1/2/3, orders 0–3, bounds DCT2/DST2, with and without a batch dimension. Before this fix the Linear cases failed 160 of them. The oracle itself lives in fastfields-cpu-lib (
tests/test_pushpull_backward.cpp) since that is where these kernels are reachable from a test.Workstream: claude-jitfields-to-fastfields
🤖 Generated with Claude Code
Generated by Claude Code