fix(sc): give vLLM workers the nemo_gym extra by default instead of swapping the env at runtime - #4043
Merged
cspades merged 1 commit intoSep 7, 2026
Conversation
…wapping the env at runtime Token capture swapped the VllmAsyncGenerationWorker registry entry to VLLM_GYM at setup time. Worker venvs are cached by actor class name, so a venv prebuilt with plain --extra vllm (which is what the Dockerfile bakes) was reused as-is and the nemo_gym import failed. This is why the SingleController L1 job fails even on a freshly built container. Make VLLM_EXECUTABLE use VLLM_GYM so the baked venv already has nemo_gym, and drop the runtime registry override and the error wrapper that described the old behavior. Signed-off-by: Terry Kong <terryk@nvidia.com>
Collaborator
Author
|
/ok to test 3e547fa |
cspades
merged commit Sep 7, 2026
3e547fa
into
cye/rl_mllm_omni_mm_nocolgym_rebase_pr2957
31 of 33 checks passed
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
2 participants
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.
What does this PR do?
Targets the head branch of #4009. Fixes the
L1_Functional_Tests_SingleControllerfailure seen in https://github.com/NVIDIA-NeMo/RL/actions/runs/34093342031/job/101689581550:Root cause
Token capture (#3837) swapped the
VllmAsyncGenerationWorkerregistry entry toPY_EXECUTABLES.VLLM_GYMat setup time. But worker venvs are cached by actor class name only, and the venv builder returns early as soon asbin/pythonexists. The Dockerfile bakes that venv with plain--extra vllm, so the runtime swap never takes effect: the baked venv is reused as-is and thenemo_gymimport fails.That is why this fails on a freshly built container. Rebuilding the image reproduces the bug rather than fixing it. It was not caught before because no CI run of #3837, and no main run since it merged, ever got past L0 to run this job.
Fix
VLLM_EXECUTABLEnow usesPY_EXECUTABLES.VLLM_GYM(vllm + nemo_gym extras), so the venv the Dockerfile prefetches already hasnemo_gym.ACTOR_ENVIRONMENT_REGISTRYoverride insetup_single_controller. We should not change a worker's environment after the venv cache has been built.try/exceptthat rewrote thenemo_gymimport error. It described the old behavior and is no longer reachable.The venv fingerprint commit already on this branch (32fbaef) would also have hidden this symptom, because the digest includes the uv command string. It is left alone here, but it is not needed for this bug.
Testing
ruff checkandruff format --checkpass on the edited files.L1_Functional_Tests_SingleController, which is where the token-capture test lives.