Skip to content

ggml: CONCAT backward slices a contiguous gradient (a transposed one read the wrong elements) - #46

Merged
joelteply merged 2 commits into
feat/props-weight-residencyfrom
fix/concat-backward-contiguous
Oct 7, 2026
Merged

joelteply merged 2 commits into
feat/props-weight-residencyfrom
fix/concat-backward-contiguous

Conversation

@joelteply

Copy link
Copy Markdown

The bug. CONCAT's backward slices the gradient with ggml_view_4d(grad, …, grad->nb[1..3], offset), and a view's first stride is always the element size. The training attention transposes V for its KQV product (v = ggml_cont(ggml_transpose(v)) in build_attn_mha). So V's gradient reaches the walk's concat ([cached prefix | this chunk's V]) through a transpose, with a first stride that is not the element size, and the slice reads the right number of elements from the wrong places. K is only permute(0,2,1,3), which keeps the first stride, so K came through intact.

Measured (1.5B Q4_K_M on the 5090, test-walk-exact on the walk branch, with the SGD step read off the adapter, so step = −lr·g):

  • The exact walk's cross-chunk V term had the right norm (0.93) and was orthogonal to the true one (cosine −0.10). K's was aligned.
  • With the fix, the exact walk in 4 chunks matches one graph at cosine 0.998, and the plain walk's step against one graph moves from cosine 0.54 to 0.84.

So every walk-trained adapter has had its attn_v gradient scrambled on every chunk that had a cached prefix. Any other concat whose gradient arrives non-contiguous was affected the same way.

The fix: slice a contiguous gradient (ggml_cont only when it isn't one already). It's one commit, standalone on the production line, and test-walk-exact mutation-checks it on #40's follow-up (it fails at cosine 0.43 without it).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc

…read the wrong elements)

The slice is ggml_view_4d(grad, ..., grad->nb[1..3], offset), and a view's first stride is the
element size. The training attention transposes V for its KQV product, so V's gradient reaches
the concat (the walk's [cached prefix | this chunk's V]) through a transpose, with a first
stride that is not the element size: the slice read the right NUMBER of elements from the wrong
places. K is only permuted (0,2,1,3), keeping its first stride, so it came through intact.

Measured on the 1.5B (test-walk-exact, SGD step = -lr*g read off the adapter): the walk's
cross-chunk V term had the right norm (0.93) and was orthogonal to the true one (cosine -0.10);
with the fix the exact walk in 4 chunks matches one graph at cosine 0.998, and the PLAIN walk's
step moved from cosine 0.54 to 0.84 against the one-graph step: every walk-trained adapter had
its attn_v gradient scrambled on every chunk with a cached prefix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc
@github-actions github-actions Bot added the ggml label Oct 6, 2026
joelteply added a commit that referenced this pull request Oct 7, 2026
…nks (hybrid models)

Fable's ask: a hybrid's linear layers carry the window in their state, so stopping the gradient
there is where most of the bias would be. The state is treated like K/V. build_rs puts a zero
GRAD leaf on the state ENTERING a chunk (the backward yields dL/d(entry state)), and the two
delta-net helpers give the state a chunk LEAVES (the conv state's tail, the delta-net's new
state) the surrogate <exit, G>, where G is the next chunk's dL/d(entry state)
(build_walk_state_exit). Each exit state feeds exactly one chunk, so G is one vector per state
tensor, overwritten as the reverse pass moves back. The forward pass decodes chunk by chunk and
snapshots sequence 0's state rows on the host at every chunk start; the reverse pass restores
each chunk's entry state before training it (the first chunk starts from zero). The delta-net
backward already returns its initial state's gradient and folds in its final state's through
the snapshot slots; the conv state goes through CONCAT (#46).

test-walk-exact on Qwen3.5-0.8B (attention + gated delta-net), the SGD step read off the adapter:
  every position labelled: exact walk in 4 chunks vs one graph: cosine 0.9980, norm 0.9997
                           (plain walk: cosine 0.83, norm 2.13)
  context + reply:         exact walk in 4 chunks vs one chunk per run: cosine 0.9985
Mutation-checked: without the state surrogate it fails (cosine 0.987, norm 0.969). The 1.5B
results are unchanged. The test's contexts carry a scratch sequence over a unified cache, which
the plain walk needs for a recurrent state; the exact walk keeps host snapshots instead and does
not require it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc
@joelteply

Copy link
Copy Markdown
Author

Review at fbd9ba5 (Fable). The fix is right: ggml_view_4d takes nb[1..3] only and assumes nb[0] is the element size, so a gradient that arrives through a transpose (V in the training attention) was sliced from the wrong addresses. Slicing a contiguous copy, with offsets from g->nb[dim], is the correct fix. ggml_is_contiguous also routes a permuted or gapped gradient through the copy, which is what we want.

One ask before merge: a regression test that fails without this hunk. This is the silent-wrong-gradient class we keep missing. Only your cosine measurement caught it, and nothing in CI would catch it if it came back. test-backend-ops' gradient mode already checks CONCAT, but with a contiguous consumer, so it passes either way. A case whose CONCAT output is consumed through a transpose (e.g. concat on dim 1, then ggml_cont(ggml_transpose(...)), then the loss), so the gradient reaches CONCAT non-contiguous, would gate it. Please confirm it fails with the ggml.c hunk reverted. Approve once that's in.

…(Fable on #46)

The output reaches the loss the way the training attention consumes a walk's [prefix | chunk]
V: permute(0,2,1,3) then transpose, so CONCAT's gradient arrives with a first stride that is
not the element size. Squared, because the harness differentiates a SUM (gradient all ones),
and a scrambled tensor of ones is still ones. With the fix reverted: MAA 13.5, FAIL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc
@joelteply

Copy link
Copy Markdown
Author

Approve at b8c1f93 (Fable). Verified on the M5 (CPU build of this head): test-backend-ops grad -p ne_b_d gives 193/193 with the fix. With the ggml.c hunk reverted, the new case fails with CONCAT(ne_a=[4,3,5,1],ne_b_d=2): [CONCAT] MAA = 13.756570002 > 0.000100000 FAIL, and every other CONCAT case still passes, so the new case is the only gate on this bug. The SQR trick is the right call: a SUM's all-ones gradient survives any scramble.

@joelteply

Copy link
Copy Markdown
Author

Done (b8c1f93): test_concat_grad_transposed in test-backend-ops. CONCAT's output reaches the loss through permute(0,2,1,3) then a transpose, the way the training attention consumes the walk's V, so the gradient arrives with a first stride that isn't the element size. The output is squared, because the harness differentiates a SUM (all-ones gradient) and a scrambled tensor of ones is still ones; my first version without the square passed with the fix reverted, so it caught nothing. Now, with the fix reverted: [CONCAT] MAA = 13.548662675 > 0.0001 FAIL; with it, OK. (test-backend-ops grad segfaults for every op on this Windows CPU build, ADD included after 51 passing cases, so I ran it filtered with -p; that's a harness problem here, separate from this.)

@joelteply
joelteply merged commit ba5af84 into feat/props-weight-residency Oct 7, 2026
10 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant