[Task Clean-up][Assets] Serve the Shadow Hand from one asset with native PhysX tendons - #7161
Conversation
| """ | ||
| if isinstance(target, torch.Tensor): | ||
| offset = -target | ||
| elif isinstance(target, (int, float)): |
| init_state=SHADOW_HAND_CFG.init_state.replace(pos=init_pos, rot=init_rot), | ||
| ) | ||
|
|
||
| def _for_engine(physics: str): |
There was a problem hiding this comment.
please expand the preset and don't return preset
There was a problem hiding this comment.
if doesn't exceed the length we can just inline it
4cfdae3 to
029ab63
Compare
|
run-ci |
1aaf6d8 to
1902d03
Compare
The hand now spawns from the unified asset, so the reference frames it renders moved with it. Seven of the eight combinations changed; newton/newton/tiled renders byte-identically and is left as it was.
The unified asset renders the hand in its own materials, so the reference frames the rendering-correctness suite compares against no longer match. Twenty-four of the seventy-six combinations moved; the rest are untouched. Verified by re-running the suite against the regenerated references: 10 passed, 1 skipped.
The OVPhysX and OVRTX combinations render the hand from the same unified asset, so their references moved with it. Twenty-three images across ovphysx-newton_renderer, ovphysx-ovrtx_renderer and newton-ovrtx_renderer. Verified by re-running the kitless suite against them: 20 passed, 2 skipped.
…tendon-codex-base
…tendon-codex-base
|
run-ci |
ooctipus
left a comment
There was a problem hiding this comment.
Thanks for addressing all the concerns approved
…tendon-codex-base
… term The term clamped every raw action to [-1, 1], which asserts that every caller sends normalized policy output; no other action term bounds its input, and the RL wrappers clip nothing by default. The bound belongs to the task, so the Shadow configs carry clip over the tendon limits and the term applies scale and offset directly. The result is unchanged for these tasks, since clip equals the mapped span. scale and offset also take the scalar-or-dict forms a joint term takes, parsed the same way, and the IO descriptor carries them as plain values.
|
run-ci |
| @@ -0,0 +1,32 @@ | |||
| # Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). | |||
There was a problem hiding this comment.
Can merge this with commands.py?
| @@ -0,0 +1,272 @@ | |||
| # Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). | |||
There was a problem hiding this comment.
Let's not use _ file name. Also @ooctipus has comment, if you follow that this file might not be needed either
…tendon-codex-base
The Shadow Hand's tendon-coupled fingers diverge past their joint limits under the default explicit integrator, which corrupted memory and surfaced as a SIGSEGV in an unrelated collision kernel. Measured over 260 steps against a 1.571 rad limit: euler reaches 3.275 with 15 joints out of range, implicitfast 0.944 with none, matching develop's 1.041. The demo also never commanded the tendons, so the eight joints they span took no command at all, and posed the hand with the reorientation task's spawn pose, which left the palm facing down beside the Allegro. Resolve the MuJoCo actuator rows of an articulation spawned outside a cloned scene, where the importer leaves actuator_world at -1 and partitioning by world selected nothing. Without it the tendon control never binds and commanding a tendon reports the asset as authoring no tendon actuator.
|
run-ci |
## Summary - Bumps Newton from `1.5.1` to `1.6.0rc1`, the first release carrying [newton#4017](newton-physics/newton#4017), unblocking the MuJoCo tendon adapter that PR #7161 needs. - `warp-lang` and `mujoco`/`mujoco-warp` move with it, because `1.6.0rc1` requires them. - Pins `newton[sim]` in `[project].dependencies` as well, so a wheel built from this metadata resolves the fix rather than falling back to the newest stable. - Updates test literals in `isaaclab` and `isaaclab_newton` for Newton's new actuator class names, and asks for a determinism guarantee in the manipulation determinism test. # Description `resolve_fixed_tendon_actuator_columns` reads `mujoco.actuator_trntype` through an `ArticulationView`. Every Newton 1.5.x release rejects that with *"has custom frequency 'mujoco:actuator' which is not supported by ArticulationView"*; newton#4017 makes `ArticulationView` generic over custom frequencies. `1.6.0rc1` (published 2026-09-03) is the first release containing that fix — verified two ways: the merge commit `4055a159` is an ancestor of `v1.6.0rc1`, and `custom_frequency_label_attributes`, the symbol the fix introduces in `newton/_src/sim/model.py`, is present at `v1.6.0rc1` and absent at `v1.5.1`. No `1.6.0` final exists yet, and no 1.5.x backport carries the fix, so `1.6.0rc1` is the only released option. The release requires `warp-lang>=1.17.0` and `mujoco-warp~=3.12.0`, so those pins move too: | Package | Before | After | | --- | --- | --- | | `newton[sim]` | `1.5.1` | `1.6.0rc1` | | `warp-lang` | `1.16.0` | `1.17.0` | | `mujoco` | `3.11.0` | `3.12.0` | | `mujoco-warp` | `3.11.0` | `3.12.0` | `warp-lang` is pinned to the `1.17.0` release rather than left as `>=1.17.0`: Newton's prerelease enables prerelease resolution, and an unbounded bound then resolves to a nightly (`1.18.0.dev20260903`). ## Wheel installs get the pin too `[tool.uv].override-dependencies` only reaches `uv sync`, so it does not constrain a wheel built from this metadata. `1.6.0rc1` is a prerelease, which a default resolve skips, so a loose bound resolved `newton 1.5.1` — without newton#4017, the fix this bump exists to deliver. Since `wheel.yml` builds on every push to `develop`, that would have shipped. `[project].dependencies` now pins `newton[sim]==1.6.0rc1`, matching the override. A default resolve of the new metadata returns `1.6.0rc1` under both `uv pip compile` and `pip download`, with no `--pre` flag. `isaacsim-core==6.0.1.0` requires `newton[sim]==1.2.1`, so installing the `isaacsim` extra still needs an override — exactly as on `develop`, which carries an equally exact pin in the same place. ## Newton actuator class rename `1.6.0rc1` introduces `DrivePD` / `DriveNeuralMLP` / `DriveNeuralLSTM` alongside the existing `ControllerPD` / `ControllerNeural*`. Both names remain importable, so this is **not** a breaking API change and no production code changes. Tests in `isaaclab` and `isaaclab_newton` assert on the class name Newton *constructs with*, which switched to the `Drive*` form, so those literals are updated. ## Determinism `test_manipulation_env_determinism[cuda-Isaac-Open-Drawer-Franka]` failed on this branch in CI. Two separate questions: whether the bump caused it, and why it failed at all. **Not caused by the bump.** Each arm was run 5x on the same machine, because a single run of an intermittent test samples a coin rather than measuring it: | GPU | develop pins (`newton 1.5.1` / `warp 1.16.0` / `mujoco 3.11.0`) | this PR (`newton 1.6.0rc1` / `warp 1.17.0` / `mujoco 3.12.0`) | | --- | --- | --- | | RTX PRO 6000 Blackwell | 3 / 5 failed | 3 / 5 failed | | L40 | 3 / 3 passed | 4 / 4 passed | Both pin sets fail at the same rate on Blackwell and both are clean on L40, so the failure is hardware-dependent, not version-dependent. Intermediate Newton revisions (the commits before and at [newton#4119](newton-physics/newton#4119), the MuJoCo 3.12 bump) show the same behaviour, so neither Warp 1.17, MuJoCo 3.12, nor #4119 is implicated. **Why it failed.** The test asserted bit-reproducible rewards without ever asking for a determinism guarantee. Newton defaults to `wp.DeterministicMode.NOT_GUARANTEED`, under which Warp's atomics may accumulate in any order, so two runs of the same environment are not required to agree — the assertion was stricter than the configuration it ran under, and whether it held came down to GPU scheduling. The Newton-backed case now passes `deterministic_mode="run_to_run"`, which is exactly the guarantee this test asserts: one device, one process, run twice. `deterministic_mode` is a Newton setting, so the PhysX-backed `IsaacContrib-Lift-Cube-Franka` case leaves it unset and the helper keeps asserting that the mode is only applied to a `NewtonCfg` backend. This is a test-configuration fix, not a masking of the intermittency: the guarantee the assertion depends on is now actually requested. Related: [#7561](#7561) addresses the adjacent layer and does not overlap this PR. Newton's solvers take a per-module `deterministic` argument, which is what `deterministic_mode` reaches and what this PR sets; Newton's sensor and geometry kernels take no such argument and fall back to the `warp.config.deterministic` global, which #7561 raises from the same config field in the RL entrypoint. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [ ] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Validation - `uv lock --check` — clean; the relock moves exactly the four packages above, and the `[project].dependencies` pin moves exactly one further lockfile line - Default resolve of the published metadata returns `newton==1.6.0rc1` under both `uv pip compile` and `pip download`, with no `--pre` flag - `uv run --frozen python -m pytest source/isaaclab/test/cli/test_wheel_builder_metadata.py source/isaaclab/test/cli/test_uv_run_pyproject.py` — 21 passed, which enforces that the two generated `uv-overrides.txt` files match the override table and that `[tool.isaaclab.versions]` mirrors the dependency pins - `uv run isaaclab -f` — passed - Full Docker + Tests suite — **48 passed / 0 failed** on `b184335e80b`
|
run-ci |
|
run-ci |
|
@hujc7 are you planning to address the comments around _mjwarp_view_compat.py? |
All the comments should be addressed. For the shim, it will stay as a WAR for newton 1.5.1/0, the cleanup is ready for develop branch once this gets cherry-picked |
|
Backported to |
…ive PhysX tendons (#7161) # Description The Shadow Hand shipped as two assets, one per engine, with the PhysX tendons synthesised at spawn — a per-environment patch that never completed scene construction at 4096 envs. One asset now serves both, its `Physics` USD variant selecting the engine, with the four tendons authored natively. `set_fixed_tendon_position_target_index` / `_mask` on `BaseArticulation` let a task command a tendon without naming a backend. The spawn-time tendon writers are not used here: the asset carries its four tendons natively and runtime control goes through the tensor API. Their namespace fix and per-instance selection live in #7527. Both engines now spawn the hand at the asset's own pose. PhysX previously overrode `init_state` with an identity rotation, so its hand sat differently from Newton's; removing that override leaves one placement for both. The cube starts 98 mm from the fingertips instead of 132 mm, which is a task change, not a rendering one — **existing PhysX checkpoints are not comparable across this PR.** The two engines' variants are aligned to 0.00 mm on palm, ffdistal, thdistal, forearm and wrist. > [!NOTE] > **No dependency change — this runs on the pinned Newton 1.5.1 and `uv.lock` is untouched.** > [newton#4017](newton-physics/newton#4017) makes `ArticulationView` > generic over custom frequencies, but it was deliberately not backported to the 1.5 patch series, > so it arrives with 1.6. `mjc_view_compat.py` back-fills that surface on 1.5, which keeps the > tendon adapter written once against the 1.6 API instead of carrying two code paths. It is deleted by > **#7468**, which is held until the pinned Newton reaches 1.6. Fixes # (issue) ## Type of change - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Validation Trained at each task's default environment count and iteration budget on Newton 1.5.1, through the compatibility shim: | task | success (mean last 20) | |---|---| | `Isaac-Shadow-Reorient` | 0.9617 | | `Isaac-Shadow-Reorient-Direct` | 0.9704 | | `Isaac-Shadow-Handover-Direct` | 1.0000 | | `Isaac-Shadow-Handover` | 0.8729 | On PhysX, the handover tasks completed ten of ten seeds at their default 2048 environments with no non-finite values. This PR redefines `success_rate` to test distance at episode end rather than latching on first contact, so these are not comparable with values recorded under the previous definition. Training budgets cut to the measured plateau: reorient 10000 → 3000, handover 5000 → 3500. Re-run at the shipping configuration on the current head, one arm per engine, task defaults: | task | engine | iters | success | |---|---|---|---| | `Isaac-Reorient-Cube-Shadow` | `newton_mjwarp` | 3000 | 0.9665 | | `Isaac-Reorient-Cube-Shadow` | `isaacsim_physx` | 3000 | 0.9083 | Suites across every touched package, all green: `isaaclab_physx` articulation 171, `isaaclab_newton` articulation 198, `isaaclab_ov` articulation 87 (cpu) + 84 (cuda), newton tendon control and reorient utils 21. The two fixed-tendon mask fixes each fail on the parent commit and pass on this one. ### Asset warnings measured against nvbug 6701561 That bug reports the Shadow Hand emitting 183 `material:binding` "outside the scope of the reference" warnings and 1225 `Failed to find rigid body ... /joints/robot0_forearm` warnings, one per environment. Running its own repro on `develop` reproduces both exactly; on this branch the rigid-body warnings are gone and the binding warnings drop to 90: | | `develop` | this PR | |---|---|---| | `outside the scope of the reference` | 183 (visual bindings) | 90 (physics bindings) | | `Failed to find rigid body .../robot0_forearm` | 1225 | 0 | The remaining 90 are a converter defect in the new asset, not a regression: the prototypes in `instances.usda` bind a stage-absolute physics material that USD cannot remap on reference, so it discards them. They are redundant — `base.usda` binds the same material at composed scope, and all 32 colliders resolve to it either way — so they are cosmetic. Removed from the published asset separately; that edit does not touch this PR. ## Rendering references The unified asset renders the hand in its own materials, so every baseline it feeds moved with it: the `isaaclab_visualizers` goldens, the `isaaclab_tasks` rendering goldens for `shadow_hand` and `registered_tasks/Isaac-Reorient-Cube-Shadow-Camera-Direct`, and that task's golden USD stage. Pixel references were taken from the rendering suites' own `comparison-images` artifacts rather than regenerated locally: they do not reproduce across GPU models, and a local L40 leaves the shaded outputs about nine percent different from the L40S the runners use while every geometric output matches exactly. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit e0d2fe0)
Description
The Shadow Hand shipped as two assets, one per engine, with the PhysX tendons synthesised at
spawn — a per-environment patch that never completed scene construction at 4096 envs. One asset
now serves both, its
PhysicsUSD variant selecting the engine, with the four tendons authorednatively.
set_fixed_tendon_position_target_index/_maskonBaseArticulationlet a taskcommand a tendon without naming a backend.
The spawn-time tendon writers are not used here: the asset carries its four tendons natively and
runtime control goes through the tensor API. Their namespace fix and per-instance selection live in
#7527.
Both engines now spawn the hand at the asset's own pose. PhysX previously overrode
init_statewith an identity rotation, so its hand sat differently from Newton's; removing that override
leaves one placement for both. The cube starts 98 mm from the fingertips instead of 132 mm, which
is a task change, not a rendering one — existing PhysX checkpoints are not comparable across
this PR. The two engines' variants are aligned to 0.00 mm on palm, ffdistal, thdistal, forearm
and wrist.
Note
No dependency change — this runs on the pinned Newton 1.5.1 and
uv.lockis untouched.newton#4017 makes
ArticulationViewgeneric over custom frequencies, but it was deliberately not backported to the 1.5 patch series,
so it arrives with 1.6.
mjc_view_compat.pyback-fills that surface on 1.5, which keeps thetendon adapter written once against the 1.6 API instead of carrying two code paths. It is deleted by
#7468, which is held until the pinned Newton reaches 1.6.
Fixes # (issue)
Type of change
Release backport
developValidation
Trained at each task's default environment count and iteration budget on Newton 1.5.1, through the
compatibility shim:
Isaac-Shadow-ReorientIsaac-Shadow-Reorient-DirectIsaac-Shadow-Handover-DirectIsaac-Shadow-HandoverOn PhysX, the handover tasks completed ten of ten seeds at their default 2048 environments with no
non-finite values.
This PR redefines
success_rateto test distance at episode end rather than latching on firstcontact, so these are not comparable with values recorded under the previous definition. Training
budgets cut to the measured plateau: reorient 10000 → 3000, handover 5000 → 3500.
Re-run at the shipping configuration on the current head, one arm per engine, task defaults:
Isaac-Reorient-Cube-Shadownewton_mjwarpIsaac-Reorient-Cube-Shadowisaacsim_physxSuites across every touched package, all green:
isaaclab_physxarticulation 171,isaaclab_newtonarticulation 198,
isaaclab_ovarticulation 87 (cpu) + 84 (cuda), newtontendon control and reorient utils 21. The two fixed-tendon mask fixes each fail on the parent commit and pass on this one.
Asset warnings measured against nvbug 6701561
That bug reports the Shadow Hand emitting 183
material:binding"outside the scope of thereference" warnings and 1225
Failed to find rigid body ... /joints/robot0_forearmwarnings, oneper environment. Running its own repro on
developreproduces both exactly; on this branch therigid-body warnings are gone and the binding warnings drop to 90:
developoutside the scope of the referenceFailed to find rigid body .../robot0_forearmThe remaining 90 are a converter defect in the new asset, not a regression: the prototypes in
instances.usdabind a stage-absolute physics material that USD cannot remap on reference, so itdiscards them. They are redundant —
base.usdabinds the same material at composed scope, and all32 colliders resolve to it either way — so they are cosmetic. Removed from the published asset
separately; that edit does not touch this PR.
Rendering references
The unified asset renders the hand in its own materials, so every baseline it feeds moved with it:
the
isaaclab_visualizersgoldens, theisaaclab_tasksrendering goldens forshadow_handandregistered_tasks/Isaac-Reorient-Cube-Shadow-Camera-Direct, and that task's golden USD stage.Pixel references were taken from the rendering suites' own
comparison-imagesartifacts ratherthan regenerated locally: they do not reproduce across GPU models, and a local L40 leaves the
shaded outputs about nine percent different from the L40S the runners use while every geometric
output matches exactly.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there