Add PhysX joint wrench sensor#5458
Add PhysX joint wrench sensor#5458AntoineRichard wants to merge 16 commits intoisaac-sim:developfrom
Conversation
…sor_cfg.py Co-authored-by: Antoine RICHARD <[email protected]> Signed-off-by: camevor <[email protected]>
Co-authored-by: Antoine RICHARD <[email protected]> Signed-off-by: camevor <[email protected]>
Add the PhysX backend implementation for JointWrenchSensor and migrate body incoming wrench observations off ArticulationData. Remove the old articulation data accessor and document the Isaac Lab 3.0 migration path.
AntoineRichard
requested review from
Mayankm96,
jtigue-bdai,
kellyguo11 and
ooctipus
as code owners
github-actions
Bot
added
documentation
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR adds a new JointWrenchSensor class with PhysX and Newton backend implementations, removing the deprecated ArticulationData.body_incoming_joint_wrench_b property. The sensor reads incoming joint reaction wrenches and exposes them as separate force [N] and torque [N·m] buffers. The implementation follows the established factory pattern for multi-backend support and includes comprehensive tests for both backends.
Architecture Impact
This is a breaking change that affects:
- Articulation data API: Removes
body_incoming_joint_wrench_bfrom all backend data classes (PhysX, Newton, OvPhysX, mock) - MDP observations:
mdp.body_incoming_wrenchnow requires aJointWrenchSensorinstead of anArticulationasset - Task environments:
inhand_manipulation_env.py,shadow_hand_vision_env.py,ant_env_cfg.py,humanoid_env_cfg.pyare updated to use the new sensor - Test infrastructure: Mock articulation data no longer includes
body_incoming_joint_wrench_b
The sensor integrates cleanly with the existing sensor base infrastructure and follows the established patterns for PhysX/Newton backend separation.
Implementation Verdict
Minor fixes needed — The implementation is solid overall, but there are semantic differences between PhysX and Newton buffer shapes that could cause user confusion, and one potential device mismatch issue.
Test Coverage
Thorough. Both backends have comprehensive test suites:
- Pre-init contract tests (data returns None)
- Initialization and shape validation
- Physical correctness tests (gravity-only, external wrench, interior joints)
- Reset behavior (full and partial env_ids reset)
- String representation
The PhysX tests verify against the raw tensor API, while Newton tests use analytical wrench calculations. This is appropriate given the different backend semantics.
CI Status
No CI checks available yet — manual verification of tests claimed in PR description.
Findings
🟡 Warning: PhysX vs Newton semantic mismatch in buffer shapes — could confuse users
PhysX sensor (source/isaaclab_physx/isaaclab_physx/sensors/joint_wrench/joint_wrench_sensor.py:112-113) reports num_bodies entries (one per link including root), while Newton sensor (source/isaaclab_newton/isaaclab_newton/sensors/joint_wrench/joint_wrench_sensor.py:118-125) reports num_joints entries (excluding FREE/FIXED joints). This means:
- PhysX:
sensor.data.force.shape == (num_envs, num_bodies, 3) - Newton:
sensor.data.force.shape == (num_envs, num_joints, 3)
The body_names property returns different-length lists. Code written for one backend may fail or produce wrong results on the other. Consider documenting this prominently in the base class docstring.
🟡 Warning: source/isaaclab/isaaclab/envs/mdp/observations.py:314-315 — Missing line break causes long line
The RuntimeError message is on a single line without proper formatting, which will fail linting:
raise RuntimeError("Joint wrench sensor data is not initialized. Call sim.reset() before reading observations.")Should be split across lines to meet line length requirements.
🔵 Improvement: source/isaaclab_tasks/isaaclab_tasks/direct/inhand_manipulation/inhand_manipulation_env.py:54-57 — Index lookup happens before sensor might exist
The finger_wrench_bodies initialization accesses self._joint_wrench_sensor.body_names which requires the sensor to be initialized. However, _joint_wrench_sensor is set in _setup_scene() which runs after __init__(). The check getattr(self, "_joint_wrench_sensor", None) will always be None at this point in __init__. This code path is dead — the indices are never populated during init.
Looking at the actual flow: _setup_scene() is called by DirectRLEnv.__init__() before returning to InHandManipulationEnv.__init__(), so this code runs after _setup_scene(). This is correct but confusing — consider moving this logic into _setup_scene() for clarity.
🔵 Improvement: source/isaaclab_newton/isaaclab_newton/sensors/joint_wrench/joint_wrench_sensor.py:206-216 — Invalidation re-registers extended state request
The _invalidate_initialize_callback method calls NewtonManager.request_extended_state_attribute("body_parent_f") again. This is correct for hot-reloading scenarios, but worth verifying that NewtonManager handles duplicate requests idempotently (doesn't allocate twice).
🔵 Improvement: source/isaaclab_physx/isaaclab_physx/sensors/joint_wrench/kernels.py:13-22 — Kernel writes to inputs
The kernel signature lists out_force and out_torque in inputs= but they are output arrays:
wp.launch(
joint_wrench_split_kernel,
dim=(self._num_envs, self._num_bodies),
inputs=[env_mask, incoming_joint_wrench, self._data._force, self._data._torque],
device=self._device,
)Should use outputs=[self._data._force, self._data._torque] for clarity and potential future Warp optimizations. The Newton kernel does this correctly at line 184-185.
🔵 Improvement: source/isaaclab/isaaclab/sensors/joint_wrench/joint_wrench_sensor_cfg.py:22-30 — Convention enum only has one value
The convention field is typed as Literal["incoming_joint_frame"] with only one option. This is forward-looking API design, but the docstring references "IsaacLab2.3" which doesn't exist yet (current is 3.0). Should say "IsaacLab 2.x" or similar.
🔵 Improvement: Documentation migration guide completeness
The migration guide at docs/source/migration/migrating_to_isaaclab_3-0.rst:336-407 provides good examples but doesn't mention that PhysX includes the root body while Newton excludes it. Users migrating from the old body_incoming_joint_wrench_b (which was PhysX-only) should be warned about this if they plan to use Newton.
Greptile SummaryThis PR adds Confidence Score: 4/5Safe to merge; all findings are style/P2 with no runtime-breaking defects. Only P2 issues present: private API access in the PhysX sensor, private sensor attribute in test helper, and a cross-backend body-availability nuance in the observations function. The Warp kernels, buffer management, and migration are correct. Tests pass (9 PhysX + Newton suites). source/isaaclab_physx/isaaclab_physx/sensors/joint_wrench/joint_wrench_sensor.py (private _backend check), source/isaaclab_newton/isaaclab_newton/sensors/joint_wrench/joint_wrench_sensor.py (joint_child indexing documentation) Important Files Changed
Sequence DiagramsequenceDiagram
participant Env as ManagerBasedEnv / DirectRLEnv
participant Sensor as JointWrenchSensor (factory)
participant PhysX as PhysX Backend
participant Newton as Newton Backend
participant Kernel as Warp Kernel
Env->>Sensor: __new__() → dispatch to backend
Sensor->>PhysX: _initialize_impl()
PhysX->>PhysX: create_articulation_view(prim_path)
PhysX->>PhysX: allocate force/torque buffers
Sensor->>Newton: _initialize_impl()
Newton->>Newton: ArticulationView + bind body_parent_f
Newton->>Newton: allocate force/torque buffers
loop Every update step
Env->>Sensor: sensor.data (triggers _update_outdated_buffers)
Sensor->>PhysX: _update_buffers_impl(env_mask)
PhysX->>PhysX: get_link_incoming_joint_force().view(spatial_vectorf)
PhysX->>Kernel: joint_wrench_split_kernel
Kernel-->>PhysX: out_force, out_torque
Sensor->>Newton: _update_buffers_impl(env_mask)
Newton->>Kernel: joint_wrench_to_incoming_joint_frame_kernel
Kernel-->>Newton: out_force (joint frame), out_torque (joint frame)
Sensor-->>Env: JointWrenchSensorData.force / .torque (ProxyArray)
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| self._physics_sim_view = SimulationManager.get_physics_sim_view() |
There was a problem hiding this comment.
_backend attribute used to validate ArticulationView creation
The check self._root_view._backend is None relies on a private implementation detail of omni.physics.tensors. If this attribute is renamed or removed in a future PhysX/Kit update, the guard will silently break (either false-positive errors or missed initialization failures). Consider using a supported public API — e.g., checking self._root_view.count > 0 or self._root_view.shared_metatype — or wrapping the creation in a try/except against the exceptions the PhysX tensor API documents for failure.
| yield sim_ctx | ||
|
|
||
|
|
||
| def _physx_incoming_joint_wrench(sensor: JointWrenchSensor) -> torch.Tensor: | ||
| """Read the raw PhysX incoming joint wrench tensor. | ||
|
|
There was a problem hiding this comment.
sensor._root_view
_physx_incoming_joint_wrench reaches into sensor._root_view (a private attribute) to reproduce the raw PhysX wrench for comparison. If _root_view is ever renamed or its shape contract changes, this test helper will break silently rather than catching a real regression. Consider exposing a thin internal-test accessor or using the sensor's own data API as the reference for the comparison where possible.
| self._sim_bind_body_parent_f = body_parent_f[:, 0] | ||
| self._sim_bind_body_q = self._root_view.get_link_transforms(state_0)[:, 0] | ||
| self._sim_bind_body_com = self._root_view.get_attribute("body_com", model)[:, 0] | ||
| self._sim_bind_joint_X_c = self._root_view.get_attribute("joint_X_c", model)[:, 0] | ||
|
|
||
| # joint_child is per-articulation; topology is identical across envs, | ||
| # so we take the first-env mapping as the 1-D kernel input. |
There was a problem hiding this comment.
joint_child extraction indexing deserves a clarifying comment
The double-index idiom get_attribute("joint_child", model)[:, 0] followed by .numpy()[0] is non-obvious. Without inline documentation of the exact shape that ArticulationView.get_attribute returns and what each dimension represents, it's hard to verify that [:, 0] selects the per-articulation joint child mapping (rather than the first joint for all envs). A brief comment explaining the returned shape and why [0] gives the 1-D joint→child body mapping would help future maintainers.
Resolve nested USD articulation roots before creating the PhysX articulation view so classic Ant and Humanoid environments can use the joint wrench sensor from the asset prim path.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The new commits have addressed the main structural concerns from the previous review. Checking the specific findings:
-
PhysX vs Newton semantic mismatch — Still present but this is a design decision documented in the class docstrings. The difference is inherent to the backends.
-
Long line in observations.py — Still present at line 314-315. The
RuntimeErrormessage should be split. -
finger_wrench_bodies initialization — Addressed. The code flow is now clearer with
_joint_wrench_sensorbeing set in_setup_scene()which is called before the index lookup code runs. -
Kernel inputs vs outputs — Still present in PhysX kernel launch at
joint_wrench_sensor.py:176-181. The arraysself._data._forceandself._data._torqueare passed ininputs=but should be inoutputs=. -
Convention docstring reference — Still says "IsaacLab2.3" which doesn't exist.
New Finding
🟡 Warning: source/isaaclab/isaaclab/envs/mdp/observations.py:314-315 — Long line will fail pre-commit
raise RuntimeError("Joint wrench sensor data is not initialized. Call sim.reset() before reading observations.")This 107-character line exceeds the typical 100-char limit. Split it:
raise RuntimeError(
"Joint wrench sensor data is not initialized. Call sim.reset() before reading observations."
)Verdict
Ship it with the minor formatting fix above. The implementation is solid, tests are comprehensive, and the architecture follows established patterns.
Resolve nested articulation roots for the Newton joint wrench sensor and let classic Ant and Humanoid Newton presets use the same wrench observation terms as PhysX.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
Checking the previous findings against the latest commit (a7f68ee):
-
Long line in observations.py:314-315 — Still present. The
RuntimeErrormessage is still on a single long line that will likely fail pre-commit checks. -
PhysX kernel inputs vs outputs — Checking
isaaclab_physx/sensors/joint_wrench/joint_wrench_sensor.py:176-181: Thejoint_wrench_split_kernellaunch passesout_forceandout_torqueininputs=rather thanoutputs=. While Warp will still function, this is semantically incorrect for kernel arrays that are written to. -
Convention docstring reference —
joint_wrench_sensor_cfg.py:29still references "IsaacLab2.3" which doesn't exist.
Implementation Verdict
Ship it — These are minor documentation/style issues that don't affect correctness. The core implementation is solid with comprehensive tests passing.
Convert PhysX incoming joint wrenches from body1 frame at the body origin into the shared child-side joint frame with torque referenced at the child-side joint anchor.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
Checking previous findings against commit da5ff06:
-
Long line in observations.py — Addressed. The
RuntimeErrormessage formatting issue is no longer present in the diff. -
PhysX kernel inputs vs outputs — Still present. In
isaaclab_physx/sensors/joint_wrench/joint_wrench_sensor.py:211-219, thejoint_wrench_split_kernellaunch still passesout_forceandout_torqueininputs=rather thanoutputs=. This is semantically incorrect but functionally works. -
Convention docstring reference — Addressed. The "IsaacLab2.3" reference has been removed from
joint_wrench_sensor_cfg.py.
New Observation
🔵 Improvement: isaaclab_physx/sensors/joint_wrench/joint_wrench_sensor.py:211-219 — For consistency with the Newton implementation (which correctly uses outputs=), consider moving self._data._force and self._data._torque to the outputs= parameter in the wp.launch() call.
Implementation Verdict
Ship it — The remaining issue is cosmetic and doesn't affect correctness. All tests pass and the implementation is solid.
2 participants
Description
Adds the PhysX backend for
JointWrenchSensor.This PR is built on top of #5412. Until that PR lands, this comparison also includes the Newton joint-wrench sensor commits from #5412.
The PhysX sensor creates a PhysX articulation view and reads incoming joint wrenches from
ArticulationView.get_link_incoming_joint_force(). It then converts the raw PhysX body1-frame, body-origin wrench into the sharedincoming_joint_frameconvention: child-side joint frame with torque referenced at the child-side joint anchor. PhysX and Newton joint-wrench sensors also resolve nested USD articulation roots before creating their articulation views so asset-level paths such as{ENV_REGEX_NS}/Robotwork for classic Ant/Humanoid assets.This also removes
ArticulationData.body_incoming_joint_wrench_bfrom the articulation data APIs, migrates manager/direct environments that used it toJointWrenchSensor, and documents the Isaac Lab 3.0 migration path. Classic Ant/Humanoid Newton presets now use the same wrench observations as PhysX and pass short RSL-RL training smokes.@camevor can you review?
Fixes # (issue)
N/A
Type of change
Screenshots
N/A
Tests
./isaaclab.sh -fdocker exec isaac-lab-base bash -lc "cd /workspace/isaaclab && ./isaaclab.sh -p -m pytest source/isaaclab_physx/test/sensors/test_joint_wrench_sensor.py"- 11 passeddocker exec isaac-lab-base bash -lc "cd /workspace/isaaclab && ./isaaclab.sh -p -m pytest source/isaaclab_newton/test/sensors/test_joint_wrench_sensor.py"- 10 passeddocker exec isaac-lab-base bash -lc "cd /workspace/isaaclab && ./isaaclab.sh -p -m pytest source/isaaclab_tasks/test/test_environments.py -k 'Isaac-Ant-v0 or Isaac-Humanoid-v0'"- 4 passeddocker exec isaac-lab-base bash -lc "cd /workspace/isaaclab && ./isaaclab.sh -p scripts/reinforcement_learning/rsl_rl/train.py --task Isaac-Ant-v0 --num_envs 16 --max_iterations 1 --headless --device cuda presets=newton agent.num_steps_per_env=8"- passeddocker exec isaac-lab-base bash -lc "cd /workspace/isaaclab && ./isaaclab.sh -p scripts/reinforcement_learning/rsl_rl/train.py --task Isaac-Humanoid-v0 --num_envs 16 --max_iterations 1 --headless --device cuda presets=newton agent.num_steps_per_env=8"- passeddocker exec isaac-lab-base bash -lc "cd /workspace/isaaclab && ./isaaclab.sh -p -m pytest source/isaaclab/test/test_mock_interfaces/test_mock_data_properties.py"- 253 passedChecklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there