-
Notifications
You must be signed in to change notification settings - Fork 533
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
reimplementation of gpu_count #3718
base: master
Are you sure you want to change the base?
reimplementation of gpu_count #3718
Conversation
222ebd5
to
154f3c7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3718 +/- ##
==========================================
- Coverage 73.07% 73.05% -0.03%
==========================================
Files 1278 1279 +1
Lines 59406 59414 +8
==========================================
- Hits 43411 43404 -7
- Misses 15995 16010 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
154f3c7
to
4dfbbbe
Compare
We need to remove gputil from the dependencies. I'm not terribly worried about Windows, as there are many years of development since last time anybody tried to get nipype working on Windows. |
Done
I meant that I don't have a computer without a nvidia GPU to test if this code fix the error reported, I assumed it was on Windows. So even a test on other OS may be useful. |
nipype/utils/gpu_count.py
Outdated
from subprocess import Popen, PIPE | ||
import os | ||
|
||
|
||
def gpu_count(): | ||
try: | ||
if platform.system() == "Windows": | ||
nvidia_smi = shutil.which('nvidia-smi') | ||
if nvidia_smi is None: | ||
nvidia_smi = ( | ||
"%s\\Program Files\\NVIDIA Corporation\\NVSMI\\nvidia-smi.exe" | ||
% os.environ['systemdrive'] | ||
) | ||
else: | ||
nvidia_smi = "nvidia-smi" | ||
|
||
p = Popen( | ||
[nvidia_smi, "--query-gpu=name", "--format=csv,noheader,nounits"], | ||
stdout=PIPE, | ||
) | ||
stdout, stderror = p.communicate() | ||
|
||
output = stdout.decode('UTF-8') | ||
lines = output.split(os.linesep) | ||
num_devices = len(lines) - 1 | ||
return num_devices | ||
except: | ||
return 0 |
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 goal is to keep it as close to the source as possible, that's fine. Here are some cleanups for your consideration:
- Use
shutil.which()
unconditionally. We can detect aFileNotFoundError
before calling the process and simply return 0. - Use
subprocess.run
, which is the recommended API. By using text mode, we don't have to handle decoding or newline normalization. - Use targeted errors.
FileNotFoundError
andPermissionError
(bothOSError
s) should catch failures to run.UnicodeDecodeError
will catch bad output. - Use
p.output.splitlines()
to get content-full lines. A trailing newline does not produce an empty string at the end of the list.
from subprocess import Popen, PIPE | |
import os | |
def gpu_count(): | |
try: | |
if platform.system() == "Windows": | |
nvidia_smi = shutil.which('nvidia-smi') | |
if nvidia_smi is None: | |
nvidia_smi = ( | |
"%s\\Program Files\\NVIDIA Corporation\\NVSMI\\nvidia-smi.exe" | |
% os.environ['systemdrive'] | |
) | |
else: | |
nvidia_smi = "nvidia-smi" | |
p = Popen( | |
[nvidia_smi, "--query-gpu=name", "--format=csv,noheader,nounits"], | |
stdout=PIPE, | |
) | |
stdout, stderror = p.communicate() | |
output = stdout.decode('UTF-8') | |
lines = output.split(os.linesep) | |
num_devices = len(lines) - 1 | |
return num_devices | |
except: | |
return 0 | |
import subprocess | |
import os | |
def gpu_count(): | |
nvidia_smi = shutil.which('nvidia-smi') | |
if nvidia_smi is None and platform.system() == "Windows": | |
nvidia_smi = f'{os.environ["systemdrive"]}\\Program Files\\NVIDIA Corporation\\NVSMI\\nvidia-smi.exe' | |
if nvidia_smi is None: | |
return 0 | |
try: | |
p = subprocess.run( | |
[nvidia_smi, "--query-gpu=name", "--format=csv,noheader,nounits"], | |
stdout=subprocess.PIPE, | |
text=True, | |
) | |
except (OSError, UnicodeDecodeError): | |
return 0 | |
return len(output.splitlines()) |
fix #3717
I tried to write a simpler implementation of
getGPUs
from gputil package.I used a separate file to include their license.
This need to be tested under windows with Python 3.12+ and I don't have such system, so it's untested for now.