[OVPHYSX] Add IMU sensor#5421
Conversation
Add nine RIGID_BODY_* aliases to isaaclab_ovphysx.tensor_types covering the rigid-actor root pose/velocity/acceleration, wrench application, and mass/inertia/COM properties. Each alias carries a shape/units docstring that Sphinx autoattribute can pick up. Extend _CPU_ONLY_TYPES with the five CPU-routed rigid-body variants so the existing GPU/CPU dispatch in _to_flat_f32 routes them correctly. Direct imports (no shim) intentionally couple this package to a future ovphysx wheel that exposes the matching TensorType enum values; see docs/superpowers/specs/2026-04-27-ovphysx-rigid-body-tensortypes-gap.md. Issue: isaac-sim#5316
Move kernels reused across multiple asset types from isaaclab_ovphysx.assets.articulation.kernels into a new isaaclab_ovphysx.assets.kernels module: _body_wrench_to_world, _scatter_rows_partial, _copy_first_body, _compose_root_com_pose, _projected_gravity, _compute_heading, _world_vel_to_body_lin, _world_vel_to_body_ang. Articulation-only kernels (joint-limit setup, FD joint acceleration, multi-body COM compose) stay in articulation/kernels.py. Articulation modules update their imports accordingly. No behavior change. This refactor unblocks the upcoming RigidObject implementation, which needs the same kernels for frame conversions and wrench packing. Issue: isaac-sim#5316
Generalize the mock binding factory to produce a rigid-object-shaped binding set (RIGID_BODY_* keys only, num_joints=0, num_bodies=1, no tendons) when called with asset_kind='rigid_object'. Default behavior is unchanged: existing articulation callers do not need updates. Make set_random_data tolerant of the smaller binding set so it works for both asset kinds. Issue: isaac-sim#5316
Add the rigid_object sub-package and the RigidObjectData class skeleton with constructor, count properties, update/invalidate hooks, and _process_cfg that fills _default_root_pose / _default_root_velocity from cfg.init_state. Add the backend-specific test file with a hasattr-based wheel gate (importorskip-then-attr on the module would AttributeError rather than skip when RIGID_BODY_* enums are absent on the wheel) and a basic-counts smoke test. Issue: isaac-sim#5316
Address review blockers on commit 65084f7: 1. _process_cfg now mirrors the articulation pattern (wp.zeros to pre-allocate, then wp.copy from wp.from_numpy with the typed dtype directly — matching articulation._process_cfg verbatim), avoiding the use-after-free that the previous reinterpret-cast-from-local idiom introduced when the float32 source went out of scope. 2. is_primed is now a guarded @Property + setter that enforces the one-way gate (False->True allowed, True->True idempotent, True->False raises ValueError), matching every other *Data class. 3. Drop unused forward-reference imports from test/assets/test_rigid_object.py so ruff/F401 passes; subsequent tasks will re-add them when the symbols are actually used. Issue: isaac-sim#5316
Replace the abstract-property stubs for root_link_pose_w, root_link_vel_w, root_com_pose_w, root_com_vel_w, and the per-axis sliced views (root_link_pos_w, root_link_quat_w, root_lin_vel_w, root_ang_vel_w, plus their root_com_* counterparts) with concrete implementations that lazy-read from RIGID_BODY_ROOT_POSE / RIGID_BODY_ROOT_VELOCITY through TensorBindings, cache for the rest of the sim step, and expose slices as zero-copy Warp views over the canonical pose/velocity buffers. Mirrors the articulation_data lazy-read + ProxyArray + sliced-view idioms with num_bodies=1 (root pose/velocity are 1-D over instances, with no body axis). Issue: isaac-sim#5316
The per-property TimestampedBuffer.timestamp fields are the freshness gate consulted by every lazy-read property. _invalidate_caches was previously clearing only the legacy _timestamps dict, leaving the buffers with their last-read timestamp; a property accessed within the same sim step (e.g. immediately after RigidObject.reset and before update(dt) advances _sim_time) would serve pre-reset data. Reset every allocated buffer's timestamp to -1.0 in _invalidate_caches so the next property access always re-reads from the binding regardless of where _sim_time stands. Add a regression test that mutates the binding in place + calls _invalidate_caches without advancing _sim_time and asserts the next read reflects the new value. Issue: isaac-sim#5316
Replace the abstract-property stubs for the body-state singleton-dim
views (body_link_pose_w, body_com_pose_w, body_*_vel_w, body_*_pos_w,
body_*_quat_w, body_*_lin_vel_w, body_*_ang_vel_w), the body
acceleration (body_link_acc_w, body_com_acc_w, plus their _lin_*/
_ang_* slices) gated on the RIGID_BODY_ACCELERATION binding, and the
body-frame derived properties (projected_gravity_b, heading_w,
root_link_lin_vel_b, root_link_ang_vel_b, root_com_lin_vel_b,
root_com_ang_vel_b).
Body-state views are zero-copy reshapes of the (N,) root buffers into
(N, 1, k) — no compute kernel needed when num_bodies=1. Body
acceleration raises NotImplementedError with a pointer to the gap
spec if the wheel lacks RIGID_BODY_ACCELERATION. Derived properties
launch the relocated _projected_gravity / _compute_heading /
_world_vel_to_body_{lin,ang} kernels from
isaaclab_ovphysx.assets.kernels.
Extend _invalidate_caches with the new TimestampedBuffer instances so
they participate in coarse cache invalidation.
Issue: isaac-sim#5316
The IsaacLab projected_gravity_b convention (per BaseRigidObjectData docstring "Projection of the gravity direction on base frame", and matching ArticulationData which normalizes gravity_np / gravity_mag before storing GRAVITY_VEC_W) is the unit direction, not the signed-magnitude. The Task 6 test assertion expected -9.81 (signed magnitude); the kernel correctly produces -1.0 (unit z-component of the gravity direction). Update the assertion and the inline comment so the test reflects the documented contract. Issue: isaac-sim#5316
Replace the abstract-property stubs for body_mass, body_inertia, and body_com_pose_b with concrete CPU-side lazy-read implementations backed by RIGID_BODY_MASS / RIGID_BODY_INERTIA / RIGID_BODY_COM_POSE bindings. Also implement body_com_pos_b and body_com_quat_b as zero-copy slice views of body_com_pose_b's transformf buffer. Properties read once on first access, cache as semi-static, and invalidate via the _invalidate_caches reset loop (driven by RigidObject mass/COM/inertia setters). Mirrors the articulation_data pattern adapted for num_bodies = 1. Issue: isaac-sim#5316
Add RigidObject class with __init__, _initialize_impl, _get_binding, and count/data accessor properties. Eagerly creates GPU bindings for RIGID_BODY_ROOT_POSE / RIGID_BODY_ROOT_VELOCITY / RIGID_BODY_WRENCH so binding-creation failures surface at init time with a clear message pointing at the gap spec. _create_buffers and _process_cfg are placeholder no-ops; Task 9 replaces them. Write paths, reset, update, find_bodies, and the deprecated state writers come in subsequent tasks (10-13). Abstract methods that must be satisfied to instantiate the class are stubbed with NotImplementedError pending those tasks. Issue: isaac-sim#5316
Renames per Marco's feedback: RIGID_BODY_ROOT_POSE -> RIGID_BODY_POSE and RIGID_BODY_ROOT_VELOCITY -> RIGID_BODY_VELOCITY throughout. "Root" is articulation vocabulary; a standalone rigid body IS the body. Shape correction: RIGID_BODY_MASS and RIGID_BODY_INV_MASS ship as (N,) not (N, 1). MockOvPhysxBindingSet allocates (N,) for both; rigid_object_data.py's body_mass property consumes (N,) and exposes (N, 1) via zero-copy reshape to satisfy the BaseRigidObjectData contract. Soften _initialize_impl error: removes the prescriptive "NOT under an articulation root" language since pattern- resolution gating is a future wheel-side selection policy. Pre-existing ruff E501 in write_root_link_state_to_sim docstring fixed as collateral.
Allocate _ALL_INDICES, _ALL_BODY_INDICES, their Warp views, the single-body (N, 1, 9) _wrench_buf staging buffer, and the instantaneous/permanent wrench composers. _process_cfg delegates to RigidObjectData._process_cfg. Issue: isaac-sim#5316
Add the twelve root-state writers (pose+velocity, actor+link+com, index+mask variants) on RigidObject, plus the shared _to_flat_f32 and _write_root_state helpers ported from articulation. Frame-conversion variants launch the relocated assets/kernels.py kernels before the binding write. Add the three deprecated compound state writers that emit DeprecationWarning and delegate to the split pose+velocity writers. Add _compose_root_link_pose_from_com kernel to assets/kernels.py to support COM->link pose inversion on the write path: link_pose = com_pose_w * inverse(com_pose_b) Issue: isaac-sim#5316
Add set_masses_{index,mask}, set_coms_{index,mask},
set_inertias_{index,mask}. Each writes through the matching
CPU-routed RIGID_BODY_* binding via _write_root_state, then
invalidates the corresponding RigidObjectData cache via
_invalidate_caches.
body_ids / body_mask parameters are accepted for parity with the
BaseRigidObject contract but unused (num_bodies = 1).
Extend _write_root_state to handle 1-D bindings (RIGID_BODY_MASS)
by detecting them and bypassing the 2-D scatter kernel path.
Issue: isaac-sim#5316
Compose instantaneous + permanent wrenches, rotate body-frame force/torque to world frame via _body_wrench_to_world (dim=(N, 1)), reshape the (N, 1, 9) staging buffer to (N, 9) zero-copy, write to RIGID_BODY_WRENCH, reset the instantaneous composer. Issue: isaac-sim#5316
Replace stubs for reset, update, and find_bodies. reset writes the default pose/velocity to the specified envs via zero-copy flat float32 views of the typed wp.transformf/wp.spatial_vectorf defaults, resets both wrench composers, and invalidates the data caches. update delegates to RigidObjectData.update. find_bodies handles the None (all bodies) fast path and delegates regex matching to resolve_matching_names for non-None inputs. The deprecated compound state writers were already implemented in Task 10 alongside the split-form writers and are not touched here. Issue: isaac-sim#5316
Add the rigid_object/__init__.pyi stub mirroring the articulation sibling, and extend isaaclab_ovphysx.assets/__init__.pyi to include RigidObject and RigidObjectData. Public imports now resolve via ``from isaaclab_ovphysx.assets import RigidObject, RigidObjectData``. Issue: isaac-sim#5316
Add the import-guarded BACKENDS.append('ovphysx') block, the
create_ovphysx_rigid_object factory, and the dispatch case so the
existing parametrized interface tests automatically cover the new
backend. Mirrors the OVPhysX articulation parametrization in
test_articulation_iface.py.
Issue: isaac-sim#5316
Mirror the Cartpole/Ant pattern by adding ovphysx variants to ObjectCfg (RigidObjectCfg using the same DexCube spawn as the physx variant) and to PhysicsCfg (OvPhysxCfg()). Default backend remains physx; existing PhysX/Newton paths are unchanged. This wires Isaac-Repose-Cube-Allegro-Direct-v0 for OVPhysX validation via ./scripts/run_ovphysx.sh source/isaaclab_tasks/isaaclab_tasks/ direct/allegro_hand/allegro_hand_env.py --num_envs 4 --headless once the wheel ships. Issue: isaac-sim#5316
Add the 0.2.0 changelog entry on isaaclab_ovphysx covering the new RigidObject/RigidObjectData classes, the RIGID_BODY_* TensorType aliases (six already-shipping + three pending wheel update), the mock-binding asset_kind extension, and the assets/kernels.py kernel relocation. Bump the matching extension.toml version. Add a patch-bump changelog entry on isaaclab_tasks for the Allegro env preset addition. Issue: isaac-sim#5316
The ovphysx wheel currently exposes 6 of the 9 RIGID_BODY_* TensorType enums (POSE, VELOCITY, WRENCH, MASS, COM_POSE, INERTIA). The remaining three (ACCELERATION, INV_MASS, INV_INERTIA) are pending an upcoming wheel update from @marcodiiga. Make the IsaacLab side wheel-update-agnostic: * Guard tensor_types.py imports of the three not-yet-shipping aliases with try/except AttributeError so isaaclab_ovphysx.tensor_types imports cleanly on today's wheel. _CPU_ONLY_TYPES filters to only the names that exist via _RIGID_BODY_OPTIONAL_CPU. * MockOvPhysxBindingSet skips the optional bindings when their alias is not defined. * RigidObjectData.body_*_acc_w now finite-differences from body_com_vel_w, mirroring Newton's pattern (kernel derive_body_acceleration_from_body_com_velocities ported into isaaclab_ovphysx.assets.kernels). Removes the NotImplementedError-on-missing-binding fallback. * update(dt) stores _last_dt and eagerly triggers body_com_acc_w each step so FD captures every transition. The forward-compat aliases stay declared (Marco will land them); when they ship, the existing TensorBinding read path will work without further IsaacLab changes. Issue: isaac-sim#5316
WrenchComposer.__init__ calls hasattr(asset.data, "body_com_pos_w"), which triggers the property chain body_com_pos_w → body_com_pose_w → root_com_pose_w, reading the RIGID_BODY_COM_POSE binding and setting _body_com_pose_b_buf.timestamp = _sim_time = 0.0. Any subsequent mutation of the binding (e.g. set_coms_index, or a test that sets the binding directly) is then invisible to _com_pose_to_link_pose because the freshness gate ``buf.timestamp >= _sim_time`` treats 0.0 ≥ 0.0 as "already fresh" and skips the read — returning stale buffer contents to the frame-conversion kernel and producing an off-by-translation result. Force a fresh read in _com_pose_to_link_pose by resetting the buffer timestamp to -1.0 immediately before calling _read_transform_binding. The frame conversion always needs the current binding value at write time; the lazy-cache is the wrong policy here. Caught by test_write_root_com_pose_to_sim_index_invokes_frame_conversion. Issue: isaac-sim#5316
When running via ./scripts/run_ovphysx.sh, the test file's unconditional AppLauncher call segfaults during pytest collection because the script's thin Kit shell (libcarb preload only, no full Kit boot) cannot host a real AppLauncher. Mirror the _kitless heuristic from test_articulation_iface so the rigid-object iface tests skip AppLauncher and substitute MagicMock isaacsim core modules in the same scenarios. The original _kitless heuristic (LD_PRELOAD == "" and EXP_PATH not set) was also incorrect for this environment: run_ovphysx.sh sets LD_PRELOAD to the ovphysx libcarb.so path, not an empty string. Fix the heuristic in both test files to check for "ovphysx" in LD_PRELOAD as the primary signal, with the bare-Python fallback retained as a secondary guard. Also extend the kitless sys.modules stub list to cover omni.physics.tensors and related Kit modules that physx_manager.py imports at module scope, which would otherwise cause a ModuleNotFoundError during collection. This unblocks running the OVPhysX-parametrized parts of the file via run_ovphysx.sh. PhysX/Newton/Mock paths are unaffected. Issue: isaac-sim#5316
Three related bugs were present in the OVPhysX RigidObject body-property write path, all contributing to the 120 test failures. First, _write_root_state's full-write path (no env_ids, no mask) had no row-count validation for 1-D bindings such as RIGID_BODY_MASS. Passing a tensor with more rows than num_instances silently reached the mock's numpy reshape and raised ValueError, but the test infrastructure expected AssertionError or RuntimeError. An explicit row-count guard now raises RuntimeError on the full-write path before any binding call. Second, the index/mask sub-write path for 1-D bindings passed the source array to binding.write() as a 2-D (K, 1) buffer (the raw shape produced by _to_flat_f32 when the caller supplies (K, 1) torch data). For the mask path this caused NumPy boolean-index assignment to fail with TypeError because it cannot scatter a 2-D array into a 1-D binding buffer. The 1-D source is now normalised to shape (K,) via a zero-copy warp array view before any write. Third, write_root_com_pose_to_sim_index and write_root_com_pose_to_sim_mask silently truncated oversized inputs inside _com_pose_to_link_pose, which hardcodes shape=(N,) for the intermediate warp array view regardless of the input size. This masked shape errors on full writes. Explicit row-count guards are now applied at the public API entry points for these two methods. Additionally, default_root_pose and default_root_vel in RigidObjectData were NotImplementedError stubs. They are now implemented to return ProxyArray wrappers over the _default_root_pose/_default_root_velocity buffers that are already populated during _process_cfg. Caught by the cross-backend TestRigidObjectWritersBody tests against the OVPhysX backend. After the fix all 372 ovphysx-parametrized cases pass. Issue: isaac-sim#5316
The mock-based test file is removed in favor of a copy of PhysX's test_rigid_object.py adapted to the kitless OVPhysX architecture: - Drop AppLauncher; mock the isaacsim and omni.* modules instead so the file imports under run_ovphysx.sh without launching Kit. - Build the test scene via MockOvPhysxBindingSet, bypassing OvPhysxManager entirely (no Kit stage export needed). - Drive sim steps via direct binding manipulation; no build_simulation_context. - Programmatically construct rigid-object shells instead of pulling DexCube USD from Nucleus. Tests that exercise OVPhysX features not yet wired (OvPhysxManager step loop, contact materials, kitless stage entry point) are explicitly xfail-marked with inline reasons. Result: 67 passed, 73 xfailed, 0 failed. See docs/superpowers/specs/2026-04-28-ovphysx-rigid-object-test-gaps.md (worktree-only, gitignored) for the consolidated gap list for Marco.
OvPhysxManager IS drivable without AppLauncher: _warmup_and_load only needs PhysicsManager._sim.stage + a couple of cfg fields, which a thin SimpleNamespace fake satisfies. Use this to convert the warmup and stage-load xfails into real-backend tests against the live ovphysx.PhysX instance. Adds _make_kitless_sim_context() helper (builds in-memory USD stage with RigidBodyAPI + CollisionAPI cube + PhysicsScene, wraps in SimpleNamespace exposing: stage, cfg.physics, cfg.device, cfg.physics_prim_path, cfg.enable_scene_query_support, cfg.dt) and a module-scoped kitless_manager_cpu fixture that drives initialize() + reset() + close(). Converts test_warmup_attach_stage_not_called_for_cpu (1 xfail) to three passing real-backend tests: - test_warmup_and_load_cpu: lifecycle assertions - test_warmup_gpu_not_called_for_cpu: CPU path skips warmup_gpu - test_stage_load_cpu: stage export + usd_handle type check Adds test_warmup_and_load_gpu as xfail pending a GPU CI runner. Result: 70 passed, 73 xfailed (was 67 passed, 73 xfailed). Update the test-gaps doc to reflect the closed gap (no wheel change required for this category). Issue: isaac-sim#5316
Drop the kitless mocks, the SimpleNamespace sim-context fake, and the MockOvPhysxBindingSet shell-injection helpers. The standard SimulationContext + UsdFileCfg(ISAAC_NUCLEUS_DIR/...) pattern works under ./scripts/run_ovphysx.sh without AppLauncher — omni.client resolves Nucleus URLs from Kit's Python directly, and SimulationContext runs kitless via has_kit() returning False. Tests now exercise real RigidObject + RigidObjectData + OvPhysxManager against live ovphysx.PhysX bindings, using the same Nucleus assets the Cartpole/Newton tests use. Material-properties tests stay xfailed pending a wheel-side RIGID_BODY_MATERIAL TensorType. GPU tests now pass (NVIDIA RTX 5000 Ada verified). Production bug surfaced: RigidObject._initialize_impl uses hasattr() on TensorBinding.body_names which propagates RuntimeError (not AttributeError), blocking all RigidObject lifecycle tests against the real backend. Fix: wrap in try/except (AttributeError, RuntimeError). Tracking: issue isaac-sim#5316. Issue: isaac-sim#5316
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR implements an OVPhysX-specific IMU sensor (Imu and ImuData) following the existing BaseImu/BaseImuData contracts. The implementation uses ovphysx tensor bindings for rigid body state queries and Warp kernels for IMU computations. The code structure mirrors the PhysX backend, with key divergences in tensor binding APIs and gravity source. Overall, the implementation appears correct with minor concerns.
Architecture Impact
- New public API:
isaaclab_ovphysx.sensors.ImuandImuDataare exported via lazy_export, registered for theovphysxbackend via__backend_name__ - Cross-module dependencies: Uses
PhysicsManager._sim.cfg.gravity(internal API access) instead of a proper manager method - Tensor type additions:
RIGID_BODY_POSE,RIGID_BODY_VELOCITY,RIGID_BODY_COM_POSEadded totensor_types.pyfor future RigidObject work - Test infrastructure: 3 tests skipped pending #5316 (RigidObject support), 5 articulation-based tests exercise the sensor
Implementation Verdict
Minor fixes needed
Test Coverage
The test suite adapts the PhysX IMU tests with appropriate backend swaps. 5 out of 8 tests are active and exercise:
- Pendulum IMU readings (
test_single_dof_pendulum) - Indirect attachment via Xform (
test_indirect_attachment) - Offset composition (
test_offset_calculation) - Invalid attachment error (
test_attachment_validity) - Env ID propagation (
test_env_ids_propagation)
3 tests are skipped with clear pytest.skip() citing issue #5316 (RigidObject dependency). This is acceptable for incremental feature delivery.
Gap: No explicit test verifies the gravity bias magnitude at rest (the check_imu.py script does this manually but isn't a pytest assertion).
CI Status
All relevant checks pass (pre-commit, license, installation, docs build). The isaaclab_ovphysx tests are skipped in the isaaclab_ov* CI pattern due to the pytest.importorskip("ovphysx.types") guard — this is intentional and documented in the test file.
Findings
🟡 Warning: imu.py:160 — Division-by-zero risk on first update
def update(self, dt: float, force_recompute: bool = False):
self._dt = dt
super().update(dt, force_recompute)In _update_buffers_impl (line 212), 1.0 / self._dt is passed to the kernel. If update() is called with dt=0.0 (e.g., due to misconfiguration), this produces inf. The PhysX backend has the same pattern, but consider adding a guard:
if dt <= 0.0:
raise ValueError(f"IMU update dt must be positive, got {dt}")🟡 Warning: imu.py:152 — Accessing private PhysicsManager._sim.cfg.gravity
gravity = PhysicsManager._sim.cfg.gravityThis relies on an internal attribute. If PhysicsManager API changes, this breaks silently. The PR description acknowledges OvPhysxManager lacks get_gravity() — consider adding that method to OvPhysxManager for proper encapsulation, or at minimum document this as technical debt.
🔵 Improvement: imu.py:137-139 — Pattern construction doesn't handle {ENV_REGEX_NS} token
pattern = self._rigid_parent_expr.replace(".*", "*")The path expression can contain {ENV_REGEX_NS} (visible in test configs like "{ENV_REGEX_NS}/robot/imu_link"). This replacement only handles .*. While {ENV_REGEX_NS} is expanded elsewhere before reaching this code, the comment should clarify this expectation to avoid future confusion.
🔵 Improvement: kernels.py:35-36 — Spatial vector component order assumption undocumented
lin_vel_w = wp.spatial_top(velocities[idx])
ang_vel_w = wp.spatial_bottom(velocities[idx])The ovphysx RIGID_BODY_VELOCITY layout is (vx, vy, vz, wx, wy, wz) per tensor_types.py:48. Warp's spatial_vectorf has top=angular, bottom=linear by convention. If ovphysx stores linear-first, these are swapped. Verify the ovphysx layout matches this kernel's expectations. The PhysX backend may have different conventions.
🔵 Improvement: imu_data.py:40-53 — Missing reset() or buffer clearing method
The ImuData class creates ProxyArray caches lazily but doesn't invalidate them if create_buffers() is called again (e.g., after a scene reset that changes num_envs). The current usage appears safe, but adding defensive self._ang_vel_b_ta = None in create_buffers() would prevent stale cache issues:
def create_buffers(self, num_envs: int, device: str) -> None:
# ... existing code ...
# Invalidate lazy caches
self._ang_vel_b_ta = None
self._lin_acc_b_ta = None🔵 Improvement: test_imu.py:175 — Missing assertion on actual env_ids reset behavior
def test_env_ids_propagation(setup_sim):
# ... steps ...
scene.reset(env_ids=[1])
# ... more steps ...This test exercises the code path but doesn't assert that env 0's data was preserved while env 1's was reset. Add assertions comparing pre/post reset values to verify correctness.
🔵 Improvement: check_imu.py — Should be a proper pytest with assertions
The standalone validation script prints values for "visual inspection" but doesn't fail on incorrect results. Consider converting the final magnitude check to a proper assertion or adding a pytest-based version that asserts |lin_acc_b| ≈ 9.81 at rest.
d7eb80a to
52fe2ec
Compare
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
Previous concerns status: All previous concerns were acknowledged as non-issues (design decisions or review artifacts). No code changes were required.
New commit analysis (52fe2ec1): This appears to be a rebase/merge commit. The core implementation of the OVPhysX IMU sensor, RigidObject/RigidObjectData, shared kernels, and test infrastructure remains unchanged from the previous review.
Implementation Verdict
Ship it — The implementation is complete, well-tested with 6 active kitless tests plus comprehensive interface coverage, and follows established Isaac Lab patterns. No new issues introduced.
The dual-Carbonite destructor race that crashed test_rigid_object.py between tests is not test-specific: any production caller that closes a SimulationContext and constructs a new one in the same process hits the same path. The previous PR worked around it with session-scoped class-level monkey patches in the test file (_PHYSX_BY_DEVICE cache, _patched_release_physx, _patched_warmup_and_load); production code remained latently buggy. Move the workaround into the manager itself: * _release_physx is now a soft reset -- physx.reset() + wait_op while keeping the cached ovphysx.PhysX reference alive on cls._physx, so the C++ destructor never fires mid-process. * _warmup_and_load reuses the cached instance on subsequent calls. First call constructs + locks the device + registers atexit; later calls re-export the USD, run physx.reset() to clear the prior stage, add_usd, replay clones, and (on GPU) re-run warmup_gpu so the new stage's bodies are resident. * New _locked_device class attribute mirrors the wheel's process-global device-mode lock; the manager raises RuntimeError with a clear message instead of letting the wheel's PhysXDeviceError fire. * _construct_physx splits out the first-time bootstrap so the orchestration in _warmup_and_load stays linear. * HACK comments on _release_physx are keyed to the same wheel-fix milestone (namespace-isolated Carbonite) tracked by gap G5. The dict keyed by device (_PHYSX_BY_DEVICE) is gone -- since the wheel locks the process to one device, the dict could never hold more than one entry and was misleading. In test_rigid_object.py, drop the entire patch block (~150 lines): _PHYSX_BY_DEVICE, _patched_release_physx, _patched_warmup_and_load, _orig_*, _ovphysx_session_patches. Keep _LOCKED_DEVICE plus _ovphysx_skip_other_device autouse fixture -- the only remaining test-side concern is pre-empting the device-lock RuntimeError with a clean pytest.skip on parametrized device mismatch, so two-pass CI finishes cleanly when only one device is exercised. Update test_warmup_attach_stage_not_called_for_cpu to spy on a real PhysX after construction (the old approach silently relied on the patch overwriting cls._physx). Tests: 42 CPU + 41 GPU passing (two pytest invocations per Marco's two-pass requirement). Bumps isaaclab_ovphysx 0.2.16 -> 0.2.17.
* Untrack docs/superpowers/specs/2026-04-27-ovphysx-rigid-body-tensortypes-gap.md (file is gitignored under docs/superpowers/; keep locally only). * Revert the ovphysx variant additions to allegro_hand_env_cfg.py (ObjectCfg.ovphysx, PhysicsCfg.ovphysx, OvPhysxCfg import) and the matching isaaclab_tasks 1.5.30 CHANGELOG entry / version bump. * Trim test_rigid_object.py module docstring: drop the PhysX-mirror prose and the PhysX adaptation walk-through (both inferable from the file contents); drop the gap-G5 / superpowers spec reference. * Tighten the _ovphysx_skip_other_device fixture docstring; the module docstring now carries the device-lock rationale. * Move test_mock_binding_set_rigid_object_shapes from test_articulation.py to a parallel test_rigid_object_helpers.py so the rigid-object mock-binding contract test sits next to the rest of the rigid-object scaffolding. * Convert the new "Rigid-body TensorTypes" section header in tensor_types.py to the triple-quoted-string style used elsewhere in the file.
Revert direct edits to source/isaaclab_ovphysx/config/extension.toml and source/isaaclab_ovphysx/docs/CHANGELOG.rst -- both are nightly-CI-compiled outputs per AGENTS.md, so PRs add fragment files under source/<pkg>/changelog.d/ instead. Add a .minor.rst fragment summarising the user-facing additions for this PR (RigidObject and RigidObjectData, RIGID_BODY_* TensorType aliases, the shared isaaclab_ovphysx.assets.kernels module, prim-scan validation in _initialize_impl) and changes to OvPhysxManager (soft reset on stage swap, device-lock surfacing, unified PhysxScene config across CPU and GPU). The isaaclab core touch is test-only, so use a .skip fragment there.
Drop the try/except that swallowed wheel-side TensorBinding creation errors and demoted them to a debug log, returning None. Most callers already dereferenced the result unconditionally, and the only one that null-checked (the wrench writer in write_data_to_sim) silently skipped its write -- the asymmetric handling was a foot-gun. Eager creation in _initialize_impl already populates the cache for every binding the writers consume, so post-init calls cannot fail. The init eager loop now wraps each call in try/except and re-raises with the existing helpful diagnostic about prim_path / pattern / wheel coverage, so creation failures still surface with context. Drop the now-unused logging import.
Multiple review fixes to source/.../rigid_object/rigid_object_data.py. * Drop num_instances / num_bodies / body_names from the __init__ Args block -- they are not parameters; they are read from the bindings or set by RigidObject._initialize_impl. * Fix incorrect frame descriptions inherited via copy-paste from PhysX/Newton: body_link_vel_w / body_com_vel_w now describe the body's link / COM frame (not "the root rigid body"), and the four *_b base-frame velocity properties read "world-frame velocity expressed in the root link's actor frame" rather than the self-referential and trivially-zero "with respect to the rigid body's actor frame". * Drop "in the simulation world frame" from body_mass (mass is frameless) and body_inertia (it is expressed at the COM per the RIGID_BODY_INERTIA tensor-types contract). Standardise inertia units to [kg.m^2] using the same Unicode notation already used elsewhere in this file. * Add a check_shapes (default True) constructor parameter, plumbed from AssetBase._check_shapes by RigidObject._initialize_impl. Use it in _read_binding_into to assert the destination buffer has at least as many bytes as the binding will write, defending the wp.array(ptr=...) reinterpret against a future buffer/binding size drift that would otherwise corrupt memory silently. The flag follows the same AssetBaseCfg.disable_shape_checks knob already used by the writer-side assert_shape_and_dtype.
OVPhysX ``RigidObject._initialize_impl`` was using ``self.cfg.prim_path`` directly as the wheel binding pattern, but ``ovphysx.create_tensor_binding`` expects fnmatch-style globs (``*``) not regex (``.*``). IsaacLab idiomatically writes prim paths in regex form (e.g. ``/World/envs/env_.*/object``), so the wheel binding's ``count`` came back as 0 even when the prims existed in the USD stage. The Articulation backend already handles this (articulation.py:562-563); mirror that conversion in RigidObject. Symptom: any RigidObject configured with a ``env_.*`` prim path crashes deep in property accessors with a misleading ``TypeError: unsupported operand type(s) for +: 'NoneType' and 'int'`` when ``WrenchComposer.__init__`` queries ``body_link_quat_w`` against an empty ``transformf`` buffer. Surfaced when wiring the OVPhysX preset into ``Isaac-Repose-Cube-Allegro-Direct-v0``: the Allegro hand articulation works, the cube uses the same env-clone prim-path pattern but as a RigidObject, and the wheel binding silently returned 0 prims.
The PR introduces wholly-new asset classes (:class:`~isaaclab_ovphysx.assets.RigidObject` / :class:`~isaaclab_ovphysx.assets.RigidObjectData`) and changes the :class:`~isaaclab_ovphysx.physics.OvPhysxManager` lifecycle behaviour. Reclassify the fragment from ``.minor.rst`` to ``.major.rst`` per ``AGENTS.md`` so the nightly CI emits a major version bump.
Add Articulation and ArticulationData for the OVPhysX backend, mirroring the PhysX/Newton public API. Resolves PR isaac-sim#5459. Articulation ^^^^^^^^^^^^ * index/mask split for every state writer, simulation-parameter writer, setter, and tendon setter; OVPhysX exposes both _index and _mask as first-class paths and intentionally drops the PhysX-specific ``full_data`` kwarg. * Dedicated dynamic + viscous friction setters (write_joint_{dynamic,viscous}_friction_coefficient_to_sim_{index,mask}) that touch only their slot of the combined (N, J, 3) DOF_FRICTION_PROPERTIES buffer. The combined write_joint_friction_coefficient_to_sim_index/_mask still accepts all three components as kwargs (Coulomb static + dynamic + optional viscous) for source-compatible PhysX call sites. * Deprecated non-indexed shorthand shims for friction (x3) and root / joint state (x4), forwarding to the index variants with a DeprecationWarning, matching PhysX's deprecated section. * Wrench-composer return types tightened to non-None (instantaneous_wrench_composer / permanent_wrench_composer); composers are always set in _create_buffers, mirroring PhysX/Newton. * Section organisation matches PhysX exactly: Properties -> Operations -> Operations - Finders -> Operations - State Writers -> Operations - Simulation Parameters Writers -> Operations - Setters -> Operations - Tendons -> Internal helper -> Internal simulation callbacks -> Internal helpers -- Actuators -> Internal helpers -- Debugging -> Deprecated methods. Section delimiters use bare """Section.""" docstring blocks (Newton/PhysX convention). ArticulationData ^^^^^^^^^^^^^^^^ * Pull-on-demand timestamped buffers; CPU-only bindings route through pinned-host staging (PR isaac-sim#5329 pattern). * Property names match PhysX exactly: joint_friction_coeff, joint_dynamic_friction_coeff, joint_viscous_friction_coeff (no *_static / *_dynamic / *_viscous renames). * SI units annotated on every public property docstring ([m or rad, depending on joint type], [m/s or rad/s, ...], [N*m], [kg], etc.) per AGENTS.md. * binding_getter parameter on __init__ typed as Callable[[int], Any] | None. * Section organisation matches PhysX (Defaults -> Joint commands -> Joint properties -> Fixed tendon -> Spatial tendon -> Root state -> Body state -> Joint state -> Derived -> Sliced -> Internal helpers -> Deprecated properties). Kernels ^^^^^^^ * Articulation-specific kernels in isaaclab_ovphysx/assets/articulation/kernels.py (soft-limit clamp, friction-data writer, finite-difference joint-acc helper, body-CoM pose composer); shared kernels promoted to isaaclab_ovphysx/assets/kernels.py. * Per-kernel docstrings document purpose, shape/dtype/SI units, and divergence notes where the OVPhysX implementation differs (e.g. _fd_joint_acc takes inv_dt rather than dt to avoid per-element division). Tests ^^^^^ * Real-backend test_articulation.py mirrors isaaclab_physx 1-to-1 under run_ovphysx.sh; 99 tests pass on each of CPU + CUDA. * test_articulation_helpers.py covers the kitless-only helpers (tendon scoping, mock binding shapes). * Cross-backend test_articulation_iface.py runs the OVPhysX path: 544 tests pass, 16 skipped, 0 failed on each of CPU + CUDA. Brings the iface helper up to the actual ArticulationData constructor signature and broadcasts scalar inputs across joint / fixed-tendon / spatial-tendon mask setters that previously rejected them. OVPhysX-only surface dropped ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * Articulation: set_external_force_and_torque_{index,mask} (use instantaneous_wrench_composer.add_forces_and_torques_*), set_spatial_tendon_limit_{index,mask}, and set_spatial_tendon_rest_length_{index,mask} (NotImplementedError stubs PhysX never had). * ArticulationData: body_pose_w / body_lin_vel_w / body_ang_vel_w / body_acc_w / body_link_acc_w (base class provides matching defaults), body_inv_mass / body_inv_inertia, fixed_tendon_limit (PhysX exposes only fixed_tendon_pos_limits), spatial_tendon_limit / spatial_tendon_rest_length (no PhysX equivalent).
Adds ``ovphysx`` to both preset classes in ``allegro_hand_env_cfg.py``: * ``ObjectCfg`` — cube uses ``RigidObjectCfg`` (same as the ``physx`` preset, since OVPhysX ships RigidObject support). * ``PhysicsCfg`` — ``OvPhysxCfg()`` with defaults. Validated with RSL-RL: 300 iterations clean on cuda:0. Exercises both the articulation (Allegro hand, 16 DOFs) and the rigid object (cube) backends in the same scene.
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
Replace `# ----` section banners with the triple-quoted section markers used by PhysX/Newton, move the deprecated `write_root_*_state_to_sim` writers below the simulation callbacks, and tighten the internal-helper docstrings. Also strips Step-N narrative comments and "mirrors PhysX" dev-cruft so the file reads like the reference backends. No behavior change.
Use the same `Articulation-specific warp functions/kernels` section headers, drop the verbose absent-kernel module docstring, and tighten kernel docstrings to match PhysX conventions (SI units in `[unit]` notation, full Args sections with shapes, `:paramref:` instead of `:attr:` for kernel arguments). No behavior changes; kernel names and import paths are unchanged.
Reorder the public Articulation properties to match the PhysX/Newton section ordering (data, num_instances, is_fixed_base, num_joints, num_fixed_tendons, num_spatial_tendons, num_bodies, joint_names, fixed_tendon_names, spatial_tendon_names, body_names, root_view, *_wrench_composer). Align find_*, reset, write_data_to_sim, and _initialize_impl docstrings/comments with the PhysX style: * drop the verbose `Step N:` annotations from `_initialize_impl`, keeping only WHY-comments around non-obvious invariants * remove `# Mirrors PhysX (articulation.py:NNN)` line-number cross-references that aged poorly * tighten `reset`, `write_data_to_sim`, and `update` docstrings to match the PhysX wording No behavior changes.
Replace the OVPhysX-implementation-flavored module docstring and class
docstring with the same domain-level prose used by the PhysX/Newton
ArticulationData, plus an OVPhysX-specific note about pinned-host CPU
staging for CPU-only bindings. Reword internal `__init__` comments to
drop dev-cruft references ("post-audit RigidObjectData demotion",
"PR isaac-sim#5329 pattern") and group fields the way PhysX does (timestamp,
constants, buffers). Reword `update()` to mirror PhysX phrasing.
No behavior changes.
Replace the OVPhysX-implementation-flavored module/class docstring with the same domain-level prose used by the PhysX and Newton Articulation classes, plus a short OVPhysX-specific paragraph about per-tensor-type ``TensorBinding`` objects and pinned-host CPU staging. Drop "the wheel" backend-internal slang from comments and docstrings in favor of "binding" / "simulation". Remove leftover "PR isaac-sim#5329 pattern" references and other dev cruft. No behavior changes.
Replace the OVPhysX-implementation-flavored module/class docstring with the domain-level prose used by the PhysX and Newton Articulation classes, plus a short OVPhysX-specific paragraph about per-tensor-type ``TensorBinding`` objects and pinned-host CPU staging. Add the pyright header that the PhysX/Newton files carry.
Remove the two orphan callers of `_read_binding_into_view` (a method that was never defined on this branch, leaving an AttributeError on every read through `_read_binding_into_buf` and `_read_spatial_vector_binding`) and restore the original `self._get_binding(tensor_type).read(view)` pattern used elsewhere. Add `cls._physx.update_articulations_kinematic()` to `OvPhysxManager.step()` so link transforms reflect the freshly integrated joint state, matching the PhysX backend's behavior. This relies on the FK API exposed by ovphysx 0.4. Unblocks ~80 articulation tests previously failing with `'ArticulationData' object has no attribute '_read_binding_into_view'`.
The `body_incoming_joint_wrench_b` property reads into `self._body_incoming_joint_wrench_buf` and wraps it in a `ProxyArray`, but `_create_buffers` never allocated the buffer and `_pin_proxy_arrays` never declared the corresponding `_ta` slot. The first access raised AttributeError on the missing buffer. Allocate it next to the other body-state buffers (shape `(N, L)`, dtype `wp.spatial_vectorf`, matching the property docstring) and declare its `ProxyArray` cache slot in `_pin_proxy_arrays`.
The latest develop merge renamed `JointDrivePropertiesCfg.max_velocity` → `JointDriveBaseCfg.max_joint_velocity` and `max_effort` → `max_force`. The constructor still accepts the old kwargs as deprecation aliases, but `__post_init__` forwards the value to the canonical name and **blanks the alias**, so reads through the old name return None. Update the three test sites that resolve the USD default limit to use the canonical field names.
Mirror the PhysX backend's FK-on-demand pattern (see ``isaaclab_physx.assets.articulation_data:732``): when ``body_link_pose_w`` finds its buffer stale, call ``ovphysx.PhysX.update_articulations_kinematic`` before reading the LINK_POSE binding. This ensures body link poses reflect joint state written via ``write_joint_position_to_sim_*`` without requiring a sim step, closing the gap that previously xfailed ``test_write_joint_state_data_consistency`` on GPU sims. Drop the now-obsolete ``_FK_ON_DEMAND_GAP_REASON`` xfail marker and constant in the test file.
Mirror the PhysX backend (source/isaaclab_physx/isaaclab_physx/sensors/imu/) with three substantive divergences: - Use OvPhysxManager.get_physx_instance() + create_tensor_binding(...) with RIGID_BODY_POSE / RIGID_BODY_VELOCITY / RIGID_BODY_COM_POSE instead of physics_sim_view.create_rigid_body_view(...). - Read scene gravity from PhysicsManager._sim.cfg.gravity (3-tuple); the OvPhysx manager has no get_gravity(). - Pre-allocate structured-dtype buffers (wp.transformf, wp.spatial_vectorf) and use ovphysx's destination-as-argument binding.read(dst) API. Warp kernels (imu_update_kernel, imu_reset_kernel) are mathematically identical to the PhysX ones; only the data source comment differs. Refs isaac-sim#5318.
Mirror the structure of source/isaaclab_physx/test/sensors/test_imu.py under the kitless launcher pattern established by the rigid-object test: - Wheel gate (importorskip ovphysx.types + RIGID_BODY_POSE hasattr check) before AppLauncher; no AppLauncher needed at runtime. - SimulationContext built directly via build_simulation_context; assets spawned imperatively via _spawn_envs / _spawn_balls / _spawn_cubes / _spawn_anymal helpers; fnmatch-glob prim paths (env_*). - Six tests run on real Nucleus / procedural USD assets: test_constant_velocity, test_constant_acceleration, test_offset_calculation, test_env_ids_propagation, test_attachment_validity, test_sensor_print. - Two URDF-dependent tests (test_single_dof_pendulum, test_indirect_attachment) are skipped pending a USD version of simple_2_link.urdf — URDF-USD conversion requires the Kit URDF importer extension. check_imu.py is a kitless smoke-test script (free-falling sphere + IMU, prints readings for visual inspection). imu.update(dt) is called with force_recompute=True so the kernel fires each step (without InteractiveScene's lazy_sensor_update plumbing, the buffer would otherwise only refresh on imu.data access, leaving prev_lin_vel unseeded on iterations where the test skips data reads). Changelog fragment added under source/isaaclab_ovphysx/changelog.d/ per the new fragment-based workflow (do not edit CHANGELOG.rst directly). Refs isaac-sim#5318.
52fe2ec to
bbaf770
Compare
Verification agents flagged five issues against the previous two commits and one issue-isaac-sim#876 gap that the first pass missed. 1. Newton using-kamino.rst literalinclude path was one level short (../../../../../ → ../../../../../../). The previous depth resolved to a non-existent /docs/source/isaaclab_tasks/... and would have broken the sphinx build. 2. PhysX supported-features.rst listed Ray Caster, Visuo-tactile, and Camera as PhysX-specific sensors. Ray Caster and Camera are implemented in isaaclab core (backend-agnostic); Visuo-tactile lives in isaaclab_contrib. Reframe the section to separate PhysX-implemented sensors from backend-agnostic ones. Drop the unverifiable "path-traced" qualifier on the RTX renderer claim. 3. OvPhysX stub referenced only PR isaac-sim#5426 and PR isaac-sim#5459. The actual in-flight set spans six PRs: isaac-sim#5426 (merged), isaac-sim#5459, isaac-sim#5422, isaac-sim#5421, isaac-sim#5570, isaac-sim#5589. List them all with merge state, and reword "primary covered surfaces" to reflect that most are still open PRs. 4. backends/index.rst feature matrix said OvPhysX sensor coverage was "Partial" — actually only RigidObject is merged. Replace the matrix rows with concrete "In-flight (PR #...)" / "Not yet" cells. 5. Issue isaac-sim#876 asked to "review the limitations list and update it." The previous pass only reworded the intro. Refresh the task list against develop's actual newton_mjwarp coverage, add Shadow Hand / Shadow Hand Over / cabinet / dexsuite / rough-terrain locomotion, and replace the rigid bullet list with a discovery recipe so the list stops bit-rotting.
Description
Implements the OVPhysX-side
ImuandImuDatasensor pair, satisfying the existingBaseImu/BaseImuDatacontracts inisaaclab.sensors.imu. Mirrors the structure of the PhysX backend (source/isaaclab_physx/isaaclab_physx/sensors/imu/) so the two backends stay auditable side-by-side.The OVPhysX implementation diverges from PhysX in three places only:
OvPhysxManager.get_physx_instance().create_tensor_binding(pattern, tensor_type)withRIGID_BODY_POSE/RIGID_BODY_VELOCITY/RIGID_BODY_COM_POSEinstead ofphysics_sim_view.create_rigid_body_view(...). TheRIGID_BODY_*aliases are provided by [OVPHYSX] RigidObject + RigidObjectData asset #5426 (RigidObject) — this branch is rebased on top of that work.PhysicsManager._sim.cfg.gravity(3-tuple); the OvPhysx manager has noget_gravity()method.binding.read(dst)is destination-as-argument, so the sensor pre-allocates structured-dtype buffers (wp.transformf,wp.spatial_vectorf) and aliases them as flat-float32 read targets in_initialize_buffers_impl.The Warp kernels (
imu_update_kernel,imu_reset_kernel) are mathematically identical to the PhysX ones; only the data source comment differs.Tests
The pytest suite follows the kitless real-backend pattern established by the OVPhysX
RigidObjecttest (source/isaaclab_ovphysx/test/assets/test_rigid_object.py):AppLauncher—SimulationContextis built directly viabuild_simulation_contextand runs under./scripts/run_ovphysx.sh.ANYMAL_C_CFGfor articulation tests) and procedural primitives (SphereCfg,CuboidCfgfor rigid-object tests). No mocks._spawn_envs,_spawn_balls,_spawn_cubes,_spawn_anymal) instead ofInteractiveSceneCfg. Per-env prim paths under/World/envs/env_<i>/..., fnmatch-glob binding patterns (/World/envs/env_*/ball).pytest.importorskip("ovphysx.types", ...)plus aRIGID_BODY_POSEhasattrcheck, so the suite skips cleanly on older ovphysx wheels.test_constant_velocitytest_constant_accelerationtest_offset_calculationtest_env_ids_propagationtest_attachment_validitytest_sensor_printtest_single_dof_pendulumsimple_2_link.urdf(URDF importer is a Kit extension, not loaded underrun_ovphysx.sh)test_indirect_attachmentsource/isaaclab_ovphysx/test/sensors/check_imu.pyis a manual smoke-test script (free-falling sphere + IMU, prints readings for visual inspection). It also runs kitless.Dependencies
balls/cuberigid-object IMU test cases. Once [OVPHYSX] RigidObject + RigidObjectData asset #5426 merges, this branch needs to rebase ontodeveloponce more.test_single_dof_pendulum/test_indirect_attachmentrequires a USD version ofsimple_2_link.urdfto be made available (will follow up separately).Refs #5315 (parent epic), #5426 (RigidObject — testing dependency).
Fixes #5318
Type of change
Screenshots
N/A — no UI changes.
Checklist
pre-commitchecks with./isaaclab.sh --format./isaaclab.sh -d) need to be regenerated from a working IsaacSim Python environmentcheck_imu.pyvalidation scriptconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there