F12/F33: SGLang base_gpu_id is not 0 in RLix mode — doc/code mismatch (draft, needs direction)#31
Draft
TianyeGGBond wants to merge 1 commit into
Conversation
The placement_provider docstring claimed SGLang base_gpu_id "MUST be 0" via per-worker CVD, but rollout actors use NOSET_VISIBLE_DEVICES and the launch passes the physical GPU id. Describe actual behavior. See PR body for the code-fix alternative (Option B) that needs GPU validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mismatch
The F12 design docstring promises that in RLix / shared-PG mode SGLang runs with
base_gpu_id=0(each rollout worker gets its ownCUDA_VISIBLE_DEVICES, so post-CVD it only seescuda:0..tp-1). The rollout code does not implement this —base_gpu_idis the physical GPU id.Docstring (
miles/ray/placement_provider.py, F33 note, before this PR):Actual code (
miles/ray/rollout.py):env_varsusesNOSET_VISIBLE_DEVICES_ENV_VARS_LIST(all"1") — Ray does not setCUDA_VISIBLE_DEVICESfor the rollout actor. Unlike the train path inactor_group.py, there is no per-worker CVD.base_gpu_id = int(reordered_gpu_ids[gpu_index])is the physical id (e.g.2for inference pool[2, 3]).sglang_engine._to_local_gpu_id(base)returns the id unchanged when CVD is unset, sobase_gpu_idstays physical, not0.Net: for
infer_device_mapping=[2, 3]SGLang launches withbase_gpu_id=2, contradicting "MUST be 0".Options
Option A — fix the docs (this draft). One-hunk change to the
placement_provider.pyF33 note describing actual behavior (base_gpu_id= physical id; rollout actors do not get per-worker CVD). Zero behavior change.Option B — fix the code to honor
base_gpu_id=0. Give each rollout actor a per-workerCUDA_VISIBLE_DEVICES(mirroring the train path) and passbase_gpu_id=0. This is a real GPU-scheduling change and needs validation on multi-GPU hardware (colocate vs non-colocate, tp>1, multi-node); it should not land unverified. Not included here.Ask
Which direction do you want? If A, this draft can be reviewed as-is. If B, I'll open a separate PR once it can be validated on GPUs.