-
Notifications
You must be signed in to change notification settings - Fork 320
[Feat] Per Stage Runtime Env #1623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
5271c7c
Add support for per-stage pip specifications and virtual environments
suiyoubi 9d1fd4e
add test
suiyoubi b3624fc
ruff
suiyoubi e2ae9a2
comments resolved
suiyoubi b25df9f
comments resolve
suiyoubi eaaf1d4
Merge branch 'main' into aot/runtime_env
suiyoubi e243755
comments resolve
suiyoubi 98470ca
normalize pip specifications before creating virtual environments.
suiyoubi b7b2351
Merge branch 'main' of github.com:NVIDIA-NeMo/Curator into aot/runtim…
suiyoubi fc8f510
Add Path import and update ProcessingStage class
suiyoubi 2666b24
ruff check
suiyoubi f4ef209
ruff
suiyoubi e971625
ruff check
suiyoubi fab1b90
Add runtime environment conflict check in RayDataStageAdapter and cle…
suiyoubi e89badb
fix
suiyoubi 8062427
Refactor pip_specs to use ClassVar for type hinting in VersionStage1 …
suiyoubi a60e271
Merge branch 'main' into aot/runtime_env
ayushdg 297b8e4
refactor to use runtime_env
suiyoubi d88d678
add more tests
suiyoubi dcf7143
remove ensurepip and fix it with --seed in dockerfile
suiyoubi 7164dd0
add extra_env_vars
suiyoubi beb11ac
Refactor extra_env_vars to be a settable attribute in CuratorRuntimeEnv
suiyoubi 952e77f
Update CI workflow to include 'uv venv --seed' command for improved t…
suiyoubi a882ad4
limit to two test case
suiyoubi 6792826
use _with for unit test
suiyoubi 00dd3b9
Merge branch 'main' into aot/runtime_env
suiyoubi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| # Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved. | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| """Tests for per-stage runtime_env support. | ||
|
|
||
| Verifies that stages can declare different runtime_env (pip/uv packages) and | ||
| that Ray's native runtime_env creates isolated venvs per actor on each node. | ||
| Also verifies that runtime_env is additive: base-env packages remain importable. | ||
| """ | ||
|
|
||
|
suiyoubi marked this conversation as resolved.
|
||
| from typing import Any | ||
|
|
||
| import pandas as pd | ||
| import pytest | ||
|
|
||
| from nemo_curator.backends.base import BaseExecutor | ||
| from nemo_curator.backends.ray_data import RayDataExecutor | ||
| from nemo_curator.backends.xenna import XennaExecutor | ||
| from nemo_curator.pipeline.pipeline import Pipeline | ||
| from nemo_curator.stages.base import ProcessingStage | ||
| from nemo_curator.stages.resources import Resources | ||
| from nemo_curator.tasks import DocumentBatch | ||
|
|
||
|
|
||
| class RecordPackagingVersionStage(ProcessingStage[DocumentBatch, DocumentBatch]): | ||
| """Records the packaging library version visible to this worker. | ||
|
|
||
| The column name is derived from self.name so multiple instances with | ||
| different runtime_env can coexist in the same pipeline. | ||
| Also checks whether loguru (a Curator dep, not a Ray dep) is importable. | ||
| """ | ||
|
|
||
| name = "record_packaging_version" | ||
| resources = Resources(cpus=0.5) | ||
| batch_size = 1 | ||
|
|
||
| def inputs(self) -> tuple[list[str], list[str]]: | ||
| return ["data"], [] | ||
|
|
||
|
suiyoubi marked this conversation as resolved.
|
||
| def outputs(self) -> tuple[list[str], list[str]]: | ||
| return ["data"], [] | ||
|
|
||
| def process(self, task: DocumentBatch) -> DocumentBatch: | ||
| import packaging | ||
|
|
||
| try: | ||
| from loguru import logger | ||
|
|
||
| loguru_available = logger is not None | ||
| except ImportError: | ||
| loguru_available = False | ||
|
|
||
| batch = task.to_pandas().copy() | ||
| batch[f"{self.name}_version"] = packaging.__version__ | ||
| batch[f"{self.name}_loguru_available"] = loguru_available | ||
| return DocumentBatch( | ||
| task_id=task.task_id, | ||
| dataset_name=task.dataset_name, | ||
| data=batch, | ||
| _metadata=task._metadata, | ||
| _stage_perf=task._stage_perf, | ||
| ) | ||
|
|
||
|
|
||
| def _make_initial_task() -> DocumentBatch: | ||
| return DocumentBatch( | ||
| task_id="runtime_env_test", | ||
| dataset_name="test", | ||
| data=pd.DataFrame({"text": ["hello"]}), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "backend_config", | ||
| [ | ||
| pytest.param((RayDataExecutor, {}), id="ray_data"), | ||
| pytest.param((XennaExecutor, {"execution_mode": "streaming"}), id="xenna_streaming"), | ||
| ], | ||
| indirect=True, | ||
| ) | ||
| class TestPerStageRuntimeEnv: | ||
| """Stages with different runtime_env see different package versions.""" | ||
|
|
||
| backend_cls: type[BaseExecutor] | None = None | ||
| config: dict[str, Any] | None = None | ||
| results: list[DocumentBatch] | None = None | ||
|
|
||
| @pytest.fixture(scope="class", autouse=True) | ||
| def backend_config(self, request: pytest.FixtureRequest, shared_ray_cluster: str): | ||
| """Execute a 3-stage pipeline: base env, packaging==23.2 (pip), packaging==24.0 (uv).""" | ||
| backend_cls, config = request.param | ||
| request.cls.backend_cls = backend_cls | ||
| request.cls.config = config | ||
|
|
||
| base_stage = RecordPackagingVersionStage().with_(name="base_env") | ||
| stage_v232 = RecordPackagingVersionStage().with_( | ||
| name="pinned_v232", | ||
| runtime_env={"pip": ["packaging==23.2"]}, | ||
| ) | ||
| stage_v240 = RecordPackagingVersionStage().with_( | ||
| name="pinned_v240", | ||
| runtime_env={"uv": ["packaging==24.0"]}, | ||
| ) | ||
|
|
||
| pipeline = Pipeline(name="runtime_env_test", stages=[base_stage, stage_v232, stage_v240]) | ||
| request.cls.results = pipeline.run(backend_cls(config), initial_tasks=[_make_initial_task()]) | ||
|
|
||
| def test_output_count(self): | ||
| assert self.results is not None | ||
| assert len(self.results) == 1 | ||
|
|
||
| def test_base_env_uses_installed_version(self): | ||
| """Stage with no runtime_env should see the base environment's packaging version.""" | ||
| df = self.results[0].to_pandas() | ||
| assert "base_env_version" in df.columns | ||
| assert df["base_env_version"].iloc[0] # non-empty | ||
|
|
||
| def test_pinned_v232(self): | ||
| df = self.results[0].to_pandas() | ||
| assert df["pinned_v232_version"].iloc[0] == "23.2" | ||
|
|
||
| def test_pinned_v240(self): | ||
| df = self.results[0].to_pandas() | ||
| assert df["pinned_v240_version"].iloc[0] == "24.0" | ||
|
|
||
| def test_all_three_versions_differ(self): | ||
| """Base env, 23.2, and 24.0 should all be distinct.""" | ||
| df = self.results[0].to_pandas() | ||
| versions = { | ||
| df["base_env_version"].iloc[0], | ||
| df["pinned_v232_version"].iloc[0], | ||
| df["pinned_v240_version"].iloc[0], | ||
| } | ||
| assert len(versions) == 3, f"Expected 3 distinct versions, got {versions}" | ||
|
|
||
| def test_runtime_env_is_additive(self): | ||
| """Stages with runtime_env can still import base-env packages (loguru is a Curator dep, not Ray).""" | ||
| df = self.results[0].to_pandas() | ||
| assert bool(df["pinned_v232_loguru_available"].iloc[0]) | ||
| assert bool(df["pinned_v240_loguru_available"].iloc[0]) | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so... unit test are not running on the docker image (without this I got
pip not founderror)