Add configurable sum gradient reduction - #8232
Conversation
Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a3b05de83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._unmanaged_backward_count = 0 | ||
| self.skipped_steps = 0 | ||
| self.gradient_average = True | ||
| self.gradient_average = config_class.gradient_allreduce_op == GRADIENT_ALLREDUCE_OP_MEAN |
There was a problem hiding this comment.
Propagate sum mode into DeepCompile reductions
When ZeRO stage 1 or 2 runs with an active DeepCompile backend, allreduce_gradients() returns at engine.py:2842 before any of the new sum-aware paths execute. DeepCompile instead keeps pre_div_reduce = true (csrc/compile/deepcompile.cpp:21) and unconditionally divides every bucket by the process-group size in CustomOpExecutor::applyPreDivision() (csrc/includes/deepcompile.h:590-600), so gradient_allreduce_op="sum" silently still produces a mean and scales updates down by the data-parallel world size. Pass this mode into the compiled reducer or reject the combination during configuration.
Useful? React with 👍 / 👎.
| return get_scalar_param(param_dict, GRADIENT_PREDIVIDE_FACTOR, GRADIENT_PREDIVIDE_FACTOR_DEFAULT) | ||
|
|
||
|
|
||
| def get_gradient_allreduce_op(param_dict): |
There was a problem hiding this comment.
Add the required Signed-off-by trailer
The reviewed commit is a non-merge commit, but its message contains no Signed-off-by trailer. Add the trailer using the configured Git identity before merging so the commit satisfies the repository's commit and CI requirements.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
| <= ZeroStageEnum.max_stage), "DeepSpeedConfig: Maximum supported ZeRO stage is {}".format( | ||
| ZeroStageEnum.max_stage) | ||
|
|
||
| if (self.gradient_allreduce_op == GRADIENT_ALLREDUCE_OP_SUM |
There was a problem hiding this comment.
any reason why zero3 is not supported at this moment?
There was a problem hiding this comment.
Yes. I limited this PR to ZeRO stages 0-2 because ZeRO-3 uses a separate reduction and partition path: reduce_scatter_coalesced() pre-divides by world size, and the Stage 3 optimizer currently does not receive gradient_average. Supporting sum safely would require plumbing the mode through the Stage 3 optimizer and auditing its regular, contiguous, and quantized/all-to-all reduction paths, with dedicated ZeRO-3 tests. For this PR I chose to reject the combination during config validation instead of silently returning mean semantics. I can handle ZeRO-3 in a follow-up if that scope is desired.
| else: | ||
| values.mul_(1. / (dp_world_size)) | ||
| else: | ||
| values.mul_(1. / (dp_world_size)) |
There was a problem hiding this comment.
is this where we divide by dp world size to make sure the gradients used for optimizer is still scaled?
There was a problem hiding this comment.
Yes—this is the sparse-gradient path. With the default mean mode, self.gradient_average is true, so the values are divided by the data-parallel world size before all-gather. With gradient_allreduce_op="sum", self.gradient_average is false, this block is skipped, and the optimizer receives the unnormalized sum. The dense and ZeRO stages 1/2 paths use the same guard.
Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
| (False, False, True, 1.0), | ||
| ], | ||
| ) | ||
| class TestGradientAllreduceOp(DistributedTest): |
There was a problem hiding this comment.
is this overall simplistic? maybe add a real training as well and compare the losses are close for each step with the same model with or without this knob on?
pengdurice
left a comment
There was a problem hiding this comment.
at the same time, maybe a loss comparison figure with our without enabling this will be useful.
pengdurice
left a comment
There was a problem hiding this comment.
this change should be optimizer agnostic. should we test that using different optimizers, e.g. AdamW and Muon, things all work fine?
Signed-off-by: Wang Zupeng <zupenwang@gmail.com>
|
@pengdurice Thanks — I agree the original single-step SGD test was too narrow. |
Thank you for the reply!
|
|
@pengdurice Done — the follow-up now uses the repository I also added the loss-comparison figure and the updated validation results to the PR description. The targeted tests pass 23/23, and the complete changed test file passes 98 tests with one conditional skip. Thanks! |
would you mind push your change to remote? I cannot find new commit yet |
|
Thanks! The change has been pushed to the PR's fork branch ( |


Summary
gradient_allreduce_opconfiguration with"mean"as the default and"sum"as the new optionAddresses #7107.
Motivation
Some distributed objectives, including contrastive learning over globally gathered embeddings, require summing data-parallel gradients rather than averaging them. Today users need to rescale the loss manually to cancel DeepSpeed's world-size normalization.
This change makes the reduction semantics explicit while keeping the current behavior as the default.
Validation
Tested on two NVIDIA GeForce RTX 3090 GPUs with PyTorch 2.13.0+cu130:
pytest --forked -q tests/unit/v1/zero/test_zero.py::TestGradientAllreduceOp— 18 passedtests/unit/runtime/test_ds_config_dict.py— 8 passed; the new ZeRO-1/2 DeepCompile cases both failed against the prior head because no error was raisedpytest --forked -q tests/unit/v1/zero/test_zero_coalesce_grad_reduction.py::TestCoalesceCombinations— 12 passedpytest --forked -q tests/unit/runtime/sparse_tensor/test_averaging_sparse_gradients.py— 1 passedpre-commithooks, including YAPF, flake8, codespell, license,check-torchdist, andcheck-torchcuda— passedThe distributed test matrix covers ZeRO stages 0/1/2, mean and sum reductions, reduce-scatter, gradient predivide, prescale, and non-contiguous gradient fallback.
Real-training equivalence
A deterministic two-rank, five-step
SimpleModelregression compares the default MEAN reduction with SUM while normalizing only the SUM backward loss byworld_size, so both modes provide identical gradients to the optimizer.23 passedtests/unit/v1/zero/test_zero.py:98 passed, 1 skippedAcross all five optimizer/stage configurations and all five training steps, the observed loss, full-gradient, and full-parameter differences were zero.
Limitations
gradient_allreduce_op="sum"is intentionally not supported with ZeRO stage 3, ZenFlow, or DeepCompile. These combinations fail during configuration instead of silently applying mean semantics.