[Feat] Per Stage Runtime Env - #1623
Conversation
Signed-off-by: Ao Tang <aot@nvidia.com>
Greptile SummaryAdds per-stage Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/consistency issues. Previous rounds of review addressed the substantive concerns. The two remaining gaps (RAFT/shuffle paths not propagating runtime_env, and overwrite vs. merge in the Ray Data adapter) are unlikely to affect real users in this iteration since RAFT/shuffle stages don't need pip isolation and mixed use of both mechanisms is not a documented pattern. The test fragility is low-risk given modern packaging versions.
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ProcessingStage\nruntime_env: ClassVar] --> B{Which backend?}
B -->|RayData| C[RayDataStageAdapter.process_dataset]
C --> D[Build ray_remote_args\nfrom ray_stage_spec]
D --> E{stage.runtime_env set?}
E -->|Yes| F[ray_remote_args\n\x5b'runtime_env'\x5d = stage.runtime_env]
E -->|No| G[Pass as-is]
F --> H[dataset.map_batches\n**concurrency_kwargs]
G --> H
B -->|RayActorPool| I[_create_actor_pool]
I --> J{stage.runtime_env set?}
J -->|Yes| K[actor.options\nruntime_env=stage.runtime_env]
J -->|No| L[actor.options\nno runtime_env]
K --> M[ActorPool]
L --> M
B -->|Xenna| N[XennaStageAdapter.env_info]
N --> O{stage.runtime_env set?}
O -->|Yes| P[CuratorRuntimeEnv\nwraps full Ray dict]
O -->|No| Q[return None]
P --> R[to_ray_runtime_env\nmerges extra_env_vars]
R --> S[Ray actor with\nisolated venv]
Reviews (19): Last reviewed commit: "Merge branch 'main' into aot/runtime_env" | Re-trigger Greptile |
Signed-off-by: Ao Tang <aot@nvidia.com>
Signed-off-by: Ao Tang <aot@nvidia.com>
|
/claude review |
Signed-off-by: Ao Tang <aot@nvidia.com>
Signed-off-by: Ao Tang <aot@nvidia.com>
Signed-off-by: Ao Tang <aot@nvidia.com>
Signed-off-by: Ao Tang <aot@nvidia.com>
Signed-off-by: Ao Tang <aot@nvidia.com>
…an up imports in base.py Signed-off-by: Ao Tang <aot@nvidia.com>
…and VersionStage2 classes Signed-off-by: Ao Tang <aot@nvidia.com>
Signed-off-by: Ao Tang <aot@nvidia.com>
|
/ok to test beb11ac |
| def __init__(self, runtime_env: dict[str, Any]) -> None: | ||
| self._runtime_env = runtime_env | ||
| # Xenna's actor pool both reads and writes extra_env_vars on the runtime env object, | ||
| # so this must be a plain settable attribute, not a read-only property. | ||
| self.extra_env_vars: dict[str, str] = dict(runtime_env.get("env_vars", {})) | ||
|
|
||
| def to_ray_runtime_env(self) -> ray.runtime_env.RuntimeEnv: | ||
| return ray.runtime_env.RuntimeEnv(**self._runtime_env) |
There was a problem hiding this comment.
to_ray_runtime_env() silently drops Xenna's extra_env_vars mutations
__init__ copies runtime_env["env_vars"] into self.extra_env_vars as a separate dict. The comment confirms Xenna both reads and writes that attribute at actor startup (e.g., injecting per-actor CUDA device IDs). But to_ray_runtime_env() reconstructs RuntimeEnv from the original self._runtime_env snapshot and never consults self.extra_env_vars, so any env vars Xenna injects are silently discarded before Ray ever receives the runtime environment.
| def __init__(self, runtime_env: dict[str, Any]) -> None: | |
| self._runtime_env = runtime_env | |
| # Xenna's actor pool both reads and writes extra_env_vars on the runtime env object, | |
| # so this must be a plain settable attribute, not a read-only property. | |
| self.extra_env_vars: dict[str, str] = dict(runtime_env.get("env_vars", {})) | |
| def to_ray_runtime_env(self) -> ray.runtime_env.RuntimeEnv: | |
| return ray.runtime_env.RuntimeEnv(**self._runtime_env) | |
| def to_ray_runtime_env(self) -> ray.runtime_env.RuntimeEnv: | |
| merged = {**self._runtime_env, "env_vars": self.extra_env_vars} | |
| return ray.runtime_env.RuntimeEnv(**merged) |
…est setup. Signed-off-by: Ao Tang <aot@nvidia.com>
|
/ok to test 952e77f |
| - name: Run tests ${{ matrix.folder }} (CPU) | ||
| timeout-minutes: 40 | ||
| run: | | ||
| uv venv --seed |
There was a problem hiding this comment.
Hmmm why do we need this here? @thomasdhc shouldn't the ci tests run on the docker image itself?
There was a problem hiding this comment.
I don't think so... unit test are not running on the docker image (without this I got pip not found error)
| stage1 = VersionStage1().with_(runtime_env={spec_type: ["packaging==23.2"]}) | ||
| stage2 = VersionStage2().with_(runtime_env={spec_type: ["packaging==24.0"]}) |
There was a problem hiding this comment.
Why do we need these two stages too, and not just do BaseEnvStage().with_(...)?
There was a problem hiding this comment.
So I want to have both stages need to write to different columns (stage1_packaging_version vs stage2_packaging_version) to preserve both versions in the final result for comparison.
Unless we drop the "both versions in one result" design and run two separate pipelines instead:
stage = BaseEnvStage() # one reusable class
result1 = Pipeline(..., stages=[stage.with_(runtime_env={spec_type: ["packaging==23.2"]})]).run(...)
result2 = Pipeline(..., stages=[stage.with_(runtime_env={spec_type: ["packaging==24.0"]})]).run(...)
assert result1[0].to_pandas()["base_packaging_version"].iloc[0] == "23.2"
assert result2[0].to_pandas()["base_packaging_version"].iloc[0] == "24.0"
Is this what you mean ?
|
/ok to test a882ad4 |
Signed-off-by: Ao Tang <aot@nvidia.com>
|
/ok to test 6792826 |
praateekmahajan
left a comment
There was a problem hiding this comment.
🙏 thank you for your work here
|
/ok to test 00dd3b9 |
Overview
Enables per-stage runtime environments in NeMo Curator pipelines. Each stage can declare a distinct set of Python packages via a `runtime_env` class variable, and Ray will create an isolated virtualenv for that stage's workers — allowing incompatible library versions to coexist in the same pipeline.
Changes
Stage API
Backend support (all three backends)
runtime_envforwarded viaray_remote_argstomap_batchesCuratorRuntimeEnvduck-type bridges Xenna'senv_infoto Ray's fullruntime_envdictruntime_envpassed to actor.options()at pool creationInfrastructure
docker/Dockerfile:uv venv --seedso pip is present in the container venv and Ray's cloned worker venvs inherit itcicd-main.yml:uv venv --seedbeforeuv syncso pip is available in CI's uv-managed.venv.rayignore: excludesuv.lockto prevent uv from enforcing locked transitive dep versions that would override the stage's declared versionsHow it works
Ray's native
runtime_envcreates an isolated virtualenv per unique spec set under/tmp/ray/session_latest/runtime_resources/pip/<hash>/virtualenv, cached for the lifetime of the Ray session. No driver-side venv creation orPYTHONPATHmanipulation is needed.Tests
tests/pipelines/test_per_stage_runtime_env.py— parametrized over RayData (uv) and Xenna streaming (pip), runs a 3-stage pipeline with:packaging==23.2stagepackaging==24.0stageVerifies each stage sees its declared version and that base-env packages (loguru) remain importable in isolated envs.