Fix Newton self-collisions being uncontrolled via the deprecated PhysX cfg

[AntoineRichard]

Summary

• ArticulationRootPropertiesCfg/PhysxArticulationRootPropertiesCfg.enabled_self_collisions only authored physxArticulation:enabledSelfCollisions. Newton's schema resolver checks newton:selfCollisionEnabled first and only falls back to the PhysX attribute when it's unauthored, so the deprecated cfg's setting silently never reached Newton simulations.
• modify_articulation_root_properties now mirrors enabled_self_collisions onto newton:selfCollisionEnabled (applying NewtonArticulationRootAPI) whenever it's set, so the deprecated cfg controls self-collisions on both backends.
• Migrated ALLEGRO_HAND_CFG, SHADOW_HAND_CFG, SHADOW_HAND_NEWTON_CFG, and KUKA_ALLEGRO_CFG off the deprecated cfg to explicit PhysxArticulationCfg + NewtonArticulationCfg fragments. Several of the reorientation/handover tasks built on these hands (Isaac-Reorient-Cube-Allegro*, Isaac-Reorient-Cube-Shadow-Direct*, Isaac-Shadow-Handover-Direct) default to the newton_mjwarp preset, so this was a real training-time gap, not just a hypothetical one.
• Adds isaaclab_physx/isaaclab_newton as declared dependencies of isaaclab_assets (needed for the fragment imports above).

Test plan

• test_physx_articulation_root_writes_self_collisions extended to assert newton:selfCollisionEnabled / NewtonArticulationRootAPI are authored; confirmed it fails without the fix and passes with it.
• source/isaaclab/test/sim/test_schemas.py (43 tests) and test_articulation_fragments.py (26 tests) pass.
• source/isaaclab_assets test suite passes (test_asset_configs loads all three migrated robot configs).
• test_initialization_hand_with_tendons (Shadow Hand, PhysX backend) passes with the fragment-based articulation_props.
• uv run isaaclab -f passes (formatting, changelog fragments).

[greptile-apps]

Greptile Summary

The PR fixes deprecated self-collision configuration under Newton and migrates affected hand assets to backend-specific articulation fragments.

• Mirrors legacy enabled_self_collisions values to the Newton articulation schema.
• Migrates Allegro, Kuka-Allegro, and Shadow Hand configurations to explicit PhysX and Newton fragments.
• Declares the corresponding backend packages as isaaclab_assets dependencies and extends schema coverage.

Confidence Score: 5/5

The PR appears safe to merge with no concrete blocking or non-blocking defects identified.

The compatibility shim follows the established schema-authoring pattern, the fragment lists are supported by the spawner contract, and affected assets retain their prior PhysX settings while gaining an explicit Newton setting.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/sim/schemas/schemas.py	Mirrors the deprecated self-collision setting onto the Newton schema while preserving the existing PhysX write.
source/isaaclab/test/sim/test_schemas.py	Extends regression coverage to verify both backend attributes and API schemas are authored.
source/isaaclab_assets/isaaclab_assets/robots/allegro.py	Replaces the legacy articulation configuration with explicit PhysX and Newton fragments.
source/isaaclab_assets/isaaclab_assets/robots/kuka_allegro.py	Preserves PhysX solver settings while explicitly enabling Newton self-collisions.
source/isaaclab_assets/isaaclab_assets/robots/shadow_hand.py	Migrates both Shadow Hand variants to supported backend-specific articulation fragments.
source/isaaclab_assets/pyproject.toml	Declares the backend packages required by the migrated robot configuration imports.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  C["Hand articulation configuration"] --> P["PhysxArticulationCfg"]
  C --> N["NewtonArticulationCfg"]
  P --> PA["physxArticulation:enabledSelfCollisions"]
  N --> NA["newton:selfCollisionEnabled"]
  L["Deprecated articulation configuration"] --> S["Compatibility shim"]
  S --> PA
  S --> NA

Loading

_{Reviews (1): Last reviewed commit: "Merge branch 'develop' into antoiner/fix..." | Re-trigger Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

The Newton compatibility fix is directionally correct, but the shared writer mirrors self-collision settings for every configuration exposing enabled_self_collisions, not only the deprecated compatibility configurations targeted by the PR.

• Design and architecture: The backend-agnostic writer now lets a non-deprecated PhysxArticulationCfg author Newton schema state. This creates cross-backend coupling and makes conflicting PhysX/Newton fragments order-dependent. The compatibility mirror should be restricted to the deprecated configuration types.
• API: modify_articulation_root_properties gains broader behavior than documented: a PhysX-only fragment can apply NewtonArticulationRootAPI and set newton:selfCollisionEnabled. The public asset fragment lists are otherwise a supported configuration form.
• Implementation: The non-None guard, idempotent schema application, explicit attribute name, and regression assertion are appropriate. The type or configuration scope of the mirror must be narrowed so explicit backend fragments remain isolated and their values cannot overwrite one another based on ordering.

Minor fixes needed. Posted 1 actionable finding inline.

Automated review; human maintainers own approval decisions.

[isaaclab-review-bot]

🟡 Warning · Design Architecture — Newton mirror also fires for PhysX fragment

The shim keys on the field name, not on the deprecated cfg type, so it also runs for the non-deprecated PhysxArticulationCfg fragment introduced here, stamping NewtonArticulationRootAPI and authoring newton:selfCollisionEnabled from a PhysX-namespaced cfg. With the new [PhysxArticulationCfg, NewtonArticulationCfg] list form the outcome becomes order-dependent whenever the two disagree (e.g. Newton self_collision_enabled=False silently overwritten). Gate the mirror on the deprecated cfg types only.

[ooctipus]

I wonder would this not work if just delete it, the physx resolver should pick it up?

[AntoineRichard]

PhysX picks it up, but newton does not, the issue is that if there is a newton authored property, then it takes precendence over the PhysX one.

[marcodiiga]

[P1] Avoid removing referenced property specs during relocation — With a standard UsdFileCfg reference, these Newton attributes are authored in the referenced layer. FlattenTo(parent_prim) succeeds, but articulation_prim.RemoveProperty(...) edits only the current target and returns false, so this block raises and aborts spawning. I reproduced this with public usd-core==25.11 and newton-usd-schemas==0.4.1. The shared PhysicsManager._relocate_articulation_root removes the applied schema but deliberately does not remove individual properties. Please reuse that helper or omit these removals and add a referenced-layer regression. Also account for raw apiSchemas metadata: before codeless registration, GetAppliedSchemas() hides the Newton token, which can leave it on the child and produce two roots when the stage is reopened with the schema registered.

[marcodiiga]

[P1] Do not overwrite an explicit Newton value from the supported PhysX cfg — This writer receives the non-deprecated PhysxArticulationRootPropertiesCfg as well as the deprecated alias. If an asset authors newton:selfCollisionEnabled=False, setting the PhysX field to true now silently replaces that Newton choice, although the class documents one cfg/namespace and NewtonArticulationRootPropertiesCfg owns the Newton side. Since the affected assets are already migrated to an explicit PhysX + Newton fragment pair, please either scope this compatibility mirror to the deprecated alias or document the supported PhysX cfg as dual-authoring and add a precedence regression.

[marcodiiga]

[P2] Regenerate the lockfile for these package edges — The committed isaaclab-assets block in uv.lock still lists only isaaclab and isaaclab-contrib, so package-specific frozen sync/export metadata does not record these two new requirements. The branch and current develop both need a lock refresh, but these missing dependency edges are introduced here; please regenerate uv.lock (or coordinate the refresh with the current base).

[marcodiiga]

[P2] Document the public config-shape migration — These exported hand configs change spawn.articulation_props from a cfg object to a list. Downstream code that customizes them through cfg.spawn.articulation_props.enabled_self_collisions = ... now raises AttributeError. The fragment pair is the intended abstraction, but the changelog should mark the shape change and show how to update or replace the appropriate fragment.

[marcodiiga]

Reviewed current head 01257e8. No P0 or critical release blocker found, so approving. The PhysX + Newton fragment pair is the repository’s intended extensible abstraction: subclasses are accepted and each fragment writes an isolated namespace. The Newton schema is supplied by the existing Apache-2.0 codeless newton-usd-schemas package; this PR embeds no schema code or data. I left actionable comments for referenced/unregistered-schema relocation, mirror scope, lock metadata, and migration guidance. The tests verify USD authoring and root placement; an end-to-end Newton collision assertion remains worthwhile.