[USD] Select tendon instances per fragment and fix the physxTendon attribute namespace by hujc7 · Pull Request #7527 · isaac-sim/IsaacLab

hujc7 · 2026-09-03T08:51:08Z

Summary

A PhysX tendon fragment now names the instances it tunes (instance_names), so two tendons rooted on one joint can be configured independently; the writer is a core applier driven by static cfg data and runs under Kit PhysX and OVPhysX alike.
Tendon properties written at spawn land in physxTendon:<instance>:*, the namespace PhysX reads. Both writers spelled PhysxTendonAxisRootAPI:<instance>:*, which PhysX ignores, so spawn-time tendon tuning never took effect.

Description

Split out of #7161, where the namespace defect surfaced; that PR's asset carries its tendons natively and does not use these writers.

MultiApplyFragment (core mixin, one field) adds the instance coordinate, instance_names ("x", ["x", "y"], None = every instance; unset or [] raise); schema and namespace stay on the base metadata (_usd_applied_schema, _usd_namespace). apply_schema_instances (core) selects the prim's <Schema>:<instance> tokens by that field and writes <namespace>:<instance>:<camelCase(field)>. No schema-registry lookup at runtime: OVPhysX ships PhysxSchema as a codeless plugin, so typed accessors (PhysxTendonAxisRootAPI.Get) exist only under Kit. A Kit test checks the cfg data against the schema definition instead.
PhysxFixedTendonCfg / PhysxSpatialTendonCfg mix it in, set func = apply_schema_instances and gain lower_limit / upper_limit; apply_fixed_tendon / apply_spatial_tendon are removed (they only bound the cfgs to a PhysX-side writer). One fragment per API: the root cfg carries tendon-wide dynamics; per-axis coupling stays asset-authored.
Legacy modify_*_tendon_properties keep whole-subtree broadcast and write the corrected namespace; the spatial writer targets attachment roots only.

Fixes # (none)

Type of change

Bug fix
New feature
Breaking change: instance_names is required on the two PhysX tendon fragments (migration: instance_names=None); apply_fixed_tendon / apply_spatial_tendon removed.

Release backport

Backport this pull request to the active release branch after it merges into develop

Test plan

Kit: source/isaaclab/test/sim/test_tendon_fragments.py, 29 passed (CI image latest-develop)
Kitless: source/isaaclab/test/sim/test_multi_apply_fragments.py, 9 passed
isaaclab -f clean

Checklist

I have read and understood the contribution guidelines
I have run the pre-commit checks with ./isaaclab.sh --format
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump extension.toml — CI handles that)
I have added my name to the CONTRIBUTORS.md or my name already exists there

…amespace A fixed- or spatial-tendon fragment now carries the instance coordinate of the multiple-apply schema it tunes (`instance_names`), so two tendons rooted on the same joint can be configured independently instead of every instance being written. The mechanism is a core mixin, `MultiApplyFragment`, plus one applier, `apply_multi_apply`, driven by static cfg data (schema token, attribute template, selection). It does not consult the schema registry at runtime: OVPhysX ships PhysxSchema as a codeless plugin, so typed accessors exist only under Kit. A Kit test checks the cfg data against the schema definition instead. Both tendon writers spelled attribute names from the schema class name (`PhysxTendonAxisRootAPI:<inst>:stiffness`), which PhysX never reads; they now write `physxTendon:<inst>:stiffness`. The spatial writer targets attachment roots only, since leaves do not declare the tendon dynamics. `apply_fixed_tendon` / `apply_spatial_tendon` are removed: they only bound the fragments to a PhysX-side writer that no longer exists. `lower_limit` and `upper_limit` complete the root API's tuning properties.

hujc7 · 2026-09-03T08:52:22Z

run-ci

greptile-apps · 2026-09-03T09:10:00Z

Greptile Summary

The PR introduces a generic USD multiple-apply fragment applier and uses it to select individual PhysX tendon instances while correcting tendon property namespaces. It also limits spatial-tendon dynamics writes to attachment roots, adds fixed-tendon limits, removes superseded PhysX-specific appliers, and adds focused Kit and Kitless coverage.

Adds MultiApplyFragment, apply_multi_apply, and applied-schema instance resolution.
Routes fixed and spatial PhysX tendon fragments through instance-aware attribute writing.
Corrects authored attributes to physxTendon:<instance>:<property>.
Adds lower_limit and upper_limit configuration fields.
Updates exports, API documentation, changelogs, and regression tests.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete correctness, security, or compatibility defect remaining after review.

The instance-aware applier validates explicit selection, writes schema-verified attribute names, preserves family-writer traversal semantics, and is covered across selected, broadcast, missing, root-only, and legacy parity cases.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/sim/schemas/schemas.py	Adds generic multiple-apply instance selection and corrects legacy fixed and spatial tendon attribute writing and traversal status.
source/isaaclab/isaaclab/sim/schemas/schemas_cfg.py	Introduces the multiple-apply fragment contract and its explicit instance-selection field.
source/isaaclab_physx/isaaclab_physx/sim/schemas/schemas_cfg.py	Migrates PhysX tendon fragments to the generic applier and adds fixed-tendon length limits.
source/isaaclab_physx/isaaclab_physx/sim/schemas/schemas.py	Removes the superseded backend-specific tendon appliers while retaining the PhysX joint writer.
source/isaaclab/test/sim/test_multi_apply_fragments.py	Covers instance filtering, broadcast selection, invalid selections, missing schemas, and generic attribute templates.
source/isaaclab/test/sim/test_tendon_fragments.py	Verifies corrected PhysX namespaces, root-only spatial writes, subtree behavior, schema metadata, and legacy-fragment parity.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    C[Tendon fragment configuration] --> S{instance_names}
    S -->|specific name or list| F[Filter applied schema instances]
    S -->|None| A[Select every applied instance]
    F --> W[Format schema-owned attribute name]
    A --> W
    W --> U[Author physxTendon instance property on USD prim]
    U --> P[PhysX reads configured tendon dynamics]

_{Reviews (1): Last reviewed commit: "Select tendon schema instances per fragm..." | Re-trigger Greptile}

isaaclab-review-bot

Isaac Lab Review Bot

The multi-apply tendon design and corrected physxTendon:<instance>:* namespace are coherent, but the PR removes two public PhysX appliers without deprecation and omits required SI units from two new public fields.

Design and architecture: The core MultiApplyFragment and apply_multi_apply abstraction preserves the core/backend dependency direction while allowing PhysX fragments to provide static schema and attribute-template metadata. Instance selection and root-only spatial tendon tuning align with the changed producer paths and tests.
API: The new core exports and instance_names migration are documented. However, apply_fixed_tendon and apply_spatial_tendon are removed from isaaclab_physx.sim.schemas in one step despite previously being exported public APIs and valid callable strings. Retain deprecated forwarding wrappers before removal. Also document [m] on the new public PhysxFixedTendonPropertiesCfg.lower_limit and upper_limit fields.
Implementation: Instance resolution, selection validation, template formatting, corrected tendon namespaces, and spatial-root filtering are consistently implemented and covered by focused tests. The remaining implementation work is limited to compatibility wrappers and unit annotations.

Minor fixes needed. Posted 2 actionable findings inline.

Automated review; human maintainers own approval decisions.

isaaclab-review-bot · 2026-09-03T09:12:56Z

-:mod:`isaaclab.sim.schemas`. This module additionally hosts the PhysX-specific fragment
-applier funcs that override :attr:`~isaaclab.sim.schemas.SchemaFragment.func` for the
-joint-drive and multi-instance tendon schemas, keeping the backend func out of the core package.
+:mod:`isaaclab.sim.schemas`. This module hosts the PhysX joint-drive applier, which keeps the


🟡 Warning · Api — Public tendon appliers removed without deprecation

apply_fixed_tendon and apply_spatial_tendon were exported from isaaclab_physx.sim.schemas (module __all__ and the package stub) and were the default func string on the two tendon fragments. Deleting them outright breaks external imports and any stored "isaaclab_physx.sim.schemas:apply_fixed_tendon" string at string_to_callable time, contrary to the rule requiring a prior deprecation and migration path. Keep thin wrappers that delegate to apply_multi_apply and emit a DeprecationWarning.

isaaclab-review-bot · 2026-09-03T09:12:56Z

    """Spring rest length of the tendon."""

+    lower_limit: float | None = None
+    """Lower limit of the tendon's length."""


🔵 Suggestion · Api — Document units on new tendon limit fields

The new public lower_limit/upper_limit on PhysxFixedTendonPropertiesCfg are tendon lengths but their docstrings omit the SI unit, while the identical fields on PhysxFixedTendonCfg document [m]. Repository guidelines require inline [unit] notation for public physical quantities; annotate both as [m] so the two cfg surfaces agree.

The multiple-apply schema work (instance selection on the tendon fragments, the attribute-namespace fix in both writers, and the schema helpers) is not used by the Shadow Hand migration: the asset carries its tendons natively and runtime control goes through the tensor API. It continues in isaac-sim#7527. The schema modules, their tests and docs return to their develop state. The tendon-naming change in the PhysX and OVPhysX articulations stays; it now parses the applied-schema tokens inline instead of importing the helper that moved with the other PR.

…adata The two PhysX tendon fragments now carry the multiple-apply data the way every other fragment carries its metadata: `_usd_applied_schema` names the schema whose instances are tuned (it is never applied here) and `_usd_namespace` the attribute namespace, written as `<namespace>:<instance>:<camelCase(field)>`. Each declares its `instance_names` field and points `func` at `apply_multi_apply`, so the hierarchy stays single-inheritance and the public additions are the applier and `resolve_applied_schema_instances`. The kitless test moves from `PhysicsDriveAPI`, whose layout put the instance mid-name, to core USD's `CollectionAPI`, which lays out attributes like the PhysX tendon schemas do.

…ti-apply-instances

hujc7 · 2026-09-03T10:18:55Z

run-ci

… mixin `apply_schema_instances` replaces `apply_multi_apply`, whose name said "apply" in two senses. `MultiApplyFragment` returns as a one-field mixin holding only `instance_names`: the concept is generic to multiple-apply schemas, so it does not belong on a tendon cfg, while schema and namespace stay on the base metadata as for every fragment and `func` stays explicit on the concrete cfgs. The applier types its parameter on the mixin and derives the non-property fields from it instead of spelling them. The applied-schema token parser becomes private: its only callers are the three writers in this module. Two Kit selection tests that duplicated the kitless coverage are dropped in favour of the subtree test.

hujc7 · 2026-09-03T10:56:58Z

run-ci

…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 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

…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 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)

github-actions Bot added documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team labels Sep 3, 2026

isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 3, 2026

hujc7 marked this pull request as ready for review September 3, 2026 09:07

hujc7 requested a review from a team September 3, 2026 09:07

hujc7 requested review from AntoineRichard, StafaH, fatimaanes, kellyguo11, ooctipus and pbarejko as code owners September 3, 2026 09:07

isaaclab-review-bot Bot reviewed Sep 3, 2026

View reviewed changes

hujc7 mentioned this pull request Sep 3, 2026

[Task Clean-up][Assets] Serve the Shadow Hand from one asset with native PhysX tendons #7161

Merged

8 tasks

hujc7 added 2 commits September 3, 2026 03:18

Merge remote-tracking branch 'upstream/develop' into jichuanh/usd-mul…

aa38e96

…ti-apply-instances

isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 3, 2026

ooctipus mentioned this pull request Sep 3, 2026

[USD] Make PhysX tendon fragments schema- and instance-specific #7534

Merged

5 tasks

isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 3, 2026

hujc7 closed this Sep 4, 2026

Navigation Menu

Uh oh!

[USD] Select tendon instances per fragment and fix the physxTendon attribute namespace - #7527

[USD] Select tendon instances per fragment and fix the physxTendon attribute namespace#7527
hujc7 wants to merge 4 commits into
isaac-sim:developfrom
hujc7:jichuanh/usd-multi-apply-instances

hujc7 commented Sep 3, 2026 •

edited

Loading

Uh oh!

hujc7 commented Sep 3, 2026

Uh oh!

greptile-apps Bot commented Sep 3, 2026

Uh oh!

isaaclab-review-bot Bot left a comment

Uh oh!

isaaclab-review-bot Bot Sep 3, 2026

Uh oh!

isaaclab-review-bot Bot Sep 3, 2026

Uh oh!

hujc7 commented Sep 3, 2026

Uh oh!

hujc7 commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hujc7 commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Description

Type of change

Release backport

Test plan

Checklist

Uh oh!

hujc7 commented Sep 3, 2026

Uh oh!

greptile-apps Bot commented Sep 3, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

isaaclab-review-bot Bot left a comment

Choose a reason for hiding this comment

Isaac Lab Review Bot

Uh oh!

isaaclab-review-bot Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

isaaclab-review-bot Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

hujc7 commented Sep 3, 2026

Uh oh!

hujc7 commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hujc7 commented Sep 3, 2026 •

edited

Loading