Skip to content

fix(optimizer): route ETP-only experts through expert groups - #84

Merged
yushengsu-thu merged 1 commit into
fix-layerwisefrom
jiajun/fix-layerwise-expert-tagging
Aug 19, 2026
Merged

fix(optimizer): route ETP-only experts through expert groups#84
yushengsu-thu merged 1 commit into
fix-layerwisefrom
jiajun/fix-layerwise-expert-tagging

Conversation

@guapisolo

Copy link
Copy Markdown

Depends on: #82
Stacked on the parent PR's branch. Review / merge the parent first.

Summary

Route ETP-only expert parameters through expert reduction and LayerWise ownership.

Symptom & Reproduction

  • Symptom: At parent head 3d107754c, TP2/ETP1/EP1 expert parameters carry allreduce=True, so DDP puts them in dense buffers and LayerWise uses the dense ownership plane.
  • Reproduction: Run NVIDIA_PYTORCH_VERSION=25.06 torchrun --standalone --nproc-per-node=2 -m pytest -q tests/unit_tests/test_muon_optimizer.py::test_real_moe_ddp_layerwise_muon_expert_ownership_tp2_etp1_ep1; the parent behavior violates the expert-tag and expert-buffer assertions, while this commit passes them.

Root Cause

  1. ColumnParallelLinear derived param.allreduce from EP > 1 only.
  2. DistributedDataParallel selected dense buffers for those mislabeled parameters.
  3. _get_param_groups propagated dense ownership into LayerWiseDistributedOptimizer.

Fix

Port the producer predicate is_expert and (EP > 1 or ETP != TP) from NVIDIA/Megatron-LM#5916 at head 6ccd20e2005b0698e908d42db771e2f22b0281e8 across native, Transformer Engine, grouped, legacy, and ModelOpt linear layers. The TE helper also restores numbered-parameter TP metadata and preserves SwiGLU partition stride; DDP remains unchanged because it already honors param.allreduce.

Verification

  • pytest -q tests/unit_tests/transformer/moe/test_grouped_mlp.py -k 'expert_parameter_attributes' tests/unit_tests/post_training/test_modelopt_module_spec.py -k 'expert_parameter_attributes or modelopt_expert_linear': 9 passed.
  • The 2-GPU native, TE CPU-init, and ETP-only DDP collective set: 3 passed on each rank; the optional legacy GroupedMLP runtime case skipped because grouped_gemm is unavailable.
  • The 2-GPU TE stride=2 plus real MoE end-to-end set: 2 passed on each rank.
  • The 8-GPU real MoE → DDP → Muon/Adam → LayerWise test: all eight ranks passed exact buffer, owner, norm, clipping, update, and EDP replica assertions.
  • git diff --check, isort --check-only, and python -m compileall -q: passed for the changed files.

Review Focus

  • _set_expert_parameter_attributes: numbered weight/bias matching, row-bias replication, and partition_stride preservation.
  • use_expert_pgs: identical topology semantics across every expert parameter producer.
  • test_real_moe_ddp_layerwise_muon_expert_ownership_tp2_etp1_ep1: deterministic gradients still exercise real MoE construction, DDP collectives, optimizer grouping, ownership, clipping, and updates.

Tag native, Transformer Engine, grouped, and ModelOpt expert parameters for expert data-parallel reduction whenever EP > 1 or ETP differs from TP. Preserve TE tensor-parallel metadata, including SwiGLU partition stride, and add direct plus real MoE-to-DDP-to-LayerWise Muon regression coverage.
@guapisolo

Copy link
Copy Markdown
Author

@codex review

@yushengsu-thu
yushengsu-thu merged commit c479401 into fix-layerwise Aug 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants