Repository navigation
[Core] Add initial foundational configurations and flags for fork server #67356
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -277,6 +277,16 @@ def set_num_threads_env_vars_if_unset() -> List[str]: | |
| num_threads = str(max(math.floor(num_assigned_cpus), 1)) | ||
| for name in unset_env_vars: | ||
| os.environ[name] = num_threads | ||
| if "OMP_NUM_THREADS" in unset_env_vars: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the design doc, if I'm understanding the diagram correctly, the forkserver with the specified modules is started before the env vars is configured. If the user specifies
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it won't be ignored. This current change is only here to handle the case where the user doesn't set Followup changes will apply |
||
| # If modules like torch were pre-imported before fork(), their internal thread | ||
| # pool is already initialized and ignores subsequent os.environ["OMP_NUM_THREADS"] | ||
| # updates. | ||
| for module_name in ray_constants.THREAD_POOL_MODULES: | ||
| if module_name in sys.modules: | ||
| try: | ||
| sys.modules[module_name].set_num_threads(int(num_threads)) | ||
| except Exception: | ||
| pass | ||
| return unset_env_vars | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import os | ||
| import sys | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest | ||
|
|
||
| import ray | ||
| from ray._private.utils import set_num_threads_env_vars_if_unset | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "num_threads_env_vars,preset_omp,expected_ret,expected_torch_threads", | ||
| [ | ||
| (None, None, ["OMP_NUM_THREADS"], 4), | ||
| ("OMP_NUM_THREADS,RAYON_NUM_THREADS", "2", ["RAYON_NUM_THREADS"], None), | ||
| ], | ||
| ) | ||
| def test_set_num_threads_env_vars_if_unset_updates_thread_pool_modules( | ||
| monkeypatch, | ||
| num_threads_env_vars, | ||
| preset_omp, | ||
| expected_ret, | ||
| expected_torch_threads, | ||
| ): | ||
| """Test that set_num_threads_env_vars_if_unset calls .set_num_threads() on loaded | ||
| THREAD_POOL_MODULES only when OMP_NUM_THREADS is set by Ray. | ||
| """ | ||
| if num_threads_env_vars is None: | ||
| monkeypatch.delenv("RAY_NUM_THREADS_ENV_VARS", raising=False) | ||
| else: | ||
| monkeypatch.setenv("RAY_NUM_THREADS_ENV_VARS", num_threads_env_vars) | ||
| if preset_omp is None: | ||
| monkeypatch.delenv("OMP_NUM_THREADS", raising=False) | ||
| else: | ||
| monkeypatch.setenv("OMP_NUM_THREADS", preset_omp) | ||
| monkeypatch.delenv("RAYON_NUM_THREADS", raising=False) | ||
|
|
||
| mock_ctx = MagicMock() | ||
| mock_ctx.worker.mode = ray._private.worker.WORKER_MODE | ||
| mock_ctx.get_assigned_resources.return_value = {"CPU": 4} | ||
| mock_torch = MagicMock() | ||
| with patch("ray.get_runtime_context", return_value=mock_ctx), patch.dict( | ||
| sys.modules, {"torch": mock_torch} | ||
| ): | ||
| assert set_num_threads_env_vars_if_unset() == expected_ret | ||
| if expected_torch_threads is not None: | ||
| assert os.environ["OMP_NUM_THREADS"] == str(expected_torch_threads) | ||
| mock_torch.set_num_threads.assert_called_once_with(expected_torch_threads) | ||
| else: | ||
| assert os.environ["OMP_NUM_THREADS"] == preset_omp | ||
| mock_torch.set_num_threads.assert_not_called() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(pytest.main(["-sv", __file__])) |
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.
If the validated module doesn't actually exist, what's our plan for surfacing the import error to the user?
Uh oh!
There was an error while loading. Please reload this page.
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.
This is actually being taken care of in the follow-up PR (#67514 but WIP), it will catch and log preload import errors in the fork server logs without failing server startup. If the task or actor imports that missing module at runtime, Python will raise
ModuleNotFoundErrorto the user. Please let me know if you have thoughts on this!