Articulation Reordering Part 6/8: OVPhysX backend#6339
Conversation
01f4fb8 to
5299b96
Compare
… utilities (#6334) # Description Moves the rigid object and rigid object collection interface test factories into shared underscore-prefixed utility modules so later parts can reuse them. Pure code movement with no behavior change and no ordering content. Part 1/8 of the series splitting #6248 into independently reviewable units. #6248 remains open, untouched, as the reference implementation; this stack was cut from its final tree after a `develop` sync (Kamino Core, Newton bump, OVPhysX 0.5.1). ## Stack - **Part 1/8 (this PR)**: #6334 — Extract rigid object interface test utilities - Part 2/8: #6335 — Core ordering foundation - Part 3/8: #6336 — Newton backend - Part 4/8: #6337 — PhysX backend - Part 5/8: #6338 — Extract articulation interface test utilities - Part 6/8: #6339 — OVPhysX backend - Part 7/8: #6340 — Cross-backend ordering interface tests - Part 8/8: #6341 — Events, consumers, and documentation ## Type of change - Test/documentation/refactor change ## 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 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] My name exists in `CONTRIBUTORS.md` ## Test Plan - [x] `./isaaclab.sh -p -m pytest source/isaaclab/test/assets/test_rigid_object_iface.py` -> 1478 passed, 12 xfailed - [x] `./isaaclab.sh -p -m pytest source/isaaclab/test/assets/test_rigid_object_collection_iface.py` -> 1902 passed; the 7 failures + 636 errors are pre-existing on `develop` in the local environment (OVPhysX 0.4.13 wheel vs the 0.5.x binding API from #6314) — fail-set identical to the `develop` baseline - [x] `./isaaclab.sh -f` -> all hooks pass --------- Signed-off-by: Antoine RICHARD <antoiner@nvidia.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
c61b283 to
ffdf559
Compare
# Description Introduces the public joint/body ordering machinery in `isaaclab` core: the `ArticulationNameMap` type and builder, the ordering preset helper, the Warp reorder kernels with an explicit visibility contract, the symbolic convention resolvers (`physx`, `mjwarp`, `robot_schema`), the `ArticulationCfg.joint_ordering`/`body_ordering` fields, the `BaseArticulation`/`BaseArticulationData` plumbing that backends wire into, and the API reference entries. **Transitional semantics:** no backend consumes the machinery yet, so configuring an ordering in this intermediate state is a silent no-op until Parts 3/4/6 land. This is called out in the cfg field docstrings and resolves quickly as the stack merges. Part 2/8 of the series splitting #6248 into independently reviewable units. #6248 remains open, untouched, as the reference implementation; this stack was cut from its final tree after a `develop` sync (Kamino Core, Newton bump, OVPhysX 0.5.1). ## Stack - Part 1/8: #6334 — Extract rigid object interface test utilities - **Part 2/8 (this PR)**: #6335 — Core ordering foundation - Part 3/8: #6336 — Newton backend - Part 4/8: #6337 — PhysX backend - Part 5/8: #6338 — Extract articulation interface test utilities - Part 6/8: #6339 — OVPhysX backend - Part 7/8: #6340 — Cross-backend ordering interface tests - Part 8/8: #6341 — Events, consumers, and documentation > [!NOTE] > All parts target `develop`, so until the preceding parts merge this PR's diff includes them. The true delta of this part is [this compare](AntoineRichard/IsaacLab@antoiner/articulation-reordering-p1...antoiner/articulation-reordering-p2). ## Type of change - New feature (non-breaking change which adds functionality) ## 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 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] My name exists in `CONTRIBUTORS.md` ## Test Plan - [x] `./isaaclab.sh -p -m pytest source/isaaclab/test/assets/test_articulation_ordering.py` -> 67 passed - [x] `./isaaclab.sh -p -m pytest source/isaaclab/test/assets/test_articulation_ordering_kernels.py` -> 24 passed - [x] `./isaaclab.sh -f` -> all hooks pass
bd3ad28 to
6f9c1fa
Compare
# Description Wires ordering into the Newton backend read/write paths. Non-identity orderings register an unconditional post-step publish that reorders the core state buffers inside the stepped (and CUDA-graph-captured) region, keeping the documented Tier-1 capture-safety contract without Python read guards; `NewtonManager` gains the matching `register`/`unregister_post_step_callback` API. Actuator torque telemetry reads live backend state through the ordering map. Also carries the shared branching USD test fixture used by cross-backend convention tests here and in later parts. Part 3/8 of the series splitting #6248 into independently reviewable units. #6248 remains open, untouched, as the reference implementation; this stack was cut from its final tree after a `develop` sync (Kamino Core, Newton bump, OVPhysX 0.5.1). ## Stack - Part 1/8: #6334 — Extract rigid object interface test utilities - Part 2/8: #6335 — Core ordering foundation - **Part 3/8 (this PR)**: #6336 — Newton backend - Part 4/8: #6337 — PhysX backend - Part 5/8: #6338 — Extract articulation interface test utilities - Part 6/8: #6339 — OVPhysX backend - Part 7/8: #6340 — Cross-backend ordering interface tests - Part 8/8: #6341 — Events, consumers, and documentation > [!NOTE] > All parts target `develop`, so until the preceding parts merge this PR's diff includes them. The true delta of this part is [this compare](AntoineRichard/IsaacLab@antoiner/articulation-reordering-p2...antoiner/articulation-reordering-p3). ## Type of change - New feature (non-breaking change which adds functionality) ## 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 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] My name exists in `CONTRIBUTORS.md` ## Test Plan - [x] `./isaaclab.sh -p -m pytest source/isaaclab_newton/test/assets/test_articulation_ordering_kernels.py` -> 2 passed - [x] `./isaaclab.sh -p -m pytest source/isaaclab_newton/test/assets/test_newton_actuators_newton.py` -> 76 passed - [x] `./isaaclab.sh -p -m pytest source/isaaclab_newton/test/assets/test_articulation.py` -> 257 passed, 8 skipped, 1 xfailed - [x] `./isaaclab.sh -f` -> all hooks pass
ooctipus
left a comment
There was a problem hiding this comment.
Agent review: this backend's new code replicates six patterns flagged on Part 4 (#6337) — listing them once here instead of re-raising each inline, so both backends can change together:
- Unreachable staging None-guards — 16 sites across the two files here (PhysX had 6); staging is installed at init whenever ordering resolves, same argument as the #6337 thread.
- The dead
had_*release/reseed lifecycle (~60 lines in_configure_ordering_buffers) — the #6337 thread already noted "same on OVPhysX"; whichever resolution lands (wire a real re-resolve lifecycle in core, or drop the plumbing) should be chain-wide. articulation_data.py:69— repeats the factually wrong claim that Newton's ordered shadows are "refreshed lazily on first access"; they're published every step by a post-step callback (see the #6337 comment on the same sentence).articulation_data.py:845—root_com_pose_w's ternary re-derives the identity branch_backend_body_com_pose_balready takes internally; the base property can be used unconditionally.- ~13 boolean gates re-derive
data.has_joint_ordering/has_body_ordering— you already agreed to sweep these on #6337. - The joint-axis gate-and-gather / write-buffer template is hand-written as near-twin variants here (e.g.
_refresh_joint_pos/_refresh_joint_veland their write-buffer getters are verbatim twins), where the PhysX file already parameterized the same pattern (_refresh_joint_state_user,_get_joint_state_write_buffer).
| joint_vel_backend = self._data._get_joint_vel_write_buffer(joint_selection_is_partial) | ||
| has_joint_ordering = self.data.joint_ordering is not None | ||
| wp.launch( | ||
| ordering_kernels.write_joint_state_user_to_backend_with_indices, |
There was a problem hiding this comment.
Agent review: this deprecated shim previously delegated in two lines to the _index sub-writers; it now carries its own 36-line hand-optimized fused launch that near-verbatim copies write_joint_state_to_sim_mask's body. A compatibility shim whose own docstring tells users to migrate doesn't need a private optimized implementation — restoring the delegation deletes the copy.
| if effort_binding is not None: | ||
| torque = self._data._applied_torque | ||
| shape = self._effort_binding.shape | ||
| self._effort_write_view = wp.array( |
There was a problem hiding this comment.
Agent review: after this rewiring, _effort_write_view/_pos_target_write_view/_vel_target_write_view are never dereferenced as arrays — their only remaining use is is not None in write_data_to_sim. Constructing pointer-aliased wp.array(ptr=...) wrappers (and _make_write_view) purely as availability sentinels can collapse to three booleans.
| sim, num_articulations, device | ||
| ): | ||
| """Reversed body ordering: an external wrench composes from the backend-order link pose and | ||
| ``write_data_to_sim`` no longer refreshes the public ``body_link_pose_w`` shadow (finding [27]). |
There was a problem hiding this comment.
Agent review: these docstrings cite numbered findings from a private review document ("finding [27]" etc.) that future readers can't resolve — worth inlining the one-line rationale instead of the reference.
| "docs/superpowers/specs/2026-04-28-ovphysx-wheel-gaps-for-marco.md." | ||
| ) | ||
|
|
||
| _ANYMAL_PHYSX_JOINT_NAMES = ( |
There was a problem hiding this comment.
Agent suggestion: third hand-copied set of the ordering test fixtures (ANYmal/Panda name tuples, branching-fixture path helper + ground-truth orders) after the Newton and PhysX test files. Same home question as raised on #6338 — an installable shared location would let Parts 4/6 keep one copy.
# Description Wires ordering into the PhysX backend read/write paths using the shared kernels: fused user-and-backend target writes, lazily refreshed timestamped `body_mass`/`body_inertia` (see the changelog `Changed` entry for the semantics change vs develop), and ordering maps resolved at initialization. Part 4/8 of the series splitting #6248 into independently reviewable units. #6248 remains open, untouched, as the reference implementation; this stack was cut from its final tree after a `develop` sync (Kamino Core, Newton bump, OVPhysX 0.5.1). ## Stack - Part 1/8: #6334 — Extract rigid object interface test utilities - Part 2/8: #6335 — Core ordering foundation - Part 3/8: #6336 — Newton backend - **Part 4/8 (this PR)**: #6337 — PhysX backend - Part 5/8: #6338 — Extract articulation interface test utilities - Part 6/8: #6339 — OVPhysX backend - Part 7/8: #6340 — Cross-backend ordering interface tests - Part 8/8: #6341 — Events, consumers, and documentation > [!NOTE] > All parts target `develop`, so until the preceding parts merge this PR's diff includes them. The true delta of this part is [this compare](AntoineRichard/IsaacLab@antoiner/articulation-reordering-p3...antoiner/articulation-reordering-p4). ## Type of change - New feature (non-breaking change which adds functionality) ## 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 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] My name exists in `CONTRIBUTORS.md` ## Test Plan - [x] `./isaaclab.sh -p -m pytest source/isaaclab_physx/test/assets/test_articulation.py` -> 239 passed - [x] `./isaaclab.sh -p -m pytest source/isaaclab_physx/test/assets/test_newton_actuators_physx.py` -> 44 passed - [x] `./isaaclab.sh -f` -> all hooks pass
Move the articulation interface test factories into a shared utility module mirroring the rigid object extraction, with ordering-aware construction hooks used by the later cross-backend ordering tests. Part 5 of the articulation reordering series.
The ordering core deleted the _cache_ordering_maps guard machinery; mock articulation factories now configure backend staging directly, matching the backend initialization paths.
6f9c1fa to
f0ed1bc
Compare
Extract common test bootstrapping and ordering assertions so backend tests share one implementation. Guard optional PhysX setup and make unsupported mock ordering explicit.
Apply repository formatting and import ordering after the new helper files became tracked.
Greptile SummaryThis PR wires articulation joint/body ordering into the OVPhysX backend's read and write paths — covering joint-target reordering in
Confidence Score: 3/5There is an ordering asymmetry: write_joint_state_to_sim_mask was upgraded to reorder joint state before writing to OVPhysX, but write_joint_state_to_sim_index was not, so any reset or initialisation call that routes through the index path with a non-identity joint ordering will silently inject joint positions and velocities in the wrong order into the physics engine. The core ordering kernels (_fd_joint_acc_ordered, _body_wrench_to_world_ordered, reorder_joint_targets_user_to_backend) are logically sound and the FK-aware refresh mechanism is correct. The gap is the incomplete update to write_joint_state_to_sim_index, which is a write path that callers (including reset logic) use frequently. Under a non-identity ordering, this produces silently wrong physics state from the very first step. source/isaaclab_ovphysx/isaaclab_ovphysx/assets/articulation/articulation.py — write_joint_state_to_sim_index needs the same ordering treatment applied to write_joint_state_to_sim_mask in this PR. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User as User Code
participant Art as Articulation
participant Data as ArticulationData
participant Kern as kernels
participant OVP as OVPhysX
Note over Art,OVP: write_data_to_sim() — ordering-aware write path
User->>Art: write_data_to_sim()
Art->>Data: _backend_body_link_pose_w (backend order)
Data-->>Art: poses (backend order)
Art->>Kern: _body_wrench_to_world_ordered(force_b[user], torque_b[user], poses[backend], user_to_backend)
Kern-->>Art: wrench_buf (backend order)
Art->>OVP: set_attribute(LINK_WRENCH, wrench_buf)
alt joint_ordering is not None
Art->>Kern: reorder_joint_targets_user_to_backend(applied_torque, pos_target, vel_target, backend_to_user)
Kern-->>Art: effort_backend, pos_target_backend, vel_target_backend
end
Art->>OVP: set_attribute(DOF_ACTUATION_FORCE / POSITION_TARGET / VELOCITY_TARGET)
Note over Art,OVP: update() — FD joint acceleration
User->>Art: update(dt)
Art->>Data: update(dt)
Data->>Data: "_sim_timestamp += dt"
Data->>Data: joint_acc (lazy FD refresh)
alt joint_ordering is not None
Data->>OVP: read DOF_VELOCITY → _joint_vel_backend (backend order)
Data->>Kern: _fd_joint_acc_ordered(cur_vel_backend, user_to_backend, prev_vel_user, inv_dt)
Kern-->>Data: out_user (joint acc in public order)
else
Data->>OVP: read DOF_VELOCITY → _joint_vel_buf (user order)
Data->>Kern: _fd_joint_acc(joint_vel, prev_vel, inv_dt)
Kern-->>Data: joint_acc
end
Note over Art,OVP: body state read — FK-aware
User->>Data: body_link_pose_w
Data->>Data: _ensure_fk_fresh()
alt "_fk_timestamp < _sim_timestamp"
Data->>OVP: update_articulations_kinematic()
end
Data->>OVP: read LINK_POSE → backend buffer
alt body_ordering is not None
Data->>Kern: reorder_2d_backend_to_user
Kern-->>Data: body_link_pose_w (public order)
end
Data-->>User: body_link_pose_w
%%{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"}}}%%
sequenceDiagram
participant User as User Code
participant Art as Articulation
participant Data as ArticulationData
participant Kern as kernels
participant OVP as OVPhysX
Note over Art,OVP: write_data_to_sim() — ordering-aware write path
User->>Art: write_data_to_sim()
Art->>Data: _backend_body_link_pose_w (backend order)
Data-->>Art: poses (backend order)
Art->>Kern: _body_wrench_to_world_ordered(force_b[user], torque_b[user], poses[backend], user_to_backend)
Kern-->>Art: wrench_buf (backend order)
Art->>OVP: set_attribute(LINK_WRENCH, wrench_buf)
alt joint_ordering is not None
Art->>Kern: reorder_joint_targets_user_to_backend(applied_torque, pos_target, vel_target, backend_to_user)
Kern-->>Art: effort_backend, pos_target_backend, vel_target_backend
end
Art->>OVP: set_attribute(DOF_ACTUATION_FORCE / POSITION_TARGET / VELOCITY_TARGET)
Note over Art,OVP: update() — FD joint acceleration
User->>Art: update(dt)
Art->>Data: update(dt)
Data->>Data: "_sim_timestamp += dt"
Data->>Data: joint_acc (lazy FD refresh)
alt joint_ordering is not None
Data->>OVP: read DOF_VELOCITY → _joint_vel_backend (backend order)
Data->>Kern: _fd_joint_acc_ordered(cur_vel_backend, user_to_backend, prev_vel_user, inv_dt)
Kern-->>Data: out_user (joint acc in public order)
else
Data->>OVP: read DOF_VELOCITY → _joint_vel_buf (user order)
Data->>Kern: _fd_joint_acc(joint_vel, prev_vel, inv_dt)
Kern-->>Data: joint_acc
end
Note over Art,OVP: body state read — FK-aware
User->>Data: body_link_pose_w
Data->>Data: _ensure_fk_fresh()
alt "_fk_timestamp < _sim_timestamp"
Data->>OVP: update_articulations_kinematic()
end
Data->>OVP: read LINK_POSE → backend buffer
alt body_ordering is not None
Data->>Kern: reorder_2d_backend_to_user
Kern-->>Data: body_link_pose_w (public order)
end
Data-->>User: body_link_pose_w
|
f0ed1bc to
89a73c5
Compare
Cover the isaaclab_newton and isaaclab_physx test changes so the changelog gate recognizes every package touched by P5.
89a73c5 to
1261aff
Compare
Match the established rigid-object test helper conventions for the mock backend label and private test access. Add a regression check that keeps the three interface utilities consistent.
Translate between public and backend order on the OVPhysX read and write paths: fused user-and-backend writes with persistent target staging separated from transient applied-torque scratch, forward kinematics aware body state refresh, and a fused ordered joint acceleration kernel. Part 6 of the articulation reordering series.
The ordering core rework single-homed the joint and body ordering maps on ArticulationData and removed the mirrored `_has_*` flags and `_*_user_to_backend` maps, the `_cache_ordering_maps` machinery, the `ArticulationNameMap` identity/name fields, and the dtype-suffixed write-kernel overloads. Repoint the OVPhysX articulation and data classes to the new contracts: - Read ordering presence via `data.joint_ordering` / `data.body_ordering` is-not-None checks; conditional sites dereference the map directly, while always-launch sites use the new `_*_user_to_backend_map()` helpers so publish, wrench, and telemetry gathers keep a valid map. - Configure backend staging through `_ordering_configure_backend_staging` and, now that `_install_ordering_flags` is gone, recover the previous per-axis ordering from the persistent backend staging buffers so a cleared ordering still releases its buffers on rebind. - Replace the dtype-suffixed overload launches with the `write_*_user_to_backend_*` launch wrappers, passing the dtype the old suffix implied and dropping the explicit launch dim. Adapt the OVPhysX asset tests to assert through the single-homed ordering maps, since a non-None map now always denotes a real permutation.
1261aff to
5e2de0e
Compare
Description
Wires ordering into the OVPhysX backend read/write paths: fused user-and-backend writes with persistent target staging separated from transient applied-torque scratch, FK-aware body state refresh, and a fused ordered joint acceleration kernel.
Part 6/8 of the series splitting #6248 into independently reviewable units. #6248 remains open, untouched, as the reference implementation; this stack was cut from its final tree after a
developsync (Kamino Core, Newton bump, OVPhysX 0.5.1).Stack
Note
All parts target
develop, so until the preceding parts merge this PR's diff includes them. The true delta of this part is this compare.Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdTest Plan
PhysX.set_cpu_modeAttributeError from the OVPhysX 0.4.13 wheel vs Fetch OVPhysX wheelhouse CI through NGC + 0.5.1 bump #6314's manager code (develop shows the identical signature on its 99 pre-existing live tests); the same tests passed on the pre-sync monolithic branch./isaaclab.sh -f-> all hooks pass