[PyTorch] Add architecture gate to NVFP4 split_quantize RHT path - #3265
Open
davidkny22 wants to merge 1 commit into
Open
[PyTorch] Add architecture gate to NVFP4 split_quantize RHT path#3265davidkny22 wants to merge 1 commit into
davidkny22 wants to merge 1 commit into
Conversation
split_quantize with RHT-enabled NVFP4 quantizers dispatches unconditionally to the grouped Hadamard transform kernels, which are SM100 only, so GroupedLinear under the NVFP4 recipe fails at runtime on sm_120/sm_121. Apply the same architecture band the single-tensor path uses and fall back to per-tensor quantize outside it. Signed-off-by: David Kogan <davidkny22@gmail.com>
Contributor
Greptile SummaryAdds an architecture-gated fallback for NVFP4 split quantization with RHT.
Confidence Score: 5/5The PR appears safe to merge, with the fallback correctly populating each preallocated split through the existing per-tensor NVFP4 quantization path. The architecture check matches the existing fused-RHT eligibility band, while unsupported architectures avoid the grouped kernel and retain the expected output storage and metadata semantics. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["NVFP4 split_quantize with RHT"] --> B{"SM architecture 100–110?"}
B -->|Yes| C["Grouped RHT quantization"]
B -->|No| D["Quantize each split independently"]
C --> E["Return split NVFP4 tensors"]
D --> E
Reviews (1): Last reviewed commit: "Add architecture gate to the NVFP4 split..." | Re-trigger Greptile |
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.
Description
GroupedLinearunder the NVFP4 recipe fails at runtime on sm_120/sm_121.split_quantizewith RHT-enabled quantizers goes straight tosplit_quantize_nvfp4_impl_with_rht_helper, which calls the grouped Hadamard transform kernels. Those are SM100 only, so the launch fails:The single-tensor path does not have this problem.
NVFP4Quantizerchecksis_eligible_for_rht_cast_fusion, which gates on100 <= sm_arch <= 110, and uses the unfused RHT kernels otherwise. This adds the same check tosplit_quantize_nvfp4_impland quantizes the splits per tensor when it fails. That is slower than a fused grouped kernel would be, but it makes the configuration work.I repeated the band instead of calling
is_eligible_for_rht_cast_fusionbecause that function also checks shape alignment, and reusing it would change SM100 behavior. Happy to factor the band into a shared helper if you prefer.Fixes #3219
Type of change
Changes
transformer_engine/pytorch/csrc/extensions/cast.cpp: architecture gate and per-tensor fallback insplit_quantize_nvfp4_impl.tests/pytorch/nvfp4/test_nvfp4_group_quantize.py: test that RHT split_quantize matches per-tensor quantization, on either dispatch route.Checklist:
No documentation changes needed. On the last box, see below.
Testing done
GB10 (DGX Spark), sm_121a, CUDA 13.2, driver 580.95.05, main @ f1d5f8d built with
NVTE_CUDA_ARCHS=121a. cpplint, black and clang-format are clean on the changed files.The new test fails on all three parametrizations without the fix and passes with it. It is not gated on architecture, so it exercises the grouped kernels on SM100 and the fallback elsewhere.
The rest of
tests/pytorch/nvfp4/test_nvfp4_group_quantize.pygoes from 274 failures to 120. The 154 that start passing are the RHT cases withoptimize_for_gemm=False, which is what this patch covers.The 120 that remain fail before the patch too, for a different reason. They are all
optimize_for_gemm=True: swizzled scale factor emission is gated on the same architecture band, so the outputs here carry compact scale factors, but the test swizzles the reference wheneveroptimize_for_gemmis set.tests/pytorch/nvfp4/test_nvfp4_rht_quantize_exact.pyis 16 failures and 284 passes both before and after, so the single-tensor path is unaffected. Those 16 are small numeric mismatches already on main.The reproducer in #3219 fails before the patch and passes after: per-split rowwise dequant cos 0.995463 to 0.995507, norm ratio 0.999135 to 1.001009.
cc @zhongbozhu @cael-ling