Skip to content

[OVPHYSX] ContactSensor#5422

Open
AntoineRichard wants to merge 126 commits into
isaac-sim:developfrom
AntoineRichard:antoiner/feat/ovphysx_contactsensor
Open

[OVPHYSX] ContactSensor#5422
AntoineRichard wants to merge 126 commits into
isaac-sim:developfrom
AntoineRichard:antoiner/feat/ovphysx_contactsensor

Conversation

@AntoineRichard
Copy link
Copy Markdown
Collaborator

@AntoineRichard AntoineRichard commented Apr 28, 2026

Description

Implements ContactSensor, ContactSensorCfg, and ContactSensorData for the isaaclab_ovphysx backend, mirroring the existing PhysX implementation.

The contact sensor reports normal contact forces in the world frame using the ovphysx 0.3.7 ContactBinding API (PhysX.create_contact_binding / read_net_forces / read_force_matrix). Optional pose tracking is wired through a RIGID_BODY_POSE TensorBinding.

Validation environment: Isaac-Velocity-Flat-Anymal-C-Direct-v0 (Anymal-C foot-contact tracking for locomotion).

v1 scope (this PR):

  • Net contact forces + history
  • Force matrix (filtered partner forces) + history
  • Pose tracking (track_pose)
  • Air/contact time tracking + compute_first_contact / compute_first_air
  • Reset semantics + native handle teardown on simulation stop

Deferred (raise NotImplementedError if cfg flag enabled):

  • track_contact_points
  • track_friction_forces

These features are blocked on tensor-friendly per-sensor read APIs in ovphysx (ContactBinding.read_contact_points / read_friction_forces). A maintainer-facing spec listing the missing APIs is attached at docs/superpowers/specs/2026-04-27-ovphysx-contact-api-gaps.md. The data properties still return None per the BaseContactSensorData contract; the implementation only raises if the cfg flag is set.

Fixes #5325
Parent issue: #5315

Stacked on:

Dependencies:

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

N/A — backend feature, no UI surface.

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 (changelog fragment at source/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_contactsensor.minor.rst, docstrings, gap spec for the maintainer of ovphysx)
  • My changes generate no new warnings
  • I have added tests that prove my feature works (9 tests adapted from the PhysX backend; 3 skipped for the deferred features so they're trivially un-skip-able once ovphysx ships the missing APIs)
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file (changelog fragment, minor tier — version bump compiled by the nightly CI workflow per the upstream convention)
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Notes for the reviewer

  • The Warp kernels (sensors/contact_sensor/kernels.py and the shared sensors/kernels.py) are physics-engine-agnostic and ported verbatim from the PhysX backend — only module docstrings differ.
  • ContactSensorCfg and ContactSensorData are also near-verbatim mirrors; the only edits are the backend-specific class_type pointer and the shared-kernels import path.
  • The ovphysx-specific glue lives in _initialize_impl (USD prim discovery + regex→fnmatch glob conversion + create_contact_binding + optional RIGID_BODY_POSE binding), _create_buffers (pre-allocated DLPack read buffers), _update_buffers_impl (per-step reads), and _invalidate_initialize_callback (native handle release).
  • Tests not yet executed. They are currently authored in the Isaac-Sim/AppLauncher style (mirroring isaaclab_physx/test/sensors/test_contact_sensor.py); they need to be re-adapted to the kitless ./scripts/run_ovphysx.sh -m pytest flow and the _ovphysx_sim_context helper used by the articulation/rigid-object suites in [OVPHYSX] Articulation rewrite (data class + asset class + kernels) #5459. Will land as a follow-up commit on this branch once that adaptation is reviewed.

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
@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label Apr 28, 2026
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
Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Choose a reason for hiding this comment

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

🤖 Isaac Lab Review Bot

Summary

This PR implements ContactSensor, ContactSensorCfg, and ContactSensorData for the ovphysx backend, providing contact force reporting via the ovphysx ContactBinding API. The implementation mirrors the PhysX backend structure, supporting net forces, force matrix filtering, history buffers, air/contact time tracking, and pose tracking. Two features (track_contact_points, track_friction_forces) are explicitly deferred with NotImplementedError. The implementation appears structurally sound but has several bugs in the Warp kernels and test setup that need addressing.

Architecture Impact

  • Cross-module: The sensor integrates with isaaclab.sensors.contact_sensor.BaseContactSensor and isaaclab.scene.InteractiveScene. The ContactSensorCfg uses lazy class resolution via "{DIR}.contact_sensor:ContactSensor".
  • Data flow: Contact data flows from ovphysx ContactBinding.read_net_forces() → flat Warp buffers → structured ContactSensorData buffers via Warp kernels.
  • Dependency: Blocked on #5316 for test execution (tests use RigidObject which requires ovphysx RigidObject support).

Implementation Verdict

Significant concerns — Several Warp kernel bugs will cause incorrect behavior or crashes at runtime. The tests are well-structured but cannot run until #5316 lands.

Test Coverage

The test file is comprehensive, ported from the PhysX backend with appropriate modifications. However:

  • Tests are not executable due to dependency on #5316
  • 3 tests properly skipped for deferred features
  • No unit tests for the Warp kernels in isolation
  • @flaky decorator on contact time tests suggests known instability

CI Status

  • Check for Broken Links: failure — Likely unrelated to this PR (documentation link issue)
  • Most CI jobs are skipped — expected given the test dependency on #5316
  • pre-commit: success confirms formatting is correct

Findings

🔴 Critical: contact_sensor.py:309-310 — reset() passes None to optional buffers causing kernel crash

The reset_contact_sensor_kernel is launched with self._data._friction_forces_w and self._data._contact_pos_w as outputs, but these are None when track_friction_forces=False and track_contact_points=False. Warp kernels cannot safely write to None arrays. The kernel guards (if friction_forces_w:) only prevent the write, but the array is still passed as an argument.

outputs=[
    ...
    self._data._friction_forces_w,  # None when track_friction_forces=False
    self._data._contact_pos_w,      # None when track_contact_points=False
],

Fix: Create empty placeholder buffers or restructure the kernel to not require these arguments when disabled.

🔴 Critical: kernels.py:229-230 — force_matrix_w_history accessed without null guard

In update_net_forces_kernel, when net_forces_matrix_flat is not None but force_matrix_w_history could be None (if history tracking is disabled differently than matrix tracking), the kernel will crash:

if net_forces_matrix_flat:
    for f in range(num_filter_shapes):
        force_matrix_w[env, sensor, f] = net_forces_matrix_flat[src_idx, f]
        for i in range(history_length - 1, 0, -1):
            force_matrix_w_history[env, i, sensor, f] = ...  # No null guard!

Looking at _create_buffers(), both are allocated together, so this may be safe in practice, but the asymmetric guarding is fragile.

🔴 Critical: contact_sensor.py:271-278 — Timestamp arrays passed incorrectly to kernel

The kernel expects wp.array(dtype=wp.float32) for timestamp and timestamp_last_update, but self._timestamp and self._timestamp_last_update from the base class are likely per-environment scalars or structured differently. Need to verify these match the expected (num_envs,) shape.

inputs=[
    ...
    self._timestamp,           # What shape/type is this?
    self._timestamp_last_update,
]

🟡 Warning: contact_sensor.py:207-208 — Pose binding pattern may match unintended prims

The pose binding uses a wildcard pattern f"{base_glob}/*" which could match more prims than intended if the scene has siblings under the base path:

single_pose_pattern = f"{base_glob}/*"
self._pose_binding = physx_instance.create_tensor_binding(
    pattern=single_pose_pattern,
    tensor_type=TT.RIGID_BODY_POSE,
)

The count check at line 215-221 catches mismatches, but a more precise pattern construction (using the actual body names) would be safer.

🟡 Warning: contact_sensor_data.py:25-35 — pose_w property launches kernel on every access

Every call to pose_w launches a Warp kernel to concatenate position and quaternion:

@property
def pose_w(self) -> ProxyArray | None:
    if self._pose_w is None:
        return None
    wp.launch(
        concat_pos_and_quat_to_pose_kernel,  # Launched every access!
        ...
    )

This is inefficient if accessed multiple times per step. Consider caching or only recomputing when data is dirty.

🟡 Warning: check_contact_sensor.py:51 — Test imports ovphysx before checking availability

The test file calls pytest.importorskip("ovphysx.types") but then has unconditional imports from isaaclab_ovphysx.sensors at line 53. If ovphysx is not installed, the skip happens but the subsequent import will still execute during collection:

pytest.importorskip("ovphysx.types", reason="ovphysx wheel not installed")
from isaaclab_ovphysx.sensors import ContactSensorCfg  # noqa: E402

This should work due to lazy export, but the order is fragile.

🔵 Improvement: kernels.py:280-292 — Duplicate kernel definition

concat_pos_and_quat_to_pose_kernel is defined both in contact_sensor/kernels.py and sensors/kernels.py. The one in contact_sensor_data.py imports from sensors/kernels.py, while contact_sensor.py also defines it locally via the import. This duplication will cause maintenance issues.

# In sensors/kernels.py
@wp.kernel
def concat_pos_and_quat_to_pose_kernel(...):

# Also in contact_sensor/kernels.py (line 280)
@wp.kernel  
def concat_pos_and_quat_to_pose_kernel(...):

Fix: Remove the duplicate in contact_sensor/kernels.py and import from sensors/kernels.py.

🔵 Improvement: contact_sensor.py:92-94 — Config mutation in __init__

Mutating the config object's attributes in __init__ is a side effect that could surprise callers:

if self.cfg.max_contact_data_count_per_prim is None:
    self.cfg.max_contact_data_count_per_prim = 4
if self.cfg.force_threshold is None:
    self.cfg.force_threshold = 1.0

Fix: Use local variables or @property accessors with defaults instead of mutating the config.

Discovers sensor bodies via USD prim walk + PhysxContactReportAPI schema
check, converts IsaacLab regex paths to ovphysx fnmatch globs, creates
the ContactBinding and optional RIGID_BODY_POSE tensor binding, then
pre-allocates Warp read buffers for net forces, force matrix, and poses.
Port the 9 contact sensor tests from the PhysX backend test file,
adapting them to use isaaclab_ovphysx.sensors.ContactSensorCfg
(which wires class_type to the ovphysx ContactSensor).

Three tests that depend on track_friction_forces (not yet supported
in ovphysx v1) are marked @pytest.mark.skip.  The contact-time helper
is adapted to never enable track_contact_points or track_friction_forces
so the basic contact-time tests continue to provide real coverage.
Replace try/except/pass patterns with contextlib.suppress
as required by ruff SIM105 rule. Also add missing contextlib
import.
The post-articulation upstream uses changelog fragments under
source/<pkg>/changelog.d/; restore the direct CHANGELOG.rst and
extension.toml edits and add a minor-tier fragment that the nightly
CI workflow will compile.
Drop AppLauncher and the PhysX-style setup_simulation fixture in favour
of the kitless _ovphysx_sim_context helper (matches test_rigid_object.py
shipped in isaac-sim#5459) and the autouse device-lock fixture for ovphysx<=0.3.7.

Also drop the disable_contact_processing parametrize axis: toggling the
global Carbonite '/physics/disableContactProcessing' flag requires the
settings manager which is not available in kitless mode.  test_no_contact_reporting
now exercises the same semantic via a no-filter cfg instead.

The three v2-deferred tests (track_friction_forces / track_contact_points)
keep their pytest.mark.skip but their bodies are fully kitless so they
can be un-skipped once the ovphysx wheel ships the missing per-sensor
read APIs (see docs/superpowers/specs/2026-04-27-ovphysx-contact-api-gaps.md).

Run via:
    ./scripts/run_ovphysx.sh -m pytest source/isaaclab_ovphysx/test/sensors/test_contact_sensor.py -v
The PhysX-only PRIM_DELETION callback in
isaaclab.sensors.SensorBase._register_callbacks was gated on
'physx' in physics_mgr_cls.__name__.lower(), which also matches
OvPhysxManager.  In kitless ovphysx mode the resulting
'from isaaclab_physx.physics import IsaacEvents' raises
ModuleNotFoundError because omni.physics.tensors isn't loaded.

Switch to an exact name match so the import only fires for the
PhysX backend.  Add an isaaclab changelog fragment.

Fixes the test-time import error surfaced by
test_contact_sensor.py under ./scripts/run_ovphysx.sh.
Three places matched ovphysx as physx via substring 'physx in
manager_name.lower()'.  Switch to exact name matches and explicitly
raise on unknown managers:
  - SensorBase._register_callbacks (already fixed in prior commit;
    extended changelog)
  - AssetBase.set_debug_vis (omni.kit.app import gate)
  - SceneDataProvider._get_backend (factory dispatch)

Also broaden InteractiveScene.physics_scene_path to fall back to a
bare UsdPhysics.Scene prim (TypeName='PhysicsScene') when no prim
with PhysxSceneAPI is present.  Kitless ovphysx does not load the
omni.physx schema, so the scene prim only carries the stock USD
type.  PhysX-backed flows still prefer PhysxSceneAPI.
The ovphysx ContactBinding API explicitly states that no USD schema
beyond the rigid bodies themselves is needed for contact reporting
(ovphysx/api.py:2186-2189).  The PhysxContactReportAPI check was
copied verbatim from the PhysX backend but is irrelevant for ovphysx
and silently breaks the kitless flow where the PhysxSchema USD
plugin is not registered (so AddAppliedSchema('PhysxContactReportAPI')
in activate_contact_sensors is a no-op).

Switch the discovery loop to require UsdPhysics.RigidBodyAPI (the
USD-core rigid body schema, applied by the spawner regardless of
PhysX availability).  Update the error message accordingly.
The ovphysx wheel ships its own PhysxSchema USD plugin under
ovphysx/plugins/usd/PhysxSchema, but in kitless runs it is not
auto-registered (omni.physx normally does that in Kit-based runs).
Without registration, prim.AddAppliedSchema('PhysxContactReportAPI')
silently writes the schema name to the apiSchemas listOp but the wheel's
C++ contact-binding check fails to match the schema on the prim and
emits 'Failed to find contact report API at ...' — the binding then
returns sensor_count=0.

Register the plugin once in OvPhysxManager.initialize() (idempotent;
guarded by a class-level flag).  This is the canonical bootstrap for
any sensor that depends on Physx-prefix schemas under kitless ovphysx,
not just ContactSensor.

Also switch ContactSensor._initialize_impl's discovery check from the
filtered prim.GetAppliedSchemas() to the unfiltered
prim.GetPrimTypeInfo().GetAppliedAPISchemas() so the Python-side
discovery sees the same schemas the wheel does.  The filtered API
hides Physx-prefix schemas in kitless mode because the C++ schema
type registration only happens when the omni.physx plugin loads the
PhysxSchema library — distinct from the plugInfo.json registration we
just added.
clear_callbacks() reset _callback_id back to 0, so subsequent
register_callback() calls handed out IDs that collided with old
CallbackHandle.id values still held by not-yet-garbage-collected
sensors / assets.  When the old object's __del__ eventually ran it
deregistered the NEW callback by ID — leaving the new sensor's
_initialize_callback wired up but never fired.

The symptom under kitless OVPhysX was a sensor that registered its
callback, then PHYSICS_READY dispatched, then sensor.reset()
crashed with 'has no attribute _ALL_ENV_MASK' (init never ran).
The PhysX backend hid this because its callback registry is the
Carbonite event bus rather than this Python dict.

Keep _callback_id monotonic across clear_callbacks invocations to
preserve uniqueness for the lifetime of the process.
* Add 'ovphysx' field to PhysicsCfg in AnymalD flat_env_cfg.py
* Add 'ovphysx' field to VelocityEnvContactSensorCfg in velocity_env_cfg.py
* events.py randomize_rigid_body_material: explicit OvPhysxManager branch
  (no-op + warning; PhysX impl assumes root_view.link_paths which OvPhysx's
  per-tensor-type bindings dict does not satisfy)
* ContactSensor._initialize_impl: override _num_envs from binding sensor_count
  (clone_usd=False on OvPhysX leaves env_1..N out of USD, so the parent
  class's USD walk reports num_envs=1 even with N=4096 cloned physics envs)
… layout

ovphysx ContactBinding returns sensors in pattern-major order:
the flat index for (env, sensor) is sensor*num_envs + env, NOT the
PhysX env-major env*num_sensors + sensor that update_net_forces_kernel
inherited from the PhysX port.

Symptom on AnymalD locomotion: body indices were scrambled, so foot
contacts were attributed to the wrong body (often the base), tripping
Episode_Termination/base_contact and resetting episodes after ~4 steps.

Verified by inspecting cb.sensor_paths for a 3-env, 2-body scene:
  ['/World/envs/env_0/A', '/World/envs/env_1/A', '/World/envs/env_2/A',
   '/World/envs/env_0/B', '/World/envs/env_1/B', '/World/envs/env_2/B']
i.e. each per-body sensor pattern is fully expanded across envs before
the next pattern starts.

Pass num_envs through update_net_forces_kernel and switch its src_idx
to the pattern-major formula.
Existing tests are all single-body per ContactSensor; pattern-major
vs. env-major flat-buffer indexing collapses to the same address when
there is only one body, so they couldn't catch the kernel formula bug
fixed in the previous commit.

test_multi_body_per_sensor_indexing exercises a single ContactSensor
with prim_path='{ENV_REGEX_NS}/Cube_.*' that resolves to two cubes per
env (one on the ground, one floating).  After the scene settles, the
on-ground cube reports a non-zero net force and the floating cube
reports zero.  If the kernel indexing is wrong, the floating cube
picks up a phantom contact force from another env's grounded cube and
the assertion fires with a diagnostic message pointing at the
pattern-major / env-major mismatch.

Verified to fail (sum-abs ~3.0) on the pre-fix kernel and pass after
re-applying sensor*num_envs+env.
Without an 'ovphysx' field on RoughPhysicsCfg, 'presets=ovphysx' falls
back to the 'default' PhysxCfg for the physics stack while still picking
the ovphysx ContactSensor (which we already wired into
VelocityEnvContactSensorCfg).  The OvPhysx ContactSensor's
_initialize_impl then calls OvPhysxManager.get_physx_instance() and
raises 'OvPhysxManager has not been initialized yet.' on PhysX-backed
rough envs (e.g. Isaac-Velocity-Rough-Anymal-D-v0).

Add OvPhysxCfg() to RoughPhysicsCfg so 'presets=ovphysx' selects the
ovphysx physics stack for rough locomotion the same way it already does
for the AnymalD flat env.
Add the ovphysx entry to the FrameView factory and teach _get_backend
to recognise OvPhysxManager so calls to FrameView(...) return the
OVPhysX-backed view instead of falling through to FabricFrameView.
Implement a Warp-native batched-prim view that mirrors
NewtonSiteFrameView's site-resolution approach against the OVPhysX
scene data provider's body_q array. Each prim resolves at init to a
(body_idx, site_local) pair via USD ancestor walk; world poses are
computed on GPU as body_q[bid] * site_local. Scales and visibility
delegate to an internal UsdFrameView. The view defers initialization
to PhysicsEvent.PHYSICS_READY when the SDP is not yet built.

Adds contract-suite coverage via the shared frame_view_contract_utils
plus OVPhysX-specific tests for factory dispatch, deferred-init
guarding, and the requires_newton_model error path.
Read body poses directly from an OVPhysX RIGID_BODY_POSE tensor binding
(mirroring the ContactSensor data path) instead of going through the
scene data provider's Newton state. Scenes that don't declare
requires_newton_model=True (e.g. headless training without a Newton-style
visualizer) can now use OvPhysxFrameView -- the previous design raised at
PHYSICS_READY because the SDP exposes body_q only when the Newton model
build is requested.

Site discovery handles both InteractiveScene modes:
  * clone_usd=True: every env has USD prims, one per env matches the
    pattern and resolves directly to its rigid-body ancestor.
  * clone_usd=False: only env_0 has authored USD prims; env_1..N are
    physics-layer clones. The binding row count becomes the authoritative
    site count, and per-env site paths are synthesized from the env_0
    template prim with env_0 replaced by each row's env_id.

The Warp kernels are unchanged. count, prim_paths, and prims honor the
clone_usd=False expansion. Tests updated to drive _pose_buf directly
(detach the binding after one warm-up read) instead of mutating SDP
body_q, and the now-obsolete requires_newton_model error test is removed.
Under OVPhysX's clone_usd=False scenes (the default for InteractiveScene),
USD only holds env_0 -- env_1..N are physics-layer clones via physx.clone().
SensorBase.__init__ derives _num_envs from len(find_matching_prims(...)) so
any FrameView-using sensor (RayCaster, MultiMeshRayCaster, Camera) sees
_num_envs=1 even when the scene has many envs. The sensor's
_reset_mask_torch is then sized 1, and the first reset(env_ids=[0..N-1])
triggers a CUDA assert from out-of-bounds indexing.

Walk the call stack at construction time to capture the sensor that owns
this view, then at the end of _initialize_impl re-allocate its env-sized
buffers (_ALL_ENV_MASK, _reset_mask + torch view, _is_outdated, _timestamp,
_timestamp_last_update) to match the OVPhysX RIGID_BODY_POSE binding's
row count. Duck-typed -- works for any SensorBase subclass without an
isaaclab.sensors import dependency. Mirrors the local fix the OVPhysX
ContactSensor already applies to itself at contact_sensor.py:240-248.

This is a hack confined to the OVPhysX backend. A cleaner long-term fix
would source _num_envs from InteractiveScene.num_envs (or move the
under-cloning behaviour to be opt-in for rendering rather than backend-
keyed), but both are larger changes that touch core IsaacLab.

Verified: Anymal-D rough velocity env with presets=ovphysx + 64 envs
now initializes past env.reset() without the CUDA assert.
…/Newton

InteractiveScene now USD-replicates the per-env asset subtree for every
backend including OVPhysX (clone_usd=True). OVPhysX's add_usd + physx.clone
tolerates the extra USD content at runtime, so the per-env USD prims layer
cleanly on top of the physics-side replicas. Sensors that discover
_num_envs via find_matching_prims now see N prims under OVPhysX too,
matching the behaviour they already had under PhysX and Newton.

Removes the two local workarounds that the previous clone_usd=False
behaviour had forced:

* OvPhysxFrameView no longer walks the call stack to patch the caller
  sensor's _num_envs (the hack from commit 3fd4f48). _num_envs now
  reaches the binding row count naturally.
* OvPhysX ContactSensor drops the binding_num_envs-driven re-allocation
  of _ALL_ENV_MASK / _reset_mask / _is_outdated / _timestamp /
  _timestamp_last_update, replacing it with an assertion that the
  binding row count agrees with SensorBase._num_envs.

Parity fixes inspired by a PhysX/Newton review:

* Rename the OVPhysX update_net_forces_kernel to
  update_net_forces_ovphysx_kernel. The PhysX kernel of the same name
  has a different signature and index convention; the explicit suffix
  prevents accidental cross-imports from compiling silently.
* Add _set_debug_vis_impl and a no-op _debug_vis_callback to the OVPhysX
  ContactSensor. The kitless OVPhysX flow has no Kit renderer, so the
  hooks are wired to log a one-shot warning rather than silently no-op
  when cfg.debug_vis=True.
* OvPhysxContactSensor.body_names now returns list[str] (never None)
  and raises a clear RuntimeError if accessed before initialization,
  matching the PhysX contract.
* OvPhysxFrameView._initialize_impl rejects prim_paths that resolve to
  a rigid body with a clear ValueError, mirroring the
  NewtonSiteFrameView guard. FrameView is for non-physics sensor frames;
  callers should use RigidObject/Articulation for body-level control.
* OvPhysxFrameView.get_scales / set_scales docstrings now make explicit
  that these read/write the static USD xformOp:scale attribute and do
  not propagate to PhysX collision-shape scale (unlike Newton's
  shape_scale path).
InteractiveScene now USD-replicates every env's asset subtree for all
backends (clone_usd=True from the previous commit), but OvPhysxManager
was still handing the full N-env stage to physx.add_usd. The wheel
loaded all N USD-defined bodies as independent prims, so the subsequent
physx.clone() ran onto already-populated targets and never produced the
clone-lineage that the wheel's create_tensor_binding fast path expects.
At 4096 envs this turned every binding-creation call into a multi-second
USD enumeration -- the hang in articulation init.

Re-shape _warmup_and_load to export an env_0-scoped USD file:

1. Export the full stage to disk (existing flatten-and-write).
2. Re-open the exported file as an Sdf.Layer.
3. Delete every /World/envs/env_<i>  prim spec for i != 0 from the layer.
4. Re-export.

The live USD stage held by SimulationContext is untouched -- sensors
(RayCaster, Camera, ContactSensor discovery) still see N envs and
discover _num_envs = N correctly. Only the file passed to the wheel is
scoped to env_0 + globals. physx.clone() then repopulates env_1..N at
the physics layer with proper lineage, and create_tensor_binding walks a
1-USD-path result that auto-extends across the N clones -- the fast
path that clone_usd=False used to give us implicitly.

Net effect: keeps the previous commit's clone_usd=True flip (sensor
parity across backends) while restoring OVPhysX's per-env scaling. No
test changes required; the FrameView/ContactSensor suite stays at
27/27 pass on CPU.
Adds a Limitations section to _export_env0_only_stage covering the
three assumptions the workaround makes:

* Homogeneous envs -- per-env USD-authored physics overrides (mass,
  friction, collision filters under env_<i!=0>) are dropped from the
  file handed to physx.add_usd. Sensors and visualizers still see
  them in the live stage, so a divergence is possible. Per-env state
  has to be written via the runtime APIs instead.
* Global path convention -- physics-relevant prims must live outside
  /World/envs (or under env_0) to survive the export.
* Static topology -- envs added/removed after warmup require a
  re-warmup with a re-exported stage.
@AntoineRichard AntoineRichard force-pushed the antoiner/feat/ovphysx_contactsensor branch from d1fbcd2 to 0ef602d Compare May 13, 2026 09:57
@kellyguo11 kellyguo11 moved this from In progress to In review in Isaac Lab May 14, 2026
AntoineRichard added a commit to AntoineRichard/IsaacLab that referenced this pull request May 15, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

2 participants