S1-26: OUT_PROD_ID CPU kernel — the activation half of MUL_MAT_ID's backward#21
Open
dillon-blake wants to merge 2 commits into
Open
S1-26: OUT_PROD_ID CPU kernel — the activation half of MUL_MAT_ID's backward#21dillon-blake wants to merge 2 commits into
dillon-blake wants to merge 2 commits into
Conversation
…backward
d_b[k,l,t] = sum_{i : i % ne_b1 == l} sum_j as[k,j, ids[i,t]] * grad[j,i,t]
`as` may be QUANTIZED, and that is the whole difficulty rather than a corner case. In a
LoRA MoE graph the base expert stacks are frozen Q4_K. They take no gradient themselves,
but the activations flowing INTO them do, because an earlier layer's LoRA is upstream --
so d(b) has to propagate back THROUGH a quantized weight. Same dequantize-a-row-at-a-time
path as OUT_PROD, accumulating in F32 (ADR-0002).
Threaded by token: one writer per output column, no atomics, no barrier.
MODE_GRAD CANNOT CHECK THE QUANTIZED PATH, and it took a while to work out why.
ggml's quantized matmul quantizes the ACTIVATIONS on the fly to vec_dot_type before the
dot product. So the forward is a STAIRCASE in b: its finite difference at the FD step
scale measures quantization edges, not a derivative. The analytic gradient is the
gradient of the intended smooth function; the harness evaluates the actual quantized one.
They disagree by construction (measured MAA 0.028-0.071 -- and the kernel is exact).
Upstream reached the same conclusion and encoded it without a word of comment:
// test_mul_mat
if (!ggml_is_quantized(type_a)) {
ggml_set_param(b); // b is a param ONLY when the weight is not quantized
}
So the quantized cases are deliberately absent from MODE_GRAD, and the dequantize path is
verified a stronger way instead: OUT_PROD_ID with a quantized `as` against OUT_PROD_ID with
that same `as` dequantized to F32. BIT-EXACT on q8_0, q4_K and q4_0.
TWO TEST FIXES, both found by measuring rather than assuming.
1. The token count is 32, not 5. mean_abs_asymm divides by (gn + ga), NOT by
(|gn| + |ga|), so any output element whose true gradient is near zero sends the ratio
to infinity. This op manufactures such elements: d_as[k,j,e] sums only over the slots
that routed to expert e, so with 5 tokens and 3 experts it is a sum of ONE OR TWO
random products. One element with asymm ~700 drags the mean of 90 to ~8.
5 tokens -> 3/10 runs fail, MAA up to 7.9 (kernel exact throughout)
32 tokens -> 0/30 runs fail
Condition the test; do not widen the tolerance until the flapping stops.
2. max_maa_err = 5e-3, measured on both sides:
worst FD noise, 40 runs 4.5e-4
ignore grad entirely 9.21
ignore the ne_b1 broadcast 3.39 / 1.15
read grad row 0 always 4.41
route every slot everywhere 5.30
10x above the noise floor, 200-1800x below every real defect. Five mutations
introduced, five caught.
Both kernels verified against a naive DOUBLE reference across 15 configurations covering
both broadcast modes: relative error ~1e-7, i.e. float32 epsilon. That reference is what
proved the FD failures were the harness and not the arithmetic -- though note it shares
the kernels' assumption about the broadcast rule, so it cannot check THAT. The rule was
confirmed by reading the forward: row_mapping.i1 is the SLOT index (the in-code comment
calling it "selected expert index" is wrong), so the forward's b column is slot % ne_b1.
…byte stride
Found by adversarial review, with a working reproduction. Two real bugs, both silent,
both in the kernels I had just declared verified.
1. GRAD (src1) was indexed as g_col[j] -- a hard-coded 4-byte dim-0 stride.
ggml's autodiff hands out-prod-shaped ops TRANSPOSED grads as a matter of course:
the MUL_MAT backward passes ggml_transpose(grad) straight to ggml_out_prod, which is
exactly why ggml_compute_forward_out_prod_f32 -- the kernel mine is modelled on --
reads src1 through `i1*nb10` rather than indexing a float*. I dropped that.
A TRANSPOSE node reaching here has nb[0] == 8. Reproduced: 24 of 36 output elements
wrong, max abs err 9.2. No assert fires. The run just trains on a wrong gradient.
2. IDS (src2) was indexed as ids_t[i], likewise hard-coded.
The FORWARD reads ids through nb[0]
(ggml_compute_forward_mul_mat_id: `*(int32_t *)(ids->data + iid1*nb[1] + id*nb[0])`)
and ggml_mul_mat_id puts NO contiguity constraint on ids. So a strided ids is a legal
tensor that the forward computes CORRECTLY and the backward misread -- crediting the
wrong experts with each other's gradients. Reproduced: forward bit-identical across
two ids layouts, backward disagreeing by 1.03.
Both are now read through their nb[0]. b keeps a contiguity assert, because it is the
VECTOR operand of ggml_vec_mad_f32 -- a strided read there is not merely wrong, it is
unrepresentable.
WHY NOTHING CAUGHT THIS. test-backend-ops only ever builds a CONTIGUOUS grad, so no test
in the suite could see it -- and neither could my double-precision reference, which built
its own contiguous tensors. Every check I had said the kernels were exact, and they were,
on the only inputs anyone was feeding them.
So the guard is a new test case, not a comment. grad_transposed routes the objective
through cont(transpose(out)), which makes ggml_build_backward_expand hand MUL_MAT_ID's
backward a grad whose op is TRANSPOSE. Reintroducing the exact bug:
contiguous-grad cases (the entire old suite) -> PASS. invisible.
transposed-grad cases (the new guard) -> FAIL, MAA 21.0 / 2.57 / 2.55
Re-verified after the fix: double-precision reference across 15 configs (rel ~1e-7),
bit-identical across 1/2/4/8 threads, quantized-vs-dequantized bit-exact on q8_0/q4_K/q4_0,
strided ids identical to contiguous ids, MUL_MAT_ID grad 0/20 runs failing, the grad
allowlist and the MUL_MAT_ID forward unregressed.
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
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 #20 (S1-27). With this,
MUL_MAT_IDhas a complete backward.asmay be quantized, and that is the whole difficulty rather than a corner case. In a LoRA MoEgraph the base expert stacks are frozen Q4_K. They take no gradient themselves, but the activations
flowing into them do — because an earlier layer's LoRA is upstream. So
d(b)must propagate backthrough a quantized weight: the same dequantize-a-row-at-a-time path as
OUT_PROD, accumulating inF32 per ADR-0002.
MODE_GRAD cannot check the quantized path, and it took a while to work out why
ggml's quantized matmul quantizes the activations on the fly to
vec_dot_typebefore the dotproduct. So the forward is a staircase in
b: its finite difference at the FD step scale measuresquantization edges, not a derivative. The analytic gradient is the gradient of the intended smooth
function; the harness evaluates the actual quantized one. They disagree by construction — measured
MAA 0.028–0.071, with the kernel exact throughout.
Upstream reached the same conclusion and encoded it without a word of comment:
So the quantized cases are deliberately absent from MODE_GRAD, and the dequantize path is verified
a stronger way instead:
OUT_PROD_IDwith a quantizedas, againstOUT_PROD_IDwith that sameasdequantized to F32. Bit-exact on q8_0, q4_K and q4_0.
Two test fixes, both from measuring rather than assuming
The token count is 32, not 5.
mean_abs_asymmdivides by(gn + ga)— not(|gn| + |ga|)— soany output element whose true gradient is near zero sends that ratio to infinity. This op manufactures
such elements:
d_as[k,j,e]sums only over the slots that routed to experte, so with 5 tokens and 3experts it is a sum of one or two random products. A single element with asymm ≈ 700 drags the mean
of 90 up to ≈ 8.
Condition the test; don't widen the tolerance until the flapping stops.
max_maa_err = 5e-3, measured on both sides:gradentirelyne_b1broadcastgradrow 0 always10× above the noise floor, 200–1800× below every real defect. Five mutations introduced, five
caught.
Verification
Both kernels checked against a naive double-precision reference across 15 configurations covering
both broadcast modes: relative error ~1e-7, i.e. float32 epsilon. That reference is what proved the FD
failures were the harness and not the arithmetic.
Note its one blind spot, stated honestly: it shares the kernels' assumption about the broadcast rule,
so it cannot check that. The rule was confirmed by reading the forward instead —
row_mapping.i1isthe slot index (the in-code comment calling it "selected expert index" is wrong), so the forward's
bcolumn isslot % ne_b1.