Skip to content

pr-set: pin the Metal NVFP4 kernels (#330) - #331

Merged
danielhanchen merged 3 commits into
masterfrom
pinset/pin-metal-nvfp4
Oct 29, 2026
Merged

danielhanchen merged 3 commits into
masterfrom
pinset/pin-metal-nvfp4

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Pins #330 by @Lyxot (metal: add NVFP4 mul_mat, mul_mat_id and get_rows kernels) at 3adf93fa5, so the macOS prebuilt runs NVFP4 weights on the GPU instead of silently on the CPU (unslothai/unsloth#15409).

Upstream state

Upstream Metal has no NVFP4 kernels. ggml_metal_supports_mul_mat_op() rejects an NVFP4 src0 for MUL_MAT and MUL_MAT_ID, and GET_ROWS rejects it too, on every tag from b11922 to b11933 and on current master. The loader then places those weights in CPU buffers while still reporting every layer as offloaded. MiniMax-M3.1 UD-Q4_K_XL and UD-Q8_K_XL have all 171 routed-expert tensors in NVFP4, so on a Mac the experts run on the CPU. As #330 notes, ggml-org#20456 adds Metal NVFP4 upstream, but it is not merged and is written against the old single-file shader layout, so it no longer applies.

#330 adds the kernels for every path the three ops can take: mul_mv, mul_mv_ext, mul_mv_id, the few-row mul_mv_mma kernels, mul_mm, mul_mm_id and get_rows, and drops the NVFP4 rejections in supports_op. Thanks to @Lyxot, who also tested it on an M3 Max.

Changes

  • scripts/unsloth/pr-set.json: append unslothai/llama.cpp#330 @ 3adf93fa5ae019c153c8c1ace3a698189b7db41d after vulkan: copy a misaligned top_k input to an aligned scratch instead of aborting #328.
  • scripts/unsloth/feature-checks.json: unslothai#330 under unchecked. The behaviour is covered by test-backend-ops MUL_MAT, MUL_MAT_ID and GET_ROWS with -p nvfp4, which need a Metal device, and no runner in the pipeline has one. A backend-op check would pass on CUDA, which already has NVFP4 upstream, so it cannot tell the pin apart.

#330 is based on base/upstream-d854d98ba, an ancestor of every tag simmed below, and only touches ggml/src/ggml-metal. No other pin touches that directory. The pin is dropped once upstream merges ggml-org#20456 or an equivalent.

Evidence

Pin set = master + this pin (25 pins). audit: 24/25 clean, #330 clean, feature-checks coverage ok. The one FIX row is #320 (pin != PR head, stale base), which is the same on master.

tag #330 merge #330 pin_contract merge_checks
b11922 clean intact clean
b11925 (aged) clean intact (115 lines) clean
b11933 (newest) clean intact (115 lines) clean

The full set does not merge on these tags with or without this pin: ggml-org#24423, ggml-org#25731, #241 and #298 are refused on all three, and #251 as well on b11933. On b11925 and b11933 the refused list and the failing contract rows are the same as for master's pin set alone. Those pins need their own repins before any nightly can go green. This PR does not change that.

Because of that, the CPU checks ran on b11925 plus this pin alone:

check result
CPU gate (llama, test-llama-archs, test-backend-ops, test-mtmd-impl) ok
test-chat all passed
test-backend-ops -b CPU -p nvfp4 MUL_MAT / MUL_MAT_ID / GET_ROWS 45 / 73 / 8 OK, 0 failed (CPU against CPU, so it cannot reach the Metal code)
feature_matrix (CPU) 5 features ok; the ones that fail need pins not in this tree (ggml-org#24423, ggml-org#25731, #70, #176, #298)

The Linux CPU build does not compile ggml/src/ggml-metal, so this mainly shows that the pin breaks nothing else. The Metal evidence is from GitHub macOS arm64 runners (macos-15 and macos-26, Apple Paravirtual GPU), #330's head against its own base d854d98ba. Both arms were built from the same source, with a CI-only switch that turns on simdgroup ops on the virtual GPU. The model is a tiny random-weight Qwen3-MoE with Q8_0 weights and NVFP4 routed experts, the same layout as UD-Q4_K_XL.

check base (d854d98) with #330
test-backend-ops -b MTL0 -p nvfp4 MUL_MAT 0 run, 78 not supported 45/45 passed, 33 not supported (all type_b=f16, same rule as MXFP4)
MUL_MAT_ID 0 run, 73 not supported 73/73 passed
GET_ROWS 0 run, 8 not supported 8/8 passed
MXFP4 control (MUL_MAT / MUL_MAT_ID / GET_ROWS) 88/88, 98/98, 8/8 88/88, 98/98, 8/8
expert weights CPU_Mapped MTL0_Mapped
tg64 t/s, 2 rounds x 3 reps (macos-15) 41.7, 42.7 80.1, 100.4
tg64 t/s (macos-26) 72.6, 65.0 138.6, 141.1
pp128 t/s (macos-15) 675, 437 4208, 5185
pp128 t/s (macos-26) 934, 1278 5120, 4631
mean KLD vs an F32 CPU reference of the same weights (macos-15 / macos-26) 0.0042 / 0.0042 0.0010 / 0.0010
same top token vs that reference 92.1% / 92.2% 97.6% / 97.6%

The KLD and top-token numbers match an independent port of the same kernels to 4 decimals on the same model, and the speed is within runner noise of it. The UE4M3 scale decode (ue4m3_to_fp32 in kernels/common.h) agrees with the CPU ggml_ue4m3_to_fp32 on all 256 byte values, including 0x7F (decodes to 0) and scale bytes with bit 7 set (ignored on both sides).

Not run here: a CUDA build of the merged tree and the GPU feature checks (no local GPU in this session; the pin touches no CUDA source), bare-metal Apple Silicon (covered by the author's M3 Max run), and the ROCm, Vulkan, Windows and ARM legs, which only the nightly builds.

@danielhanchen danielhanchen self-assigned this Oct 27, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T16:05:16.946583Z f2b6c31 PR opened
🔒 Security Review ✅ Completed 2026-10-27T16:08:12.135717Z f2b6c31 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

# Conflicts:
#	scripts/unsloth/feature-checks.json
#	scripts/unsloth/pr-set.json
@danielhanchen
danielhanchen merged commit 04d7180 into master Oct 29, 2026
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant