Respect ROCR_VISIBLE_DEVICES on AMD GPU device discovery by tbennun · Pull Request #142292 · pytorch/pytorch

tbennun · 2024-12-07T02:30:46Z

Reland of #140320 after failing test on trunk. Fixes potential environment clobbering in test, makes ROCr+HIP devices (if specified together) more robust to index errors.

Fixes #140318

cc @jeffdaily @sunway513 @jithunnair-amd @pruthvistony @ROCmSupport @dllehr-amd @jataylo @hongxiayang @naromero77amd

pytorch-bot · 2024-12-07T02:30:49Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/142292

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit f1ec857 with merge base b0c3f48 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

tbennun · 2024-12-07T02:31:36Z

@pytorchbot label "topic: not user facing"

tbennun · 2024-12-07T02:39:35Z

FYI (previous approvers) @jataylo @jithunnair-amd @eqy @jeffdaily

torch/cuda/__init__.py

tbennun · 2024-12-20T00:46:43Z

torch/cuda/__init__.py

-            else:
+            rocr_count = len(rocr_devics.split(","))
+            if hip_devices is not None:
+                if hip_devices.startswith("GPU-"):


could HIP_VISIBLE_DEVICES be set to 0,GPU-uuid,2? If so, this check should be on any(dev for dev in hip_devices.split(","))

@jeffdaily Although not a common use case it could be possible for the user to use ROCR_VISIBLE_DEVICES in combination with HIP_VISIBLE_DEVICES with UUIDs

e.g. may use ROCR to expose only GPUs "0,1" in some environment but the user in that environment may want to use UUIDs in HIP_VISIBLE_DEVICES to ensure they run on the requested gpu.

I was wrong about ROCR env var only taking ints. It can take UUIDs also. So ROCR and HIP env vars are similar today, but ROCR env var activates first lower-level than HIP env var so it will reduce the number of devices that HIP can select from. I think the best we can do now is go back to the simple check that HIP env var isn't asking for more devices than are visible.

torch/cuda/__init__.py

tbennun · 2024-12-20T20:05:16Z

@jeffdaily @jataylo I think this is ready for testing before merging, apart from @jataylo 's comment on mixing UUIDs and indices in HIP_VISIBLE_DEVICES.

jataylo

Looks good to me, we're good to go. Lets run CI.

(Slight unknown to me is how ROCR_VISIBLE_DEVICES will operate inside the os.env inside the UT, but if we see some issues there lets just remove the test)

jataylo · 2024-12-23T10:28:21Z

@jeffdaily can we get your approval then lets reland this and open a cherry pick for release 2.6

jataylo · 2024-12-23T17:39:04Z

@atalman @huydhn Hi both, could you help us re-land this PR, we would like to get this bug fix into 2.6 cherry picks also.

huydhn · 2024-12-23T17:57:59Z

@pytorchbot merge

pytorchmergebot · 2024-12-23T18:00:08Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2024-12-24T15:55:45Z

Successfully rebased rocr-visible-devices onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout rocr-visible-devices && git pull --rebase)

jataylo · 2024-12-24T23:35:00Z

@pytorchbot merge

pytorchmergebot · 2024-12-24T23:37:32Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

jataylo · 2024-12-31T11:01:25Z

@pytorchbot cherry-pick --onto release/2.6

pytorch-bot · 2024-12-31T11:01:27Z

❌ 🤖 pytorchbot command failed:

@pytorchbot cherry-pick: error: the following arguments are required: -c/--classification

usage: @pytorchbot cherry-pick --onto ONTO [--fixes FIXES] -c
                               {regression,critical,fixnewfeature,docs,release}

Try @pytorchbot --help for more info.

jataylo · 2024-12-31T11:01:56Z

@pytorchbot cherry-pick --onto release/2.6 -c critical

Reland of #140320 after failing test on trunk. Fixes potential environment clobbering in test, makes ROCr+HIP devices (if specified together) more robust to index errors. Fixes #140318 Pull Request resolved: #142292 Approved by: https://github.com/jataylo, https://github.com/huydhn, https://github.com/jeffdaily Co-authored-by: Jack Taylor <108682042+jataylo@users.noreply.github.com> Co-authored-by: Jeff Daily <jeff.daily@amd.com> (cherry picked from commit c0d7106)

pytorchbot · 2024-12-31T11:06:56Z

Cherry picking #142292

The cherry pick PR is at #144026 and it is recommended to link a critical cherry pick PR with an issue. The following tracker issues are updated:

[v.2.6.0] Release Tracker #142814 (comment)

Details for Dev Infra team

Raised by workflow job

Respect ROCR_VISIBLE_DEVICES on AMD GPU device discovery (#142292) Reland of #140320 after failing test on trunk. Fixes potential environment clobbering in test, makes ROCr+HIP devices (if specified together) more robust to index errors. Fixes #140318 Pull Request resolved: #142292 Approved by: https://github.com/jataylo, https://github.com/huydhn, https://github.com/jeffdaily Co-authored-by: Jack Taylor <108682042+jataylo@users.noreply.github.com> Co-authored-by: Jeff Daily <jeff.daily@amd.com> (cherry picked from commit c0d7106) Co-authored-by: Tal Ben-Nun <tbennun@users.noreply.github.com>

) Respect ROCR_VISIBLE_DEVICES on AMD GPU device discovery (pytorch#142292) Reland of pytorch#140320 after failing test on trunk. Fixes potential environment clobbering in test, makes ROCr+HIP devices (if specified together) more robust to index errors. Fixes pytorch#140318 Pull Request resolved: pytorch#142292 Approved by: https://github.com/jataylo, https://github.com/huydhn, https://github.com/jeffdaily Co-authored-by: Jack Taylor <108682042+jataylo@users.noreply.github.com> Co-authored-by: Jeff Daily <jeff.daily@amd.com> (cherry picked from commit c0d7106) Co-authored-by: Tal Ben-Nun <tbennun@users.noreply.github.com> (cherry picked from commit 23e390c)

…covery (pytorch#144026) (#1895) Respect ROCR_VISIBLE_DEVICES on AMD GPU device discovery (pytorch#142292) Reland of pytorch#140320 after failing test on trunk. Fixes potential environment clobbering in test, makes ROCr+HIP devices (if specified together) more robust to index errors. Fixes pytorch#140318 Pull Request resolved: pytorch#142292 Approved by: https://github.com/jataylo, https://github.com/huydhn, https://github.com/jeffdaily Co-authored-by: Jack Taylor <108682042+jataylo@users.noreply.github.com> Co-authored-by: Jeff Daily <jeff.daily@amd.com> (cherry picked from commit c0d7106) Co-authored-by: Tal Ben-Nun <tbennun@users.noreply.github.com> (cherry picked from commit 23e390c) Fixes #ISSUE_NUMBER Co-authored-by: pytorchbot <soumith+bot@pytorch.org>

tbennun requested review from eqy and syed-ahmed as code owners December 7, 2024 02:30

pytorchbot added the open source label Dec 7, 2024

pytorch-bot bot added the topic: not user facing topic category label Dec 7, 2024

tbennun mentioned this pull request Dec 9, 2024

Respect ROCR_VISIBLE_DEVICES on AMD GPU device discovery #140320

Closed

jataylo requested changes Dec 9, 2024

View reviewed changes

torch/cuda/__init__.py Outdated Show resolved Hide resolved

jataylo requested changes Dec 9, 2024

View reviewed changes

torch/cuda/__init__.py Outdated Show resolved Hide resolved

janeyx99 added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Dec 9, 2024

tbennun commented Dec 20, 2024

View reviewed changes

torch/cuda/__init__.py Outdated Show resolved Hide resolved

tbennun commented Dec 20, 2024

View reviewed changes

torch/cuda/__init__.py Outdated Show resolved Hide resolved

tbennun commented Dec 20, 2024

View reviewed changes

torch/cuda/__init__.py Show resolved Hide resolved

jeffdaily requested a review from jataylo December 20, 2024 22:28

jataylo approved these changes Dec 20, 2024

View reviewed changes

pruthvistony added module: rocm AMD GPU support for Pytorch rocm This tag is for PRs from ROCm team ciflow/rocm Trigger "default" config CI on ROCm labels Dec 23, 2024

jataylo requested review from atalman and huydhn December 23, 2024 17:38

huydhn approved these changes Dec 23, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Dec 23, 2024

pytorchmergebot added the merging label Dec 23, 2024

jataylo and others added 11 commits December 24, 2024 15:55

[ROCm] respect ROCR_VISIBLE_DEVICE setting during _parse_visible_devices

b909389

Linting

3f68fae

Add environment variable test

d4516f3

Improve handling of HIP/ROCr visible devices when both are given

795b23d

Fix test starting conditions, make ROCr/hip device list more robust

e6fe087

Lint

83a197c

correct logic for both env vars being set

3c358d6

Apply suggestions from code review

0cd01c2

remove exception for setting ROCR env var and HIP env var with UUIDs

56c45d1

one more time

256dada

Fix CUDA_VISIBLE_DEVICES

f1ec857

pytorchmergebot force-pushed the rocr-visible-devices branch from 7893dbb to f1ec857 Compare December 24, 2024 15:55

pytorchmergebot added the merging label Dec 24, 2024

pytorchmergebot added the Merged label Dec 25, 2024

pytorchmergebot closed this in c0d7106 Dec 25, 2024

pytorchmergebot removed the merging label Dec 25, 2024

pytorchbot mentioned this pull request Dec 31, 2024

[v.2.6.0] Release Tracker #142814

Closed

jataylo mentioned this pull request Feb 12, 2025

[release/2.5] [CP] Respect ROCR_VISIBLE_DEVICES on AMD GPU device discovery (#144026) ROCm/pytorch#1895

Merged

Conversation

tbennun commented Dec 7, 2024 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Dec 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/142292

✅ No Failures

Uh oh!

tbennun commented Dec 7, 2024

Uh oh!

tbennun commented Dec 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tbennun Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

jataylo Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

jeffdaily Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tbennun commented Dec 20, 2024

Uh oh!

jataylo left a comment

Choose a reason for hiding this comment

Uh oh!

jataylo commented Dec 23, 2024

Uh oh!

jataylo commented Dec 23, 2024

Uh oh!

huydhn commented Dec 23, 2024

Uh oh!

pytorchmergebot commented Dec 23, 2024

Merge started

Uh oh!

pytorchmergebot commented Dec 24, 2024

Uh oh!

jataylo commented Dec 24, 2024

Uh oh!

pytorchmergebot commented Dec 24, 2024

Merge started

Uh oh!

jataylo commented Dec 31, 2024

Uh oh!

pytorch-bot bot commented Dec 31, 2024

Uh oh!

jataylo commented Dec 31, 2024

Uh oh!

pytorchbot commented Dec 31, 2024

Cherry picking #142292

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tbennun commented Dec 7, 2024 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Dec 7, 2024 •

edited

Loading