Skip to content

Fix qvm_split_k incorrect batch stride calculation - #3497

Merged
angeloskath merged 2 commits into
mainfrom
qvm-split-k
May 11, 2026
Merged

angeloskath merged 2 commits into
mainfrom
qvm-split-k

Conversation

@angeloskath

Copy link
Copy Markdown
Member

Fixes #3480 .

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@angeloskath
angeloskath merged commit 241f1bf into main May 11, 2026
16 checks passed
@angeloskath
angeloskath deleted the qvm-split-k branch May 11, 2026 18:42
sinistersoundworks pushed a commit to sinistersoundworks/Mac-MLX that referenced this pull request Aug 25, 2026
…urces so they apply

The release scan added in magicnight#108 listed 33 upstream fixes the fork does not
carry. Triaging the three that touch quantized matmul — the path most users
actually run — found two that reach us and one that cannot.

ml-explore/mlx#3497: qvm mishandles a stride-0 batch dimension. That is exactly
the shape mlx-swift-lm's quantizedScaledDotProductAttention produces, where a
quantized KV cache is broadcast across GQA repeat groups with expandDimensions.
Only the non-transposed product (scores @ values) reaches the affected kernel,
and only for M >= 2, which is why ordinary single-token decode never showed it
while speculative decoding sits squarely in range. Not hardware-gated —
upstream reproduced on an M4 Pro. Measured here before the fix: max |diff|
0.139 at N=2048 and 0.104 at N=4096 against a dequantized reference, versus
~2e-4 quantization noise, with M=1 correct — matching upstream's numbers and
its stated boundary.

ml-explore/mlx#3631: NAX qmm edge-tile bounds computed in short(), wrapping
past 32767, corrupting a column band for unaligned N above 2^15. Reached
through an lm_head whose vocab is unaligned. MiniCPM3-4B's 73448 is one, and
one this project lists as supported; every other vocab we support is a multiple
of 64 and takes the aligned branch. Measured here before the fix: max |diff|
236 at N=73448, K=2560, M=16.

ml-explore/mlx#3854 is NOT carried: upstream states affine quantization is
unaffected, and every model this project loads is affine.

Bumping the submodule was not enough. mlx-swift does not compile the
submodule's Metal kernel headers — it embeds them, stringified, into
checked-in files under Source/Cmlx/mlx-generated, and the JIT compiles those.
After the bump both tests still failed with byte-identical numbers, because the
generated copies still held the pre-fix kernel text. The fork now regenerates
them. #3810 hid this trap earlier: it changed host C++, which is compiled from
source, so a plain submodule bump was sufficient there and looked like the rule.

QuantizedMatmulParityTests pins both, using upstream's own repro shapes, and
includes the M=1 case as a control so the M>=2 boundary is asserted rather than
assumed.
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.

[BUG] quantized_matmul produces wrong results for GQA expand_dims broadcasting when M < vector_limit

2 participants