Fix incorrect nvfp4 quantized_matmul through the split-K path#3854
Open
metascroy wants to merge 1 commit into
Open
Fix incorrect nvfp4 quantized_matmul through the split-K path#3854metascroy wants to merge 1 commit into
metascroy wants to merge 1 commit into
Conversation
qmm_splitk caps split_k only by the quantization group count (K / group_size), not by the kernel K-tile width BK (32). For nvfp4 (group_size=16) this yields a per-partition K of 16 < BK; fp_qmm_t_splitk then reads a full BK-wide K-tile with no K bound, spilling past the partition into the next group's weights and fp8 scales. The result is corrupted (non-uniform ~2x error) with NaN/inf on the last partition. Affine (group_size >= 32) is unaffected. Require each K partition to be a whole number of BK-wide tiles as well as whole quantization groups by aligning split_k to max(group_size, BK). Only changes behavior for group_size < 32.
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.
Summary
nvfp4quantized matmuls that take the split-K path (qmm_splitk/fp_qmm_t_splitk) produce incorrect results — non-uniform ~2x error andNaN/inf.affineis unaffected.Root cause
qmm_splitkcapssplit_kby the quantization-group count but not by the kernel's K-tile widthBK(=32):For
nvfp4(group_size == 16) this can pick a per-partitionKsmaller thanBK. Example:K=64,group_size=16→split_k=4,k_partition_size=16 < BK=32.The
qmm_tkernels tile K inBK-wide chunks and do not bound the K dimension —load_safe/load_unsafeonly bound M/N, andQuantizedBlockLoaderadvances scales byscale_step = BCOLS/group_sizeperBKstep. So when a partition is smaller thanBK, the single tile load reads a fullBK-wide slice that spills past the partition into the next group's packed weights and fp8 scales (and past the buffer on the last partition →NaN/inf).affinenever hits this becausegroup_size >= 32keeps partitions>= BK(orsplit_kcollapses to 1 and falls back toqmm).Fix
Require each K partition to be a whole number of
BK-wide tiles as well as whole quantization groups, by aligningsplit_ktomax(group_size, BK):This only changes behavior for
group_size < 32(i.e.nvfp4); forgroup_size >= 32it is identical to today. It preserves split-K where valid (e.g. theK=64case still runs 2-way withpartition=32=BK) and otherwise falls back toqmmvia the existingsplit_k <= 1path.Testing
nvfp4quantized_matmulwithM >= vector_limit,transpose=True, single batch (e.g.x=[32,64],w=[128,64],group_size=16): incorrect (NaN + ~2x error) before, matches the reference after.affineand largergroup_sizeunchanged.Open questions for reviewers
BKis hardcoded as32here to match theqmm_t_splitktemplate default (consistent with the existing localbm/bn = 32). Happy to use a named constant / template-derivedBKif preferred.max(group_size, BK)is correct because all supported group sizes (16/32/64/128) are in a divisor relationship with 32;std::lcm(group_size, BK)would be the fully general form.mxfp4(samefp_qmm_t_splitkkernel) get a regression test too? Its defaultgroup_size=32avoids the sub-BKpartition.