Find ROCm on Fedora #5705
Find ROCm on Fedora #5705
Conversation
|
@trixirt - can you please complete the CLA? |
| @@ -0,0 +1,28 @@ | |||
| # Copyright (c) Tom Rix | |||
There was a problem hiding this comment.
Hi @trixirt - can you please run the formatting check (pre-commit run --all-files) - to pass that check on this file?
There was a problem hiding this comment.
pre-commit errors out with
File "<string>", line 22, in <module>
File "/home/trix/.cache/pre-commit/repoyr3bl7h1/yapf/__init__.py", line 35, in <module>
from yapf.yapflib import errors
File "/home/trix/.cache/pre-commit/repoyr3bl7h1/yapf/yapflib/errors.py", line 16, in <module>
from lib2to3.pgen2 import tokenize
ModuleNotFoundError: No module named 'lib2to3'
[end of output]
note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error
Fedora/Rawhide is on python 3.13
The docs on lib2to3 say
removed from 3.13
There was a problem hiding this comment.
Running on 3.12 the license check fails because i believe this line
copyright check
It will pass if I use the Microsoft boilerplate.
There are examples of non Microsoft copyrights in the code, so I wonder how they passed this check.
There was a problem hiding this comment.
@trixirt - our check requires there to be a license file on top of each file, we can use the standard Microsoft one here since that's most fitting here if you could add it? Or I can add a dummy test_rocm file with the header as well if that helps.
There was a problem hiding this comment.
@trixirt - curious if you had any time to consider making these changes to pass the formatting check?
op_builder/builder.py
Outdated
| rocm_info = Path("/opt/rocm/bin/rocminfo") | ||
| if (not rocm_info.is_file()): | ||
| rocm_info = Path("rocminfo") | ||
| rocm_wavefront_size_cmd = str(rocm_info) + " | grep -Eo -m1 'Wavefront Size:[[:space:]]+[0-9]+' | grep -Eo '[0-9]+'" |
There was a problem hiding this comment.
This should become available in torch.cuda.get_device_properties with pytorch/pytorch#128449
|
@microsoft-github-policy-service agree |
|
Sorry for delay. |
ROCm is packaged natively on Fedora. It's install location do not match the AMD release. So add some Fedora specific logic to find the ROCm version and use rocminfo when attempts to use the AMD release fail. Signed-off-by: Tom Rix <trix@redhat.com>
|
I have dropped the tests. |
Understood, I'm still trying to confirm more about the copyright issue, but haven't made much progress yet. I appreciate the changes you've contributed though, and I'll work on getting this merged this week. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
ROCm is packaged natively on Fedora. It's install location do not match the AMD release.
So add some Fedora specific logic to find the ROCm version and use rocminfo when attempts to use the AMD release fail.