[OVPHYSX] ContactSensor#5422
Conversation
There was a problem hiding this comment.
🤖 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.BaseContactSensorandisaaclab.scene.InteractiveScene. TheContactSensorCfguses lazy class resolution via"{DIR}.contact_sensor:ContactSensor". - Data flow: Contact data flows from ovphysx
ContactBinding.read_net_forces()→ flat Warp buffers → structuredContactSensorDatabuffers via Warp kernels. - Dependency: Blocked on #5316 for test execution (tests use
RigidObjectwhich requires ovphysxRigidObjectsupport).
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
@flakydecorator 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: successconfirms 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: E402This 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.0Fix: Use local variables or @property accessors with defaults instead of mutating the config.
7981eed to
42001ea
Compare
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR implements ContactSensor, ContactSensorCfg, and ContactSensorData for the OVPhysX backend, mirroring the PhysX implementation. The sensor reports normal contact forces using the ovphysx.ContactBinding API.
Update (f495912)
133 commits since last review. Key feature-related changes:
Final Alignment with PhysX/Newton (f495912)
- Aligned OVPhysX ContactSensor parity with PhysX/Newton contracts
- Fixed
configclassimport path for develop branch compatibility (2578b48)
Multi-Body Sensor Indexing Fix (05c4053)
- Critical fix: Corrected flat-buffer indexing from env-major to pattern-major layout
- OVPhysX
ContactBindingreturnsflat_idx = sensor * num_envs + env(pattern-major) - PhysX uses
flat_idx = env * num_sensors + sensor(env-major) - Added dedicated regression test (
8e9783d) to catch layout mismatches
Infrastructure Improvements
- Wired OvPhysX preset into AnymalD Flat config (
9bfc8d2) - Auto-register PhysxSchema USD plugin in OvPhysxManager (
7ee5897) - Fixed PhysicsManager.clear_callbacks ID-recycle bug (
b1a1e59)
Code Review Findings
✅ Strengths
-
Clean API Symmetry: The sensor mirrors PhysX
ContactSensornearly 1:1, enabling backend-agnostic user code. -
Proper Unsupported Feature Handling:
track_contact_pointsandtrack_friction_forcesraiseNotImplementedErrorearly in__init__, before USD discovery — fail-fast is correct. -
Pattern-Major Indexing: The
update_net_forces_ovphysx_kernelcorrectly usessrc_idx = sensor * num_envs + envfor OVPhysX's flat buffer layout. -
Callback ID Recycling Fix:
PhysicsManager.clear_callbacks()no longer resets_callback_id = 0, preventing stale callback deregistration bugs in multi-scene tests. -
PhysxSchema Registration:
OvPhysxManager._ensure_physx_schemas_registered()handles kitless flows whereomni.physxdoesn't load the schema automatically. -
Comprehensive Test Suite: 1000+ line test file covering contact/air time, cube stack filtering, multi-body indexing, threshold validation — with proper device-lock fixture for OVPhysX's process-global device binding.
⚠️ Suggestions (Non-Blocking)
-
Duplicate Kernel Definition (lines ~300 in
contact_sensor/kernels.pyandsensors/kernels.py):concat_pos_and_quat_to_pose_kernelis defined in both files- The
contact_sensor/kernels.pyimports from the shared module but also has a local copy - Consider removing the duplicate from
contact_sensor/kernels.py
-
Force Threshold Comparison (kernels.py line ~268):
in_contact = wp.length_sq(net_forces_w[env, sensor]) > contact_force_threshold * contact_force_threshold
- Using squared threshold is computationally efficient ✓
- Document that
force_thresholdcfg is applied as squared magnitude
-
_num_filter_shapesInitialization (_create_buffers, line ~290):- The
filter_countis read from binding:self._contact_binding.filter_count if self.cfg.filter_prim_paths_expr else 0 - Consider validating that
filter_countmatches expectedlen(filter_prim_paths_expr)per sensor
- The
-
Debug Visualization Warning (
_set_debug_vis_impl):- Uses module-level
_warned_debug_vis_unavailableattribute viagetattr - Consider using a class attribute with
cls._warned_...for cleaner state management
- Uses module-level
📝 Documentation
- Changelog fragments present and well-structured ✓
- Docstrings comprehensive with parameter descriptions ✓
- Gap spec reference (
docs/superpowers/specs/2026-04-27-ovphysx-contact-api-gaps.md) properly documented for deferred features ✓
Test Coverage Analysis
| Test | Status | Coverage |
|---|---|---|
test_cube_contact_time |
✅ | Net forces, air/contact time, history |
test_sphere_contact_time |
✅ | Same as cube with sphere primitive |
test_cube_stack_contact_filtering |
✅ | Force matrix, filter patterns |
test_multi_body_per_sensor_indexing |
✅ | Pattern-major layout validation |
test_no_contact_reporting |
✅ | Unfiltered sensor behavior |
test_sensor_print |
✅ | __str__ method |
test_contact_sensor_threshold |
✅ | USD attribute verification |
test_friction_reporting |
⏭️ Skipped | v1 limitation |
test_invalid_prim_paths_config |
⏭️ Skipped | v1 limitation |
test_invalid_max_contact_points_config |
⏭️ Skipped | v1 limitation |
The skipped tests preserve the test bodies so they can be trivially un-skipped when ovphysx ships tensor-friendly friction/contact-point APIs.
CI Status Note
Check changelog fragmentsfailing — likely needs fragment regeneration after rebase- Other jobs pending/running
Verdict
LGTM — The ContactSensor implementation is complete and well-tested. The pattern-major indexing fix is correct and validated. The code follows Isaac Lab conventions and maintains API symmetry with PhysX.
The PR is ready to merge once:
- CI passes (changelog fragment check)
- Parent PR #5459 merges (per stacking note)
Update (e6fc1e0): Import order fix & changelog addition.
- Test import fix: Moved
pytest.importorskip("ovphysx.types")before allisaaclab_ovphysximports and deferredwp.init()— CI environments without the ovphysx wheel now skip cleanly instead of failing on import. - Changelog fragment: Added
antoiner-feat-ovphysx_contactsensor.rstdocumenting the newovphysxpreset for locomotion velocity tasks.
Both changes are housekeeping. LGTM stands.
Update (84f6e28): randomize_rigid_body_material physics backend detection refactor.
- events.py: Simplified manager detection logic in
RandomizeRigidBodyMaterialImpl.__init__:- Changed from exact name matching to substring matching with priority order
- Checks
"ovphysxmanager"first (to avoid false positive from"physx" in ...) - Then uses
"newton" in ...and"physx" in ...for Newton subclasses (NewtonMJWarpManager,NewtonKaminoManager, etc.) and PhysX variants - Manager name is now lowercased before comparison for case-insensitive matching
- This unifies the dispatch pattern and handles future Newton subclasses automatically without explicit enumeration.
Minor maintenance change, no functional impact on ContactSensor. LGTM holds.
498e4b0 to
25e8f3c
Compare
Greptile SummaryThis PR introduces the
Confidence Score: 3/5The core data-flow is sound, but the shared result buffer between compute_first_contact and compute_first_air means a locomotion policy calling both in the same step silently gets wrong answers for the first call. The shared _first_transition buffer is a real behavioral defect: both compute_first_contact and compute_first_air write into and return the same Warp array, so the earlier call is clobbered whenever a policy queries both signals in one control step — a common pattern in foot-contact locomotion. The duplicate kernel in contact_sensor/kernels.py causes unnecessary Warp JIT overhead. The contact_pos_w NaN-vs-zero inconsistency is latent but should be addressed before the deferred v2 feature lands. Tests are unrun in the current state, so runtime correctness of the pattern-major index remapping and the binding fan-out logic remains unverified. contact_sensor.py (shared transition buffer), kernels.py (dead kernel copy and NaN reset mismatch) Important Files Changed
Sequence DiagramsequenceDiagram
participant SC as SimulationContext
participant OPM as OvPhysxManager
participant CS as ContactSensor
participant CB as ContactBinding
participant PB as PoseBinding (opt)
participant WK as Warp Kernels
SC->>OPM: initialize(sim_context)
OPM->>OPM: _ensure_physx_schemas_registered()
SC->>OPM: reset()
OPM->>OPM: _warmup_and_load() to PhysX instance
SC->>CS: initialize()
CS->>CS: _initialize_impl()
CS->>OPM: get_physx_instance()
CS->>CS: discover sensor bodies (USD prim walk)
CS->>CB: physx_instance.create_contact_binding(sensor_patterns, filter_patterns)
CS->>CS: override _num_envs from binding.sensor_count
opt track_pose
CS->>PB: physx_instance.create_tensor_binding(RIGID_BODY_POSE)
end
CS->>CS: _create_buffers()
loop Every physics step
SC->>OPM: step_sync()
SC->>CS: update() to _update_buffers_impl()
CS->>CB: read_net_forces(net_forces_flat_buf)
opt filter_prim_paths_expr
CS->>CB: read_force_matrix(force_matrix_flat_buf)
end
CS->>WK: update_net_forces_kernel(pattern-major to env,sensor)
opt track_pose
CS->>PB: read(poses_flat_buf)
CS->>WK: split_flat_pose_to_pos_quat()
end
end
SC->>OPM: stop()
OPM->>CS: _invalidate_initialize_callback()
CS->>CB: destroy()
opt track_pose
CS->>PB: destroy()
end
OPM->>OPM: _release_physx() physx.reset() keep C++ instance
Reviews (1): Last reviewed commit: "Patch caller sensor _num_envs from OvPhy..." | Re-trigger Greptile |
| wp.launch( | ||
| compute_first_transition_kernel, | ||
| dim=(self._num_envs, self._num_sensors), | ||
| inputs=[float(dt + abs_tol), self._data._current_contact_time], | ||
| outputs=[self._data._first_transition], | ||
| device=self._device, | ||
| ) | ||
| return self._data._first_transition_ta | ||
|
|
||
| def compute_first_air(self, dt: float, abs_tol: float = 1.0e-8) -> ProxyArray: | ||
| """Boolean mask (as float) of bodies that broke contact within ``dt`` [s]. | ||
|
|
||
| Args: | ||
| dt: Time window since contact break [s]. | ||
| abs_tol: Absolute tolerance for the comparison [s]. | ||
|
|
||
| Returns: | ||
| Boolean tensor (1.0/0.0) of shape ``(num_envs, num_sensors)``. | ||
|
|
||
| Raises: | ||
| RuntimeError: If :attr:`ContactSensorCfg.track_air_time` is False. | ||
| """ | ||
| if not self.cfg.track_air_time: | ||
| raise RuntimeError( | ||
| "The contact sensor is not configured to track air time." | ||
| " Please enable 'track_air_time' in the sensor configuration." | ||
| ) | ||
| wp.launch( | ||
| compute_first_transition_kernel, | ||
| dim=(self._num_envs, self._num_sensors), | ||
| inputs=[float(dt + abs_tol), self._data._current_air_time], | ||
| outputs=[self._data._first_transition], | ||
| device=self._device, | ||
| ) | ||
| return self._data._first_transition_ta |
There was a problem hiding this comment.
Shared
_first_transition buffer clobbers results across calls
compute_first_contact and compute_first_air both write into self._data._first_transition and return the same self._data._first_transition_ta ProxyArray. In a locomotion policy that needs both signals simultaneously (e.g. rewarding foot-contact establishment while also penalizing staying airborne), the first call's result is silently overwritten when the second call runs the kernel into the same buffer. The caller has no way to detect this — both return the same object. Two separate buffers (e.g. _first_contact and _first_air) and two ProxyArray wrappers are needed to make both values usable in the same step.
| @wp.kernel | ||
| def concat_pos_and_quat_to_pose_kernel( | ||
| pos: wp.array2d(dtype=wp.vec3f), | ||
| quat: wp.array2d(dtype=wp.quatf), | ||
| pose: wp.array2d(dtype=wp.transformf), | ||
| ): | ||
| """Concatenate position and quaternion to pose. | ||
|
|
||
| Args: | ||
| pos: Position array. Shape is (N, B). | ||
| quat: Quaternion array. Shape is (N, B). | ||
| pose: Pose array. Shape is (N, B). | ||
| """ | ||
| env, sensor = wp.tid() | ||
| pose[env, sensor] = wp.transform(pos[env, sensor], quat[env, sensor]) |
There was a problem hiding this comment.
Duplicate
concat_pos_and_quat_to_pose_kernel — dead code and double Warp JIT
An identical implementation of this kernel already lives in isaaclab_ovphysx/sensors/kernels.py and is what contact_sensor_data.py actually imports (via from isaaclab_ovphysx.sensors.kernels import concat_pos_and_quat_to_pose_kernel). The copy at the bottom of this file is never imported, but Warp still JIT-compiles it as a separate kernel object on module load, doubling compile time for this kernel. The dead copy should be removed.
| if friction_forces_w: | ||
| for f in range(num_filter_objects): | ||
| friction_forces_w[env, sensor, f] = wp.vec3f(0.0) | ||
|
|
||
| if contact_pos_w: | ||
| for f in range(num_filter_objects): | ||
| contact_pos_w[env, sensor, f] = wp.vec3f(0.0) |
There was a problem hiding this comment.
reset_contact_sensor_kernel sets contact_pos_w to zeros, but it was initialized with NaN
ContactSensorData.create_buffers initialises _contact_pos_w to wp.vec3f(math.nan, math.nan, math.nan) (line 290-293 in contact_sensor_data.py) to signal "no contact point available". After a reset the kernel writes wp.vec3f(0.0) here, so the post-reset value diverges from the initial value. Downstream code relying on NaN to detect "no contact" would see false zeros after the first reset. This code path is currently unreachable in v1 (blocked by the NotImplementedError on track_contact_points), but should be fixed before the deferred feature lands to avoid a silent regression.
| base_glob = self.cfg.prim_path.rsplit("/", 1)[0] | ||
| base_glob = re.sub(r"\{ENV_REGEX_NS\}", "*", base_glob) | ||
| base_glob = re.sub(r"\.\*", "*", base_glob) | ||
| sensor_patterns = [f"{base_glob}/{name}" for name in body_names] | ||
|
|
||
| # Build filter patterns (flat: len = n_sensors * filters_per_sensor). | ||
| filter_globs = [ | ||
| re.sub(r"\.\*", "*", re.sub(r"\{ENV_REGEX_NS\}", "*", expr)) for expr in self.cfg.filter_prim_paths_expr | ||
| ] | ||
| filters_per_sensor = len(filter_globs) | ||
| if filters_per_sensor > 0: | ||
| filter_patterns: list[str] | None = filter_globs * self._num_sensors | ||
| else: | ||
| filter_patterns = None |
There was a problem hiding this comment.
filter_patterns does not account for multi-env when sizing the binding
filter_patterns = filter_globs * self._num_sensors produces num_sensors × num_filters entries. The comment on max_count (line 219) explicitly acknowledges that at binding-creation time self._num_envs reflects only env_0 because USD-discovery sees only the template prim. If the ovphysx create_contact_binding API expects filter_patterns sized as num_sensors × num_envs × num_filters (analogous to sensor_patterns), the current list is undersized by a factor of the real env count. The binding's own expansion semantics are not documented here; a brief comment explaining that the binding handles per-clone fan-out would remove the ambiguity.
d1fbcd2 to
0ef602d
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.
0ef602d to
d5d5dd4
Compare
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.
This reverts commit 4652794.
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.
Two small fixes needed after the rebase onto current develop: * ``isaaclab.utils`` no longer re-exports the ``configclass`` decorator directly; the import has to be ``from isaaclab.utils.configclass import configclass``. Touched ContactSensorCfg and the contact-sensor test module. * The OVPhysX wheel is now invocable through the standard Kit Python entrypoint (``./isaaclab.sh -p``); the kitless ``run_ovphysx.sh`` wrapper is no longer required. Updated the test-module docstring to reflect that.
Three small parity fixes that came out of a code review against the PhysX/Newton implementations: * Rename ``update_net_forces_kernel`` to ``update_net_forces_ovphysx_kernel``. PhysX ships a kernel of the same name with a different signature (env-major vs sensor-major index order); the explicit suffix prevents a confusing compile error if someone cross-imports. * ``ContactSensor.body_names`` now returns ``list[str]`` (never ``None``) and raises a clear ``RuntimeError`` if accessed before initialization, matching the PhysX contract. * 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 implementations log a one-shot warning and otherwise no-op, but the hooks are wired so ``cfg.debug_vis=True`` surfaces an explicit message instead of silently doing nothing.
d5d5dd4 to
f495912
Compare
The CI ``isaaclab_ov*`` filter collects the OVPhysX contact-sensor
tests in Docker images that do not yet bundle the ``ovphysx`` wheel,
causing ``ModuleNotFoundError`` at collection time and a red required
check.
Match the precedent set by
``source/isaaclab_ovphysx/test/assets/test_articulation_helpers.py``
and ``test_rigid_object_helpers.py``: move the existing
``pytest.importorskip("ovphysx.types", ...)`` guard above the
``from isaaclab_ovphysx ...`` imports so the file is skipped before
the missing wheel can break collection.
Also add the missing ``isaaclab_tasks`` changelog fragment for the
``AnymalD Flat`` and ``LocomotionVelocityRoughEnvCfg`` ovphysx-preset
wiring that PR 5422 ships.
The previous exact-name check (``manager_name == "NewtonManager"``) broke ``NewtonMJWarpManager``, ``NewtonKaminoManager``, ``NewtonXPBDManager``, and ``NewtonFeatherstoneManager`` — all subclasses of ``NewtonManager``. The original substring match handled them; the exact-match check rejected them and fell into the catch-all ``ValueError`` raise. Restore the original ``.lower()`` substring matching, but check the ``ovphysxmanager`` exact name *before* the substring branches: without that ordering ``"physx" in "ovphysxmanager"`` would route OVPhysX to the PhysX impl (which assumes a ``root_view`` with ``.link_paths``, absent on OVPhysX's per-tensor-type bindings dict). Reproduced locally by running ``test_rendering_dexsuite_kuka[newton-isaacsim_rtx-rgb]`` against the contact-sensor branch — failed with the documented ``ValueError`` before the fix, passes the dispatch path after.
…5637) # Description Restructures the physics-backend documentation to give each backend a first-class home and to add the cross-backend orientation pages asked for in [isaac-sim/IsaacLab-Internal#876](isaac-sim/IsaacLab-Internal#876). **Structure** ``` docs/source/overview/core-concepts/physical-backends/ ├── index.rst ← user-facing hub + feature-support matrix ├── solver-comparison.rst ← cross-backend behavioural differences ├── physx/ │ ├── index.rst │ ├── installation.rst │ ├── configuration.rst ← PhysxCfg tuning knobs │ └── supported-features.rst ├── newton/ │ ├── index.rst │ ├── installation.rst │ ├── supported-features.rst (was experimental-features/.../limitations-and-known-bugs.rst) │ ├── mjwarp-solver.rst (was experimental-features/.../solver-transitioning.rst) │ └── kamino-solver.rst (was experimental-features/.../using-kamino.rst) └── ovphysx/ └── index.rst ← stub flagged as highly experimental; tracking issue #5634 ``` **Highlights** - The Newton subdir moves wholesale out of `experimental-features/` via `git mv`; framing changes from "experimental feature branch" to "beta backend." The Experimental Features toctree now contains only `bleeding-edge`. - Per-solver Newton pages (`mjwarp-solver`, `kamino-solver`) replace the old `solver-transitioning` / `using-kamino` files, matching #876's "sub-sections for the Newton solvers" ask. - New PhysX page set written from `isaaclab_physx.physics.PhysxCfg`, mirroring the Newton structure. - OvPhysX stub references the full in-flight PR set (#5421, #5422, #5426, #5459, #5570, #5589) and is gated by a follow-up issue (#5634) for expansion after those land. - `solver-comparison.rst` covers friction, contact pipeline, restitution, stabilization, convergence, articulation coordinates, substepping, and GPU buffers across PhysX TGS, Newton MJWarp, and Newton Kamino. Each cell points at the concrete config attribute that controls the behavior, with a porting checklist at the end. - Newton's `supported-features.rst` list refreshed against `develop`'s actual `newton_mjwarp` coverage (adds Shadow Hand, Shadow Hand Over, cabinet, dexsuite, rough-terrain locomotion). Replaces the bit-rotting bullet list with a discovery recipe (`grep -rln newton_mjwarp source/isaaclab_tasks/`). - Cross-references updated in `docs/index.rst`, `core-concepts/index.rst`, `multi_backend_architecture.rst`, `features/visualization.rst`, and `overview/reinforcement-learning/rl_existing_scripts.rst`. Fixes isaac-sim/IsaacLab-Internal#876 ## Type of change - Documentation update ## Screenshots N/A — docs-only restructure. New page tree visible from the table-of-contents in `core-concepts/`. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works *(N/A — docs-only)* - [ ] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package *(N/A — docs-only; matches precedent of #5512 and 4aeb4d6 which shipped without fragments)* - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
Description
Implements
ContactSensor,ContactSensorCfg, andContactSensorDatafor theisaaclab_ovphysxbackend, mirroring the existing PhysX implementation.The contact sensor reports normal contact forces in the world frame using the ovphysx 0.3.7
ContactBindingAPI (PhysX.create_contact_binding/read_net_forces/read_force_matrix). Optional pose tracking is wired through aRIGID_BODY_POSETensorBinding.Validation environment:
Isaac-Velocity-Flat-Anymal-C-Direct-v0(Anymal-C foot-contact tracking for locomotion).v1 scope (this PR):
track_pose)compute_first_contact/compute_first_airDeferred (raise
NotImplementedErrorif cfg flag enabled):track_contact_pointstrack_friction_forcesThese 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 atdocs/superpowers/specs/2026-04-27-ovphysx-contact-api-gaps.md. The data properties still returnNoneper theBaseContactSensorDatacontract; the implementation only raises if the cfg flag is set.Fixes #5325
Parent issue: #5315
Stacked on:
[OVPHYSX] Articulation rewrite) — this branch is rebased on top ofantoiner/feat/ovphysx_articulation. Reviewers can focus on the last 13 commits (everything fromScaffold isaaclab_ovphysx sensors sub-packageonward); commits before that are inherited from [OVPHYSX] Articulation rewrite (data class + asset class + kernels) #5459. Will be re-rebased ondeveloponce [OVPHYSX] Articulation rewrite (data class + asset class + kernels) #5459 merges.Dependencies:
develop2026-04-20).RigidObjectlands as part of [OVPHYSX] Articulation rewrite (data class + asset class + kernels) #5459 — no separate gate.Type of change
Screenshots
N/A — backend feature, no UI surface.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_contactsensor.minor.rst, docstrings, gap spec for the maintainer of ovphysx)config/extension.tomlfile (changelog fragment, minor tier — version bump compiled by the nightly CI workflow per the upstream convention)CONTRIBUTORS.mdor my name already exists thereNotes for the reviewer
sensors/contact_sensor/kernels.pyand the sharedsensors/kernels.py) are physics-engine-agnostic and ported verbatim from the PhysX backend — only module docstrings differ.ContactSensorCfgandContactSensorDataare also near-verbatim mirrors; the only edits are the backend-specificclass_typepointer and the shared-kernels import path._initialize_impl(USD prim discovery + regex→fnmatch glob conversion +create_contact_binding+ optionalRIGID_BODY_POSEbinding),_create_buffers(pre-allocated DLPack read buffers),_update_buffers_impl(per-step reads), and_invalidate_initialize_callback(native handle release).AppLauncherstyle (mirroringisaaclab_physx/test/sensors/test_contact_sensor.py); they need to be re-adapted to the kitless./scripts/run_ovphysx.sh -m pytestflow and the_ovphysx_sim_contexthelper 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.