Skip to content

Added explicit check to raise ValueError on CPU devices#1003

Open
zer-art wants to merge 5 commits intogoogle-deepmind:mainfrom
zer-art:fix/restrict-testspeed-gpu
Open

Added explicit check to raise ValueError on CPU devices#1003
zer-art wants to merge 5 commits intogoogle-deepmind:mainfrom
zer-art:fix/restrict-testspeed-gpu

Conversation

@zer-art
Copy link
Contributor

@zer-art zer-art commented Jan 12, 2026

Summary

This PR updates mujoco_warp/testspeed.py to prevent crashes when running on non-GPU devices (e.g., macOS CPU).

As discussed in this comment, testspeed.py calls 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:

  • Added a device check at the entry point of testspeed.py.
  • Raises a clear ValueError: testspeed available for gpu only if 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 10

Output (New Behavior):
ValueError: testspeed available for gpu only

if _CLEAR_KERNEL_CACHE.value:
wp.clear_kernel_cache()

if "cpu" in (_DEVICE.value or "", str(wp.get_device())):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does "cpu" in (_DEVICE.value, wp.get_device()) work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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":

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. thanks!

@thowell
Copy link
Collaborator

thowell commented Jan 29, 2026

@zer-art lets sync this pr branch with main and then we should be good to merge. thanks!

@thowell
Copy link
Collaborator

thowell commented Feb 5, 2026

@zer-art will probably need to sync with main again for the github ci. thanks!

@zer-art
Copy link
Contributor Author

zer-art commented Feb 6, 2026

Done! I've synced the branch with main again. Thanks for the heads-up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants