Fix nvfp4 quantize and dequantize on the CPU for element counts that are not a multiple of 32 - #4385
Closed
kapellirohith wants to merge 1 commit into
Closed
Fix nvfp4 quantize and dequantize on the CPU for element counts that are not a multiple of 32#4385kapellirohith wants to merge 1 commit into
kapellirohith wants to merge 1 commit into
Conversation
…are not a multiple of 32 The 4-bit fp fallbacks grouped the flattened values into fixed blocks of 32 before packing them into uint32 words. nvfp4 is the only mode with group size 16, so it is the only one whose element count can be a multiple of 16 but not of 32, and those shapes made the reshape fail. Group by the 8 nibbles that share a word instead, and view the packed words as bytes without grouping. Outputs are unchanged for every shape that already worked.
kapellirohith
force-pushed
the
nvfp4-cpu-shape
branch
from
August 23, 2026 19:22
eff5bae to
2465196
Compare
Member
|
Please hold on creating new PRs before we review your previous ones, we can't review as fast as you create because: they are really minor problems that no ones cares in practice, it takes a lot of time to verify whether the fix is correct, and we can't trust any word in the PR description. |
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
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.
Problem
mx.quantizeandmx.dequantizeinnvfp4mode reject shapes on the CPU that the GPU accepts. Same op, same input, two backends disagree.Base
d9077d831Chip M3 Pro
macOS 26.6.1
Before:
After: both succeed, and the scales and dequantized values agree.
dequantizefails the same way on the same shapes (Cannot reshape array of size 66 into shape (16,4)). The minimal case is(1, 16), which asks for a reshape into(0, 4, 8), a literal zero dimension.Cause
The 4-bit packing in the composed fp fallbacks groups the flattened values into fixed blocks of 32 before packing them into
uint32words:ops.cpp:5189, infp_quantize:reshape(wq, {-1, 4, 8}, s)ops.cpp:5435, infp_dequantize:view(reshape(out, {-1, 4}, s), int8, s)Both require the flattened element count to be a multiple of 32.
fp_quantizereshapes to{-1, group_size}first (ops.cpp:5136), so that count isR * Kfor a matrix ofRrows andKcolumns.nvfp4is the only mode withgroup_size16, so it is the only one whose element count can be a multiple of 16 but not of 32.mxfp4andmxfp8usegroup_size32, and every affine group size is a multiple of 32, so none of them can reach this. Withinnvfp4the failing set is exactly those inputs whose flattened element count is divisible by 16 but not by 32.This inconsistency was noted in passing in #3912 while fixing the GPU side of the same
group_size = 16tail. This is the op-layer half of it.Fix
Group by the 8 nibbles that actually share a word, and view the packed words as bytes without grouping them first.
On the quantize side the
8is load-bearing: it matchesshifts, which ispower(2, arange(0, 32, 4))and therefore 8 wide, and the sum is over the last axis. The4was arbitrary. Row-major flattening makes{-1, 8}and{-1, 4, 8}produce the same flat word sequence before the finalreshape(new_shape).On the dequantize side
gatherbuilds its output shape asindices.shapefollowed byslice_sizes, sogather(lut, idx, 0, {1}, s)appends a trailing axis of size 1 andconcatenate({lo, hi}, -1, s)interleaves the low and high nibble per byte regardless of the leading shape. The grouping reshape was only there to giveviewa shape;viewrescales the last axis on its own, so it is not needed.View::eval_cpu(mlx/backend/cpu/primitives.cpp:367) handles a non-contiguous input itself: it shares the buffer with rescaled leading strides when the last axis is contiguous or the array is row contiguous, and otherwise makes a contiguous copy. Dropping the reshape therefore does not change what the byte view walks.Both forms walk the same flat sequence, so the result is unchanged wherever the old form was legal.
Testing
All numbers measured against
d9077d831.Both hunks are required. Applying either one alone leaves the new test failing 6/6 on both devices, so neither changed line is dead.
nvfp4quantize, CPU-only failuresnvfp4dequantize, CPU-only failuresmxfp4/mxfp8/affinetest_quantized, JITThe grid is 4 modes x 2 ops x rows 1 to 40 x K in {16, 32, 48, 64, 80, 96, 112, 128} x {float32, float16, bfloat16}. That is 3840 records, 1680 skipped because
K % group_size != 0, and 2160 evaluated. The 240 failures per op are 80 per dtype, which is the 20 odd row counts times the 4 K values whereK / 16is odd, exactly the set predicted above. No shape fails on the Metal backend before or after.The byte-identity check hashes every quantize and dequantize output on both builds across 15 row counts, 9 K values, 5 higher-rank shapes, 4 modes, 3 dtypes and both backends. Of the 3954 outputs that exist on both, none changed. The 198 that newly appear are 99 quantize and 99 dequantize, all
nvfp4, all CPU, which is 33 tail shapes times 3 dtypes times 2 ops. Nothing regressed.Under
DEVICE=cputhe suite has 8 failures. They are identical to a pristine rebuild of the same base: 6test_conv2d_winograd_batch_tilingsubtests,test_fft_too_large, andtest_export_import.test_leaks. All pre-existing and unrelated.The equivalence argument was also checked directly rather than only reasoned about, by rebuilding both graph forms with
mxops and comparing them: 13 quantize and 26 dequantize comparisons on contiguous inputs, plus 7 non-contiguous packed views (row slice, strided rows, reversed, column slice, strided last axis, swapaxes, broadcast). Zero mismatches.CUDA validation is source-level only. No NVIDIA hardware was used.
Tests
test_nvfp4_element_count_alignmentcovers(1, 16),(3, 16),(33, 16),(5, 80),(7, 112)and(1, 1, 16), which include the zero-dimension minimal case and a higher-rank shape, plus the controls(2, 16)and(1, 32)whose count is already a multiple of 32, each in float32, float16 and bfloat16. Inputs are built from the fp4 lookup table with the group maximum forced to 6, so the scale is exact and the round trip is exact, matching the construction the neighbouringtest_nvfp4_quantize_dequantizealready uses. The test asserts the CPU round trip is exact and, where a GPU is available, that the CPU and GPU scales and dequantized values are equal.It asserts on scales and dequantized values rather than on packed code bytes. The CPU
argminpicks lut index 0 (+0.0) where the GPU encoder keeps the sign bit and emits index 8 (-0.0), on shapes that already worked as well as new ones, so packed bytes are not a valid equality target on either backend.Checklist
pre-commit run --all-filesto format my code / installed pre-commit prior to committing changes