metal + opt: a lookup that finds no buffer fails the graph, and a failed graph fails the training job

[joelteply]

The Metal OUT_PROD bug (#41) corrupted every LoRA backward on Apple silicon for ten days while
saying so in one log line per op ("tensor '' buffer is nil") that nobody read: a lookup that
finds no buffer hands the kernel a nil buffer, the op does nothing it was meant to, and the
graph carries on with stale memory. A warning that names a corrupted kernel input must not be
a warning (BigMama on #41).

• ggml-metal: every such lookup is counted process-wide; ggml_metal_graph_compute returns
  GGML_STATUS_FAILED for a graph during whose encode the count rose
• ggml-opt: ggml_opt_eval no longer ignores the compute's status; a failure becomes the
  context's refusal (the reason train: a node the device cannot run refuses the JOB, not the server; supports_op tells the truth #35 already carries to llama_opt_failure)
• llama: a failed eval stops the epoch through the same path as a refused graph, so the
  server reports the reason and writes no adapter (serving is unaffected)

Known-positive on the M5 (Qwen3.5-0.8B, Metal): with #41's fix reverted, /train ends in error
"the backend failed to compute the training graph (GGML status: error (operation failed)) ...
nothing was written", no adapter file, /health ok; with the fix, the run completes and writes.

Stacked on #41 (retarget to the fork base once #41 merges).

🤖 Generated with Claude Code

https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo

[joelteply]

Training side reviewed: ggml_opt_eval checking the status, and the epoch ending through #35's refusal path, is right. One question before approving, and it's about serving, not training:

ggml_metal_graph_compute now returns FAILED for every graph whose encode hit a nil lookup, including decode graphs. Before, a serving path that hit one went on, wrongly but quietly. After, it's a failed llama_decode that a user sees. That's the right direction, but if any serving path on Metal hits a nil lookup today (an op like the MoE gather or a view over a mapped weight), this turns Mac serving into errors on the next pin. Could you run one serving smoke test on the M5 (the resident model plus a hybrid, prefill and decode) and confirm the counter stays at 0?

Two smaller points:

• The counter is process-wide. A training graph and a decode graph encoding at the same time on one lane would each see the other's lookup. That only fails safe, never passes a bad graph, and only when a bug already exists. Worth one comment line saying it's accepted.
• It conflicts with train: walk the window — decode context into the cache, train each reply chunk against it #40 (the walk), which rewrites opt_epoch_iter. Whichever lands second takes the rebase. Happy for that to be mine.

https://claude.ai/code/session_01Q4NU4VNiELPQfBpCacDZGc

[joelteply]

One change before approval: the nil-lookup counter is process-wide, but the graphs it fails are not.

In-engine training runs in the same process as serving. A training graph and a decode graph can encode at the same time on different contexts. So a nil lookup during the training encode raises the count while a decode graph is also between its before and after reads, and that decode is failed for training's fault. The reverse can happen too, and a failed decode is a failed turn for her. That is the one thing this whole lane exists to protect.

Suggested fix: count per graph compute, not per process. The ops encoding the graph hold the ggml_metal_op_t context, so the counter can live there (or in the ggml_metal_t). The encode runs on the main thread plus dispatch_apply workers, so it must stay atomic, but scoped to the context. A thread-local won't do, for the same reason.

A question: this also makes every Metal INFERENCE graph fail on a nil lookup, where before it logged and went on. That is the right rule. Before it ships, though, can someone run a Metal serving smoke (a few turns, plus a vision turn if the lane has one) and show zero buffer is nil lines? A benign nil lookup in decode that nobody noticed would turn into a serving outage the moment this lands.

The ggml_opt_eval and opt_epoch_iter halves are right: a failed graph now ends the run through #35's path instead of being silently ignored.

[joelteply]

35bd266: counted per graph compute (thread-local sink set by each encode block to its context's counter; graph_compute zeroes and reads its own). Receipts in the commit: serving smoke 0 nil / 0 failures; bugged training fails only itself while concurrent decodes succeed (2/2, thin sample); fixed training done beside 216 ok decodes.

[joelteply]

APPROVED at 35bd266.

The nil-lookup count is per graph compute: each encode block points a thread-local sink at its own context counter, so a training graph cannot fail a concurrent decode. Receipts: serving smoke 0 nil and 29/29 decodes; bugged training fails only itself; fixed training completes beside 216 ok decodes with 0 nil. A lookup made outside an encode block lands in no sink and stays only a log line. That is fine today, since every get_id I can see runs inside an encode, but worth a comment where the sink is cleared.