[Fix] add_new_robot tutorial wheel-velocity action on GPU device#5776
Draft
hujc7 wants to merge 1 commit into
Draft
[Fix] add_new_robot tutorial wheel-velocity action on GPU device#5776hujc7 wants to merge 1 commit into
hujc7 wants to merge 1 commit into
Conversation
scripts/tutorials/01_assets/add_new_robot.py built the Jetbot wheel- velocity action with torch.Tensor([[10.0, 10.0]]) — a CPU tensor — and forwarded it to the deprecated set_joint_velocity_target wrapper. The underlying wp.launch in isaaclab_physx rejects CPU torch tensors at kernel-argument type check, so the tutorial errored at runtime. Allocate the two wheel-velocity templates once on sim.device, reuse them across the loop iterations, and call the recommended set_joint_velocity_target_index variant (silences the deprecation warning the bug report also captured). The reset block in the same file already used the GPU device through data.default_joint_pos.torch.clone(); the per-step action block was the only remaining CPU-tensor site. This pattern is unique to add_new_robot.py — every other tutorial script under scripts/tutorials/ either pulls tensors from .torch accessors (already on device) or constructs them with torch.tensor(..., device=sim.device). Confirmed by a grep across the tutorials tree. Refs NVBug 6156303
There was a problem hiding this comment.
Code Review Summary
This PR correctly fixes a runtime error in the add_new_robot.py tutorial caused by passing CPU tensors to GPU Warp kernels. The fix is well-structured and follows established patterns in other Isaac Lab tutorials.
Strengths
- Root cause addressed: Pre-allocating tensors on
sim.deviceensures compatibility with the GPU-based asset layer. - API modernization: Migrating from the deprecated
set_joint_velocity_target()toset_joint_velocity_target_index()addresses the deprecation warning noted in the bug report. - Efficient pattern: Allocating tensors once outside the loop rather than per-iteration is the correct approach.
- Thorough PR description: The root cause analysis and historical context (PRs #289, #4551, #4911) help reviewers understand why this bug existed and why only this tutorial was affected.
Minor Observations
No blocking issues found. The implementation is clean and matches the patterns used in other tutorials (e.g., run_articulation.py).
CI Note
The "Check for Broken Links" CI failure is unrelated to this PR — it's caused by an external site (threedworld.org) returning HTTP 403.
LGTM from a code perspective.
|
|
||
| # wheel-velocity templates allocated once on the simulation device; the joint | ||
| # target setters dispatch to GPU Warp kernels and reject CPU tensors. | ||
| straight_action = torch.tensor([[10.0, 10.0]], device=sim.device) |
There was a problem hiding this comment.
Good practice: pre-allocating these tensors on the correct device outside the loop avoids per-iteration allocation overhead and ensures GPU compatibility.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Summary
scripts/tutorials/01_assets/add_new_robot.pyerrored at runtime: the wheel-velocity action was a CPUtorch.Tensorandset_joint_velocity_targetdispatches to a GPU Warp kernel that rejects CPU tensors at type check.sim.device, reuse across the loop, and callset_joint_velocity_target_index(also silences the deprecation warning visible in the bug report's first line).2. Why it broke
torch.Tensor([[10.0, 10.0]])— CPU literal. The asset API at the time accepted CPU torch tensors.wp.to_torchand_indexvariants forwrite_*_to_sim), but did not touch the per-step action block — that remained CPU.The bug has therefore been latent since #4551 and surfaced now that more users are running the tutorial on v3.0.0-beta.
3. Why only this tutorial
A grep across
scripts/tutorials/**.pyfor tensor literals fed intoset_joint_*_target*/write_*_to_sim_indexshowsadd_new_robot.pyis the only file constructing tensors without adevice=kwarg. Every other tutorial either reads from a.torchaccessor (already on device) or usestorch.tensor(..., device=sim.device).4. Test plan
torch.Tensor→wp.launch→argument 'in_data' expects an array of type array(ndim=2, dtype=float32), but passed value has type Tensor..cuda()) succeeds../isaaclab.sh -p scripts/tutorials/01_assets/add_new_robot.py --num_envs 1 --headlessruns through several reset cycles.5. Out of scope
isaaclab_physxsilently accepts CPU torch tensors at the Python boundary and only fails insidewp.launch. Auto-moving CPU tensors toself.deviceinset_joint_*_target_indexwould prevent users from hitting this from other scripts. Not in scope here per the bug-report preference for a tutorial-only fix; flagging for follow-up.Refs NVBug 6156303