[PyTorch] Pair delayed-scaling FP8 recompute metadata per module - #3394
[PyTorch] Pair delayed-scaling FP8 recompute metadata per module#3394nvegesna-netizen wants to merge 5 commits into
Conversation
Greptile SummaryThe PR pairs delayed-scaling FP8 metadata stashes with recomputation per module and avoids creating unconsumable recompute state when checkpointing begins with autograd disabled.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Checkpoint as te.checkpoint
participant Module as FP8 Module
participant Buffer as Recompute FIFO
Caller->>Checkpoint: forward with autograd enabled
Checkpoint->>Module: checkpoint phase 1
Module->>Buffer: stash delayed-scaling metadata
Caller->>Checkpoint: backward
Checkpoint->>Module: recompute phase 2
Module->>Buffer: restore oldest matching metadata
Module->>Module: execute recompute forward
Module->>Module: restore updated forward state
Reviews (4): Last reviewed commit: "Address FP8 recompute review feedback" | Re-trigger Greptile |
|
After a longer discussion with Codex, we came to the following conclusion:
What do you think? |
|
Thank you for the careful review. I agree with the core conclusion. I could not identify a supported execution path where a delayed-scaling module legitimately appears in checkpoint phase 2 without having appeared and stashed in phase 1. Such a path would require divergent checkpoint execution, FP8 state replacement, or another invariant violation, and it should fail rather than silently recompute using live metadata. I revised the PR accordingly:
if is_fp8_activation_recompute_enabled():
FP8GlobalStateManager.copy_forward_fp8_meta_tensors_for_recompute(self.fp8_meta)Testing that minimal change independently confirmed your analysis: all ten original eval/mode-change cases pass without the additional module state. That discriminator also exposed a separate phase-1-without-phase-2 case. If I addressed that at the checkpoint boundary rather than in the module. For TE-containing callables, The revised validation covers 18 focused cases across:
For the explicit-gradient cases, output, input gradient, and weight gradient match direct execution, and the recompute FIFO remains empty. The broader recompute/checkpoint selection completes with 450 passes and 90 expected capability skips. One boundary is now documented in the PR description: if checkpoint is entered under outer The current head is |
pggPL
left a comment
There was a problem hiding this comment.
overall looks good, left some comments about the tests and nits
| context_fn = kwargs.pop("context_fn", noop_context_fn) | ||
| determinism_check = kwargs.pop("determinism_check", "default") | ||
| debug = kwargs.pop("debug", False) | ||
|
|
| # Activation recomputation is used and this is the first forward phase. | ||
| if self.training and is_fp8_activation_recompute_enabled(): | ||
| # Every delayed-scaling module in the first checkpoint phase must stash. | ||
| # Checkpoint phase, rather than module training mode, determines whether |
| assert ref_observed == [(True, False, False)] | ||
| assert observed == ref_observed | ||
|
|
||
|
|
There was a problem hiding this comment.
Do you think we need so much tests for one line fix? My agent says yes, but I'm sceptical about it.
There was a problem hiding this comment.
Agreed. I reduced this to six focused cases covering eval pairing with and without a mode transition across both checkpoint paths, no grad entry, and forward context preservation.
|
/te-ci pytorch |
The delayed-scaling stash and its two restore sites made independent decisions. Module mode changes between the original forward and checkpoint replay could therefore leak a stash or restore one that was never created. Track pending stashes per module and record whether each prepare_forward call swapped one in, so end_forward performs exactly the matching restore. Stash every delayed-scaling FP8 module encountered in checkpoint phase 1: in reentrant checkpointing the original forward runs under no_grad, so an eval module receiving an intermediate tensor has no module-local autograd signal even though backward will replay it. Tests cover training and eval modules, both checkpoint implementations, mode changes in both directions, repeated iterations, multi-module reentrant replay, and exact FIFO drainage. Signed-off-by: Nitin Vegesna <nvegesna@nvidia.com>
Signed-off-by: Nitin Vegesna <nvegesna@nvidia.com>
Signed-off-by: Nitin Vegesna <nvegesna@nvidia.com>
Signed-off-by: Nitin Vegesna <nvegesna@nvidia.com>
Signed-off-by: Nitin Vegesna <nvegesna@nvidia.com>
966baaa to
aceb903
Compare
|
/te-ci pytorch |
1 similar comment
|
/te-ci pytorch |
|
My agent says: this path can still create unreachable FP8 recompute stashes for reentrant checkpoints when grad mode is enabled but none of the autograd Function inputs require gradients. In that case this Function cannot receive backward, so phase 2 cannot happen; stale FIFO entries can later be consumed by a valid training recompute.
The relevant line is outside the current PR diff, so GitHub cannot attach an inline suggestion to it. Suggested source-only change: diff --git a/transformer_engine/pytorch/distributed.py b/transformer_engine/pytorch/distributed.py
@@ -373,7 +373,9 @@ class _CheckpointFunction(torch.autograd.Function):
torch_gpu_amp_ctx, torch_cpu_amp_ctx = _get_active_autocast_contexts()
with torch.no_grad(), forward_ctx:
- with activation_recompute_forward(activation_recompute=True, recompute_phase=False):
+ with activation_recompute_forward(
+ activation_recompute=any(ctx.needs_input_grad), recompute_phase=False
+ ):
outputs = run_function(*args, **kwargs) |
Description
Fix delayed-scaling FP8 metadata stash and restore pairing when a checkpointed module is in eval mode or changes mode between the original forward and recompute forward.
The original forward stashed metadata only when
self.trainingwas true, while recompute restored metadata from every FP8 module in the recompute phase. An eval module could therefore try to restore a stash it never created. Module training mode is not a valid pairing signal because a module may change mode before recompute.Every delayed-scaling FP8 module in checkpoint phase 1 now stashes its metadata, independent of module training mode. Phase 2 retains the existing strict FIFO restore behavior, so an execution mismatch remains visible rather than being silently skipped.
When
te.checkpoint()is entered with outer autograd disabled, no backward recompute is normally possible. TE-containing callables therefore execute directly under the supplied forward context, avoiding FP8 recompute snapshots that could never be consumed. Non-TE callables continue to use native PyTorch checkpointing.Type of change
Changes
Validation
Grad mode boundary
Checkpoint entry grad mode is authoritative. If
te.checkpoint()is called under outertorch.no_grad()but itscontext_fnor callable explicitly re-enables gradients internally, execution remains numerically correct, but the direct forward path bypasses activation checkpointing and may retain more activations.Callers that need checkpoint recomputation for such a gradient-enabled region should enable gradients around the checkpoint call itself:
Checklist