Fix Demo Scripts (h1_locomotion.py and quadcopter.py)#5866
Conversation
Signed-off-by: Yize Wang <yizew@nvidia.com>
Signed-off-by: Yize Wang <yizew@nvidia.com>
Signed-off-by: Yize Wang <yizew@nvidia.com>
Signed-off-by: Yize Wang <yizew@nvidia.com>
Signed-off-by: Yize Wang <yizew@nvidia.com>
Greptile SummaryThis PR fixes two demo scripts that were broken by API changes:
Confidence Score: 4/5Both script fixes are targeted and demonstrated working via screenshots; the changes are small and low-risk. The quadcopter fix hardcodes instance [0] for mass fetching, which is fine for the current single-robot demo but differs from a fully general approach. The h1 fix defers the metadata.version call to init rather than module-level, a minor inconsistency with the rest of the codebase. Neither causes incorrect behavior in the current scripts. No files require special attention beyond the two demo scripts. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph quadcopter.py
A[sim.reset] --> B["robot.data.body_mass.torch[0].sum()"]
B --> C[robot_mass scalar]
C --> D["forces[...,2] = robot_mass * gravity / 4.0"]
D --> E[robot hover loop]
end
subgraph h1_locomotion.py
F["parse_rsl_rl_cfg(TASK, args_cli)"] --> G["metadata.version('rsl-rl-lib')"]
G --> H["handle_deprecated_rsl_rl_cfg(agent_cfg, version)"]
H --> I[get_published_pretrained_checkpoint]
I --> J[OnPolicyRunner.load]
J --> K[H1 demo loop]
end
Reviews (1): Last reviewed commit: "Remove Warp Import From quadcopter.py" | Re-trigger Greptile |
| # Fetch relevant parameters to make the quadcopter hover in place | ||
| prop_body_ids = robot.find_bodies("m.*_prop")[0] | ||
| robot_mass = robot.root_view.get_masses().sum() | ||
| robot_mass = robot.data.body_mass.torch[0].sum() |
There was a problem hiding this comment.
Using
[0] to index the first instance ties the hover-force calculation to a single environment's mass. The body_mass tensor has shape (num_instances, num_bodies), so summing over instance 0 is correct for this single-robot demo, but the mean across all instances (or a per-instance sum) would be more robust if the demo is ever extended to spawn multiple robots. For a stricter fix, an assertion or the use of .mean(dim=0).sum() would make the intent clearer.
| robot_mass = robot.data.body_mass.torch[0].sum() | |
| robot_mass = robot.data.body_mass.torch.sum(dim=1).mean() |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| from importlib import metadata | ||
|
|
||
| sys.path.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../..")) |
There was a problem hiding this comment.
The other RSL-RL scripts (
play.py, train.py) call metadata.version("rsl-rl-lib") once at module level and reuse the variable, which surfaces a PackageNotFoundError immediately on import rather than deferred to first construction of H1RoughDemo. Calling it inline inside __init__ delays the failure and also re-evaluates the version string on every new demo object. Storing it in a module-level constant matches the established pattern.
| from importlib import metadata | |
| sys.path.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../..")) | |
| from importlib import metadata | |
| _RSL_RL_INSTALLED_VERSION = metadata.version("rsl-rl-lib") | |
| sys.path.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), "../..")) |
There was a problem hiding this comment.
Code Review Summary
This PR fixes two broken demo scripts by updating deprecated APIs. The changes are straightforward and correct for their intended use case.
✅ Overall Assessment: Ship it
The fixes address real issues (broken demo scripts) with minimal, focused changes. Both API updates are appropriate and follow current Isaac Lab patterns.
Detailed Analysis
scripts/demos/quadcopter.py
Change: robot.root_view.get_masses().sum() → robot.data.body_mass.torch[0].sum()
✅ Correct fix. The root_view API was deprecated in favor of the unified data.body_mass tensor interface. The [0] indexing is appropriate here since this demo spawns exactly one robot instance (num_envs defaults to 1 for this script).
scripts/demos/h1_locomotion.py
Change: Added handle_deprecated_rsl_rl_cfg(agent_cfg, metadata.version("rsl-rl-lib"))
✅ Correct fix. The handle_deprecated_rsl_rl_cfg function handles RSL-RL configuration compatibility across version boundaries (< 4.0.0, ≥ 4.0.0, ≥ 5.0.0). This matches the pattern used in the canonical play.py and train.py scripts.
Minor Observations (non-blocking)
🔵 Style consistency — scripts/demos/h1_locomotion.py:88
The version string is evaluated inline (metadata.version("rsl-rl-lib")) within __init__. The canonical RSL-RL scripts (play.py, train.py) evaluate this at module level for earlier failure on missing packages. For a demo script, inline evaluation is acceptable, but hoisting to module level would be more consistent with the codebase pattern.
🔵 Hardcoded instance index — scripts/demos/quadcopter.py:77
Using [0] assumes single-instance execution. This is fine for the current demo (which only spawns one robot), but a comment noting this assumption would help maintainers if the demo is later extended to multi-instance.
Checklist Verification
- ✅ License headers present
- ✅ Changes are minimal and focused
- ✅ Pre-commit checks passed (per PR description)
⚠️ No tests added — acceptable for demo script bug fixes; the screenshots in the PR description serve as manual verification⚠️ CHANGELOG not updated — these are fixes to demo scripts, not library APIs, so this is reasonable
LGTM with the minor style observations noted above. Nice fix! 🚀
| # Fetch relevant parameters to make the quadcopter hover in place | ||
| prop_body_ids = robot.find_bodies("m.*_prop")[0] | ||
| robot_mass = robot.root_view.get_masses().sum() | ||
| robot_mass = robot.data.body_mass.torch[0].sum() |
There was a problem hiding this comment.
what's the main reason behind why this is needed?
There was a problem hiding this comment.
This is not necessary. The shape of robot.data.body_mass.torch is (1, 5). I wanted to highlight this is the total mass of the first quadcopter (though there are no ones in this case). With or without [0], the result is equivalent for this mono-instance demo.
…nvs, shutdown error, mjcf fix (#5888) # Description Cherry-pick PRs from develop: - #5866 - #5879 - #5882 - #5884 - #5889 --------- Signed-off-by: Yize Wang <yizew@nvidia.com> Signed-off-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: YizeWang <37894497+YizeWang@users.noreply.github.com> Co-authored-by: Yize Wang <yizew@nvidia.com>
Description
This PR fixes demo scripts that were using outdated tensor/view APIs.
The quadcopter demo now reads robot mass through the public
robot.data.body_massAPI. The H1 locomotion demo now applies the RSL-RL deprecated config handler based on the installedrsl-rl-libversion. This PR also adds Yize Wang toCONTRIBUTORS.md.Fixes NV Bugs:
Type of change
Screenshots
Fixed scripts:


Validation of other scripts:










Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there