Skip to content

Commit eb1c677

Browse files
Thread pg_collection through wrap_model_chunks_with_ddp (NVIDIA#5328)
Signed-off-by: ykarnati <ykarnati@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0022550 commit eb1c677

2 files changed

Lines changed: 107 additions & 6 deletions

File tree

‎megatron/training/training.py‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,8 +1551,9 @@ def wrap_model_chunks_with_ddp(
15511551
to DDP. ``False`` keeps LayerWise on its legacy sync path.
15521552
DP: The DDP class to construct (``DistributedDataParallel`` or an FSDP
15531553
variant).
1554-
pg_collection: Optional :class:`ProcessGroupCollection`. Forwarded to
1555-
FSDP-style DPs only.
1554+
pg_collection: Optional :class:`ProcessGroupCollection`. When provided,
1555+
forwarded to both standard DDP and FSDP variants, and used to source
1556+
DP world sizes for layout computation.
15561557
bucket_sizes: Optional per-chunk bucket size override; defaults to
15571558
``[ddp_config.bucket_size] * len(model_chunks)``.
15581559
disable_bucketing_per_chunk: Optional per-chunk disable_bucketing flag;
@@ -1583,10 +1584,17 @@ def wrap_model_chunks_with_ddp(
15831584
else:
15841585
compute_layout = None
15851586
if compute_layout is not None:
1586-
data_parallel_world_size = mpu.get_data_parallel_world_size(
1587-
with_context_parallel=True
1587+
layout_pgs = (
1588+
pg_collection
1589+
if pg_collection is not None
1590+
else ProcessGroupCollection.use_mpu_process_groups()
15881591
)
1589-
expert_data_parallel_world_size = mpu.get_expert_data_parallel_world_size()
1592+
assert layout_pgs.dp_cp is not None, (
1593+
"wrap_model_chunks_with_ddp requires a dp_cp process group to size "
1594+
"the distributed-optimizer parameter layout"
1595+
)
1596+
data_parallel_world_size = get_pg_size(layout_pgs.dp_cp)
1597+
expert_data_parallel_world_size = get_pg_size(getattr(layout_pgs, "expt_dp", None))
15901598
for i, (chunk, bucket_size) in enumerate(zip(model_chunks, bucket_sizes)):
15911599
all_params = [p for p in chunk.parameters() if p.requires_grad]
15921600
per_chunk_layouts[i] = compute_layout(
@@ -1603,7 +1611,8 @@ def wrap_model_chunks_with_ddp(
16031611
model_chunks, per_chunk_layouts, disable_bucketing_per_chunk
16041612
):
16051613
chunk_kwargs = {}
1606-
if pg_collection is not None and DP is not DDP:
1614+
# TorchFSDP takes process_group, not pg_collection.
1615+
if pg_collection is not None and not (HAVE_FSDP2 and DP is torch_FSDP):
16071616
chunk_kwargs["pg_collection"] = pg_collection
16081617
if layout is not None:
16091618
chunk_kwargs["full_param_layout"] = layout
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
3+
"""Tests for ``wrap_model_chunks_with_ddp`` pg_collection threading.
4+
5+
These cover the training-loop helper directly (not the MIMO models), verifying
6+
that an explicit ``pg_collection`` built from a ``HyperCommGrid`` (no
7+
``parallel_state`` globals) is forwarded to standard ``DistributedDataParallel``,
8+
and that the default ``pg_collection=None`` path still falls back to MPU groups.
9+
"""
10+
11+
import pytest
12+
import torch
13+
import torch.nn as nn
14+
15+
from megatron.core import parallel_state
16+
from megatron.core.distributed import DistributedDataParallel, DistributedDataParallelConfig
17+
from megatron.core.hyper_comm_grid import HyperCommGrid
18+
from megatron.core.process_groups_config import ProcessGroupCollection
19+
from megatron.core.transformer import TransformerConfig
20+
from tests.unit_tests.test_utilities import Utils
21+
22+
pytestmark = pytest.mark.skipif(torch.cuda.device_count() < 8, reason="Requires 8 GPUs")
23+
24+
25+
class _TinyModel(nn.Module):
26+
def __init__(self):
27+
super().__init__()
28+
self.fc1 = nn.Linear(16, 8)
29+
30+
def forward(self, x):
31+
return self.fc1(x)
32+
33+
34+
def _make_chunk():
35+
return _TinyModel().bfloat16().cuda()
36+
37+
38+
def _config():
39+
return TransformerConfig(num_attention_heads=1, num_layers=1)
40+
41+
42+
class TestExplicitPgCollection:
43+
"""Drive the helper with a HyperCommGrid-derived pgc, no parallel_state."""
44+
45+
def test_explicit_pg_collection_forwarded_to_ddp(self):
46+
from megatron.training.training import wrap_model_chunks_with_ddp
47+
48+
Utils.initialize_distributed() # torch.distributed only, no model-parallel state
49+
# One pure-DP grid over the 8 ranks; tp/pp/ep are size-1 (no model parallelism).
50+
grid = HyperCommGrid([1, 1, 1, 8], ["tp", "pp", "ep", "dp"], backend="nccl")
51+
dp = grid.create_pg("dp")
52+
pg_collection = ProcessGroupCollection(
53+
tp=grid.create_pg("tp"),
54+
pp=grid.create_pg("pp"),
55+
ep=grid.create_pg("ep"),
56+
dp=dp,
57+
dp_cp=dp,
58+
expt_dp=dp,
59+
)
60+
61+
ddp_config = DistributedDataParallelConfig(use_distributed_optimizer=True)
62+
wrapped = wrap_model_chunks_with_ddp(
63+
[_make_chunk()], _config(), ddp_config, pg_collection=pg_collection
64+
)
65+
66+
chunk = wrapped[0]
67+
assert isinstance(chunk, DistributedDataParallel)
68+
# DDP was constructed against the grid-derived dp_cp group, not parallel_state.
69+
assert chunk.dp_cp_group is dp
70+
grid.destroy()
71+
72+
73+
class TestDefaultPath:
74+
"""Isolated back-compat check: pg_collection=None falls back to MPU globals."""
75+
76+
def teardown_method(self, method):
77+
Utils.destroy_model_parallel()
78+
79+
def test_default_path_uses_mpu(self):
80+
from megatron.training.training import wrap_model_chunks_with_ddp
81+
82+
Utils.initialize_model_parallel(
83+
tensor_model_parallel_size=1, pipeline_model_parallel_size=1
84+
)
85+
ddp_config = DistributedDataParallelConfig(use_distributed_optimizer=True)
86+
wrapped = wrap_model_chunks_with_ddp([_make_chunk()], _config(), ddp_config)
87+
88+
chunk = wrapped[0]
89+
assert isinstance(chunk, DistributedDataParallel)
90+
assert chunk.dp_cp_group is parallel_state.get_data_parallel_group(
91+
with_context_parallel=True
92+
)

0 commit comments

Comments
 (0)