Skip to content

Add GPU placement validation before starting rollout engines - #1934

Merged
zhuzilin merged 2 commits into
THUDM:mainfrom
fmh66:fix-rollout-gpu-placement-validation
May 25, 2026
Merged

Add GPU placement validation before starting rollout engines#1934
zhuzilin merged 2 commits into
THUDM:mainfrom
fmh66:fix-rollout-gpu-placement-validation

Conversation

@fmh66

@fmh66 fmh66 commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1896.

This PR adds an explicit GPU placement boundary check in ServerGroup.start_engines() before creating SGLang Ray actors.

Previously, when the placement group GPU slots and SGLang engine configuration were inconsistent, start_engines() could fail with an internal:

IndexError: list index out of range

at reordered_gpu_ids[gpu_index].

With this change, invalid configurations fail earlier with a ValueError that includes the relevant rollout configuration context.

Changes

  • Add a lightweight validation helper for rollout server group GPU indices.
  • Validate gpu_offset + num_engines * num_gpu_per_engine <= len(reordered_gpu_ids) before indexing placement group GPU IDs.
  • Include actionable fields in the error message:
    • worker_type
    • gpu_offset
    • num_gpus_per_engine
    • num_engines
    • required_gpu_slots
    • len(reordered_gpu_ids)
    • rollout_num_gpus
    • rollout_num_gpus_per_engine
  • Add unit coverage for valid configs, empty groups, and invalid configs with clear error context.

Testing

  • python -m pytest tests/test_rollout_validation.py
  • Manual validation of the helper behavior for both valid and invalid configurations.

@zhuzilin
zhuzilin merged commit e713457 into THUDM:main May 25, 2026
26 checks passed
This was referenced May 27, 2026
Merged
Closed
aoshen02 added a commit to vllm-project/vime that referenced this pull request Jun 1, 2026
@aoshen02 @claude
feat: validate rollout server-group GPU placement (slime #1934, #1944)
dd70973 
Port of THUDM/slime#1934 (add GPU placement validation before starting rollout
engines) + #1944 (register its test to CI). 🔧 PORT — slime/→vime/ rewrite.

- vime/ray/rollout_validation.py (new): pure, engine-agnostic
  validate_server_group_gpu_indices() — raises a descriptive ValueError when the
  rollout server group's GPU slots (gpu_offset + num_engines*num_gpu_per_engine)
  exceed the available reordered GPU ids. The error message's arg hints were
  genericized for vime's vLLM rollout (dropped the sglang-specific
  `--sglang-config server_groups` reference).
- vime/ray/rollout.py: call the validator in ServerGroup.start_engines right
  after unpacking the placement group, before creating VLLMEngine actors.
- tests/test_rollout_validation.py (new): pytest unit tests (accept valid /
  allow empty / reports config context) with a __main__ pytest entrypoint.
- .github/workflows/pr-test.yml.j2 (+regenerated pr-test.yml): register
  test_rollout_validation.py in the 0-GPU cpu test matrix.

(black also wrapped one pre-existing long _start_router line in rollout.py,
required for the changed-file format check to pass.)

Refs: THUDM/slime#1934, THUDM/slime#1944, #107

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
@aoshen02 aoshen02 mentioned this pull request Jun 1, 2026
Closed
aoshen02 added a commit to vllm-project/vime that referenced this pull request Jun 5, 2026
@aoshen02 @claude
feat: validate rollout server-group GPU placement (slime #1934, #1944)
4d42658 
Port of THUDM/slime#1934 (add GPU placement validation before starting rollout
engines) + #1944 (register its test to CI). 🔧 PORT — slime/→vime/ rewrite.

- vime/ray/rollout_validation.py (new): pure, engine-agnostic
  validate_server_group_gpu_indices() — raises a descriptive ValueError when the
  rollout server group's GPU slots (gpu_offset + num_engines*num_gpu_per_engine)
  exceed the available reordered GPU ids. The error message's arg hints were
  genericized for vime's vLLM rollout (dropped the sglang-specific
  `--sglang-config server_groups` reference).
- vime/ray/rollout.py: call the validator in ServerGroup.start_engines right
  after unpacking the placement group, before creating VLLMEngine actors.
- tests/test_rollout_validation.py (new): pytest unit tests (accept valid /
  allow empty / reports config context) with a __main__ pytest entrypoint.
- .github/workflows/pr-test.yml.j2 (+regenerated pr-test.yml): register
  test_rollout_validation.py in the 0-GPU cpu test matrix.

(black also wrapped one pre-existing long _start_router line in rollout.py,
required for the changed-file format check to pass.)

Refs: THUDM/slime#1934, THUDM/slime#1944, #107

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
aoshen02 added a commit to vllm-project/vime that referenced this pull request Jun 7, 2026
@aoshen02 @claude
feat: validate rollout server-group GPU placement (slime #1934, #1944)
4d1b04e 
Port of THUDM/slime#1934 (add GPU placement validation before starting rollout
engines) + #1944 (register its test to CI). 🔧 PORT — slime/→vime/ rewrite.

- vime/ray/rollout_validation.py (new): pure, engine-agnostic
  validate_server_group_gpu_indices() — raises a descriptive ValueError when the
  rollout server group's GPU slots (gpu_offset + num_engines*num_gpu_per_engine)
  exceed the available reordered GPU ids. The error message's arg hints were
  genericized for vime's vLLM rollout (dropped the sglang-specific
  `--sglang-config server_groups` reference).
- vime/ray/rollout.py: call the validator in ServerGroup.start_engines right
  after unpacking the placement group, before creating VLLMEngine actors.
- tests/test_rollout_validation.py (new): pytest unit tests (accept valid /
  allow empty / reports config context) with a __main__ pytest entrypoint.
- .github/workflows/pr-test.yml.j2 (+regenerated pr-test.yml): register
  test_rollout_validation.py in the 0-GPU cpu test matrix.

(black also wrapped one pre-existing long _start_router line in rollout.py,
required for the changed-file format check to pass.)

Refs: THUDM/slime#1934, THUDM/slime#1944, #107

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
CalvinXKY pushed a commit to vllm-project/vime that referenced this pull request Jun 7, 2026
@aoshen02 @claude
[Sync][B] models/backends: MiniMax-M2.5 + rollout GPU-placement valid…
32a8523 
…ation (slime #1929/#1992/#1934/#1944) (#143)

* feat: validate rollout server-group GPU placement (slime #1934, #1944)

Port of THUDM/slime#1934 (add GPU placement validation before starting rollout
engines) + #1944 (register its test to CI). 🔧 PORT — slime/→vime/ rewrite.

- vime/ray/rollout_validation.py (new): pure, engine-agnostic
  validate_server_group_gpu_indices() — raises a descriptive ValueError when the
  rollout server group's GPU slots (gpu_offset + num_engines*num_gpu_per_engine)
  exceed the available reordered GPU ids. The error message's arg hints were
  genericized for vime's vLLM rollout (dropped the sglang-specific
  `--sglang-config server_groups` reference).
- vime/ray/rollout.py: call the validator in ServerGroup.start_engines right
  after unpacking the placement group, before creating VLLMEngine actors.
- tests/test_rollout_validation.py (new): pytest unit tests (accept valid /
  allow empty / reports config context) with a __main__ pytest entrypoint.
- .github/workflows/pr-test.yml.j2 (+regenerated pr-test.yml): register
  test_rollout_validation.py in the 0-GPU cpu test matrix.

(black also wrapped one pre-existing long _start_router line in rollout.py,
required for the changed-file format check to pass.)

Refs: THUDM/slime#1934, THUDM/slime#1944, #107

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>

* [Sync][B] MiniMax-M2.5 support (slime #1929, minus #1992-deleted convert scripts)

Port THUDM/slime #1929 MiniMax-M2.5: megatron layer spec
(vime_plugins/models/minimax_m2.py), mbridge online-sync bridge
(vime_plugins/mbridge/minimax_m2.py), megatron->hf converter, and the
minimax-m2 / run-minimax-m2 launchers (rollout path translated sglang->vLLM:
--sglang-* -> --vllm-*, pkill sglang -> pkill vllm serve). The two
convert-minimax-m2-*.sh scripts added by #1929 are intentionally omitted
(deleted by follow-up #1992).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>

* fix(B): restore slime's server_groups hint in GPU-placement error (translate --sglang-config -> --vllm-config)

slime's validate_server_group_gpu_indices error tells the user to align
--rollout-num-gpus, --rollout-num-gpus-per-engine, AND --sglang-config
server_groups. server_groups is a framework (slime/vime) concept that vime
fully carries under --vllm-config (vime/backends/vllm_utils/vllm_config.py:
ServerGroupConfig/VllmConfig). The mega-B port had dropped the third hint for
a generic phrase; restore it as the faithful 1:1 translation --vllm-config
server_groups.

---------

Signed-off-by: aoshen02 <aoshen@inferact.ai>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zhuzilin zhuzilin zhuzilin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] test_qwen2.5_0.5B_ppo_critic_only_short.py fails with IndexError at start_engines after #1866 (multi-role megatron config + stale sglang defaults)

2 participants

@fmh66 @zhuzilin