Skip to content

Fixed fp32 residuals - #3504

Merged
deepakn94 merged 6 commits into
NVIDIA:mainfrom
mkhona-nvidia:mkhona/fp32_residuals
Feb 25, 2026
Merged

Fixed fp32 residuals#3504
deepakn94 merged 6 commits into
NVIDIA:mainfrom
mkhona-nvidia:mkhona/fp32_residuals

Conversation

@mkhona-nvidia

@mkhona-nvidia mkhona-nvidia commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

Fix FP32 residual stream activations in Megatron which currently silently break.

Problem

The existing fp32_residual_connection config flag was broken, it never actually kept the residual stream in fp32. The core issue was in _bias_dropout_add_func (used by every layer type), which downcast the fp32 residual to match the computation dtype (bf16/fp16):

Previously: defeated fp32 residuals by casting residual DOWN to x's dtype:

residual = residual if residual.dtype == x.dtype else residual.to(x.dtype)

This meant:
The embedding layer correctly emitted fp32 tensors when fp32_residual_connection=True

But the very first bias_dropout_add call cast the residual back to bf16. The fp32 residual stream was lost from layer 0 onwards
Additionally, the transformer layer never cast its residuals to fp32 — only the Mamba layer had its own residual_in_fp32 flag, which also got defeated by the same downcast.

Solution

Reverse the casting direction in _bias_dropout_add_func: instead of downcasting the residual, upcast x (and bias) to match the residual's dtype. This preserves the fp32 residual stream through every bias-dropout-add call.

megatron/core/fusions/fused_bias_dropout.py Upcast x/bias to residual dtype instead of downcasting residual to x dtype

megatron/core/transformer/transformer_layer.py Cast residual to fp32 at all 4 residual sites (self-attn, cross-attn, MLP, MoE router) when fp32_residual_connection=True

megatron/core/ssm/mamba_layer.py Respect config.fp32_residual_connection in addition to the existing residual_in_fp32 flag

megatron/core/transformer/transformer_config.py Auto-override pipeline_dtype to torch.float when fp32_residual_connection=True to prevent PP communication dtype mismatches

Why the old approach was wrong

The original downcast was added to prevent pipeline parallel communication hangs (the residual would upcast the output to fp32, mismatching the bf16 PP recv buffers). The correct fix is to set pipeline_dtype = torch.float when fp32 residuals are enabled, which this PR does automatically in TransformerConfig.__post_init__. The inference wrapper (abstract_model_inference_wrapper.py) already handled this correctly.

⚠️ For major changes (either in lines of code or in its impact), please make sure to first share a design doc with the team. If you're unsure what's the best way to do so, contact the @mcore-oncall.

Contribution process

flowchart LR
    A[Pre-checks] --> B[PR Tests]
    subgraph Code Review/Approval
        C1[Expert Review] --> C2[Final Review]
    end
    B --> C1
    C2 --> D[Merge]
Loading

Pre-checks

  • I want this PR in a versioned release and have added the appropriate Milestone (e.g., Core 0.8)
  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code Typing guidelines
  • I have added relevant documentation
  • I have run the autoformatter.sh on my PR

Code review

The following process is enforced via the CODEOWNERS file for changes into megatron/core. For changes outside of megatron/core, it is up to the PR author whether or not to tag the Final Reviewer team.

For MRs into `main` branch

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!

(Step 1): Add PR label Expert Review

(Step 2): Collect the expert reviewers reviews

  1. Attach the Expert Review label when your PR is ready for review.
  2. GitHub auto-assigns expert reviewers based on your changes. They will get notified and pick up your PR soon.

⚠️ Only proceed to the next step once all reviewers have approved, merge-conflict are resolved and the CI is passing.
Final Review might get declined if these requirements are not fulfilled.

(Step 3): Final Review

  1. Add Final Review label
  2. GitHub auto-assigns final reviewers based on your changes. They will get notified and pick up your PR soon.

(Optional Step 4): Cherry-pick into release branch

If this PR also needs to be merged into core_r* release branches, after this PR has been merged, select Cherry-pick to open a new PR into the release branch.

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.com or zijiey@nvidia.com.

Merging your PR

Any member of core-adlr and core-nemo will be able to merge your PR.

Signed-off-by: mikail <mkhona@nvidia.com>
@mkhona-nvidia
mkhona-nvidia requested review from a team as code owners February 19, 2026 23:15
@copy-pr-bot

copy-pr-bot Bot commented Feb 19, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@svcnvidia-nemo-ci
svcnvidia-nemo-ci requested a review from a team February 19, 2026 23:15
@mkhona-nvidia mkhona-nvidia self-assigned this Feb 19, 2026

@duncanriach duncanriach left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff. Thank you

# When fp32 residual connections are enabled, pipeline parallel communication must
# use fp32 to match the dtype of the residual stream between pipeline stages.
if self.fp32_residual_connection and self.pipeline_dtype is not None:
self.pipeline_dtype = torch.float

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a warnings.warn here so users aren't confused when their pipeline dtype changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized the warning shouldn't be output when self.pipeline_dtype is already torch.float.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please only warn on a single rank if possible. We don't want output spam.

@mkhona-nvidia mkhona-nvidia Feb 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a warnings.warn all over that config file, not just the change I made. Shall we make another PR that fixes this for all the others?

(made a change, please let me know if that is ok, it needs log_single_rank)

@janEbert janEbert added the Expert Review [deprecated] Apply this label to indicate that your PR is ready for expert review. label Feb 20, 2026
@janEbert

Copy link
Copy Markdown
Contributor

/ok to test e0e74fd

@svcnvidia-nemo-ci svcnvidia-nemo-ci added this to the Core 0.16 milestone Feb 20, 2026
@janEbert
janEbert enabled auto-merge February 20, 2026 00:30
@deepakn94
deepakn94 disabled auto-merge February 20, 2026 02:57
@mkhona-nvidia mkhona-nvidia added the Final Review PR is in the "final review" stage label Feb 20, 2026
Signed-off-by: mikail <mkhona@nvidia.com>
Signed-off-by: mikail <mkhona@nvidia.com>
@janEbert

Copy link
Copy Markdown
Contributor

/ok to test c15e1dd

Signed-off-by: mikail <mkhona@nvidia.com>
@mkhona-nvidia

Copy link
Copy Markdown
Contributor Author

/ok to test 5580aee

@deepakn94
deepakn94 added this pull request to the merge queue Feb 24, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/22373578757

Merged via the queue into NVIDIA:main with commit 08857d9 Feb 25, 2026
77 of 81 checks passed
ko3n1g pushed a commit to ko3n1g/Megatron-LM that referenced this pull request Feb 26, 2026
Signed-off-by: mikail <mkhona@nvidia.com>
BoxiangW pushed a commit to BoxiangW/Megatron-LM that referenced this pull request Mar 4, 2026
Signed-off-by: mikail <mkhona@nvidia.com>
yangbofun pushed a commit to xlm-research/Megatron-LM that referenced this pull request May 22, 2026
Signed-off-by: mikail <mkhona@nvidia.com>
yezhengmao1 added a commit to yezhengmao1/Megatron-LM that referenced this pull request May 30, 2026
…ar to unblock fp32_residual_connection

When fp32_residual_connection=True the residual stream is fp32 and
bias_dropout_add propagates that dtype to hidden_states. In the TE GPT
layer spec input_layernorm/pre_mlp_layernorm is IdentityOp (folded into
the fused TELayerNormColumnParallelLinear), so nothing casts back to
params_dtype before this op. TE's set_activation_dtype strictly rejects
input dtype != param dtype outside autocast, so the forward dies on the
very first layer of a bf16 model with --fp32-residual-connection.

Downcast input to layer_norm_weight.dtype at the top of forward.
Residual-accumulation precision is preserved upstream in bias_dropout_add
(fixed by NVIDIA#3504); only the input to LN+GEMM is brought back to
params_dtype, which is what the op is configured to run in anyway.

Add TestFp32ResidualTELayerNormLinear covering the path: SelfAttention
from the TE submodules with bf16 params, fed an fp32 hidden_states.
Without this fix the forward raises in TE; with the fix it returns a
bf16 output of the expected shape.
terminator123 pushed a commit to 021ai/Megatron-LM that referenced this pull request Aug 3, 2026
Signed-off-by: mikail <mkhona@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Expert Review [deprecated] Apply this label to indicate that your PR is ready for expert review. Final Review PR is in the "final review" stage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants