Skip to content

fix(metal): int4-g64 (fmt=4) models never used the GPU — enable MoE experts + fused attention - #829

Open
aaristov wants to merge 2 commits into
JustVugg:devfrom
aaristov:fix/metal-i4-gpu-path
Open

fix(metal): int4-g64 (fmt=4) models never used the GPU — enable MoE experts + fused attention#829
aaristov wants to merge 2 commits into
JustVugg:devfrom
aaristov:fix/metal-i4-gpu-path

Conversation

@aaristov

@aaristov aaristov commented Aug 4, 2026

Copy link
Copy Markdown

Problem

With an i4 snapshot (int4 weights + per-group scales = fmt=4, e.g. GLM-5.2-i4), the Metal backend executed no decode work at all:

  • moe_submit (backend_metal.mm) gated fmt != 1 && fmt != 2 && fmt != 6 → every routed-expert block on every layer silently fell back to CPU (backend_metal.h documented fmt=4 batched-MoE as "future work").
  • Both fused-attention gates in colibri.c required kv_b.fmt==2 exactly → attention fell back too (a_qabs/a_ctx only decoded per-row scales).

Since the [METAL] mode: … banner prints regardless, users saw a "GPU-enabled" build running 100% on CPU, with no visible signal — the METAL: fallback counters are printed by profile_print, which serve mode never called.

Fix

Routed experts

  • moe_gemv gains an fmt=4 branch (per-group scale folded into the MAC, mirroring mm_gemv's existing fmt=4 logic; gsz at buffer index 10).
  • moe_submit / coli_metal_moe_block[_begin] take a gs4 parameter; allowlist now {1,2,4,6} with an even-positive group-size requirement.
  • MB_BUILD (colibri.c) captures the experts' group size and poisons it on any mismatch → clean CPU fallback, never wrong results.

Fused attention

  • a_deqrow (shared by a_qabs/a_ctx) decodes per-row (fmt=2) or per-group (fmt=4) kv_b scales; kvb_gs threaded through AttnW and both entry points, validated in encode_attention. colibri.c gates relaxed to kv_b.fmt==2||fmt==4.

Observability

  • run_serve now calls profile_print at exit under PROF=1, so the cumulative METAL:/METAL-ATTN: GPU-vs-fallback counters are finally reachable in serve mode (stdin is at EOF by then; no protocol interleaving).

Launcher (coli)

  • Splash strapline was hardcoded ("GLM-5.2 · 744B MoE · int4 · streaming CPU") — now derived from the model dir's config.json and the requested backend env, so it no longer claims CPU under COLI_METAL=1.
  • Chat status lines were invisible: the stderr drain blocked on read() until EOF (engine never closes stderr), and the "ready in Xs" detector looked in stderr for a line printf'd to stdout. Both fixed; [METAL]/[CUDA] whitelisted.

Tests (make metal-test, all green)

  • run_moe_g4: two batched-MoE fmt=4 cases vs a per-group CPU reference (nerr ~3e-6).
  • run_attn(kvb_g4=1): three fused-attention cases with grouped kv_b (nerr ~5e-6).
  • Renamed the test-local FP8 helper to ref_fp8_nblk: quant.h gained an fp8_nblk with a different return type, which broke the metal-test build (C++ can't overload on return type).

Measured (M4 base / 34 GB / GLM-5.2-i4 391 GB, NGEN=8, RAM_GB=24)

before after
METAL: counters (all CPU) blocchi GPU 1248 | fallback CPU 0 | expert su GPU 9973
METAL-ATTN: layer GPU 546 (100% of decode layers fused)
GPU utilization (ioreg) 1–13% (≈idle) 91–94%
decode p50 11.4 s/forward 5.9 s/forward (with PIPE=1 DIRECT=1)

Remaining time on this host is expert I/O (24 GB/token over a ~3 GB/s SSD at ~5% cache hit), not compute — RAM-rich machines should see proportionally more of the fix.

CPU-only build (make colibri) verified clean as well.

🤖 Generated with Claude Code

aaristov and others added 2 commits August 4, 2026 23:06
- The splash strapline was hardcoded ("GLM-5.2 · 744B MoE · int4 · streaming
  CPU") regardless of the loaded model or backend. It is now derived from the
  model dir's config.json (name + expert count) and the requested backend
  (COLI_METAL/COLI_CUDA/COLI_VULKAN); the old text remains only as fallback
  for model-less commands. The engine's own [METAL]/[CUDA] line stays the
  confirmation that the tier actually engaged.
- The stderr drain called p.stderr.read(), which only returns at EOF; the
  engine stays alive on stdin, so every load-time status line was invisible
  (the bounded 1s wait always expired against an empty file). Now reads
  readline() in a loop with per-line write+flush.
- The "ready in Xs" detector scanned stderr for "loaded in ...", but that
  line goes to stdout and was being discarded by stream_turn. Now captured
  from the preamble and matched there.
- Added [METAL]/[CUDA] to the chat status-line whitelist.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sed attention

With an i4 snapshot (int4 weights, per-group scales = fmt=4, e.g. GLM-5.2-i4)
the Metal backend never executed any decode work: moe_submit gated
fmt != 1/2/6 and both fused-attention gates required kv_b.fmt==2 exactly, so
every routed-expert block and every attention layer silently fell back to
CPU. The [METAL] banner printed regardless, making the idle GPU hard to see.

Routed experts:
- moe_gemv gains an fmt=4 branch (per-group scale folded into the MAC,
  mirroring mm_gemv's; gsz at buffer index 10).
- moe_submit / coli_metal_moe_block[_begin] take a gs4 parameter; the fmt
  allowlist now includes 4 (even positive group size required, else CPU).
- colibri.c's MB_BUILD captures the experts' group size and poisons it on
  any mismatch (gs4=-1 -> moe_submit refuses -> whole subset stays on the
  CPU path, never wrong results).

Fused attention:
- a_deqrow (shared by a_qabs/a_ctx) now decodes per-row (fmt=2) or
  per-group (fmt=4) kv_b scales; kvb_gs threaded through AttnW and
  coli_metal_attn_decode / coli_metal_layer_decode, validated in
  encode_attention. Both colibri.c gates relaxed to kv_b.fmt 2 or 4.

Observability:
- run_serve now calls profile_print at exit under PROF=1: the cumulative
  METAL:/METAL-ATTN: counters were structurally unreachable in serve mode
  (only the oracle/generate exit paths printed them), so a served session
  could never show GPU-vs-fallback truth. stdin has hit EOF by then, so the
  frames cannot interleave with protocol a client is parsing.

Tests (make metal-test, all green):
- run_moe_g4: two batched-MoE fmt=4 cases vs a per-group CPU reference.
- run_attn(kvb_g4=1): three fused-attention cases with grouped kv_b.
- test-local FP8 helper renamed ref_fp8_nblk: quant.h:483 gained fp8_nblk
  returning int64_t, colliding with the test's self-contained int version
  (C++ cannot overload on return type), which broke the metal-test build.

Measured on an M4 / 34 GB with GLM-5.2-i4 (391 GB, NGEN=8, RAM_GB=24):
before: METAL blocks all-CPU, decode p50 11.4 s/forward; after: blocchi GPU
1248 | fallback CPU 0, METAL-ATTN layer GPU 546 (100% of decode layers),
p50 5.9 s/forward with PIPE=1 DIRECT=1 (remaining time is expert I/O on
this RAM-constrained host, not compute).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@JustVugg

JustVugg commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Welcome, and thank you — this is a real fix for a real defect, and you found the same root cause I did when I went through #813 independently:

-  if (!g_dev || (fmt != 1 && fmt != 2 && fmt != 6)) return nil;
+  if (!g_dev || (fmt != 1 && fmt != 2 && fmt != 4 && fmt != 6)) return nil;

I have to tell you something before you invest more in it, and I would rather you hear it from me now than from a merge conflict later.

#587 is doing the same work, and has been for two weeks. @RDouglasSharp opened it against #585 on 21 July. It touches the same four files this does — backend_metal.h, backend_metal.mm, colibri.c, tests/test_backend_metal.mm — carries 27 comments of review, and on 1 August he wrote "Rebased onto current dev and green… Ready to merge". dev then moved again underneath him.

That is not your fault. Nothing on #813 or #585 pointed at #587 as work-in-progress, and the only reason I know is that I compared the file lists. The gap is mine to close, and I have asked myself the same question about DeepSeek twice this week.

Your PR is not redundant, though, and this is the part worth keeping. #587 fixes the decode path. Yours also fixes why nobody noticed for two weeks:

the METAL: fallback counters are printed by profile_print, which serve mode never called

That is the actual bug behind #813. The banner said "GPU-enabled", the counters that would have contradicted it were never printed, and a user running coli serve on an i4 snapshot had no way to learn their GPU was idle. run_serve calling profile_print under PROF=1 is a small change with more value than it looks, and I do not see it in #587.

There is also MB_BUILD poisoning the group size on a per-expert mismatch so the fallback is clean rather than wrong — that instinct, fail to CPU rather than compute garbage, is exactly right for a mixed-format container.

What I am proposing, and it is a proposal — you two decide, not me:

@RDouglasSharp has seniority here by two weeks and 27 comments, so #587 lands the decode path first. @aaristov, would you be willing to rebase this onto it and keep the observability and the MB_BUILD guard as a focused follow-up? That is a smaller PR, it reviews in one pass, and it closes #813 properly rather than by side effect.

If instead you two look at the two diffs and conclude the reverse — that this one is the better base and #587 should reduce to its delta — say so and I will take that. You are both closer to the Metal code than I am, and the last two overlaps on this repo were settled better by the contributors than by me.

One thing I can promise: whoever ends up rebasing will not be doing it because I let it sit. Both of you have a decision within a day.

Your CI had never run, incidentally — it was held in action_required, which GitHub applies to a first contribution from a fork and shows nowhere on the PR page. I approved it: 13 checks, no failures.

@JustVugg

Copy link
Copy Markdown
Owner

@aaristov this is wanted — it is the decode-path half of what #918 now proposes for prefill, and both should share the same expert-view plumbing — but it's marked CONFLICTING against dev. A rebase would put it back in the review queue; ping here if anything in the conflict looks like it came from our side and we'll help untangle it.

@JustVugg
JustVugg changed the base branch from main to dev August 11, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Difetto verificato nel codice metal Backend Metal/Apple needs-rebase Confligge, serve rebase dell'autore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants