Repository navigation
[dev] Restore the forward's FP8 autocast state in tensor_parallel.checkpoint recompute - #7538
Conversation
|
/ok to test 913513f |
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
195650d to
419f57f
Compare
Signed-off-by: HaochenYuan <haocheny@nvidia.com>
|
/review model=claude mode=strict |
svcnvidia-nemo-ci
left a comment
There was a problem hiding this comment.
Strict review passed: I found no significant issues. LGTM.
Findings: CRITICAL 0 · IMPORTANT 0 · SUGGESTION 0
What I checked
-
The fix copies an existing pattern.
CheckpointFunction.forwardrecords whether FP8 is on and which recipe is active. It then runs underactivation_recompute_forward(recompute_phase=False).backwardre-entersfp8_autocastwith that recipe and usesrecompute_phase=True. This is the same handling, in the same context order, asCheckpointWithoutOutputFunction.forwardandCheckpointWithoutOutput._recompute. -
The contexts are entered in the right order. In both phases
fp8_ctxis entered beforeactivation_recompute_forward, so the recompute flag sees FP8 as enabled. -
Non-FP8 paths are unchanged.
bool(HAVE_TE and ...)stops early when TE is missing, and runs that are not under FP8 usenullcontext(). Behaviour without TE or without FP8 is therefore the same as before. -
Every new field is used.
ctx.fp8andctx.fp8_recipeare read inbackward, andfwd_ctx,fp8_ctxandrecompute_ctxare each entered. -
Only the intended call sites change. These sites already switch to
te_checkpointunderconfig.fp8 or config.fp4:- block/full recompute in
recompute.py - MLP recompute in
transformer_layer.py - MoE and shared-expert recompute in
moe_layer.py(also L887-L901)
The change therefore only reaches the sites that always call
tensor_parallel.checkpoint, which are the ones the PR targets:core_attninattention.py(L579)absorbed_mla.py- GDN in
gdn.py
- block/full recompute in
-
No reachable nesting. TE resets its recompute flags when the context exits instead of restoring the previous values, so nesting one of these contexts inside another would clear the outer region's flags. I found no reachable path where that happens:
- The selective sites need
recompute_granularity == 'selective', so they can't run inside a full-recomputete_checkpoint. - The
CheckpointWithoutOutput()nested inside the GDN region (thegdn_norm_outpath) is built withfp8=False, so it never touches those flags.
- The selective sites need
-
The tests match the change. The new tests in
test_random.pycover three things:- the recompute sees the forward's FP8 state, with a no-FP8 control;
- the FP8 state is restored when
backwardis called directly; - a TE
Linearwith FP8 parameters andfuse_wgrad_accumulationgives the same output,x.gradandmain_gradwith and without checkpointing, under both MXFP8 and delayed scaling.
Without the fix, the FP8-parameter case fails with the
main_gradAttributeError whatever the tolerances, so the test does guard against a regression.
I checked this by reading the source only; I did not run the tests.
Risk: low. The change is small and local. It mirrors existing in-repo handling, and it only changes behaviour for tensor_parallel.checkpoint regions whose forward ran under FP8. Those regions now recompute the way the forward ran, which is what te_checkpoint and CheckpointWithoutOutput already do.
Review generated by claude
…IA#7521) Faithful dev port of main PR NVIDIA#7521 at cf479fe: the opt-in mhc_norm_eps_inside_sqrt and mhc_keep_mappings_in_fp32 precision controls, mhc_learned_output_contract (mean contraction when False) in the HybridStack post-process and the MTP layer, an FP32 projection/RMS recomputed from the activation-dtype input, viewless mHC full-recompute boundaries, forward autocast replay in tensor_parallel.checkpoint, and the PR's tests. Dev adaptations: - Field names: dev calls the mHC switches enable_hyper_connections and num_residual_streams (main: enable_mhc_connections, mhc_num_residual_streams); the PR's code and tests use the dev names. - hyper_connection.py: dev did not import functools.partial, so it is imported. Dev's proj.view(s, b, proj.shape[-1]) return and torch.compile comments are kept; dev-only NativeHAggregateInto is unchanged. - random.py: the PR's autocast restore is entered together with dev's FP8 recompute state from NVIDIA#7538 (with torch.enable_grad(), autocast_context, fp8_ctx, recompute_ctx), in place of main-only suspend_tensor_observations(); autocast_context is built after dev's FP8 block. - recompute.py: only the viewless checkpoint boundary and its import are ported; the PR's capability dispatch is already on dev (see below). - transformer_block.py: main's TransformerBlock runs full recompute through recompute.checkpointed_forward, so the PR's viewless boundary covers GPT blocks there. Dev's TransformerBlock keeps its own _checkpointed_forward, so the same block (PR comment verbatim) opens its checkpoint_handler, with nonlocal hidden_states. The PR's TP2 test needs it: dev's input_expand returns a view that dev does not make viewless. - multi_token_prediction.py: the re-indented hc_head setattr lines keep dev's single-quoted 'sequence_parallel' (main's MTP uses double quotes; the PR only re-indents them). - test_hybrid_mhc.py is not on dev and is added with the tests the PR adds or changes (test_constructor_and_sharded_state, test_output_contract, test_wrapped_residual_layer_does_not_double_count_input, test_forward_backward, test_full_recompute_matches_forward_backward, test_full_recompute_real_moe_matches_forward_backward) plus main's scaffolding they use (_DummyHybridLayer, _get_pg_collection, _get_dummy_submodules, _get_config, _get_stack, TestHybridStackMHC setup). HyperConnectionHybridLayer is imported from hybrid_block; the layer_config_list assertions (main-only per-layer configs) are dropped. In test_wrapped_residual_layer_does_not_double_count_input, hyper_connection(hidden) replaces hyper_connection(hidden, return_residual=True), as dev's forward always returns the residual, and after the rename black joins its hidden = torch.randn(...) call onto one line. - test_transformer_block.py: also imports ModuleSpec (already imported on main) and does not add BaseTransformerLayer to the transformer_layer import (main imports it for its _ResidualObservationLayer helper, which dev's file does not have); the TP2 test sits before dev's first test class. Already on dev: - moe/token_dispatcher.py: the PR sets MoEAlltoAllTokenDispatcher.probs to None at the end of combine_postprocess. Dev PR NVIDIA#8231 already releases probs, routing_map and the permutation mappings (after record_stream) through _release_routing_tensors() right after the unpermute in combine_postprocess, and the flex dispatcher's router probs through release_router_probs(), so the hunk is not ported. test_full_recompute_real_moe_matches_forward_backward keeps the PR's probs-release assertion, which NVIDIA#8231's release satisfies. - recompute.py: the supports_hybrid_recompute_kwargs dispatch and the PR's comments around it. - layers/hybrid_hyper_connection.py (main-only module): its supports_hybrid_recompute_kwargs = True is already set on dev's HyperConnectionHybridLayer in hybrid_block.py. Not ported: - recompute.py packed_sequence_cp_metadata forwarding: main-only (dev's checkpointed_forward has no such argument). - Removal of the mHC + full recompute NotImplementedError and of its test_hyper_connection_recompute.py case: neither exists on dev (dev's test_config_rejects_unsupported_combinations has no full-recompute case). - tests/unit_tests/determinism/kernels/* (test_native_mhc_dtype_casts_replay_fwd_bwd, test_alltoall_dispatcher_releases_router_probs_after_forward): the kernels/ harness is absent on dev, whose tests/unit_tests/determinism has correctness/ and shared helpers but no kernels/. - Tests in main's test_hybrid_mhc.py that the PR does not touch. - Inherited from the PR head and left for a follow-up commit: the PR does not update tests/unit_tests/fusions/test_fused_mhc_kernels.py. native_proj_rms now returns FP32 proj and r, so TestNativeProjRms::test_fwd_bwd_vs_torch_reference (BF16 inputs, compared with the BF16 _ref_proj_rms by a dtype-checking assert_close) fails here, as it does on main at the PR head. The fused-mHC tests that feed native_proj_rms output into the kernels are not yet checked against the FP32 projection contract. - Dev-only paths the PR does not reach are left for follow-up commits: NativeHAggregateInto (mHC attention CUDA graph split) rounds FP32 coefficient products in the BF16 slot dtype when mhc_keep_mappings_in_fp32 is set; dev's GPT TransformerBlock always uses the learned contraction (main's always averages); and dev's overlap_moe_expert_parallel_comm full-recompute segment replay (RecomputeSegment.recompute in model_chunk_schedule_plan.py) re-runs layers under get_fp8_context only, without the forward autocast state that the PR restores in tensor_parallel.checkpoint. Upstream-PR: NVIDIA#7521 Upstream-Base: 915b1d2 Upstream-Head: cf479fe Scope: faithful dev port; deviations are listed above Ported-by: Robin Zhang <robinz@nvidia.com> Signed-off-by: Hollow Man <hollowman@opensuse.org> Signed-off-by: Robin Zhang <robinz@nvidia.com>
What does this PR do?
CheckpointFunction.backward()re-runs the checkpointed function from inside autograd's backward, i.e. outside thefp8_autocastthe forward ran under; it only restored the RNG state. A Transformer Engine module inside the regiontherefore recomputes with FP8 disabled: with FP8 parameters TE dequantizes the weight and the wgrad-fusion binding fails (
AttributeError: 'Tensor' object has no attribute 'main_grad'); with BF16 parameters the recompute silently runs in BF16 while the forward ran in FP8.This affects the sites that call
tensor_parallel.checkpointdirectly:--recompute-modules core_attn(attention.py,absorbed_mla.py) and--recompute-modules gdn(GatedDeltaNet / KimiDeltaAttention). The block-level sites route tote_checkpointunder FP8, andCheckpointWithoutOutputalready re-enters the FP8 state itself.Fix
forward()recordsFP8GlobalStateManager.is_fp8_enabled()and the recipe and runs underactivation_recompute_forward(recompute_phase=False);backward()re-entersfp8_autocast(enabled=True, fp8_recipe=<recorded>)withactivation_recompute_forward(recompute_phase=True)for the recompute — the same handling asCheckpointWithoutOutput._recomputeand TE'scheckpoint. Without TE, or when the forward ran outside an FP8 autocast, nothing changes.Tests
tests/unit_tests/tensor_parallel/test_random.py: the recompute must see the forward's FP8 state (and none when there was none); a TELinearwith FP8 parameters and fused wgrad accumulation insidetensor_parallel.checkpointmust match the un-checkpointed pass bit for bit. Both fail ondevas described above and pass with this change (torchrun --nproc-per-node 2 -m pytest tests/unit_tests/tensor_parallel/test_random.py -k "fp8 or test_checkpoint").Validation (1 GB200 node, mxfp8, TE 2.20)
dsaattention variant), FP8 parameters, selectivecore_attnrecomputemain_gradAttributeError)gdnrecompute vs the same run without recomputeBehaviour change
Any
tensor_parallel.checkpointregion whose forward runs under FP8 now recomputes in FP8 instead of BF16. This matcheste_checkpointandCheckpointWithoutOutput; for FP8 + `recompute the gradients change (tomatch the forward) and the FP8-parameter crash disappears. Delayed scaling reuses the forward's scaling factors through TE's recompute phases.
Issue tracking
For PRs from open-source community contributors:
Linked issue:
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-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.