Skip to content

Fix FSDP2 gradient norm/max reporting and clipping - #2900

Merged
bghira merged 2 commits into
bghira:mainfrom
hjinnkim:bugfix/fsdp-gradient-norm
Jul 27, 2026
Merged

Fix FSDP2 gradient norm/max reporting and clipping#2900
bghira merged 2 commits into
bghira:mainfrom
hjinnkim:bugfix/fsdp-gradient-norm

Conversation

@hjinnkim

@hjinnkim hjinnkim commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Fix FSDP2 gradient norm/max reporting and clipping

Summary

  • Max measurement: rank-local DTensor maxima could under-report the logical
    gradient; empty local shards and CPU-offloaded gradients were unsafe cases.
  • Norm clipping: the loop measured a max on every accumulation microstep even
    when norm clipping would recompute and overwrite it at the optimizer step.
  • Reporting: logs and webhook payloads used separate tensor/float conversion
    paths and could receive inconsistent metric types.
  • Fix: separate norm and max paths at synchronized steps, reduce max values
    across the FSDP2 mesh, reuse the norm returned by clipping, and preserve the
    existing logging API while normalizing structured progress payloads.

Changes

  • Trainer._max_grad_value(): compute the infinity norm from non-empty local
    gradients, contribute -inf for an empty DTensor shard, stage CPU scalars on
    the mesh device, and run one mesh-group all_reduce(MAX).
  • Post-backward dispatch: calculate metrics and clip only when
    accelerator.sync_gradients is true; use clip_grad_norm_()'s return value for
    norm mode instead of running a separate max traversal first.
  • Clipping paths: keep FSDP2 norm clipping on torch.nn.utils.clip_grad_norm_(),
    FSDP1 on the wrapped model method, and non-FSDP clipping through Accelerate.
    Existing optimizer exclusions and the FSDP value-clipping limitation remain
    unchanged.
  • _update_grad_metrics() and progress payloads: retain tensor objects for the
    existing accelerator log path, restore the clone_norm_value compatibility
    argument, and normalize norm values only for structured progress/webhook
    serialization.
Mode Measurement/report Clipping
Norm with clipping active Pre-clipping norm returned by clip_grad_norm_() Norm clipping runs
Value/max path Mesh-wide pre-clipping absolute maximum Non-FSDP value clipping runs; FSDP warns and skips
Accumulation microstep No metric No clipping

Result

  • Every FSDP2 rank reports the same mesh-wide absolute maximum when the max path
    is used.
  • Norm clipping performs one gradient traversal and its returned pre-clipping
    norm is reported directly.
  • Empty DTensor shards remain in the collective with -inf; CPU-offloaded scalar
    values are staged on the mesh device before reduction.
  • Accumulation-only steps avoid incomplete metrics, clipping, and unnecessary
    collectives.
  • Example: if rank 0 has local maximum 0.8, rank 1 has 3.2, and rank 2 has
    an empty shard, their contributions are 0.8, 3.2, and -inf; all ranks
    receive and report 3.2 after all_reduce(MAX).
  • Verified: 67/67 existing Trainer tests passed. A focused matrix covered
    metric contracts, norm/value clipping, empty DTensor shards, CPU-offload
    staging, and accumulation gating. A real two-GPU FSDP2 run also verified the
    mesh-wide max and post-clipping global norm.

Environment

  • Python: 3.13.14
  • PyTorch: 2.13.0+cu130
  • Accelerate: 1.14.0
  • Diffusers: 0.39.0
  • Transformers: 5.13.1
  • GPU: NVIDIA B200, driver 590.48.01

hjinnkim added 2 commits July 26, 2026 10:36
FSDP2 gradients are DTensors, so rank-local maxima can under-report the
logical gradient, empty local shards must still join collectives, and
reporting must not receive device tensors. The training loop also measured
a max on every accumulation microstep even when norm clipping would
immediately recompute and overwrite it.

- In Trainer._max_grad_value(), measure non-empty local shards, contribute
  negative infinity for empty DTensor shards, stage CPU scalars on the mesh
  device, and all-reduce MAX once across the mesh.
- At synchronized optimizer steps, reuse the norm returned by clipping in
  norm mode and run the separate max path only when clipping does not
  already provide the reported value.
- Preserve existing FSDP1, FSDP2, non-FSDP, optimizer-exclusion, and FSDP
  value-clipping policies while avoiding duplicate gradient traversal and
  collectives.
- Normalize norm and max values through one scalar path for logs, progress,
  webhooks, and regularisation metrics.
Restore the clone_norm_value keyword used by existing callers and keep value-mode grad_absmax entries as the original metric object. Continue converting norm metrics to scalars only for progress payloads that explicitly request cloning.

This keeps the FSDP2 mesh-wide max reduction and synchronized clipping changes while preserving the pre-existing logging API and tensor identity behavior exercised by the upstream trainer suite.

Copilot AI 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.

Pull request overview

This PR adjusts gradient metric computation and reporting during training—especially under FSDP2—so that gradient max/norm measurements are consistent across ranks, avoid unsafe empty/CPU-offload edge cases, and reduce redundant gradient traversals when clipping is enabled.

Changes:

  • Updates gradient metric reporting to gate metric computation/clipping on accelerator.sync_gradients, and to reuse clip_grad_norm_()’s returned pre-clipping norm in norm mode.
  • Reworks Trainer._max_grad_value() to correctly compute an infinity-norm / absolute-max style metric for DTensor gradients, including mesh-wide all_reduce(MAX) and safe handling of empty local shards.
  • Normalizes metric scalar conversion for structured progress payload construction while preserving existing accelerator logging behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +567 to 571
grad_value = self._normalize_metric_value(self.grad_norm)
if grad_value is None:
return
target_logs[f"{prefix}grad_norm"] = grad_value
elif (
@bghira bghira closed this Jul 27, 2026
@bghira bghira reopened this Jul 27, 2026
@bghira
bghira merged commit 0b14d2e into bghira:main Jul 27, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants