S1-29b: the SSM backward wiring S1-29 never landed#23
Open
dillon-blake wants to merge 1 commit into
Open
Conversation
S1-29 is marked done and delivered ONLY the CONCAT VJP. The SSM_CONV_BACK / SSM_SCAN_BACK enums, constructors and backward-switch cases its own ticket also promised were never written -- SSM_CONV_BACK appeared zero times in ggml.h. So S1-30 and S1-31 were both written against a dependency that did not exist. Two ops at the enum tail, their constructors, and the two backward cases that emit them. No kernels: CPU supports_op returns FALSE explicitly for both, which is not redundant -- the default returns TRUE, so a new op with no dispatch case is reported *supported*, gets scheduled, and hits ggml_compute_forward's `default: GGML_ABORT`. It would look implemented right up until it killed the process. Same trap S1-25 hit. SSM_CONV's backward is the convolution run backwards; only sx takes a gradient, since the conv weight is a frozen base weight on the LoRA path. SSM_SCAN has SEVEN sources and five of them take a gradient, so ggml_ssm_scan_back returns a PACKED 1-D tensor -- [d_s | d_x | d_dt | d_B | d_C] -- exactly as ggml_ssm_scan itself packs y with the final states, and the backward case views each region back onto its source. `grad` is the WHOLE packed gradient of the forward's dst, state region included: MODE_GRAD's objective sums over it, so it is NOT zero there, and a kernel that assumed otherwise would disagree with the finite difference and be wrong to. That region seeds the reverse recurrence at t = n_t. Decided here so S1-31 cannot get it wrong. THE TESTS CALLED ggml_set_param ZERO TIMES. So `grad -o SSM_CONV` and `grad -o SSM_SCAN` requested no gradients, compared nothing, and printed `Backend CPU: OK` -- while neither op had a backward case at all and both would have aborted the moment one was asked for. Both now ask. And every existing shape is far above grad_nmax() (10000), so they would have been SILENTLY SKIPPED even with a param. Tiny shapes added for both, and for SSM_SCAN in both the Mamba-1 (head_dim == 1, per-state A) and Mamba-2 (head_dim > 1, scalar A) branches -- they are different code in the kernel. The xbc_overlap variants make x, B and C views of one tensor, and ggml_set_param asserts op == GGML_OP_NONE, so those stay eval-only. Stated in the code rather than hidden: it is a real llama.cpp layout and its backward aliasing is exactly what goes wrong quietly. S1-31 owns it. Backward graphs now BUILD for both ops and report not-supported instead of aborting. Full 21-op grad sweep 0/8 failing; SSM forwards unregressed.
This was referenced Jul 14, 2026
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Stacked on #22 (S1-28).
S1-29 is marked done and delivered only the CONCAT VJP. The
SSM_CONV_BACK/SSM_SCAN_BACKenums, constructors and backward-switch cases its own ticket also promised were never written —SSM_CONV_BACKappeared zero times inggml.h. So S1-30 and S1-31 were both written against a dependency that did not exist.Two ops at the enum tail, their constructors, and the two backward cases that emit them. No kernels — CPU
supports_opreturns false explicitly for both, which is not redundant: the default returnstrue, so a new op with no dispatch case is reported supported, gets scheduled, and hitsggml_compute_forward'sdefault: GGML_ABORT. Same trap S1-25 hit.The packed-gradient decision, made here so S1-31 cannot get it wrong
SSM_SCANhas seven sources and five take a gradient, soggml_ssm_scan_backreturns a packed 1-D tensor —[d_s | d_x | d_dt | d_B | d_C]— exactly asggml_ssm_scanitself packsywith the final states.gradis the whole packed gradient of the forward's dst, state region included. MODE_GRAD's objective sums over it, so it is not zero there — a kernel that assumed otherwise would disagree with the finite difference and be right to. That region seeds the reverse recurrence att = n_t.The tests called
ggml_set_paramzero timesSo
grad -o SSM_CONVandgrad -o SSM_SCANrequested no gradients, compared nothing, and printedBackend CPU: OK— while neither op had a backward case at all and both would have aborted the moment one was asked for.And every existing shape is far above
grad_nmax()(10000), so they'd have been silently skipped even with a param. Tiny shapes added — and for SSM_SCAN in both the Mamba-1 (head_dim == 1, per-state A) and Mamba-2 (head_dim > 1, scalar A) branches, which are different code in the kernel.The
xbc_overlapvariants makex,BandCviews of one tensor, andggml_set_paramassertsop == GGML_OP_NONE, so those stay eval-only. Stated in the code rather than hidden — it is a real llama.cpp layout and its backward aliasing is exactly what goes wrong quietly. S1-31 owns it.Backward graphs now build for both ops and report not-supported instead of aborting. Full 21-op grad sweep 0/8 failing; SSM forwards unregressed.