Conversation
Signed-off-by: xiuhu17 <zhihao.wang@perplexity.ai>
for more information, see https://pre-commit.ci
Signed-off-by: xiuhu17 <zhihao.wang@perplexity.ai>
Greptile SummaryThis PR extends distributed MXFP8 master-weight casting to support row-only primary storage while preserving the existing bidirectional path.
Confidence Score: 4/5The implementation appears safe to merge, with a non-blocking gap in automated multi-rank regression coverage. The row-only omission contract is consistently implemented across amax packing, scale updates, Python fragment selection, native validation, and guarded CUDA accesses. The remaining concern is that repository CI runs the new test only as a single process, leaving the changed cross-rank behavior unprotected. Files Needing Attention: tests/pytorch/mxfp8/test_rowwise_master_cast.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
M[Distributed master-weight shard] --> A[Compute partial rowwise amax]
M --> C{Columnwise storage present?}
C -->|Yes| CA[Compute partial columnwise amax]
C -->|No| O[Use empty 0x0 omission marker]
A --> P[Pack available amax values]
CA --> P
O --> P
P --> R[Single MAX all-reduce]
R --> S[Update available inverse scales]
S --> RW[Cast rowwise fragment]
C -->|Yes| CW[Cast columnwise fragment]
C -->|No| SKIP[Skip columnwise output]
Reviews (1): Last reviewed commit: "Share the NCCL process group across row-..." | Re-trigger Greptile |
| if owned: | ||
| torch.cuda.set_device(int(os.getenv("LOCAL_RANK", "0"))) | ||
| if "RANK" in os.environ: | ||
| torch.distributed.init_process_group("nccl") | ||
| else: | ||
| torch.distributed.init_process_group( | ||
| "nccl", store=torch.distributed.HashStore(), rank=0, world_size=1 | ||
| ) | ||
| yield torch.distributed.group.WORLD |
There was a problem hiding this comment.
The L0 MXFP8 suite collects this test using single-process pytest, while the L1 distributed suite does not select or launch it. As a result, the rank-partitioned branches—including the unaligned shard boundary, empty tail ranks, and mixed row-only/bidirectional layout—are not exercised in repository CI. This is non-blocking, but future regressions in the distributed behavior changed here could go undetected. Please register this test with the distributed suite or add equivalent self-launched multi-rank coverage.
Knowledge Base Used: Verification and CI matrix
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| * \param[in] input Input tensor (continuous segment of flattened original tensor). | ||
| * \param[in,out] amax_rowwise Output tensor for row-wise maximum absolute values. | ||
| * \param[in,out] amax_colwise Output tensor for column-wise maximum absolute values. | ||
| * Shape [0, 0] skips column-wise computation. |
There was a problem hiding this comment.
Checking for empty 2D shape is not great. Just check for has_data being false (empty NVTETensor).
| * original tensor. To skip column-wise computation, pass output_colwise with shape [0] | ||
| * and scale_inv_colwise with shape [0, 0]. Row-wise outputs remain required. |
| "Wrong padding of amax_colwise's cols"); | ||
| NVTE_CHECK(amax_colwise.data.shape[1] >= cols, "Invalid cols"); | ||
| NVTE_CHECK(amax_colwise.dtype() == input.dtype(), "Wrong dtype of amax_colwise"); | ||
| // Empty views can have a non-null data pointer: use shape to detect omission. |
There was a problem hiding this comment.
If there is an issue of the pointer passed from the PyTorch side, then that should be handled in the PyTorch extension rather than WARing it here.
|
|
||
| // Load scales_colwise | ||
| { | ||
| if (output_colwise != nullptr) { |
There was a problem hiding this comment.
Please include some benchmark results on the old vs new time for the case still using columnwise output to see whether those additional conditions didn't introduce overhead. If they did then they would need to be handled via templated kernels.
| # Copyright (c) 2022-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # See LICENSE for license information. | ||
|
|
||
| """Run with pytest on one GPU or torchrun -m pytest on multiple GPUs.""" |
There was a problem hiding this comment.
Why do we need a separate test file for that? It should be part of one of the larger suites like test_sanity or test_numerics. Also, for the separate test files, they would not actually be run unless they are part of the test.sh in one of the directories in qa/.
Description
Support distributed master-weight writeback into MXFP8 primary weights whose columnwise data and scales are absent.
Optimize memory usage for: radixark/miles#615
Related initialization change: #3468
Related Megatron integration: NVIDIA/Megatron-LM#7095
The initialization change permits row-only primary storage for high-precision/dequantized backward overrides. The existing distributed cast path nevertheless dereferences columnwise scales and writes columnwise data. This PR makes the native cast path handle that layout directly, independently of how the primary was initialized.
Changes
[0, 0]columnwise amax/scales and[0]output represent omission. Detect this by shape, since an empty view may still have a backing pointer.Scope
MXFP8 distributed cast only. No initialization policy changes, TMS/backup integration, NVFP4 changes, or Megatron-specific fallback are included. Kernel support can replace the high-precision tile-reconstruction fallback discussed in Megatron PR #7095; Megatron's separate gather copy-back layout handling remains relevant.
Validation
All checks below ran on
training_gb200_dev(NVIDIA GB200), using a source-built TE wheel in an isolated environment. The Python package and native extension paths were verified to point to that build, not a preinstalled TE. Build: CUDA 13.3; runtime Torch: 2.13.0+cu130; Python 3.14.6.tests/pytorch/mxfp8/test_rowwise_master_cast.py,tests/pytorch/mxfp8/test_mxfp8_master_weight_empty_shard.py, andtests/pytorch/test_partial_cast.py.torchrun --standalone --nproc_per_node=4 -m pytest tests/pytorch/mxfp8/test_rowwise_master_cast.py: 7 passed on each rank, process exit code 0.The distributed test fixture shares one process group across its module, avoiding repeated NCCL teardown/reinitialization between parameterized cases. Environment warnings were present for Python 3.14 TorchScript deprecation and unavailable optional FlashAttention 4/CUTLASS; these tests do not use FlashAttention.
Full Megatron training, checkpoint/resharding, end-to-end FSDP, and performance measurements are outside the validation performed for this PR.
Type of change
Checklist