Repository navigation
Fix routed-expert gradient clipping for AutoEP + AutoTP folding - #8823
Conversation
Signed-off-by: 0z5a <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 352ee55cd1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| norm_groups.append(norm) | ||
| else: | ||
| norm = self.get_grad_norm_direct(self.averaged_gradients[i], self.params_in_partition[i]) | ||
| folded_experts = folding and self.has_moe_layers and self.is_moe_param_group[i] |
There was a problem hiding this comment.
Restrict folded reductions to AutoEP expert groups
When folded AutoEP is enabled in a ZeRO-1/2 model that also contains a native deepspeed.moe.layer.MoE group with enable_expert_tensor_parallelism=True, this condition selects the native group solely because the global folding spec exists. Native ETP experts can have gradients sharded over the model-parallel group, but the arguments below replace _model_parallel_all_reduce with only EDP+EP reductions, whose rank tables cover one TP lane, so TP ranks can compute partial norms and use different clipping coefficients. Restrict this branch to groups tagged with the AutoEP expert placement family, or retain the legacy model-parallel reduction for native ETP groups.
AGENTS.md reference: AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
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. |
Signed-off-by: 0z5a <[email protected]>
Incremental review and correctionsReviewed the original PR head Finding addressed: A global folding spec previously routed every MoE optimizer group through the folded EDP+EP norm path. A native MoE group using expert tensor parallelism still needs its model-parallel reduction; replacing that reduction can leave each TP lane with a partial norm and a different clipping coefficient. The correction selects the folded path only for groups whose parameters all carry The new two-rank regression uses actual ZeRO optimizers, collectives, backward and SGD updates. Native ETP gradients with shard norms 3 and 4 must produce norm 5. Adding a folded expert with norm 12 on TP rank one must produce norm 13 on both ranks, followed by the analytically expected clipped parameter update. Both ZeRO-1 and ZeRO-2 are covered, with native-only controls. Corrected in fb5d0e5. Review only this increment. Incremental validation completed on 2026-10-14 against the correction commit:
Run the contributed accelerator regression with: pytest --forked -q tests/unit/v1/moe/test_autoep_autotp_sharded_attention.py \
-k TestMixedExpertClipping
pre-commit run --files deepspeed/runtime/zero/stage_1_and_2.py \
tests/unit/v1/moe/test_autoep_autotp_sharded_attention.py \
docs/code-docs/source/autoep.rstThe mixed-expert regression validates the optimizer boundary with explicit parameter/group metadata. The new TP2/EP2 tiny GLM4-MoE cases validate the full AutoEP/AutoTP model path on the correction commit; they are not a complete mixed native/AutoEP application model or pretrained convergence test. The earlier 8/16-GPU results in the description belong to the original submitted head; they have not all been rerun for this incremental correction. AI assistance was used for this review, correction and validation. |
|
please fix the conflict |
Preserve the clipped-update and native/folded expert regressions while retaining upstream tied-head support, its HiddenParallelEmbedding import and the sequential test marker. Signed-off-by: 0z5a <[email protected]>
done. |
…epspeedai#8829) ### Summary deepspeedai#8670 left optimizer and parameter offload rejected for AutoEP + AutoTP folding ("not validated yet ... run a follow-up proof for per-family replica groups"). This PR provides that proof for ZeRO-3 with the optimizer offloaded to the CPU, and accepts that configuration: ```json { "zero_optimization": {"stage": 3, "offload_optimizer": {"device": "cpu"}}, "tensor_parallel": {"autotp_size": 2}, "expert_parallel": {"enabled": true, "autoep_size": 2} } ``` Everything else stays rejected, now with a message that names the unsupported part: optimizer offload with ZeRO stages 1 and 2, NVMe optimizer offload, parameter offload, and Muon with optimizer offload. An explicit `"device": "none"` is treated as no offload (it was rejected before). ### Why ZeRO-3 + CPU optimizer offload is already correct - ZeRO-3 applies the folded per-family gradient correction (`_apply_autoep_folding_tp_correction`: AVERAGE for TP-replicated parameters, `/tp_size` for routed experts, SKIP for TP-sharded ones) in `reduce_independent_p_g_buckets_and_remove_grads`, before every reduce-scatter. That path is the same with and without offload; offload only changes where the reduced partitions are stored. - The offloaded norm (`complete_grad_norm_calculation_for_cpu_offload`) uses the same folding-aware counting as the device norm (`_grad_norm_counts_every_rank` plus the expert-parallel all-reduce), from per-partition norms recorded at the accumulation boundary. The test below checks the claim end to end instead of relying on this reading. ### Changes - `validate_folding_global` / `validate_autoep_config`: take the offload **devices** instead of "offload configured" booleans (`offload_optimizer_device`, `offload_param_device`); accept stage 3 + `"cpu"`; reject the rest as above. Internal helpers; the two in-repo callers are updated. - `DeepSpeedEngine`: passes the configured devices. - ZeRO-3: rejects Muon with optimizer offload under folding. The offloaded Muon update gathers full gradients on its own path, which this PR does not validate (and which overlaps ongoing Muon offload work). - Docs: the AutoEP folding constraint in `autoep.rst`. ### Tests - `test_autoep_autotp_folding_config.py`: accepted and rejected offload combinations (replaces the blanket-rejection test; the duplicate in `test_autoep_autotp_grad_parity.py` is removed). - `test_autoep_autotp_sharded_attention.py`, new `TestFoldedZero3ClippedUpdate`: reuses deepspeedai#8670's per-gradient comparison against an unfolded ZeRO-0 reference and adds one clipped SGD step. For each of 4 folding shapes (world 4; TP x EP = 2x2, 2x4, 4x2, 4x4), `zero.Init` on/off, and offload on/off, it compares losses, every gradient, the global norm, and every parameter's update. Clipping (clip 1e-2, lr 1/clip) makes the update depend on the norm the optimizer computed, so a wrong offloaded norm fails the test; the comparison allows one fp32 ulp of the parameter for recovering an update as `after - before`, and asserts that a 0.1% error in the update is still detected. #### Results (on-device) Hardware: 1 node, 4x NVIDIA H100 (64 GB, NVLink NV6 between all pairs), driver 595.91.07 (CUDA 13.2), torch 2.12.1+cu130, NCCL 2.29.7, transformers 5.15.1, Python 3.12.1 (MareNostrum 5 ACC). | Command (from `tests/`) | This PR | Unpatched base `1c862f7df` | | --- | --- | --- | | `pytest unit/v1/moe/test_autoep_autotp_sharded_attention.py -k TestFoldedZero3ClippedUpdate` | 16 passed | offload off: 8 passed; offload on: 8 fail with the old "not validated yet" ValueError (the restriction this PR lifts) | | `pytest unit/v1/moe/test_autoep_autotp_sharded_attention.py -k "not TestFoldedZero3ClippedUpdate"` | 18 passed, 15 skipped, 2 failed | 18 passed, 15 skipped, 2 failed | | `pytest unit/v1/moe/test_autoep_autotp_{bf16_folding_parity,checkpoint,deepcompile_reject,dispatch,folding_config,folding_groups,grad_parity,runtime,zero1_overlap_folding}.py` | 81 passed, 9 skipped | 75 passed, 9 skipped | | `pytest unit/v1/moe/test_autoep_autotp_folding_config.py` (CPU) | 33 passed | — | The 2 failures are `TestTiedOutputHead::test_row_parallel_tied_head_is_refused[False/True]` ("AutoTP sharded an output head whose weight is tied to the input embedding"). They fail identically on the unpatched base in the same job, so they are pre-existing and unrelated to this change (likely AutoTP tied-head detection with transformers 5.15.1); not addressed here. The global norms match the reference exactly, for example folded 2.380656 vs reference 2.380656 (TP 2) and 3.552228 vs 3.552228 (TP 4). 8-rank shapes skip on the 4-GPU node. The GPU runs used base `1c862f7df`; the branch is rebased onto `5f266e26d` without conflicts (the 4 commits in between touch DeepEP slicing, XPU op builders, ZeRO-2 partition storage and a test footprint, none of the paths changed here). `pre-commit run --files` on the changed files passes (yapf, flake8, codespell, check-torchdist, check-license, check-torchcuda). ### Compatibility matrix Validated: ZeRO-3 x optimizer offload {none, cpu} x `zero.Init` {off, on} x fp32 x folding TP {2, 4} x EP {2, 4} (world 4), torch SGD on the offloaded CPU fp32 partitions. Not validated (and either rejected or left unchanged): DeepSpeedCPUAdam's offloaded step (the tests run torch SGD on the CPU fp32 partitions with `zero_force_ds_cpu_optimizer: false`; DeepSpeedCPUAdam steps the same partitions but through its own kernel), bf16/fp16 with offload, 8-rank shapes, NVMe offload (rejected), parameter offload (rejected), ZeRO-1/2 optimizer offload (rejected; its offloaded norm path is being changed by deepspeedai#8823), Muon with offload (rejected). ### Not a duplicate No open or merged PR enables offload with AutoEP + AutoTP folding (searched "folding offload", "AutoEP offload", "AutoTP offload"). deepspeedai#8823 fixes ZeRO-1/2 routed-expert clipping under folding and does not touch ZeRO-3 or offload; ZeRO-1/2 offload is left rejected so the two do not interact. Tracked in the Q4 roadmap deepspeedai#8672 (AutoEP + AutoTP folding milestone). Signed-off-by: 0z5a <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]> Co-authored-by: pengdurice <[email protected]>
Signed-off-by: 0z5a <[email protected]> # Conflicts: # tests/unit/v1/moe/test_autoep_autotp_sharded_attention.py
Head branch was pushed to by a user without write access
AutoEP + AutoTP folding gives TP peers different routed experts. ZeRO-1/2 currently filters non-TP expert parameters to TP rank zero when computing the clipping norm, then applies the ordinary expert-norm averaging path. This can omit expert owners and produce an incorrect clipping coefficient even when reconstructed logical gradients are correct.
Compute folded expert norms over the actual expert-DP partition group and EP owner group, without another TP sum or expert-norm average. Only groups tagged with the AutoEP expert placement family use this path. Dense groups and native MoE groups retain their existing reductions and native expert-norm averaging, including when native ETP and AutoEP groups coexist. ZeRO-3 is unchanged and is checked as a sibling reference. The existing folding contract rejects optimizer/parameter offload.
Refs #8672 and #8670. The live duplicate check found no open PR fixing this ZeRO-1/2 folding path. #8476, #8478 and #8691 address FP32/ZeRO-0 clipping in
runtime/utils.py; they do not change the partitioned optimizer reductions fixed here. #8739 implements DeepEP weight-gradient overlap and does not change these norm reductions.Final change
This PR contains three files:
deepspeed/runtime/zero/stage_1_and_2.py: correct the folded expert clipping reductions.tests/unit/v1/moe/test_autoep_autotp_sharded_attention.py: reconstruct full logical gradients and FP32 master parameters, then compare an actual SGD update against an unsharded ZeRO-0 reference across all 45 parameter families. Thresholds of 0.5 and 2.0 times the reference norm cover active clipping and an inactive control. Snapshots are cloned before in-place updates; distributed diagnostics finish before assertions.docs/code-docs/source/autoep.rst: document expert ownership and the reduction contract.The sharded-attention regression supports 4- and 8-rank TP2/EP4 configurations, ZeRO-1/2/3 and ZeRO-3
zero.Init. An additional two-rank analytic SGD regression covers native ETP alone and mixed native ETP/folded AutoEP groups at ZeRO-1/2.Incremental review
Incremental review report and validation · diff from the reviewed head.
The correction preserves native ETP reductions alongside folded AutoEP. On the correction commit, 8 positive cases passed on 2 H100 GPUs, and 2 published-head negative controls failed as expected. ZeRO-1/2 optimizer cases validate native norm 5, mixed norm 13 and the actual clipped-SGD updates; TP2/EP2 tiny GLM4-MoE active/inactive clipping cases validate all 45 parameter families against an unsharded reference. The old mixed cases produce partial norms 12.369317 / 12.649111 instead of 13. These runs used FP32, TF32 disabled, Torch 2.12.1+cu130 / CUDA 13.0, Transformers 5.15.1 and driver 595.91.07. Changed-file pre-commit checks and independent archive/report readback passed; owned workers and locks closed. The report records the exact execution method and CPU diagnostic separately. The 8/16-GPU results below belong to the original submitted head.
Original submitted-head validation
The original submitted three-file diff at
352ee55cd14f77f30e5170939c7e60c4d4bd7754was tested on source based on1c862f7df93f0345b977c67f5408d967b94bf4ae. The extended finite workload used 16 NVIDIA H100 GPUs across four nodes, random tiny GLM4-MoE, FP32 with TF32 disabled, Torch 2.12.1+cu130, CUDA 13.0 and Transformers 5.15.1. The original receipt did not capture the NVML driver package version.zero.Initpassed. An unfixed ZeRO-2 control failed the norm/update oracle.zero.Initpassed.git diff --checkand a clean three-way merge check against current upstream master passed.The extended distributed drivers used the same contributed reference helpers. An additional 8-H100 clipping matrix passed 16 cases and 28,800 raw arrays. All owned GPU workers and contexts closed, the original GPU locks were released, and complete outputs were archived.
Idle-owner checks exclude alignment padding using the frozen permutation formula
ceil((n_tokens + E_local*8)/8)*8; equality toE_local*8proves zero actual received rows. The original finalizer's incorrect padding assertion and failed scheduler status are preserved in the private evidence. The separate corrected reader passed against the same completed workload and unchanged numerical tolerances.These results establish tiny-model correctness, optimizer-state continuity and bounded-run stability. They do not establish pretrained-model convergence, a speedup, BF16/FP16 or Muon behavior, offload, PP/SP, DeepEP overlap, or completion of every Q4 roadmap item.
Reproduce the contributed regression and source checks:
AI assistance was used for implementation, test preparation and incremental review. The human submitter reviewed the original submission, confirmed its test results, and authorized this follow-up correction and DCO sign-off.