Skip to content

Drain predecessor reduce-scatter at dispatch time - #4940

Merged
deepakn94 merged 5 commits into
NVIDIA:mainfrom
deepakn94:dnarayanan/drain_predecessor_grad_reduce
May 27, 2026
Merged

Drain predecessor reduce-scatter at dispatch time#4940
deepakn94 merged 5 commits into
NVIDIA:mainfrom
deepakn94:dnarayanan/drain_predecessor_grad_reduce

Conversation

@deepakn94

@deepakn94 deepakn94 commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

When reduce_scatter_with_fp32_accumulation=True, each in-flight reduce-scatter pins an intermediate all-to-all output tensor until .wait() runs. With overlap_grad_reduce=True and multiple bucket groups, those buffers stayed live until end-of-step, inflating peak memory by O(num_buckets * bucket_size).

  • Mirror the existing next_param_gather_bucket_group chaining: link each bucket group to its immediate predecessor in dispatch order (forward bucket order during backward) via previous_grad_reduce_bucket_group.
  • At the start of start_grad_sync, drain the predecessor so its all-to-all buffer is freed before the new one is allocated.
  • Make finish_grad_sync idempotent (under overlap_grad_reduce) via a per-iteration grad_reduce_finished flag so the end-of-step finalize loop is a no-op for buckets already drained by their successor (and the lazy first-batch path is safe under repeated invocation).

Linkage is gated on overlap_grad_reduce && reduce_scatter_with_fp32_accumulation && num_distributed_optimizer_instances == 1 — the only config in which the extra buffer exists.

Results

8B model, 8 nodes / 64 H100s (PP=1), overlap_grad_reduce=True, 7 grad-reduce buckets. Memory measured on rank 0 at steady state.

TP Variant TFLOP/s/GPU ms/iter Max allocated (MB) Max reserved (MB)
4 bf16rs (ring) 390 6,504 18,852 19,858
4 fp32rs 340 7,453 22,693 23,698
4 bf16rs (one-shot, no drain) 383 6,614 18,852 23,712
4 bf16rs (one-shot, drain, this PR) 393 6,458 18,851 21,492
2 bf16rs (ring) 450 5,629 37,170 40,172
2 fp32rs 442 5,737 44,851 47,852
2 bf16rs (one-shot, no drain) 408 6,197 37,170 47,866
2 bf16rs (one-shot, drain, this PR) 446 5,687 37,170 43,714

Compared to bf16rs (one-shot, no drain), the drain reclaims 2.2 GB of reserved memory at TP=4 and 4.2 GB at TP=2 — the saving is larger at TP=2 because each transient all_to_all_output_tensor is correspondingly larger. Throughput also improves (+2.6% at TP=4, +9.3% at TP=2), since less allocator churn means less iteration-level overhead.

Loss curves also match well:
loss_curves_drain_vs_baseline

Test plan

  • Unit tests on H100 cluster: tests/unit_tests/distributed/test_reduce_scatter_with_fp32_accumulation.py, test_param_and_grad_buffer.py, test_distributed_data_parallel.py, test_grad_sync_with_expert_parallel.py.
  • Functional test on the FP32-accum + multi-bucket configuration that exposed the memory issue (see Results table).
  • Peak-memory comparison before/after to confirm the all-to-all buffer is released at successor dispatch, not at end-of-step (see Results table).

🤖 Generated with Claude Code

@copy-pr-bot

copy-pr-bot Bot commented May 22, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@deepakn94

Copy link
Copy Markdown
Contributor Author

/claude review

@claude claude Bot 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.

LGTM

@deepakn94

Copy link
Copy Markdown
Contributor Author

/ok to test a817431

@deepakn94

Copy link
Copy Markdown
Contributor Author

/ok to test fdfea86

@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Final Review PR is in the "final review" stage label May 26, 2026
@svcnvidia-nemo-ci svcnvidia-nemo-ci added Approved All necessary approvals have been made and removed Final Review PR is in the "final review" stage labels May 27, 2026
@svcnvidia-nemo-ci

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/26494868163

Merged via the queue into NVIDIA:main with commit 88e7ab0 May 27, 2026
78 checks passed
@deepakn94
deepakn94 deleted the dnarayanan/drain_predecessor_grad_reduce branch May 27, 2026 07:10
Victarry pushed a commit to yanring/Megatron-LM that referenced this pull request May 27, 2026
* origin/main: (50 commits)
  Drain predecessor reduce-scatter at dispatch time (NVIDIA#4940)
  ci: Add allow_failure flag to gpt and moe recipes that are failing in nightlies (NVIDIA#4905)
  fix(tests): initialize num_microbatches calculator in vision cudagraph tests (NVIDIA#4986)
  test: re-enable test_pp2_create_cudagraphs_first_stage on TE 2.15+ (NVIDIA#4985)
  ci: Add support for MBridge job gating based on PR labels  (NVIDIA#4926)
  test(ci): re-enable 8experts2parallel_multi_dist_optimizer_instances_1node (NVIDIA#4984)
  test: re-enable paged stashing MoE tests (NVIDIA#4978)
  Fix elastification unwrap_model import (NVIDIA#4972)
  Avoid offsetting functional test master port (NVIDIA#4973)
  test: enable NVTE_CUTEDSL_FUSED_GROUPED_MLP via pytest fixture (NVIDIA#4931)
  chore(beep boop 🤖): Bump  (main) (2026-05-25)
  test(release): add release goldens for deepseekv3/nemotron3 and set tp2pp2 exit-interval (NVIDIA#4932)
  Fix `get_batch` return order to ignore BlendedDataset provenance fields (NVIDIA#4952)
  ci: restore perf test torchrun logs (NVIDIA#4951)
  Various training utils (NVIDIA#4872)
  ci: Update training script paths in BERT and T5 (NVIDIA#4939)
  [MXFP8/FP4-param-gather] Post processing after forced param AG in eval (NVIDIA#4562)
  Fix mxfp8 param gather numerical issue when DP overlap is off (NVIDIA#4800)
  Add TEFusedDenseMLP for Dense+Grouped GEMM fusion on SM100+ (NVIDIA#4318) (NVIDIA#4786)
  Fix paged stashing test submodules lookup (NVIDIA#4925)
  ...

# Conflicts:
#	megatron/training/training.py
janEbert pushed a commit to janEbert/Megatron-LM that referenced this pull request Jun 2, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mathemakitten pushed a commit to mathemakitten/Megatron-LM that referenced this pull request Jun 12, 2026
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made complexity: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants