Added explicit check to raise ValueError on CPU devices#1003
Added explicit check to raise ValueError on CPU devices#1003zer-art wants to merge 5 commits intogoogle-deepmind:mainfrom
Conversation
mujoco_warp/testspeed.py
Outdated
| if _CLEAR_KERNEL_CACHE.value: | ||
| wp.clear_kernel_cache() | ||
|
|
||
| if "cpu" in (_DEVICE.value or "", str(wp.get_device())): |
There was a problem hiding this comment.
does "cpu" in (_DEVICE.value, wp.get_device()) work?
There was a problem hiding this comment.
@thowell Thanks for the review!
You were right about str()—I verified that wp.get_device() == "cpu" works directly, so the explicit cast isn't needed.
While testing the simpler in syntax, I noticed an edge case: if I am on a laptop (default CPU) but run with --device=cuda:0, wp.get_device() still returns cpu. The in check sees that and raises the error, even though I provided a valid GPU override.
Would you mind if I switched to this logic instead?
if (_DEVICE.value or wp.get_device()) == "cpu":
|
@zer-art lets sync this pr branch with main and then we should be good to merge. thanks! |
|
@zer-art will probably need to sync with main again for the github ci. thanks! |
|
Done! I've synced the branch with main again. Thanks for the heads-up! |
Summary
This PR updates
mujoco_warp/testspeed.pyto prevent crashes when running on non-GPU devices (e.g., macOS CPU).As discussed in this comment,
testspeed.pycalls CUDA functions unconditionally, which leads to errors on CPU environments. This change adds an explicit check to ensure the script is only run on GPU devices.Changes:
testspeed.py.ValueError: testspeed available for gpu onlyif the detected device is not a GPU, preventing obscure runtime errors.Verification
I verified this change by running the script in a CPU environment (macOS).
Command:
conda run -n academic python -m mujoco_warp.testspeed mujoco_warp/test_data/humanoid/humanoid.xml --nstep 10 --nworld 10Output (New Behavior):
ValueError: testspeed available for gpu only