Repository navigation
metal: OUT_PROD's transposed F32 src0 is viewed in src0's own buffer, not op's - #41
Conversation
… not op's
ggml_metal_op_out_prod built the transposed view of an F32 src0 as out_prod_view(op, ...,
s_view.data): op's buffer with src0's address. When src0 lives in another buffer, as a LoRA
weight does (blk.N.*.lora_a/b, in the adapter's buffer), ggml_metal_buffer_get_id found no
buffer holding that address ("tensor '' buffer is nil"), the S^T copy into scratch never ran,
and the mul_mm read stale scratch. Every OUT_PROD with a LoRA weight as src0 (the backward
through A/B into the activation gradient) was wrong on Metal since 973357e, deterministically
but differently per graph layout: it is why recompute ON and OFF disagreed on Metal (#37) and
why recomputing only some layers gave NaN.
The view is now based on S, which carries the buffer that holds its data.
Measured on Qwen3.5-0.8B Q8_0 (M5, Metal), window 1024, seed 7, 5165 tokens, 1 epoch:
- "buffer is nil": 70 per run before (10 per window, every run, recompute off or on); 0 after
- eval loss, recompute off: 1.9291604 before, 1.9292085 after (the old value was computed
from stale scratch)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
… on #41) Every test-backend-ops case allocated a, b and out in ONE buffer, so a kernel that resolved src0's address against op's buffer passed them all: Metal's F32 transposed path did exactly that and read stale scratch in every LoRA backward. test_out_prod gains a_own_buffer, which puts a in the weight context (its own buffer, as a LoRA weight sits in its adapter's buffer during training), at LoRA's shape (rank 8 against a 1024-wide activation), F32 and q8_0, both gradient layouts. It runs on every backend against the CPU. Known-positive on the M5: with #41's fix reverted, both F32 cases FAIL (ERR = inf, "buffer is nil"); with it, 33/33 OUT_PROD cases pass on MTL0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
|
Reviewed, approve (as a comment: GitHub won't let our shared account approve). The fix bases the view on |
|
The fix is right. The view now takes its buffer from One receipt before I approve, please: the new F32 |
… never process-wide (Cormac on #42) A process-wide counter let a training graph's nil lookup fail a decode graph encoding beside it, which costs her a turn for a fault that was not hers. Each Metal context now owns its count: every encode block (the main thread's and the dispatched ones) points a thread-local sink at its context's counter while it encodes and clears it after, a nil lookup increments the current sink, and ggml_metal_graph_compute zeroes its own count on entry and fails only its own graph. Measured on the M5 (Qwen3.5-0.8B, Metal, 2 slots decoding): - serving only, fix in: 29 decode requests ok, 0 failed, 0 "buffer is nil" - #41's fix reverted, training beside decoding: the training job fails on its first graph ("found no buffer while encoding this graph"), the decode requests in flight with it all succeed (2 ok, 0 failed: a thin sample, one failing graph) - fix in, training beside decoding: training done, 216 decode requests ok, 0 failed, 0 "buffer is nil" Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
|
Receipt asked for: with this PR's one-line fix reverted, on the M5 (MTL0) |
|
APPROVED at 40163d7. The mutation receipt settles it: with the fix reverted, both F32 own-buffer cases fail on MTL0 (ERR=inf, buffer is nil, 31/33); with it, 33/33. The test now guards this class on every backend. |
1 participant
ggml_metal_op_out_prod built the transposed view of an F32 src0 as out_prod_view(op, ...,
s_view.data): op's buffer with src0's address. When src0 lives in another buffer, as a LoRA
weight does (blk.N.*.lora_a/b, in the adapter's buffer), ggml_metal_buffer_get_id found no
buffer holding that address ("tensor '' buffer is nil"), the S^T copy into scratch never ran,
and the mul_mm read stale scratch. Every OUT_PROD with a LoRA weight as src0 (the backward
through A/B into the activation gradient) was wrong on Metal since 973357e, deterministically
but differently per graph layout: it is why recompute ON and OFF disagreed on Metal (#37) and
why recomputing only some layers gave NaN.
The view is now based on S, which carries the buffer that holds its data.
Measured on Qwen3.5-0.8B Q8_0 (M5, Metal), window 1024, seed 7, 5165 tokens, 1 epoch:
from stale scratch)
Pending in this PR or follow-ups (from review in the room): a CPU reference for the same config (running); a lookup that finds no buffer fails the training job instead of computing on stale scratch; a test-backend-ops case with src0 in its own buffer (OUT_PROD, MUL_MAT), against CPU, on every backend.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo