test(mlx): guard MLX #3912 — fp_qmm_t misreads past K when K mod 32 == 16 - #315
Merged
Merged
Conversation
…d 32 == 16 Upstream MLX ollama#3912 fixed a Metal kernel bug in fp_qmm_t that reads past the K dimension when K mod 32 == 16. Our v0.34.1 fold crossed that fix when the MLX pin moved ce916dbb -> d9add9d1, which is why gemma4 26b/31b vision embeddings changed across #302 while 12b did not. K=4304 is the vision tower's mlp.down_proj contraction dimension in the gemma4 26b/31b nvfp4 checkpoints: stored packed as [1152, 538] at 8 values per word, cross-checked by the group-16 scale tensor at [1152, 269]. 12b carries no nvfp4 vision down_proj at all — its text ones are K = 15360 and 65536, both mod 32 == 0 — so it was structurally immune. K=4288 is the nearest aligned K and is the control. Verified in both directions rather than only passing: MLX ce916dbb (pre-fix) K=4288 max 0.000152588 0/294912 over 1.0 K=4304 max 24.0494 232722/294912 -> FAIL MLX d9add9d1 (post-fix) K=4288 max 0.000152588 0/294912 K=4304 max 0.137939 0/294912 -> PASS Two things the comment block records because they are what made this expensive to find. First, the production symptom was not a crash or an obviously wrong answer: one fine-text OCR tier moved from 4 to 3 on one model in one think mode, and localising it from there took a day. Second, M must exceed get_qmv_batch_limit (13 for this K and N) or the dispatch goes to the matrix-vector kernel and fp_qmm_t is never reached — a first attempt at M=8 came back clean on the buggy library and read as a refutation of a correct diagnosis. Shrinking M silently voids this test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
glennneuber
added a commit
that referenced
this pull request
Sep 18, 2026
Root cause found by reading MLX's own upstream PR history, which this investigation never did: ollama#3912 fixed a Metal kernel bug in fp_qmm_t that reads past the K dimension when K mod 32 == 16. The v0.34.1 fold crossed that fix when the MLX pin moved ce916dbb -> d9add9d1. K = 4304 is the gemma4 26b/31b vision tower's mlp.down_proj contraction dimension, stored packed [1152, 538] at 8 values per word and cross-checked by the group-16 scale tensor at [1152, 269]. 12b carries no nvfp4 vision down_proj at all, so it was structurally immune rather than a lucky control. Confirmed at kernel level (guard test in #315): on ce916dbb, K=4304 puts 232722 of 294912 output elements more than 1.0 away from dequantize+matmul, max error 24.05; on d9add9d1 it is zero and 0.138. The K=4288 control is bit-identical across both libraries. This inverts the document. The tier drop is a downstream consequence of CORRECTING a kernel, not a defect to undo: the 9px score of 4 was measured on a build corrupting most of that matmul, and 3 is what the correct kernel produces. It also resolves what the document carried as an unexplained paradox — a more faithful encoder scoring worse — which is exactly what a correctness fix looks like from a probe that happened to favour the broken output. Prior gemma4 26b/31b vision baselines were measured through that kernel and should be re-measured rather than compared against. Renamed: the file is no longer about an open regression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
A 40-line, 0.3-second, model-free guard against the root cause found for the vision regression tracked in #310 / #312.
The bug
Upstream MLX ollama#3912 fixed a Metal kernel bug in
fp_qmm_tthat reads past the K dimension whenK mod 32 == 16. Our v0.34.1 fold crossed that fix when the MLX pin movedce916dbb → d9add9d1.K=4304is the vision tower'smlp.down_projcontraction dimension in the gemma4 26b/31b nvfp4 checkpoints — packed[1152, 538]at 8 values per word, cross-checked by the group-16 scale tensor at[1152, 269]. 12b carries no nvfp4 visiondown_projat all (its text ones are K = 15360 and 65536, bothmod 32 == 0), which is why it was structurally immune and made such a good control.Verified in both directions
A regression test that only passes proves nothing, so this was run against both libraries:
ce916dbbpre-fixd9add9d1post-fixThe control is bit-identical across both libraries, so a failure there means something other than this bug. The failure message names ollama#3912 and prints the loaded MLX pin.
Two warnings in the comment block, both earned
The production symptom was almost invisible. Not a crash, not an obviously wrong answer — one fine-text OCR tier moving 4 → 3, on one model, in one think mode. 79% of that matmul's outputs were wrong by more than 1.0 and the model still produced coherent text. Localising it from that symptom took a day of bisecting.
Mmust exceedget_qmv_batch_limit(quantized.cpp:1804), which is 13 for this K and N. A first attempt at M=8 dispatched to the matrix-vector kernel, never reachedfp_qmm_t, came back clean on the buggy library, and read as a refutation of a correct diagnosis. If this test is ever "simplified" by shrinking M, it stops testing anything — so the comment says so explicitly.Scope
Test only.
gofmtapplied,go vetclean, passes on currentmain.🤖 Generated with Claude Code