make FP4 param gather work with the mixed precisions in NVFP4 recipe#4358
Conversation
Signed-off-by: Xiaowei Ren <xren@nvidia.com>
|
This PR has been automatically converted to draft because all PRs must start as drafts. When you are ready for review, click Ready for Review to begin the review process. This will:
See the contribution guide for more details. |
|
/claude review |
There was a problem hiding this comment.
Review: Critical bug — TypeError when name is None
The name parameter defaults to None in all modified constructors, but is unconditionally used in string concatenation (e.g., name + ".self_attention"). The primary model construction path through TransformerBlock.build_layer in transformer_block.py does not pass name, so any model using the standard transformer block will crash with:
TypeError: unsupported operand type(s) for +: 'NoneType' and 'str'
This affects all 10+ files where name + "..." concatenation was added.
Suggested fix: Update transformer_block.py:build_layer to pass a name argument (e.g., name=f"decoder.layers.{layer_number}"), and add a defensive guard or make name required to prevent future callers from hitting the same issue.
|
/ok to test c6b5f06 |
Signed-off-by: Xiaowei Ren <xren@nvidia.com>
Signed-off-by: Xiaowei Ren <xren@nvidia.com>
Signed-off-by: Xiaowei Ren <xren@nvidia.com>
|
/ok to test f326ce3 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26264704092 |
|
/ok to test a394695 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26270483070 |
What does this PR do ?
Make --fp4-param-gather work with the mixed precisions in NVFP4 recipe.
Fixes #2284.
Contribution process
Pre-checks
Code review
Feel free to message or comment the @mcore-oncall to help accelerate your merge into main. The less complex your PR is, the faster it will be approved and merged!
All PRs start as draft. If you open a non-draft PR, it will be automatically converted to draft.
Step 1: Mark PR as "Ready for Review"
.github/CODEOWNERS.Final Review might get declined if these requirements are not fulfilled.
Step 2: Final Review
For PRs that change
megatron/core, once all expert reviewers have approved, theFinal Reviewlabel is applied automatically and final reviewers are assigned.For PRs outside
megatron/core, this step is skipped.Step 3: Approved
Once all required reviewers have approved, the
Approvedlabel is applied automatically.Merge
Any member of mcore-engineers will be able to merge your PR.
For MRs into `dev` branch
The proposed review process for `dev` branch is under active discussion.MRs are mergable after one approval by either
eharper@nvidia.comorzijiey@nvidia.com.