Replace AMD device env var with HIP_VISIBLE_DEVICES by vickytsang · Pull Request #51104 · ray-project/ray

vickytsang · 2025-03-05T22:09:03Z

Why are these changes needed?

Replace AMD device env var with HIP_VISIBLE_DEVICES to align with PYTORCH semantics

HIP_VISIBLE_DEVICES has the same semantics as CUDA_VISIBLE_DEVICES in PYTORCH. Using ROCR_VISIBLE_DEVICES with can cause unexpected behavior

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: root <vtsang@amd.com>

kevin85421

The changes for core looks good to me. cc @hongpeng-guo would you mind reviewing the changes of Ray Train?

hongpeng-guo

The code looks good in general! Could you please show a screen shot or log of the tests on AMD machines? Our CI env doesn't include AMD test for the moment.

Signed-off-by: root <vtsang@amd.com>

vickytsang · 2025-03-07T19:04:26Z

envvar_chk.log
pytest.log

hongpeng-guo · 2025-03-09T19:52:46Z

copy pasting the previous unit test results here for better visibility:

(py_3.9) root@hpe-hq-08:/app/ray# HIP_VISIBLE_DEVICES=0,1,2,3,4,5,6,7 CUDA_VISIBLE_DEVICES=0,1 python -m pytest -v -s python/ray/tests/accelerators/test_amd_gpu.py
Test session starts (platform: linux, Python 3.9.19, pytest 7.4.4, pytest-sugar 0.9.5)
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/app/ray/.hypothesis/examples')
rootdir: /app/ray
configfile: pytest.ini
plugins: hypothesis-5.35.1, flakefinder-1.1.0, xdist-3.3.1, cpp-2.3.0, xdoctest-1.1.0, lazy-fixture-0.6.3, httpserver-1.0.6, shutil-1.7.0, aiohttp-0.3.0, timeout-2.1.0, anyio-3.7.1, sugar-0.9.5, rerunfailures-11.1.2, sphinx-0.5.1.dev0, virtualenv-1.7.0, docker-tools-3.1.3, asyncio-0.17.0, forked-1.4.0
timeout: 180.0s
timeout method: signal
timeout func_only: False
collecting ... Usage stats collection is enabled by default for nightly wheels. To disable this, run the following command: `ray disable-usage-stats` before starting Ray. See https://docs.ray.io/en/master/cluster/usage-stats.html for more details.
2025-03-07 17:03:57,465 INFO worker.py:1842 -- Started a local Ray instance. View the dashboard at 127.0.0.1:8265

 python/ray/tests/accelerators/test_amd_gpu.py::test_visible_amd_gpu_ids ✓                                                            20% ██        Usage stats collection is enabled by default for nightly wheels. To disable this, run the following command: `ray disable-usage-stats` before starting Ray. See https://docs.ray.io/en/master/cluster/usage-stats.html for more details.
2025-03-07 17:04:02,442 INFO worker.py:1842 -- Started a local Ray instance. View the dashboard at 127.0.0.1:8265

 python/ray/tests/accelerators/test_amd_gpu.py::test_visible_amd_gpu_type ✓                                                           40% ████      Usage stats collection is enabled by default for nightly wheels. To disable this, run the following command: `ray disable-usage-stats` before starting Ray. See https://docs.ray.io/en/master/cluster/usage-stats.html for more details.
2025-03-07 17:04:07,082 INFO worker.py:1842 -- Started a local Ray instance. View the dashboard at 127.0.0.1:8265

 python/ray/tests/accelerators/test_amd_gpu.py::test_visible_amd_gpu_type_bad_device_id ✓                                             60% ██████
 python/ray/tests/accelerators/test_amd_gpu.py::test_get_current_process_visible_accelerator_ids ✓                                    80% ████████
 python/ray/tests/accelerators/test_amd_gpu.py::test_set_current_process_visible_accelerator_ids ✓                                   100% ██████████

Results (14.39s):
       5 passed

cc @vickytsang Could you help me understand why both env vars HIP_VISIBLE_DEVICES and CUDA_VISIBLE_DEVICES are needed, and how are these two variables affect the visible amd devices for each individual GPU?

vickytsang · 2025-03-10T18:56:59Z

copy pasting the previous unit test results here for better visibility:

(py_3.9) root@hpe-hq-08:/app/ray# HIP_VISIBLE_DEVICES=0,1,2,3,4,5,6,7 CUDA_VISIBLE_DEVICES=0,1 python -m pytest -v -s python/ray/tests/accelerators/test_amd_gpu.py
Test session starts (platform: linux, Python 3.9.19, pytest 7.4.4, pytest-sugar 0.9.5)
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('/app/ray/.hypothesis/examples')
rootdir: /app/ray
configfile: pytest.ini
plugins: hypothesis-5.35.1, flakefinder-1.1.0, xdist-3.3.1, cpp-2.3.0, xdoctest-1.1.0, lazy-fixture-0.6.3, httpserver-1.0.6, shutil-1.7.0, aiohttp-0.3.0, timeout-2.1.0, anyio-3.7.1, sugar-0.9.5, rerunfailures-11.1.2, sphinx-0.5.1.dev0, virtualenv-1.7.0, docker-tools-3.1.3, asyncio-0.17.0, forked-1.4.0
timeout: 180.0s
timeout method: signal
timeout func_only: False
collecting ... Usage stats collection is enabled by default for nightly wheels. To disable this, run the following command: `ray disable-usage-stats` before starting Ray. See https://docs.ray.io/en/master/cluster/usage-stats.html for more details.
2025-03-07 17:03:57,465 INFO worker.py:1842 -- Started a local Ray instance. View the dashboard at 127.0.0.1:8265

 python/ray/tests/accelerators/test_amd_gpu.py::test_visible_amd_gpu_ids ✓                                                            20% ██        Usage stats collection is enabled by default for nightly wheels. To disable this, run the following command: `ray disable-usage-stats` before starting Ray. See https://docs.ray.io/en/master/cluster/usage-stats.html for more details.
2025-03-07 17:04:02,442 INFO worker.py:1842 -- Started a local Ray instance. View the dashboard at 127.0.0.1:8265

 python/ray/tests/accelerators/test_amd_gpu.py::test_visible_amd_gpu_type ✓                                                           40% ████      Usage stats collection is enabled by default for nightly wheels. To disable this, run the following command: `ray disable-usage-stats` before starting Ray. See https://docs.ray.io/en/master/cluster/usage-stats.html for more details.
2025-03-07 17:04:07,082 INFO worker.py:1842 -- Started a local Ray instance. View the dashboard at 127.0.0.1:8265

 python/ray/tests/accelerators/test_amd_gpu.py::test_visible_amd_gpu_type_bad_device_id ✓                                             60% ██████
 python/ray/tests/accelerators/test_amd_gpu.py::test_get_current_process_visible_accelerator_ids ✓                                    80% ████████
 python/ray/tests/accelerators/test_amd_gpu.py::test_set_current_process_visible_accelerator_ids ✓                                   100% ██████████

Results (14.39s):
       5 passed

cc @vickytsang Could you help me understand why both env vars HIP_VISIBLE_DEVICES and CUDA_VISIBLE_DEVICES are needed, and how are these two variables affect the visible amd devices for each individual GPU?

master_2b56e60_pytest.log
previous pytest results from master branch attached.

CUDA_VISIBLE_DEVICES should not be needed. I added it to my command line as a way to show that the conflict between HIP_VISIBLE_DEVICES and CUDA_VISIBLE_DEVICES values did not interfere with importing ray. It should not be relevant to the outcome of the pytest results.

kevin85421 · 2025-03-11T18:03:58Z

python/ray/_private/accelerators/amd_gpu.py

+if "HIP_VISIBLE_DEVICES" in os.environ:
+    hip_val = os.environ["HIP_VISIBLE_DEVICES"]
+    if cuda_val := os.environ.get("CUDA_VISIBLE_DEVICES", None):
+        logger.warning(


Are they unequal?

The line "assert hip_val == cuda_val" from the previous commit always firing even if they are equal. I used the logger warning as a workaround. Can you suggest a better way to handle this?

Adding checks outside of functions makes it difficult to determine when they will be executed, especially since users might import packages in different orders.

How about verifying it in get_current_process_visible_accelerator_ids?

python/ray/_private/accelerators/amd_gpu.py

kevin85421 · 2025-03-11T18:08:13Z

python/ray/_private/accelerators/amd_gpu.py

+            )
+        )
+    else:
+        os.environ["CUDA_VISIBLE_DEVICES"] = hip_val


what's the reason to set CUDA_VISIBLE_DEVICES? In addition, would you mind using the existing variable instead of hard-coding CUDA_VISIBLE_DEVICES?

kevin85421

would you mind rebasing with the master branch?

Signed-off-by: vickytsang <vtsang@amd.com>

kevin85421 · 2025-03-11T22:18:03Z

python/ray/_private/ray_constants.py

Remove the changes in this file. We recently decide to move all accelerator-specific constants from ray_constants.py to xxx_xpu.py.

kevin85421 · 2025-03-11T22:28:00Z

python/ray/_private/accelerators/amd_gpu.py

+if "HIP_VISIBLE_DEVICES" in os.environ:
+    hip_val = os.environ["HIP_VISIBLE_DEVICES"]
+    if cuda_val := os.environ.get("CUDA_VISIBLE_DEVICES", None):
+        logger.warning(


Adding checks outside of functions makes it difficult to determine when they will be executed, especially since users might import packages in different orders.

kevin85421 · 2025-03-11T22:28:46Z

python/ray/_private/accelerators/amd_gpu.py

+if "HIP_VISIBLE_DEVICES" in os.environ:
+    hip_val = os.environ["HIP_VISIBLE_DEVICES"]
+    if cuda_val := os.environ.get("CUDA_VISIBLE_DEVICES", None):
+        logger.warning(


How about verifying it in get_current_process_visible_accelerator_ids?

vickytsang · 2025-03-11T23:45:29Z

Test logs from latest commits
pytest_test_0311.log
envvar_test_0311.log

kevin85421 · 2025-03-12T00:12:29Z

python/ray/_private/accelerators/amd_gpu.py

what will happen if they are not equal?

If they are not equal the assertion on line 46 will fail.

(py3.9) root@quanta-ccs-aus-f11-01:/app/ray/python/ray# HIP_VISIBLE_DEVICES=0 CUDA_VISIBLE_DEVICES=0,1 python
Python 3.9.21 | packaged by conda-forge | (main, Dec 5 2024, 13:51:40)
[GCC 13.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.

import ray
Traceback (most recent call last):
File "", line 1, in
File "/opt/conda/envs/py3.9/lib/python3.9/site-packages/ray-3.0.0.dev0-py3.9-linux-x86_64.egg/ray/init.py", line 114, in
from ray._private.worker import ( # noqa: E402,F401
File "/opt/conda/envs/py3.9/lib/python3.9/site-packages/ray-3.0.0.dev0-py3.9-linux-x86_64.egg/ray/_private/worker.py", line 1272, in
global_worker = Worker()
File "/opt/conda/envs/py3.9/lib/python3.9/site-packages/ray-3.0.0.dev0-py3.9-linux-x86_64.egg/ray/_private/worker.py", line 447, in init
ray._private.utils.get_visible_accelerator_ids()
File "/opt/conda/envs/py3.9/lib/python3.9/site-packages/ray-3.0.0.dev0-py3.9-linux-x86_64.egg/ray/_private/utils.py", line 304, in get_visible_accelerator_ids
return {
File "/opt/conda/envs/py3.9/lib/python3.9/site-packages/ray-3.0.0.dev0-py3.9-linux-x86_64.egg/ray/_private/utils.py", line 305, in
accelerator_resource_name: get_accelerator_manager_for_resource(
File "/opt/conda/envs/py3.9/lib/python3.9/site-packages/ray-3.0.0.dev0-py3.9-linux-x86_64.egg/ray/_private/accelerators/amd_gpu.py", line 46, in get_current_process_visible_accelerator_ids
assert hip_val == cuda_val
AssertionError

Oh, sorry—I'm not referring to this PR. I'm referring to what will happen to the GPUs or GPU drivers if CUDA_VISIBLE_DEVICES and HIP_VISIBLE_DEVICES are set. For example, the wrong GPUs might be used.

That is, I don't know why they should be the same if CUDA_VISIBLE_DEVICES is set.

CUDA_VISIBLE_DEVICES, HIP_VISIBLE_DEVICE can have different effects when used together with various applications and frameworks. We want to be certain of the users intent in setting these variables. Perhaps a warning would be more appropriate?

Got it. Assertion is better for me.

kevin85421 · 2025-03-12T00:12:48Z

python/ray/_private/ray_constants.py

Suggested change

kevin85421 · 2025-03-12T00:13:43Z

python/ray/train/_internal/backend_executor.py

ray_constants doesn't have HIP_VISIBLE_DEVICES_ENV_VAR.

Signed-off-by: root <root@quanta-ccs-aus-f11-01.cs-aus.dcgpu>

Signed-off-by: vickytang <vtsang@amd.com>

…/ray into amd/use-hip-visible-devices

kevin85421

Overall looks good to me

kevin85421 · 2025-03-18T04:01:12Z

python/ray/_private/accelerators/amd_gpu.py

+
+        hip_val = os.environ.get(HIP_VISIBLE_DEVICES_ENV_VAR, None)
+        if cuda_val := os.environ.get(CUDA_VISIBLE_DEVICES_ENV_VAR, None):
+            assert hip_val == cuda_val


would you mind providing an actionable error message?

Signed-off-by: root <vtsang@amd.com>

kevin85421 · 2025-03-18T19:53:49Z

cc @justinvyu @matthewdeng, would you mind reviewing the changes in Ray Train? It's just a small change. Thanks!

hongpeng-guo

LGTM

kevin85421 · 2025-03-20T00:10:25Z

This PR doesn't update doc, so I think the doc CI failure is unrelated. @edoakes @jjyao would you mind reviewing or merging this PR? Thanks!

…ct#51104) Signed-off-by: root <vtsang@amd.com> Signed-off-by: vickytsang <vtsang@amd.com> Signed-off-by: Dhakshin Suriakannu <d_suriakannu@apple.com>

Now ray uses HIP_VISIBLE_DEVICES instead of ROCR_VISIBLE_DEVICES in recent changes for AMD devices. ray-project/ray#51104 This PR integrates such changes here. Signed-off-by: Hollow Man <hollowman@opensuse.org>

…ct#51104) Signed-off-by: root <vtsang@amd.com> Signed-off-by: vickytsang <vtsang@amd.com>

Now ray uses HIP_VISIBLE_DEVICES instead of ROCR_VISIBLE_DEVICES in recent changes for AMD devices. ray-project/ray#51104 This PR integrates such changes here. Signed-off-by: Hollow Man <hollowman@opensuse.org>

Replace AMD device env var with HIP_VISIBLE_DEVICES

cb5e497

Signed-off-by: root <vtsang@amd.com>

vickytsang requested review from hongpeng-guo, justinvyu, matthewdeng, raulchen and woshiyyya as code owners March 5, 2025 22:09

hongpeng-guo requested a review from kevin85421 March 6, 2025 00:09

Add condition to avoid conflicts between xxx_VISIBLE_DEVICES env vars

820b4e8

Signed-off-by: root <vtsang@amd.com>

kevin85421 approved these changes Mar 7, 2025

View reviewed changes

hongpeng-guo reviewed Mar 7, 2025

View reviewed changes

replace assert with logger warning

49fd813

Signed-off-by: root <vtsang@amd.com>

hongpeng-guo added the go add ONLY when ready to merge, run all tests label Mar 9, 2025

kevin85421 reviewed Mar 11, 2025

View reviewed changes

kevin85421 removed the go add ONLY when ready to merge, run all tests label Mar 11, 2025

kevin85421 requested changes Mar 11, 2025

View reviewed changes

Merge branch 'master' into amd/use-hip-visible-devices

0d0c305

Signed-off-by: vickytsang <vtsang@amd.com>

kevin85421 reviewed Mar 11, 2025

View reviewed changes

kevin85421 reviewed Mar 12, 2025

View reviewed changes

vickytsang force-pushed the amd/use-hip-visible-devices branch from c24e63c to 0d0c305 Compare March 17, 2025 17:04

vickytsang and others added 4 commits March 17, 2025 10:05

Merge branch 'ray-project:master' into amd/use-hip-visible-devices

a84be30

fix rebase

3a2630e

Signed-off-by: root <root@quanta-ccs-aus-f11-01.cs-aus.dcgpu>

move HIP env var check into class method

f76036f

Signed-off-by: vickytang <vtsang@amd.com>

Merge branch 'amd/use-hip-visible-devices' of https://github.com/ROCm…

60a50b8

…/ray into amd/use-hip-visible-devices

kevin85421 reviewed Mar 18, 2025

View reviewed changes

Add assertion error message

bdb7451

Signed-off-by: root <vtsang@amd.com>

kevin85421 approved these changes Mar 18, 2025

View reviewed changes

hongpeng-guo approved these changes Mar 18, 2025

View reviewed changes

kevin85421 added the go add ONLY when ready to merge, run all tests label Mar 19, 2025

jjyao merged commit 3b9e729 into ray-project:master Mar 20, 2025
5 of 6 checks passed

This was referenced Mar 25, 2025

[Core] Add env var to force using NvidiaGPUAcceleratorManager #49148

Closed

[Core] Ray Collective Types avoids importing torch directly #49153

Closed

HollowMan6 mentioned this pull request Apr 1, 2025

Update for recent HIP_VISIBLE_DEVICES changes in ray OpenRLHF/OpenRLHF#933

Merged

gshtras mentioned this pull request Apr 30, 2025

[Core][ROCm] Setting CUDA_VISIBLE_DEVICES leads to an assertion #52701

Closed

hainesmichaelc added the community-backlog label May 22, 2025

HollowMan6 mentioned this pull request Jun 2, 2025

[Feat] Add support for RAY_EXPERIMENTAL_NOSET_*_VISIBLE_DEVICES (Fix AMD support) verl-project/verl#1465

Merged

6 tasks

2niuhe mentioned this pull request Jun 11, 2025

Conflict between ROCR_VISIBLE_DEVICES and HIP_VISIBLE_DEVICES environment variables causes Ray import error #53737

Closed

Levi080513 pushed a commit to neutree-ai/ray that referenced this pull request Jul 10, 2025

[Core] Replace AMD device env var with HIP_VISIBLE_DEVICES (ray-proje…

b858c61

…ct#51104) Signed-off-by: root <vtsang@amd.com> Signed-off-by: vickytsang <vtsang@amd.com>

Conversation

vickytsang commented Mar 5, 2025

Why are these changes needed?

Checks

Uh oh!

kevin85421 left a comment

Choose a reason for hiding this comment

Uh oh!

hongpeng-guo left a comment

Choose a reason for hiding this comment

Uh oh!

vickytsang commented Mar 7, 2025

Uh oh!

hongpeng-guo commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vickytsang commented Mar 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevin85421 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vickytsang commented Mar 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevin85421 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevin85421 commented Mar 18, 2025

Uh oh!

hongpeng-guo left a comment

Choose a reason for hiding this comment

Uh oh!

kevin85421 commented Mar 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hongpeng-guo commented Mar 9, 2025 •

edited

Loading