Skip to content

Author fixed-root-link world joint with pure USD#6536

Open
rebeccazhang0707 wants to merge 5 commits into
isaac-sim:developfrom
rebeccazhang0707:reb/schemas_kitless_fixed_joint
Open

Author fixed-root-link world joint with pure USD#6536
rebeccazhang0707 wants to merge 5 commits into
isaac-sim:developfrom
rebeccazhang0707:reb/schemas_kitless_fixed_joint

Conversation

@rebeccazhang0707

@rebeccazhang0707 rebeccazhang0707 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

modify_articulation_root_properties created the fixed-root-link world joint through omni.physx.scripts.utils.createJoint, so spawning a fixed-base articulation (fix_root_link=True) raised ModuleNotFoundError: omni.physx on kitless backends (e.g. Newton) where Kit and PhysX extensions are not loaded.

This PR replaces that call with a pure-USD _create_world_fixed_joint helper that reproduces the behavior of omni.physx.createJoint for the single-body (world-attached) case: it targets only body1 (the root link), anchors localPos0/localRot0 at the root link's world pose with scale/shear removed, sets effectively unbreakable break force/torque, walks up to the first writable ancestor when the root link lives inside an instanceable/prototype subtree, and uniquifies the joint name on collision. The same code path now runs on every backend, so Kit/PhysX behavior is unchanged.

Regression tests run on an in-memory USD stage without launching Isaac Sim / Kit, covering the world-anchored joint attributes and the instanceable-root fallback. Since this helper is the single fixed-root-link authoring path for all backends, passing without Kit also proves the kitless case.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

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
Replace the omni.physx.scripts.utils.createJoint call in
modify_articulation_root_properties with a pure-USD equivalent so the
fixed-root-link spawn path no longer depends on omni.physx. This fixes
a ModuleNotFoundError when spawning fixed-base articulations on kitless
backends (e.g. Newton), where omni.physx is unavailable.

Add regression tests on an in-memory USD stage covering the
world-anchored joint attributes and the instanceable-root fallback,
plus a changelog fragment.
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Jul 15, 2026
@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the omni.physx.scripts.utils.createJoint call in modify_articulation_root_properties with a self-contained _create_world_fixed_joint helper that authors the same UsdPhysics.FixedJoint using only public USD APIs, removing the hard omni.physx import that caused ModuleNotFoundError on kitless backends such as Newton.

  • _create_world_fixed_joint replicates the original behavior faithfully: body0 is left unset (world frame), body1 targets the articulation root, localPos0/localRot0 are derived from the root's world pose, and break thresholds are set to FLT_MAX. Instanceable/prototype ancestor traversal mirrors the original createJoint logic.
  • Two new in-memory-stage regression tests verify the world-anchored joint attributes and the instanceable-root fallback without requiring a running Isaac Sim/Kit process.

Confidence Score: 4/5

The production code in schemas.py is correct and safe to merge; the new helper faithfully reproduces the omni.physx joint-authoring logic using only public USD APIs.

The core fix is straightforward: every joint attribute matches the original createJoint call, the instanceable-ancestor walk-up is correct, and break thresholds replicate FLT_MAX. The only gaps are in the test file — the localRot0 extraction path is never exercised with a non-identity rotation, and the uniquifier deduplication loop has no test coverage. Neither gap affects the production code path.

source/isaaclab/test/sim/test_schemas_kitless_fixed_joint.py — missing coverage for the rotation extraction and uniquifier branches.

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/sim/schemas/schemas.py Replaces omni.physx.scripts.utils.createJoint with a pure-USD _create_world_fixed_joint helper; import of Gf added; all joint attributes match the original call's intent.
source/isaaclab/test/sim/test_schemas_kitless_fixed_joint.py New kitless-safe regression tests on an in-memory USD stage; covers world-anchored joint attributes and instanceable-root fallback, but the localRot0 path and uniquifier logic are not exercised.
source/isaaclab/changelog.d/schemas-kitless-fixed-joint.rst Changelog fragment correctly describes the fix and references the affected public function.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[modify_articulation_root_properties] --> B{fix_root_link set?}
    B -- No --> Z[return True]
    B -- Yes --> C{existing fixed joint?}
    C -- Yes --> D[enable/disable existing joint]
    C -- No, fix_root_link=True --> E{prim has RigidBodyAPI?}
    E -- No --> F[raise NotImplementedError]
    E -- Yes --> G[_create_world_fixed_joint]
    G --> H{prim instanceable or in prototype?}
    H -- Yes --> I[walk up to first writable ancestor]
    H -- No --> J[use articulation prim as base]
    I --> K{base == pseudo-root?}
    K -- Yes --> L[joint_base_path = empty]
    K -- No --> M[joint_base_path = ancestor path]
    J --> N[joint_base_path = articulation prim path]
    L & M & N --> O{FixedJoint name taken?}
    O -- No --> P[UsdPhysics.FixedJoint.Define]
    O -- Yes --> Q[increment uniquifier until free]
    Q --> P
    P --> R[Set body1, localPos0, localRot0, localPos1, localRot1, breakForce, breakTorque]
    R --> S[move ArticulationRootAPI to parent prim]
    S --> Z
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[modify_articulation_root_properties] --> B{fix_root_link set?}
    B -- No --> Z[return True]
    B -- Yes --> C{existing fixed joint?}
    C -- Yes --> D[enable/disable existing joint]
    C -- No, fix_root_link=True --> E{prim has RigidBodyAPI?}
    E -- No --> F[raise NotImplementedError]
    E -- Yes --> G[_create_world_fixed_joint]
    G --> H{prim instanceable or in prototype?}
    H -- Yes --> I[walk up to first writable ancestor]
    H -- No --> J[use articulation prim as base]
    I --> K{base == pseudo-root?}
    K -- Yes --> L[joint_base_path = empty]
    K -- No --> M[joint_base_path = ancestor path]
    J --> N[joint_base_path = articulation prim path]
    L & M & N --> O{FixedJoint name taken?}
    O -- No --> P[UsdPhysics.FixedJoint.Define]
    O -- Yes --> Q[increment uniquifier until free]
    Q --> P
    P --> R[Set body1, localPos0, localRot0, localPos1, localRot1, breakForce, breakTorque]
    R --> S[move ArticulationRootAPI to parent prim]
    S --> Z
Loading

Reviews (1): Last reviewed commit: "Author fixed-root-link world joint with ..." | Re-trigger Greptile

Comment on lines +40 to +60
"""Authoring must materialize a ``UsdPhysics.FixedJoint`` anchored to the world.

A world-attached fixed joint targets only ``body1`` (the root link), leaving ``body0``
empty, and anchors ``localPos0`` at the root link's world translation.
"""
stage = Usd.Stage.CreateInMemory()
prim = _make_root_prim(stage, "/World/Robot", translation=(1.0, 2.0, 3.0))

_create_world_fixed_joint(prim, stage)

joint = _find_fixed_joint(stage)
assert joint is not None, "no UsdPhysics.FixedJoint was authored"
# world-attached joint: body0 empty, body1 -> root link
assert joint.GetBody0Rel().GetTargets() == []
assert joint.GetBody1Rel().GetTargets() == [prim.GetPath()]
# localPos0 anchors the joint at the root link's world pose
local_pos0 = joint.GetLocalPos0Attr().Get()
assert math.isclose(local_pos0[0], 1.0) and math.isclose(local_pos0[1], 2.0) and math.isclose(local_pos0[2], 3.0)
# the joint must be effectively unbreakable
assert joint.GetBreakForceAttr().Get() > 1e37
assert joint.GetBreakTorqueAttr().Get() > 1e37

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 localRot0 code path is untested

Both test functions use a prim with a zero or identity rotation, so joint.GetLocalRot0Attr().Get() is always the identity quaternion and the Gf.Quatf(world_pose.ExtractRotationQuat()) branch is never exercised. If the rotation-extraction logic regresses (e.g. wrong matrix decomposition order, inadvertent RemoveScaleShear stripping the rotation for scaled parents), no test would catch it. Adding a third test that translates and rotates the prim, then asserts the authored localRot0 matches the expected quaternion, would close this gap.

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!

Comment on lines +63 to +83
def test_create_world_fixed_joint_skips_instanceable_root():
"""The joint must be authored on a writable ancestor when the root prim is instanceable.

Instanceable/instance-proxy/prototype prims are not authorable, so the joint prim must
be created under the first writable ancestor rather than under the instanceable root.
"""
stage = Usd.Stage.CreateInMemory()
UsdGeom.Xform.Define(stage, "/World")
prim = _make_root_prim(stage, "/World/Robot", translation=(0.0, 0.0, 0.0))
prim.SetInstanceable(True)
assert prim.IsInstanceable()

_create_world_fixed_joint(prim, stage)

joint = _find_fixed_joint(stage)
assert joint is not None, "no UsdPhysics.FixedJoint was authored"
# the joint prim must not be parented under the instanceable root
joint_path = joint.GetPrim().GetPath().pathString
assert not joint_path.startswith("/World/Robot/"), f"joint authored under instanceable root: {joint_path}"
# it still targets the root link
assert joint.GetBody1Rel().GetTargets() == [prim.GetPath()]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Uniquifier logic not tested

The name-deduplication loop inside _create_world_fixed_joint (FixedJointFixedJoint0 → …) is never exercised. A test that calls _create_world_fixed_joint twice on the same parent prim and asserts that two distinct joint prims exist with unique paths would cover this branch and protect against off-by-one regressions in the uniquifier counter.

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!

ooctipus added 4 commits July 15, 2026 15:53
…d_joint

fix_articulation_root reimplemented the same joint authoring logic that
_create_world_fixed_joint provides, without the instanceable-prim walk-up
or unbreakable break forces. Delegate to _create_world_fixed_joint so
both code paths share one implementation.
The local import inside fix_articulation_root was cargo-culted from
cfg-only modules that must defer Kit imports. PhysicsManager is not
imported before Kit, so a top-level import is correct and preferred.
Two issues fixed:

- Importing a `_`-prefixed symbol across module boundaries is bad design.
  Rename to `create_world_fixed_joint` (no underscore) so callers can
  import it without bypassing the privacy convention.

- The top-level import added in the previous commit was wrong: physics_manager
  is pulled in before the simulation app starts (env cfg -> manager_base ->
  PhysicsManager), so top-level imports of USD/omni modules break config
  loading. Restore the local import pattern and update the comment to
  document this constraint explicitly.
The local-import pattern with # noqa: PLC0415 was cargo-cult: cfg files
(ManagerBasedRLEnvCfg etc.) do not import physics_manager.py — they
reach manager_term_cfg.py via lazy_export, which never touches
manager_base.py. There is no pre-Kit import constraint here.

Move pxr, create_world_fixed_joint, and find_global_fixed_joint_prim to
the module-level import block and remove the now-incorrect comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

2 participants