Repository navigation
Conversation
TEFusedMLP, TEFusedMLPWithGroupedLinear and FusedSharedExpertMLP ran plain SwiGLU when activation_func_clamp_value or activation_func_tanh_clamp_scale was set. Build TE's clamped SwiGLU ops for the clamp, with the arguments FusedSharedExpertMLP and TEGroupedMLP use, and raise ValueError for SiTU-GLU, which the fused GLU paths do not implement. Signed-off-by: Felipe Sens Bonetto <felipesensbonetto@gmail.com>
This branch has not been deployed
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.
What does this PR do?
The Transformer Engine fused MLP classes now apply the SwiGLU clamp (
activation_func_clamp_value) or reject SiTU-GLU (activation_func_tanh_clamp_scale). Before this change,TEFusedMLP,TEFusedMLPWithGroupedLinearandFusedSharedExpertMLPsilently ran plain SwiGLU when either option was set.TEFusedMLPandTEFusedMLPWithGroupedLinearnow use TE's clamped SwiGLU ops. All three classes now raise aValueErrorfor SiTU-GLU, as the routed experts' fused path already declines it.What's wrong
MLP,SharedExpertMLPand the routed experts support two optional clamps on the FC1 output:activation_func_clamp_value=c(DeepSeek-V4). For SwiGLU it computessilu(min(g, c)) * clamp(u, -c, c).activation_func_tanh_clamp_scale=s_gwith an optionalactivation_func_tanh_clamp_scale_linear=s_l(Kimi-K3, TanH Sq-relu and SiTU GLU #7003). With gated silu this is SiTU-GLU:s_g*tanh(g/s_g)*sigmoid(g) * s_l*tanh(u/s_l). With a non-gated activation it is a tanh soft clamp of the input.Three fused classes build their activation without reading these options:
TEFusedMLP(a dense MLP withuse_transformer_engine_op_fuser=True)._make_te_ops_activationmaps(F.silu, gated)tote.pytorch.ops.SwiGLUwhatever the clamp settings.TEFusedMLPWithGroupedLinear(use_grouped_gemm_for_dense_mlp=True). It always appendsScaledSwiGLU(glu_interleave_size=32).FusedSharedExpertMLP(use_grouped_gemm_for_shared_expert=True). It appliesactivation_func_clamp_valuethroughScaledClampedQGeGLU, but it never checksactivation_func_tanh_clamp_scale, so a SiTU-GLU config runs plain SwiGLU.None of them raises an error or a warning. The routed experts already handle both options.
TEGroupedMLP._is_fused_impl_supportedusesScaledClampedQGeGLUfor the clamp. For SiTU-GLU it returns False ("A clamped gated activation is SiTU-GLU, which the fused GLU path does not implement"), so those layers fall back to the unfused path.Reproduction
Results:
max |TEFusedMLP - MLP|: 19.77for the clamp and19.50for SiTU-GLU.0.0for the clamp, and SiTU-GLU raisesValueError: TEFusedMLP does not support activation_func_tanh_clamp_scale: ....A longer script also compares gradients and covers the other two classes. Each fused module and its reference have the same weights. The relative error is
||a - b|| / ||b||.TEFusedMLP, SwiGLU (control), fp32SwiGLU; output, dx and dW_fc1 rel err 0TEFusedMLP, clamp 10, fp32SwiGLU; output rel err 0.616 (max abs 14.45); dx 0.679; dW_fc1 0.696ClampedSwiGLU; output 0; dx 1.1e-6; dW_fc1 1.1e-6TEFusedMLP, SiTU-GLU 4/25, fp32SwiGLU; output rel err 1.92 (max abs 20.98); dx 1.68; dW_fc1 1.74ValueErrorFusedSharedExpertMLP, SiTU-GLU 4/25, bf16 + NVFP4ScaledSwiGLU; vsSharedExpertMLPwith SiTU-GLU: 1.219; vsSharedExpertMLPwith SwiGLU: 0.248ValueErrorTEFusedMLPWithGroupedLinear, clamp 10, bf16 + NVFP4ScaledSwiGLU; vsMLPwith the clamp: 1.456; vsMLPwithout it: 0.247ScaledClampedQGeGLU; vsMLPwith the clamp: 0.277; vsMLPwithout it: 0.663In the NVFP4 rows, the fused path's error against the reference it matches (0.25 to 0.28) is mostly quantization error. For comparison, the clamped and unclamped
MLPreferences differ by 1.45 on these inputs, and the SiTU-GLU and SwiGLU references differ by 1.18. The FC1 weights are drawn so that FC1 outputs have a std of about 6 to 11, which keeps the clamps active on many elements.Impact
A config is affected when it sets
activation_func_clamp_valueoractivation_func_tanh_clamp_scaleand also enables one of these fused paths:use_transformer_engine_op_fuser=Truefor dense MLP layers;use_grouped_gemm_for_dense_mlp=True;use_grouped_gemm_for_shared_expert=True.Routed experts are not affected.
These layers train and evaluate with a different activation from the one configured, and nothing reports it. On the inputs above, the outputs differ from the configured activation by 62% to 192% (relative norm), and the gradients by 68% to 174%.
Weights trained through a fused path behave differently when loaded into the unfused classes, for example
MLP, which apply the configured activation.I have not measured the effect on a real training run.
Fix
_make_te_ops_activationtakes the clamp value. For a clamped SwiGLU it returnste.pytorch.ops.ClampedSwiGLU(limit=c, alpha=1.0, glu_linear_offset=0.0), which computessilu(min(g, c)) * clamp(u, -c, c), the same asMLP. The config already requiresglu_linear_offset == 0with the SwiGLU clamp.TEFusedMLP._make_fused_implpassesactivation_func_clamp_value.When the clamp is set,
TEFusedMLPWithGroupedLinear._make_fused_implbuildsScaledClampedQGeGLU(glu_interleave_size=32, alpha=1.0, limit=c, glu_linear_offset=0.0). These are the same argumentsFusedSharedExpertMLPandTEGroupedMLPuse.TEFusedMLP.__init__, whichTEFusedMLPWithGroupedLinearinherits, raises two errors:ValueErrorwhenactivation_func_tanh_clamp_scaleis set;RuntimeErrorwhen the clamp is set with TE < 2.17.0.dev0. That is the boundFusedSharedExpertMLPandTEGroupedMLPuse forglu_linear_offset.These checks run at construction, because the fused ops are only built on the first forward.
FusedSharedExpertMLP._validate_fused_grouped_swigluraises aValueErrorwhenactivation_func_tanh_clamp_scaleis set, next to its other config checks.I chose to reject SiTU-GLU rather than use TE's
SiTUGLUandScaledSiTUGLUops (TE 2.20) for three reasons:beta2for the linear half, so they cannot express that case.This PR stops the silent fallback until #6674 or similar work lands. That work would replace these
ValueErrors.Tests
test_te_fused_mlp_activation.py:TEFusedMLPmust reject SiTU-GLU, with and without the linear scale.TEFusedMLPmust matchMLPin fp32 on the same weights: the output, the input gradient and all parameter gradients, within 1e-4.DID NOT RAISE. The match test fails on 2048 of 2048 elements, with a max abs difference of 19.77.test_te_fused_mlp_with_grouped_linear_spec.py:_make_fused_implmust buildScaledClampedQGeGLUwith the arguments above when the clamp is set.test_shared_experts.py:test_validate_fused_grouped_swiglu_rejects_unsupported_configs.test_fused_shared_expert_rejects_situ_glu, which constructs the realFusedSharedExpertMLPwith MXFP8 and SiTU-GLU. It only checks construction: my GPU has no MXFP8, so I could not test a forward pass there.determinism/kernels/test_te_wrappers.py:test_te_fused_mlp_clamped_swiglu_replays, a bit-exact replay ofTEFusedMLPwith the clamp (bf16, clamp 1.0).Results on one GPU (RTX 5070 Ti, sm_120, torch 2.14.1+cu130, TE 2.20.2). I ran them on 8e13411. The branch is now based on 6463d38, where none of the touched files has changed:
The five related files are
test_te_fused_mlp_activation.py,test_te_fused_mlp_with_grouped_linear_spec.py,moe/test_shared_experts.py,fusions/test_te_fused_ops.pyanddeterminism/kernels/test_te_wrappers.py.The 4 remaining failures are the same before and after:
test_shared_expert_forward_backwardparametrizations need more than one GPU (world_size (1) is not divisible by 4/2).test_te_lm_head_two_uses_replaysuses MXFP8, which TE does not support on sm_120.The skipped tests need SM100, a world size divisible by EP=2, or a TE
GroupedLinearwithsplit_alignment.black 26.3.0, isort 5.13.2, ruff 0.9 and pylint 3.2.6 pass on the changed files with the repo's settings.
The RTX 5070 Ti limits these results in four ways:
ffn_hidden_size=112, which is not a multiple of 32. On sm_120, TE 2.20.2 sends GLU activations whose column count is divisible by 32 tocast_gated_tma, which fails withCUDA Error: invalid argument. This happens for plainSwiGLUtoo, so it is unrelated to this PR. TE main checks the shared-memory limit before it selects that kernel.FusedSharedExpertMLPforward needed a workaround that only applies to the repro. TE 2.20.2's unfusedScaledSwiGLUassertsscales_tensor.numel() == rows, butFusedSharedExpertMLPcaches a 1-element unit scale, so I preset that scale to one entry per row.Tested on B300 (MXFP8) and 8x H100. Same software as above (torch 2.14.1+cu130, TE 2.20.2, cudnn-frontend 1.32.0), main 6463d38 with this PR's tests against this PR's head:
NVTE_CUTEDSL_FUSED_GROUPED_MLP=1. Every case below ran on the cuDNN fused kernel (GroupedMLP_CuTeGEMMGLU, forward and backward).TEFusedMLPWithGroupedLinearwith the clamp at 10, relative error againstMLPwith the clamp (output / input gradient): 1.448 / 1.327 on main, where it matches the unclampedMLP(0.066 / 0.083), and 0.076 / 0.183 with this PR. The SwiGLU control is 0.066 / 0.083 on both.FusedSharedExpertMLPwith SiTU-GLU 4/25: on main the forward runs and is 1.179 offSharedExpertMLPwith SiTU-GLU, 0.066 off plain SwiGLU. With this PR it raisesValueError.test_shared_expert_forward_backwardcases.test_te_lm_head_two_uses_replayspasses on both. The skips are the same as on sm_120: the batch-invariant grouped-GEMM cases require exactly sm_100, and the others need EP=2 or TE features newer than 2.20.2.test_shared_expert_forward_backwardparametrizations pass on all 8 ranks, before and after.The input-gradient error with the clamp (0.183 against 0.083 for the control) comes from MXFP8. The gradient is zero on one side of the clamp, and rounding the FC1 operands moves some pre-activations across it. With the clamp at 10 and FC1 outputs of std ~11, 1522 of 131072 clamp decisions change, which alone changes dx by 0.161 (relative). Against an fp32 reference that takes its clamp decisions from MXFP8-quantized operands, the kernel's dx error is 0.106. With the clamp at 30 it is 0.084 to 0.089, the control level. An MXFP8 test of a clamped activation therefore needs a looser dx tolerance than an unclamped one, or a reference whose clamp decisions come from the quantized operands. None of the tests in this PR compares a clamped gradient under MXFP8.
tools/check_kernel_determinism_coverage.pyreports one issue.transformer_engine.pyis also a source of thepaged_stash_marker_propagationentry, and that entry's test,test_fused_activations.py, is unchanged. The newtest_te_wrappers.pycase covers the file's other two entries,transformer_engine_wrappersandte_fused_cross_entropy. This change does not touch paged stashing. Could a maintainer add thedeterminism-exemptlabel? I can't add labels myself. If you prefer, I can add a case totest_fused_activations.pyinstead.Not covered:
activation_func_clamp_value. The unfusedMLPclamps any gated activation, butTEFusedMLPstill ignores the clamp for these two. The config documents the clamp only for SwiGLU and quick-GELU, which the TE op path does not support.MLP's own fusedbias_geglu_implalso ignores the clamp, so I left this as is.activation_func_tanh_clamp_scaledocstring still mentions only the routed-expert fused path.Related open PRs:
situlu,situ_glu_beta1,situ_glu_beta2). That config predatesactivation_func_tanh_clamp_scalefrom TanH Sq-relu and SiTU GLU #7003. [MCore] Add SiTU-GLU through Transformer Engine #6674 edits the same functions intransformer_engine.pyandshared_experts.py, so whichever lands second needs a rebase.TEFusedMLP's TP group, and Use the shared expert's TP group in the fused SwiGLU path #8024 and Fix the FC1 size of the fused shared expert with tensor parallel #8145 changeFusedSharedExpertMLP. The source changes in this PR apply to each of their heads withgit apply --3waywithout conflicts. One or two of the test files conflict on each head. Those heads are older than this PR's base, and I have not checked which conflicts come from the PRs themselves.The code, the tests and this description were drafted with an AI coding agent (Claude Code).
@NVIDIA/mcore-oncall.Issue tracking
For PRs from open-source community contributors:
Linked issue: none. I found no existing issue or PR for this bug. The searches are listed below.
Duplicate search
I ran read-only searches of issues and PRs in all states on 2026-10-28, with
gh search issues --include-prs --repo NVIDIA/Megatron-LM "<query>" --limit 15:TEFusedMLP clampTEFusedMLP activationfused MLP clamp ignoredFusedSharedExpertMLPFusedSharedExpertMLPbugs), #5604 (merged; adds the class). None reports this bug.activation_func_tanh_clamp_scaleactivation_func_clamp_valueSiTUdev), #8497 (open ondev; MoK kernels, does not touch these files), #6635 and #6636 (closed)ClampedSwiGLUdev; the clamp in the routed-expert op fuser,experts.pyonly), #5941 (closed)use_transformer_engine_op_fuserTEFusedMLPWithGroupedLinearI also listed the files of #6674, #8497, #8024, #8145, #8038, #8566, #5130, #5940, #7003, #7769 and #6810 with
gh pr view <n> --json files. Only #6674 changes the activation in these classes.Code provenance
You should always make a best effort to avoid copying code from other repositories. In exceptional cases in which this is unavoidable and you copied or adapted code from an external source, provide the following details and copy to the file header:
No code was copied or adapted from an external source.
Contribution process
Pre-checks
jit_fuser/torch.compile, CUDA extension, TE or external-library dispatch, or a scatter/index accumulation), I have added or updated its bit-exact determinism test and registered it intests/unit_tests/determinism/kernels/manifest.py(guide)Code review
Feel free to message or comment
@NVIDIA/mcore-oncallto 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.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.