Repository navigation
fix: Do not pass is_first_microbatch to TE when it is not maintained - #6965
Conversation
65c071e to
3abefa0
Compare
7db7e7b to
293a85b
Compare
|
/ok to test 293a85b |
|
Good Catch~ With mtp_num_layers=2 and mtp_use_repeated_layer=True, logical MTP layers 0 and 1 invoke the same physical TE layer. Their forwards receive True then False, while backward runs in reverse, causing layer 1’s gradient to accumulate first and then be overwritten by layer 0. Currently, is_first_microbatch controls both whether TE refreshes/reuses the quantized weight cache during forward and whether fused wgrad overwrites or accumulates into main_grad during backward. This issue only occurs when we are reusing one layer for 2 layers in In this PR, it's changed to the following condition, which is still not fully fixed:
To summarize:
Adding yet another toggle to TE just for this is not recommended, so I suggest just passing |
zhongbozhu
left a comment
There was a problem hiding this comment.
Left commens~ please take a look
|
Thanks @zhongbozhu — you were right that the first version only closed the BF16 face of this. Taking your suggestion: repeated MTP layers now get
if (
module.disable_parameter_transpose_cache
or not is_first_microbatch_tracked(module.config)
or getattr(module, 'is_repeated_layer', False)
):
return None
return module.is_first_microbatchNo new TE toggle, as you asked. Why Perf cost is re-quantizing one MTP layer per forward, and Coverage. One thing I checked while scoping this, in case it saves you the question: the CUDA-graph path is unaffected. |
0a63fa3 to
47bb5c8
Compare
TEGroupedLinear.is_first_microbatch is process-local: True at construction,
False after the first forward, and never written to a checkpoint. It is only
re-armed per iteration for quantized configs, because set_is_first_microbatch
is called from the fp8/fp4/kitchen paths.
Transformer Engine derives the weight-gradient accumulation mode from it:
accumulate = fuse_wgrad_accumulation and not is_first_microbatch
so a stale True makes the wgrad GEMM overwrite main_grad with beta=0 instead
of accumulating into it. That is harmless for a module invoked once per
microbatch -- it overwrites a freshly zeroed buffer -- but destructive for one
invoked twice, such as an MTP block sharing a layer across depths under
--mtp-use-repeated-layer. Backward runs in reverse there, so the is_first=True
invocation lands second and discards the other depth's contribution.
The result is a checkpoint resume that is not bit-exact: a freshly loaded
process has is_first_microbatch=True while a process that has been training
has False, and only the fresh one corrupts its gradients. A run-to-run A/A
check cannot detect this, because both fresh processes make the same mistake
and agree.
Add is_first_microbatch_tracked(config) next to set_is_first_microbatch, which
returns whether the flag is actually maintained for this config, and pass None
to TE when it is not. TE then falls back to its own accumulate decision rather
than acting on a value nobody updates.
Verified on a three-leg resume check (train-through vs save vs resume) at 2 and
64 nodes: without this the resumed leg diverges in grad norm one step after the
resume; with it all checkpoint state shards are byte-identical.
Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
Passing None only when the flag is untracked left the quantized configs -- fp8, fp4, kitchen, quant_recipe -- still handing TE a True/False that set_is_first_microbatch does keep current. That value is honest for a module invoked once per forward pass, but a repeated MTP layer is invoked once per depth, so its first forward is paired with its last backward. The True lands on the last wgrad GEMM, which then overwrites main_grad with beta=0 and discards every gradient the later depths already accumulated. Mark the shared layer's TE submodules at construction and make the three call sites consult the marker. None rather than False, because False would keep TE reusing a quantized weight cache that nothing re-arms. The three copies of the gate are now one _resolve_is_first_microbatch helper, so the conditions cannot drift apart. split_te_layernorm_column_parallel_linear carries the marker across the split alongside the flag it shadows. Only marked when mtp_num_layers > 1; a repeated layer that runs once per pass behaves like any other layer and keeps its cache. Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
Fold the four-line marking helper back into its single caller, drop the mtp_num_layers guard, and stop carrying the marker through the layernorm-linear split. module.py is untouched again. The helper had one call site and existed only to hold a docstring, which now sits on _resolve_is_first_microbatch where the value is consumed. The guard spared the fp8 weight cache for a repeated layer with a single depth -- a configuration identical to not repeating at all -- at the cost of making the rule two clauses instead of one. The split helper is reached only from MLA latent caching at inference, which never runs a backward, so the marker there guarded nothing. Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
is_first_microbatch_unsafe announced a verdict without its reason, and "unsafe" reads as thread- or memory-safety. The flag is not unsafe here, it is inapplicable: the module is called more than once inside one microbatch, so first-forward and first-backward are different calls. is_reused_within_microbatch states that fact and leaves the verdict where it belongs, in _resolve_is_first_microbatch. Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
The previous names described the mechanism to a reader who does not exist. Anyone reviewing this already knows --mtp-use-repeated-layer, so the repo's own term for the thing is the name that needs no docstring. Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
_create_config_and_mtp_block_spec already builds the exact config the new test was assembling by hand; give it a use_repeated_layer parameter and call it. The flag has to reach TransformerConfig before get_gpt_mtp_block_spec reads it, so the builder is the only place it can go. Also drop the redundant disable_parameter_transpose_cache filter -- TE sets that attribute alongside is_first_microbatch, never apart from it -- hoist the resolver import to the top of the file next to HAVE_TE, and stop repeating the wgrad-overwrite explanation in is_first_microbatch_tracked, which now points at the TE call site that owns it. Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
Rewrite the three explanations added by this PR so they can be read without prior knowledge of the flag: state what the flag means, show its lifecycle, and drop the review-thread prose and duplicated cross-references. Comments only; no behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
47bb5c8 to
0ab7f8a
Compare
|
/ok to test 0ab7f8a |
The linkcheck job reports 404 for the TE PyTorch API page: the published URL has no user-guide/ path segment. Signed-off-by: Zhiyu Li <zhiyul@nvidia.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Zhiyu Li <zhiyul@nvidia.com>
|
/ok to test 1d1ecae |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/34568330019 |
…racked NVIDIA#6965 stopped passing is_first_microbatch to TE when the config does not maintain it. The predicate it added reads the config alone, so it answers True for every module as soon as quant_recipe is set. A recipe decides quantization per module and in both directions: it can quantize a layer the config leaves alone, and it can force a layer to high precision while the config enables fp8. A layer of the second kind has no quantized weight cache, so the flag is never refreshed for it, and TE derives the weight-gradient accumulation mode from that same argument -- accumulate = fuse_wgrad_accumulation and not is_first_microbatch -- so a stale True makes its wgrad GEMM overwrite main_grad instead of accumulating into it. Give is_first_microbatch_tracked optional module and is_context_quantized arguments and consult will_execute_quantized when a module is in hand, which is the same hook fp8_utils and the MoE experts already use. Callers with no module, namely MegatronModule.set_is_first_microbatch, keep the model-wide answer. Kitchen returns early because it drives quantization outside TE's autocast state, so there is no per-module answer to consult. Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
What does this PR do?
TEGroupedLinear.is_first_microbatch resets to True after checkpoint resume, causing the first shared-layer call to overwrite main_grad instead of accumulating into it. When an MTP layer is reused across depths, this discards one depth’s gradient and makes resumed training differ from uninterrupted training.
This only becomes destructive when the same layer is invoked multiple times, such as MTP repeated-layer sharing.
Restore or initialize the state deterministically so resume matches uninterrupted training:
Why no existing test catches it
A run-to-run A/A check cannot detect this. Both fresh processes make the same mistake and agree. It only appears between a process that has trained N steps and one that has just loaded a checkpoint, so it takes three separate processes to observe.
Fix
Two independent reasons the flag must not reach TE, both resolved to
None:is_first_microbatch_tracked(config), added next toset_is_first_microbatch, returns whether the flag is actually re-armed for this config. Outside fp8/fp4/kitchen/quant_recipeit is set once at construction and cleared by the first forward, so it is stale for the rest of the process.Truelands on the last wgrad GEMM and overwritesmain_gradwith beta=0, discarding what the later depths accumulated.mark_is_first_microbatch_unsafetags the shared layer's TE submodules at construction (only whenmtp_num_layers > 1).True/FalseNone— always accumulateTrue/FalseTrue/FalseNone— always accumulateNoneNoneNonerather thanFalse: both accumulate, butFalsealso tells TE to reuse its cached quantized weights, which would go stale across optimizer steps now that nothing re-arms the flag. The cost is re-quantizing one MTP layer per forward, and--fp8-param-gatherremoves even that by quantizing in the optimizer instead.The three copies of the gate are now one
_resolve_is_first_microbatchhelper so they cannot drift apart.Thanks @zhongbozhu for catching that the first version stopped at reason 1.
Verification
Three-leg check — train-through to step 10 / save at 5 / resume from 5 — all legs in one allocation, comparing checkpoint shards byte for byte:
iter_10Confirming arm: disabling gradient-accumulation fusion also makes the divergence vanish, which is what the mechanism predicts — with fusion off the
andshort-circuits andis_first_microbatchcan no longer select overwrite.The BF16 leg above is what the cluster runs; the quantized repeated-MTP leg is covered by unit test rather than a resume run.
test_repeated_layer_opts_out_of_is_first_microbatchbuilds a TE-spec MTP block both ways underfp8, and asserts the resolver answersNonefor every flag-carrying submodule when the layer is shared andTruewhen it is not.Issue tracking
For PRs from open-source community contributors:
Linked issue:
Contribution process
Pre-checks
Code review
Feel free to message or comment @NVIDIA/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.