Repository navigation
Conversation
all_gather_coalesced splits its parameters by partition process group and issues one all-gather per group. With AutoEP, dense and expert parameters belong to different groups. The ZeRO-3 prefetcher passes a set, whose iteration order follows object addresses and can differ across ranks, so ranks could issue the per-group all-gathers in different orders and deadlock. Training hung from the third step, once prefetching started. Sort the parameters by ds_id on entry, before grouping, instead of only inside each group. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Jin, Youzhi <youzhi.jin@intel.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
A second group over the same two ranks is enough to split the gather into two per-group collectives, so the test no longer needs four devices. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Jin, Youzhi <youzhi.jin@intel.com>
pengdurice
approved these changes
Oct 25, 2026
pengdurice
enabled auto-merge
October 25, 2026 16:24
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 25, 2026
Contributor
|
@jinyouzhi , please fix the CI issue. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes an intermittent ZeRO-3 hang when parameters live in more than one partition group (e.g. AutoEP).
all_gather_coalescednow sorts its parameters byds_idbefore splitting them by group, so every rank issues the per-group all-gathers in the same order.Related: #8060 (added per-group gathering to
all_gather_coalesced). Found while validating deepspeedai/DeepSpeedExamples#1020 (AutoEP HF → ZeRO training → rollout E2E).Bug
run_autoep_rollout_e2e.shfrom DeepSpeedExamples#1020 withZERO_STAGES=3.Root cause
params_to_prefetch(aset) toall_gather_coalesced. A set's iteration order follows object addresses, which differ per process.all_gather_coalescedbuckets params intoparams_by_groupin that iteration order. It then issues one gather per group, so the first group seen is gathered first.sorted(params, key=lambda p: p.ds_id)ran only after bucketing, inside each per-group call. It fixed the order of params within a group, not the order of the groups.It starts at step 3 because prefetching begins only once the module trace is complete. While the E2E run was hung, py-spy showed rank 2 in
_all_gather_sequentialwith a single expert param (EDP) and the other ranks in_all_gather_coalescedfor the dense params (DP).Fix
ds_idsort to the entry ofall_gather_coalesced, before bucketing.ds_idcomes from a counter (Init.param_id) assigned in construction order, so it is identical on every rank. Both the group order and the order within each group are now rank-independent.partition_parameters.py). No API, config or checkpoint change.Compatibility
ds_idorder on every rank (the fix).Test
TestMultiGroupGatherOrderintests/unit/runtime/zero/test_zero_stale_handle_wait.py(world size 2):dist.new_group([0, 1])). The two groups are distinct communicators, so the gather is split into two per-group collectives.Validation
Hardware and software: 8x Intel Arc Pro B70; driver 1.15.39122; Ubuntu 24.04; Python 3.12; torch 2.14.0.dev20260804+xpu; XCCL backend.
DS_UNITTEST_TIMEOUT=120 pytest --forked tests/unit/runtime/zero/test_zero_stale_handle_wait.py -k TestMultiGroupGatherOrder(2 ranks)pytest --forked tests/unit/runtime/zero/test_zero_stale_handle_wait.py tests/unit/runtime/zero/test_zero_allreduce_fetch_params.pySame test on CUDA: 8x NVIDIA L20, torch 2.14.1+cu130, NCCL 2.30.7.
TestMultiGroupGatherOrder, 2 ranksTestMultiGroupGatherOrder, 4-rank variant ({0,1}/{2,3}subgroups)NCCL does not hang here, likely because ProcessGroupNCCL runs each communicator on its own internal stream, so the two per-group gathers progress concurrently. On NCCL the fix only removes the rank-dependent issue order.
Not validated: ZeRO++ hpZ/qwZ and NVMe param offload were not run. They are covered only by the reasoning in the compatibility table.
CI coverage
cpu-torch-multi-latest(nightly,LOCAL_SIZE=4) runs the new test on CPU over gloo. Locally (10 runs) (LOCAL_SIZE=4 DS_ACCELERATOR=cpu) it passes with and without the fix: gloo appears to progress each process group independently, so issuing the groups in opposite order does not block there. On CPU and on NCCL the test therefore only guards the pass path; the hang reproduces on XCCL (see Validation).cpu-torch-latestadvertises one device and skips it. With 2 ranks the test runs on any 2-GPU runner. Modal's selector only scopestests/unit/v1.xpu-max1100/hpu-gaudi2are triggered by the path but run fixed lists that do not include this file.