diff --git a/megatron/core/distributed/param_and_grad_buffer.py b/megatron/core/distributed/param_and_grad_buffer.py index d34fdebaf75..50cf3e0ea37 100644 --- a/megatron/core/distributed/param_and_grad_buffer.py +++ b/megatron/core/distributed/param_and_grad_buffer.py @@ -142,9 +142,7 @@ def __init__( self.data_parallel_group = collective_group # State for bookkeeping: params is the set of parameters this bucket group is - # responsible for, params_with_grad is the set of parameters with grads - # available. When overlap_grad_reduce is True, communication (all-reduce - # or reduce-scatter) is issued when params_with_grad equals params. + # responsible for, param_to_bucket maps params to the corresponding bucket. self.param_to_bucket = {} self.params = set() for bucket in self.buckets: @@ -165,7 +163,22 @@ def __init__( if self.ddp_config.reduce_scatter_with_fp32_accumulation: dist_reduce_scatter_func = reduce_scatter_with_fp32_accumulation - self.reset() + # per_param_grad_ready_counts is a dict mapping parameters to number of times + # `register_grad_ready` is called for that parameter *when + # self.is_last_microbatch is True*. Should be 1 for most params but could be greater + # than 1 if control flow passes through the same parameter multiple times. We lazily + # populate this in the first batch, hence the .is_first_batch attribute. + # When overlap_grad_reduce is True, communication (all-reduce or reduce-scatter) + # is issued when per_param_grad_ready_counts equals golden_per_param_grad_ready_counts. + # In other words, communication is dispatched as soon as all gradients in this bucket + # are *ready*, as marked by the backward hook. + # The set of keys in per_param_grad_ready_counts should be equal to `params`. + self.golden_per_param_grad_ready_counts = {} + self.per_param_grad_ready_counts = {} + self.is_last_microbatch = True + self.is_first_batch = True + + # Other metadata to keep track of collectives. self.param_gather_handle = None self.param_gather_dispatched = False self.grad_reduce_handle = None @@ -182,7 +195,12 @@ def reset(self): """ Reset metadata in bucket group in preparation for the next iteration of training. """ - self.params_with_grad = set() + if self.is_first_batch and len(self.per_param_grad_ready_counts) > 0: + # Record golden per_param_grad_ready_counts. + assert len(self.per_param_grad_ready_counts) == len(self.params) + self.golden_per_param_grad_ready_counts = self.per_param_grad_ready_counts + self.is_first_batch = False + self.per_param_grad_ready_counts = {} self.is_last_microbatch = True def check_grads(self, check_for_nan_or_inf, check_for_large): @@ -346,6 +364,11 @@ def start_grad_sync(self): communication call. When ddp_config.overlap_grad_reduce is set to False, makes synchronous call. """ + if self.is_first_batch and self.grad_reduce_handle is not None: + # Make this start_grad_sync call a no-op if in first batch and collective has + # already been dispatched. + return + assert ( self.grad_reduce_handle is None ), "Should not have multiple communication calls outstanding at once" @@ -485,6 +508,11 @@ def finish_grad_sync(self): if not self.ddp_config.overlap_grad_reduce: self.start_grad_sync() return + # If first batch, start asynchronous communication here. register_grad_ready() launches + # asynchronous communication only once self.golden_per_param_grad_ready_counts is + # populated at the end of this first batch. + if self.is_first_batch: + self.start_grad_sync() # When using multiple DistOpt instances, we don't need to sync here as we launch # communications on a separate communication stream. if self.ddp_config.num_distributed_optimizer_instances > 1: @@ -492,7 +520,8 @@ def finish_grad_sync(self): return assert self.grad_reduce_handle is not None, ( f"Communication call has not been issued for this bucket " - f"({len(self.params_with_grad)}/{len(self.params)} params have grad available)" + f"({len(self.per_param_grad_ready_counts)}/{len(self.params)} " + "params have grad available)" ) self.grad_reduce_handle.wait() self.grad_reduce_handle = None @@ -510,11 +539,14 @@ def register_grad_ready(self, param: torch.nn.Parameter): ), "register_grad_ready() should only be called when overlap_grad_reduce is True" if self.is_last_microbatch: assert param in self.param_to_bucket, "Param is not in the bucket group" - assert param not in self.params_with_grad, "Cannot set grad twice" - self.params_with_grad.add(param) + if param not in self.per_param_grad_ready_counts: + self.per_param_grad_ready_counts[param] = 0 + self.per_param_grad_ready_counts[param] += 1 # If all params in bucket group have grads available, issue communication call. - if len(self.params_with_grad) == len(self.params): - self.start_grad_sync() + if not self.is_first_batch: + if self.per_param_grad_ready_counts == self.golden_per_param_grad_ready_counts: + assert len(self.per_param_grad_ready_counts) == len(self.params) + self.start_grad_sync() class _ParamAndGradBuffer: diff --git a/tests/unit_tests/distributed/test_grad_sync_with_expert_parallel.py b/tests/unit_tests/distributed/test_grad_sync_with_expert_parallel.py index 71e45f9d92e..e83f7142284 100644 --- a/tests/unit_tests/distributed/test_grad_sync_with_expert_parallel.py +++ b/tests/unit_tests/distributed/test_grad_sync_with_expert_parallel.py @@ -1,3 +1,5 @@ +# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. + import contextlib from typing import Optional @@ -169,15 +171,20 @@ def test_grad_sync( ) != 0 ): - # With above conditions, the data in param_and_grad_buffer.grad_data[0] equals to 1/data_parallel_word_size - # When average_in_collective=False, the grad data is always first scaled by 1/data_parallel_word_size and then summed by AR/RS - # when use_distributed_optimizer=True, only for rank=0 param_and_grad_buffer.grad_data[0] is updated, for other ranks - # another shard of grad_data is updated while param_and_grad_buffer.grad_data[0] is unchanged (=1/data_parallel_word_size) + # With above conditions, the data in param_and_grad_buffer.grad_data[0] equals + # 1/data_parallel_word_size. + # When average_in_collective=False, the grad data is always first scaled by + # 1/data_parallel_word_size and then summed by AR/RS. + # When use_distributed_optimizer=True, only for rank=0, + # param_and_grad_buffer.grad_data[0] is updated. For other ranks another shard of + # grad_data is updated while param_and_grad_buffer.grad_data[0] is unchanged + # (=1/data_parallel_word_size). non_ep_expected_grad_data_value_after_collective /= ( parallel_state.get_data_parallel_world_size() ) if ep_size > 1: - # For MoE models with exper parallelism, each expert will receive tokens from EPxETP times batches, such that the expert gradient will be EPxETP times after backward, + # For MoE models with exper parallelism, each expert will receive tokens from EPxETP + # times batches, such that the expert gradient will be EPxETP times after backward, # and the expected gradient after collective should be 1.0 as same as dense params. ep_param_and_grad_buffer.grad_data.data.fill_(float(ep_size * etp_size)) ep_expected_grad_data_value_after_collective = 1 @@ -186,14 +193,30 @@ def test_grad_sync( and (not average_in_collective) and parallel_state.get_expert_data_parallel_rank(partial_expert_data_parallel=True) != 0 ): - # With above conditions, the data in param_and_grad_buffer.grad_data[0] equals to 1/EDP - # When average_in_collective=False, the grad data is always first scaled by expert_data_parallel_size and then summed by AR/RS - # after SUM collective in expert_data_group, the scale will be 1.0. + # With above conditions, the data in param_and_grad_buffer.grad_data[0] equals 1/EDP. + # When average_in_collective=False, the grad data is always first scaled by + # expert_data_parallel_size and then summed by AR/RS. + # After SUM collective in expert_data_group, the scale will be 1.0. ep_expected_grad_data_value_after_collective /= ( parallel_state.get_expert_data_parallel_world_size() ) + register_grad_sync_context = ( + contextlib.nullcontext() if overlap_grad_reduce else pytest.raises(AssertionError) + ) + + # Call register_grad_ready for all params before starting test to seed tracking + # data structures. params = list(model.parameters()) + for param in params: + with register_grad_sync_context: + bucket_group = param_to_bucket_group[param] + bucket_group.register_grad_ready(param) + # Call reset to set .is_first_batch to False. + for param in params: + bucket_group = param_to_bucket_group[param] + bucket_group.reset() + map_bucket_to_last_param_idx = {} for i, param in enumerate(params): if not (param in param_to_bucket_group): @@ -206,9 +229,6 @@ def test_grad_sync( param_idx = 0 map_bucket_to_last_param_idx[bucket_group] = param_idx - register_grad_sync_context = ( - contextlib.nullcontext() if overlap_grad_reduce else pytest.raises(AssertionError) - ) finish_grad_sync_context = contextlib.nullcontext() if ( param_idx < (len(bucket_group.params) - 1) @@ -220,6 +240,7 @@ def test_grad_sync( with register_grad_sync_context: bucket_group.register_grad_ready(param) + with finish_grad_sync_context: # When overlap_grad_reduce is True, this should throw an assertion error until all # params in the model have registered their grad above. diff --git a/tests/unit_tests/distributed/test_param_and_grad_buffer.py b/tests/unit_tests/distributed/test_param_and_grad_buffer.py index c09e2313d8d..ac0c6a6c422 100644 --- a/tests/unit_tests/distributed/test_param_and_grad_buffer.py +++ b/tests/unit_tests/distributed/test_param_and_grad_buffer.py @@ -1,3 +1,5 @@ +# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. + import contextlib import math from typing import Optional @@ -164,7 +166,6 @@ def _pad_param_if_needed(numel_unpadded): @pytest.mark.parametrize("overlap_grad_reduce", [False, True]) @pytest.mark.parametrize("average_in_collective", [False, True]) @pytest.mark.parametrize("num_distributed_optimizer_instances", [1, 2]) -# @pytest.mark.flaky def test_grad_sync( use_distributed_optimizer: bool, overlap_grad_reduce: bool, @@ -201,10 +202,12 @@ def test_grad_sync( param_and_grad_buffer.grad_data.data.fill_(1.0) expected_grad_data_value_after_collective = 1 - # under the following conditions, the data in param_and_grad_buffer.grad_data[0] equals to 1/DP - # this is because when average_in_collective=False, the grad data is always first scaled by 1/DP and then summed by AR/RS - # and when use_distributed_optimizer=True, only for rank=0 param_and_grad_buffer.grad_data[0] is updated, for other ranks - # another shard of grad_data is updated while param_and_grad_buffer.grad_data[0] is unchanged (=1/DP) + # Data in param_and_grad_buffer.grad_data[0] is 1/DP. + # When average_in_collective=False, the grad data is always first scaled by 1/DP and then + # summed by AR/RS. + # When use_distributed_optimizer=True, only rank0's param_and_grad_buffer.grad_data[0] is + # updated; other ranks update another shard of grad_data while keeping + # param_and_grad_buffer.grad_data[0] unchanged (=1/DP). if ( use_distributed_optimizer and (not average_in_collective) @@ -215,13 +218,25 @@ def test_grad_sync( ): expected_grad_data_value_after_collective /= parallel_state.get_data_parallel_world_size() + register_grad_sync_context = ( + contextlib.nullcontext() if overlap_grad_reduce else pytest.raises(AssertionError) + ) + + # Call register_grad_ready for all params before starting test to seed tracking + # data structures. params = list(model.parameters()) + for param in params: + with register_grad_sync_context: + bucket_group = param_to_bucket_group[param] + bucket_group.register_grad_ready(param) + # Call reset to set .is_first_batch to False. + for param in params: + bucket_group = param_to_bucket_group[param] + bucket_group.reset() + for i, param in enumerate(params): assert param in param_to_bucket_group bucket_group = param_to_bucket_group[param] - register_grad_sync_context = ( - contextlib.nullcontext() if overlap_grad_reduce else pytest.raises(AssertionError) - ) finish_grad_sync_context = contextlib.nullcontext() if ( i < (len(params) - 1) @@ -233,6 +248,7 @@ def test_grad_sync( with register_grad_sync_context: bucket_group.register_grad_ready(param) + with finish_grad_sync_context: # When overlap_grad_reduce is True, this should throw an assertion error until all # params in the model have registered their grad above.