Skip to content

Commit 3051914

Browse files
authored
[ckpt] fix: Reject Energon resume after DP changes (#5637)
Signed-off-by: Yu Yao <yaoyu.094@gmail.com>
1 parent 4de0797 commit 3051914

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

src/megatron/bridge/training/checkpointing.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,6 +1870,20 @@ def maybe_load_dataloader_state(
18701870
print_rank_0(f"no dataloader state for iteration {iteration}; dataloader starts from the beginning")
18711871
return
18721872

1873+
state_file_pattern = join_paths(iter_dir, "train_dataloader_dprank*.pt")
1874+
saved_dp_size = (
1875+
len(msc.glob(state_file_pattern))
1876+
if msc is not None
1877+
else len(list(Path(iter_dir).glob("train_dataloader_dprank*.pt")))
1878+
)
1879+
current_dp_size = get_pg_size(pg_collection.dp)
1880+
if saved_dp_size != current_dp_size:
1881+
raise RuntimeError(
1882+
f"Dataloader state at {iter_dir} was saved for data-parallel size {saved_dp_size}, but the current "
1883+
f"data-parallel size is {current_dp_size}. Resuming would silently change the training data order; "
1884+
"refusing to continue."
1885+
)
1886+
18731887
dp_rank = get_pg_rank(pg_collection.dp)
18741888
data_state_load_path = join_paths(iter_dir, f"train_dataloader_dprank{dp_rank:03d}.pt")
18751889
if not is_file(data_state_load_path):

tests/unit_tests/training/test_checkpointing.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5314,7 +5314,7 @@ def _dist_initialized(self):
53145314
yield
53155315

53165316
@staticmethod
5317-
def _pg(cp: int | None = 0, pp: int = 0, tp: int = 0, dp: int = 0):
5317+
def _pg(cp: int | None = 0, pp: int = 0, tp: int = 0, dp: int = 0, dp_size: int = 1):
53185318
"""Mock a ProcessGroupCollection with the given per-dimension ranks. Only cp may be None —
53195319
modeling a collection configured without context parallelism (tp/pp/dp are always
53205320
populated); the real get_pg_rank then reads the absent cp group as rank 0."""
@@ -5326,6 +5326,7 @@ def _pg(cp: int | None = 0, pp: int = 0, tp: int = 0, dp: int = 0):
53265326
pg.pp.rank.return_value = pp
53275327
pg.tp.rank.return_value = tp
53285328
pg.dp.rank.return_value = dp
5329+
pg.dp.size.return_value = dp_size
53295330
return pg
53305331

53315332
def test_noop_when_no_path(self):
@@ -5372,6 +5373,27 @@ def test_existing_dir_missing_file_raises(self, tmp_path):
53725373
with pytest.raises(RuntimeError, match="data-parallel size"):
53735374
maybe_load_dataloader_state(train_iterator, 10, str(tmp_path), pg_collection=self._pg())
53745375

5376+
@patch("megatron.bridge.training.checkpointing.energon_torch_load")
5377+
def test_extra_saved_dp_rank_file_raises(self, mock_load, tmp_path):
5378+
"""A smaller current DP size must not silently discard saved rank state."""
5379+
train_iterator = Mock()
5380+
iter_dir = Path(get_checkpoint_name(str(tmp_path), 10))
5381+
iter_dir.mkdir(parents=True)
5382+
(iter_dir / "train_dataloader_dprank000.pt").touch()
5383+
(iter_dir / "train_dataloader_dprank001.pt").touch()
5384+
pg_collection = self._pg(dp=0, dp_size=1)
5385+
5386+
with pytest.raises(RuntimeError, match="data-parallel size"):
5387+
maybe_load_dataloader_state(
5388+
train_iterator,
5389+
10,
5390+
str(tmp_path),
5391+
pg_collection=pg_collection,
5392+
)
5393+
5394+
mock_load.assert_not_called()
5395+
train_iterator.iterable.restore_state.assert_not_called()
5396+
53755397
@patch("megatron.bridge.training.checkpointing.energon_torch_load")
53765398
def test_restores_on_every_cp_tp_pp_rank(self, mock_load, tmp_path):
53775399
"""Unlike save, restore must run on every cp/tp/pp because

0 commit comments

Comments
 (0)