Skip to content

Commit 42102ea

Browse files
authored
[training] fix: respect module-local MIMO loss normalization (#5555)
Signed-off-by: Yu Yao <yaoyu.094@gmail.com>
1 parent f97d4cd commit 42102ea

2 files changed

Lines changed: 66 additions & 6 deletions

File tree

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from megatron.core.distributed.finalize_model_grads import finalize_model_grads as _finalize_model_grads
2525
from megatron.core.models.mimo import MimoModel
2626
from megatron.core.models.mimo.config.role import MIMO_LANGUAGE_MODULE_KEY
27+
from megatron.core.utils import get_model_config
2728

2829
from megatron.bridge.models.megatron_mimo.megatron_mimo_provider import MegatronMIMOInfra
2930

@@ -258,9 +259,9 @@ def _find_module(grid):
258259
# pass num_tokens=None for them to skip the broken normalization,
259260
# then broadcast the correct total from LLM and apply it manually.
260261
#
261-
# With gradient_scaling_factor=1.0 (calculate_per_token_loss=True),
262-
# DDP does a plain SUM. After dividing by the global token count
263-
# the gradient is correct — no DP compensation factor needed.
262+
# A per-token module's DDP does a plain SUM, while a non-per-token
263+
# module's DDP averages over its DP ranks. Account for that module-local
264+
# choice before dividing by the global token count.
264265

265266
# Phase 1: gradient all-reduce for each module. Only the LLM gets
266267
# num_tokens so _finalize_model_grads can PP-broadcast + DP-all-reduce
@@ -295,7 +296,9 @@ def _find_module(grid):
295296
if module is not None and is_current_rank_in_grid(grid):
296297
module_name, _ = _find_module(grid)
297298
if module_name != MIMO_LANGUAGE_MODULE_KEY and num_tokens > 0:
298-
module.scale_gradients(1.0 / num_tokens.float().item())
299+
module_dp = _get_dp_size_from_grid(grid)
300+
ddp_mean_correction = 1.0 if get_model_config(module).calculate_per_token_loss else module_dp
301+
module.scale_gradients(ddp_mean_correction / num_tokens.float().item())
299302
else:
300303
# calculate_per_token_loss=False path.
301304
#
@@ -316,8 +319,10 @@ def _find_module(grid):
316319
)
317320

318321
module_dp = _get_dp_size_from_grid(grid)
319-
if module_dp != llm_dp:
320-
module.scale_gradients(float(module_dp) / float(llm_dp))
322+
ddp_mean_correction = 1.0 if get_model_config(module).calculate_per_token_loss else module_dp
323+
module_scale = float(ddp_mean_correction) / float(llm_dp)
324+
if module_scale != 1.0:
325+
module.scale_gradients(module_scale)
321326

322327

323328
def zero_grad_buffer_for_multimodule(module_to_grid_tuple: List[Tuple]):

‎tests/unit_tests/training/megatron_mimo/test_megatron_mimo_parallel_utils.py‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ def test_per_token_loss_path(self, mock_finalize, mock_dp, mock_in_grid, mock_di
376376
s = _build_two_module_setup(llm_dp=2, encoder_dp=4)
377377
mock_in_grid.return_value = True
378378
mock_dp.side_effect = lambda grid: s.dp_by_grid[id(grid)]
379+
s.encoder_module.config.calculate_per_token_loss = True
379380

380381
num_tokens = torch.tensor(100)
381382

@@ -405,6 +406,58 @@ def test_per_token_loss_path(self, mock_finalize, mock_dp, mock_in_grid, mock_di
405406
s.encoder_module.scale_gradients.assert_called_once_with(1.0 / 100)
406407
s.llm_module.scale_gradients.assert_not_called()
407408

409+
@pytest.mark.parametrize(
410+
("language_per_token", "encoder_per_token", "expected_gradient"),
411+
[
412+
(True, False, 4.0),
413+
(False, True, 200.0),
414+
],
415+
)
416+
@patch(f"{MODULE}.dist")
417+
@patch(f"{MODULE}.is_current_rank_in_grid")
418+
@patch(f"{MODULE}._get_dp_size_from_grid")
419+
@patch(f"{MODULE}._finalize_model_grads")
420+
def test_mixed_per_token_configs_preserve_encoder_gradient_normalization(
421+
self,
422+
mock_finalize,
423+
mock_dp,
424+
mock_in_grid,
425+
mock_dist,
426+
language_per_token,
427+
encoder_per_token,
428+
expected_gradient,
429+
):
430+
"""Module-local DDP scaling must not change the global encoder gradient."""
431+
from megatron.bridge.training.megatron_mimo_parallel_utils import finalize_model_grads_multimodule
432+
433+
s = _build_two_module_setup(llm_dp=2, encoder_dp=4)
434+
mock_in_grid.return_value = True
435+
mock_dp.side_effect = lambda grid: s.dp_by_grid[id(grid)]
436+
s.llm_module.config.calculate_per_token_loss = language_per_token
437+
s.encoder_module.config.calculate_per_token_loss = encoder_per_token
438+
439+
# Simulate the value after MCore DDP reduction for a global encoder
440+
# gradient sum of 400. Non-per-token DDP averages over encoder DP;
441+
# per-token DDP leaves the global sum intact.
442+
encoder_gradient = torch.tensor(400.0)
443+
444+
def finalize_with_module_local_ddp(model, **kwargs):
445+
if model[0] is s.encoder_module and not encoder_per_token:
446+
encoder_gradient.div_(s.dp_by_grid[id(s.encoder_grid)])
447+
448+
mock_finalize.side_effect = finalize_with_module_local_ddp
449+
s.encoder_module.scale_gradients.side_effect = encoder_gradient.mul_
450+
451+
num_tokens = torch.tensor(100) if language_per_token else None
452+
finalize_model_grads_multimodule(
453+
[MagicMock()],
454+
num_tokens,
455+
infra=s.infra,
456+
module_to_grid_tuple=s.module_to_grid_tuple,
457+
)
458+
459+
assert encoder_gradient.item() == pytest.approx(expected_gradient)
460+
408461
@patch(f"{MODULE}.dist")
409462
@patch(f"{MODULE}.is_current_rank_in_grid")
410463
@patch(f"{MODULE}._get_dp_size_from_grid")
@@ -420,6 +473,8 @@ def test_non_per_token_loss_path_applies_dp_compensation(self, mock_finalize, mo
420473
s = _build_two_module_setup(llm_dp=2, encoder_dp=4)
421474
mock_in_grid.return_value = True
422475
mock_dp.side_effect = lambda grid: s.dp_by_grid[id(grid)]
476+
s.llm_module.config.calculate_per_token_loss = False
477+
s.encoder_module.config.calculate_per_token_loss = False
423478

424479
finalize_model_grads_multimodule(
425480
[MagicMock()],

0 commit comments

Comments
 (0)