From 23d5021dd12551f857f7fdcfec2b1f47b7be2901 Mon Sep 17 00:00:00 2001 From: aoshen02 Date: Wed, 27 May 2026 01:29:31 +0000 Subject: [PATCH] fix: drop incorrect critic GPU add to rollout_num_gpus in colocate mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In colocate mode `slime/ray/placement_group.py:_create_placement_group` allocates only `actor_num_gpus_per_node * actor_num_nodes` bundles in the placement group (colocate branch). The critic shares the actor's PG (`result["critic"] = result["actor"]`) and so contributes no additional slots — actor, critic, and rollout time-share the same physical GPUs via offload/onload. The two lines being removed add `critic_num_gpus_per_node * critic_num_nodes` onto `args.rollout_num_gpus`, which over-allocates the regular `ServerGroupConfig.num_gpus` past the PG size. `start_engines` (`slime/ray/rollout.py`) then iterates `len(all_engines) = rollout_num_gpus // gpus_per_engine` and indexes `reordered_gpu_ids[gpu_index]` beyond its length, raising: IndexError: list index out of range at base_gpu_id = int(reordered_gpu_ids[gpu_index]) in ServerGroup.start_engines (slime/ray/rollout.py) Reproducer: `tests/test_qwen2.5_0.5B_ppo_critic_only_short.py` (`--advantage-estimator ppo` + `--colocate` + `--rollout-num-gpus 2` against actor 4 GPU). With the addition still in place, `rollout_num_gpus` is overridden to `4 + 4 = 8` but the PG only has 4 bundles; without it the rollout sizes correctly to the 4 colocated GPUs. This is the root cause of #1896 — PR #1934 added a defensive boundary check that surfaces the mismatch as a clearer `ValueError`, but the underlying resource math is still wrong. This patch removes the source of the mismatch instead of just catching its symptom. The blamed `if args.use_critic: args.rollout_num_gpus += ...` lines were added in 371c030e ([Fix] ppo rollout engins for distribute, #394), which only touched `arguments.py` and did not extend `placement_group.py` to match. The Miles RL framework (a downstream fork) does carry the matching `placement_group.py` change (critic gets its own bundles in colocate mode), where the addition would make sense; slime does not, so this code path was inconsistent in slime since #394. Signed-off-by: aoshen02 --- slime/utils/arguments.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/slime/utils/arguments.py b/slime/utils/arguments.py index 9b73a78221..4690a12723 100644 --- a/slime/utils/arguments.py +++ b/slime/utils/arguments.py @@ -1804,8 +1804,6 @@ def slime_validate_args(args): f"* actor_num_nodes {args.actor_num_nodes}, overriding rollout_num_gpus to match actor_num_gpus_per_node * actor_num_nodes." ) args.rollout_num_gpus = args.actor_num_gpus_per_node * args.actor_num_nodes - if args.use_critic: - args.rollout_num_gpus += args.critic_num_gpus_per_node * args.critic_num_nodes if args.offload_train is None: args.offload_train = False