Skip to content

Commit d608c62

Browse files
committed
fix(checkpoint): preserve existing FSDP v2 key conventions
Signed-off-by: Chen Cui <chcui@nvidia.com>
1 parent 31f2a61 commit d608c62

2 files changed

Lines changed: 22 additions & 14 deletions

File tree

‎src/megatron/bridge/training/checkpointing.py‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2330,14 +2330,15 @@ def apply(handler: Callable[..., tuple[dict[str, Any], dict[str, Any] | None]])
23302330
if handle_mtp_in_state_dict is not None:
23312331
apply(handle_mtp_in_state_dict)
23322332

2333-
# Globalize layer names last: earlier handlers resolve stage-local module paths.
2334-
if handle_pipeline_parallel_in_state_dict is not None:
2335-
apply(handle_pipeline_parallel_in_state_dict)
2336-
elif model_config.pipeline_model_parallel_size > 1:
2337-
raise RuntimeError(
2338-
"FSDP pipeline checkpointing requires a Megatron-Core version with "
2339-
"handle_pipeline_parallel_in_state_dict; older versions collide stage-local layer names."
2340-
)
2333+
# V2 optimizer keys are already global. Only v1 needs this final renaming.
2334+
if getattr(getattr(cfg, "ddp", None), "megatron_fsdp_version", 1) == 1:
2335+
if handle_pipeline_parallel_in_state_dict is not None:
2336+
apply(handle_pipeline_parallel_in_state_dict)
2337+
elif model_config.pipeline_model_parallel_size > 1:
2338+
raise RuntimeError(
2339+
"FSDP pipeline checkpointing requires a Megatron-Core version with "
2340+
"handle_pipeline_parallel_in_state_dict; older versions collide stage-local layer names."
2341+
)
23412342

23422343
preprocess_state_dict_for_uneven_dtensor(state_dict)
23432344

‎tests/unit_tests/training/test_checkpointing.py‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4930,8 +4930,9 @@ def test_generate_state_dict_fsdp_dtensor_no_preprocessing(self):
49304930
3.1 if hasattr(distrib_optimizer, "get_legacy_grad_dtypes") else 3.0
49314931
)
49324932

4933+
@pytest.mark.parametrize("version", [1, 2])
49334934
@patch("megatron.bridge.training.checkpointing.HAVE_MEGATRON_FSDP", True)
4934-
def test_preprocess_fsdp_dtensor_state_dict(self):
4935+
def test_preprocess_fsdp_dtensor_state_dict(self, version: int) -> None:
49354936
"""Test preprocess_fsdp_dtensor_state_dict applies all preprocessing steps."""
49364937
from unittest.mock import Mock
49374938

@@ -4940,6 +4941,7 @@ def test_preprocess_fsdp_dtensor_state_dict(self):
49404941
# Create mock model and config
49414942
mock_model = Mock()
49424943
mock_cfg = Mock()
4944+
mock_cfg.ddp.megatron_fsdp_version = version
49434945

49444946
# Mock model config without swiglu or experts
49454947
with patch("megatron.core.utils.get_model_config") as mock_get_config:
@@ -4968,11 +4970,15 @@ def test_preprocess_fsdp_dtensor_state_dict(self):
49684970
# Should call FP8 and uneven dtensor preprocessing
49694971
mock_fp8.assert_called_once()
49704972
mock_uneven.assert_called_once()
4971-
mock_pipeline.assert_called_once_with(mock_model, raw_state_dict["model"], None)
4973+
if version == 1:
4974+
mock_pipeline.assert_called_once_with(mock_model, raw_state_dict["model"], None)
4975+
else:
4976+
mock_pipeline.assert_not_called()
49724977
assert "model" in result
49734978

49744979
@pytest.mark.parametrize("pp", [1, 2])
4975-
def test_fsdp_pipeline_checkpoint_requires_core_handler(self, pp: int) -> None:
4980+
@pytest.mark.parametrize("version", [1, 2])
4981+
def test_fsdp_pipeline_checkpoint_requires_core_handler(self, pp: int, version: int) -> None:
49764982
"""Old Core remains usable for PP=1 but must not silently corrupt PP>1."""
49774983
from types import SimpleNamespace
49784984

@@ -4992,11 +4998,12 @@ def test_fsdp_pipeline_checkpoint_requires_core_handler(self, pp: int) -> None:
49924998
),
49934999
):
49945000
state = {"model": {"weight": torch.ones(1)}}
4995-
if pp > 1:
5001+
cfg = SimpleNamespace(ddp=SimpleNamespace(megatron_fsdp_version=version))
5002+
if pp > 1 and version == 1:
49965003
with pytest.raises(RuntimeError, match="collide stage-local layer names"):
4997-
checkpointing.preprocess_fsdp_dtensor_state_dict(None, state, None)
5004+
checkpointing.preprocess_fsdp_dtensor_state_dict(cfg, state, None)
49985005
else:
4999-
assert checkpointing.preprocess_fsdp_dtensor_state_dict(None, state, None) == state
5006+
assert checkpointing.preprocess_fsdp_dtensor_state_dict(cfg, state, None) == state
50005007

50015008
@patch("megatron.bridge.training.checkpointing.HAVE_MEGATRON_FSDP", True)
50025009
def test_save_fsdp_dtensor_checkpoint_preprocesses_and_saves(self, tmp_path):

0 commit comments

Comments
 (0)