Skip to content

fix(plugin): build SGLang DP-attention ParallelConfig per rank - #2785

Merged
zhuyuhua-v merged 1 commit into
mainfrom
junyyang/sglang-plugin-dp-rank-config
Oct 30, 2026
Merged

zhuyuhua-v merged 1 commit into
mainfrom
junyyang/sglang-plugin-dp-rank-config

Conversation

@junyyang-amd

Copy link
Copy Markdown
Contributor

Every SGLang+ATOM case that uses dp-attention fails at server start. Each scheduler process except DP rank 0 raises while the plugin generates its ATOM config:

File "/workspace/atom/plugin/config.py", line 616, in _generate_atom_config_from_sglang_config
  sgl_parallel_config = ParallelConfig(
File "/workspace/atom/config.py", line 1139, in __post_init__
ValueError: data_parallel_rank (3) + data_parallel_size_local (8) must not exceed data_parallel_size (8): this node's slice would run off the end of the global DP group

Rank 0 then reports Rank 0 scheduler died during initialization (exit code: -3).

Observed in the Oct 27 nightlies on MI355, for every DP case that got past model download:

Cause:

  • Under dp-attention SGLang starts one scheduler process per GPU. _normalize_sglang_parallel_config maps that to atom_dp_size = tp_size and atom_dp_rank = tp_rank.
  • The plugin passed the process rank as data_parallel_rank and the global size as data_parallel_size_local.
  • Since multi-node dp support #1603, ParallelConfig.__post_init__ reads data_parallel_rank as the first global rank of this node's slice and rejects rank + size_local > size. Native ATOM builds the config with the node's start rank and assigns each process's rank afterwards (EngineCoreManager, engine_core_mgr.py), so it is unaffected. The plugin path was not updated, and the existing plugin tests replace ParallelConfig with a fake, so they did not catch it.

Change:

  • Add _build_sglang_parallel_config(dp_size, dp_rank) in atom/plugin/config.py. It builds the config for the single-node slice starting at rank 0, then assigns the process's data_parallel_rank and data_parallel_rank_local, mirroring EngineCoreManager. data_parallel_size_local stays at the full DP size, which is what moe.py passes to MoRI as local_ep_size.
  • Rank 0 gets the same config as before. Without dp-attention the plugin still builds dp_size=1, rank=0, which is unchanged.
  • Add tests/test_plugin_sglang_parallel_config.py. It runs the real _normalize_sglang_parallel_config and the real ParallelConfig for every rank of TP4 and TP8 dp-attention, plus the non-dp-attention cases. It is under tests/ rather than tests/plugin/ because run_unit_tests.sh skips tests/plugin/.

Not handled: multi-node SGLang (nnodes > 1) with dp-attention; the plugin did not support it before this change either.

Validation:

  • The new test passes. Building the config the old way raises the error above for ranks 1 and 7 and succeeds for rank 0.
  • bash .github/scripts/run_unit_tests.sh on CPU torch: 10133 passed, 1029 skipped, 0 failed.
  • ruff check and black --check pass on the changed files.
  • The GPU DP jobs have not been rerun with this change.

Under dp-attention SGLang starts one scheduler process per GPU, and the
plugin built each process's ATOM ParallelConfig with its own global DP
rank as data_parallel_rank and the global DP size as
data_parallel_size_local. Since multi-node DP support (#1603),
ParallelConfig's constructor reads data_parallel_rank as the start of the
node's slice and rejects rank + size_local > size, so every rank but 0
failed at server start:

  ValueError: data_parallel_rank (3) + data_parallel_size_local (8) must
  not exceed data_parallel_size (8): this node's slice would run off the
  end of the global DP group

Build the config for the single-node slice starting at rank 0 and assign
the process rank afterwards, as EngineCoreManager does for native ATOM.

The regression test lives under tests/ rather than tests/plugin/, which
the CPU unit-test job skips, and uses the real ParallelConfig.
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every eligible PR before approval:

  • ✅ Pre Checkin: Black, Ruff, catalog schema validation, non-GPU unit tests

Heavy model tests:

  • ✅ Run after the PR is approved and Pre Checkin passes
  • ✅ Run immediately when an approval review is submitted
  • ✅ Can be requested before approval with labels
Label Tests
ci:full Run all heavy PR model tests: native ATOM, vLLM, and SGLang
ci:atom Run native ATOM model accuracy tests
ci:vllm Run ATOM vLLM OOT model accuracy tests
ci:sglang Run ATOM SGLang model accuracy tests

Heavy jobs are skipped when the PR is not approved and no matching ci:* label is present.
Add labels via the sidebar or gh pr edit 2785 --add-label <label>

@junyyang-amd

Copy link
Copy Markdown
Contributor Author

GPU benchmark reruns on MI355 passed for both DP-attention sizes:

Case Run Successful requests Output throughput Mean TTFT Mean TPOT
DeepSeek-R1 FP4 TP4 DP4 EP4 40827702696 640 2139.82 tok/s 393.89 ms 28.64 ms
DeepSeek-R1 FP4 TP8 DP8 EP8 40827705190 640 2688.84 tok/s 282.90 ms 22.76 ms
  • Both servers start on every DP rank. The ValueError: data_parallel_rank (...) + data_parallel_size_local (...) must not exceed data_parallel_size (...) from the Oct 27 nightlies does not appear.
  • Parameters: 1024/1024, c=64, random range ratio 0.8, one set per case. The DeepSeek-V3.2 DP cases were not run because they depend on fix(ci): use FP32 block scales for DeepSeek-V3.2 SGLang cases #2783.
  • Tested commit: e7bf89368 on temporary branches, which is this PR's head 0bd5aa873 plus one CI-only commit that lets the model download step fall back to an already complete cached model when hf download fails. The shared model cache was hitting a write quota at the time. That fallback did not trigger in either run, so the results reflect this PR's code.

@zhuyuhua-v
zhuyuhua-v merged commit 124c3b0 into main Oct 30, 2026
83 of 95 checks passed
@zhuyuhua-v
zhuyuhua-v deleted the junyyang/sglang-plugin-dp-rank-config branch October 30, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants