[OvPhysX] Add RayCaster sensor for the OVPhysX backend#5691
[OvPhysX] Add RayCaster sensor for the OVPhysX backend#5691AntoineRichard wants to merge 3 commits into
Conversation
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: OVPhysX RayCaster Sensor Implementation
This PR adds the OVPhysX backend for RayCaster, RayCasterCamera, MultiMeshRayCaster, and MultiMeshRayCasterCamera sensors, along with the supporting OvPhysxFrameView class. The implementation follows the established pattern from the PhysX and Newton backends while addressing OVPhysX-specific constraints around USD cloning and tensor bindings.
✅ Strengths
-
Clean Mixin Architecture: The
_OvPhysxRayCasterMixin(~200 lines) centralizes all backend-specific logic, allowing the four sensor classes to be simple 14-line composition modules. This mirrors the structure inisaaclab_physx. -
Proper Handling of clone_usd=True Scenes: The implementation correctly broadcasts env_0 offsets across all binding rows when only a single USD prim is discovered but the binding has multiple rows.
-
Static Frame Fallback: Non-physics sensor frames (no rigid-body ancestor) correctly fall back to a one-time USD pose snapshot via
_initialize_static_pose_tracking. -
Resource Cleanup:
_invalidate_initialize_callbackproperly destroys tensor bindings and clears mesh view buffers when the simulation stops. -
Well-Documented Limitations: The v1 single-pattern constraint for multi-mesh targets is clearly documented with
NotImplementedErrorand matches the ContactSensor limitation.
⚠️ Suggestions & Potential Issues
1. Potential Race Condition in _initialize_pose_tracking (Medium)
In ray_caster.py (lines 108-112), if _initialize_pose_tracking is called before PHYSICS_READY, OvPhysxManager.get_physx_instance() returns None and raises. Unlike OvPhysxFrameView which defers initialization via a callback, the raycaster mixin lacks this deferred-init pattern. Consider whether sensors constructed early could hit this path.
2. Missing Ray Direction Validation (Low)
The mixin reads body poses but delegates ray pattern generation to the base class. Consider whether the base class adequately validates that ray directions are normalized and non-NaN before raycasting. Degenerate ray patterns could produce silent misses.
3. get_world_poses Return Type (Low)
In ray_caster.py (lines 196-200), the Camera base classes expect the .torch pattern on SimpleNamespace, but raw tensor users might find this surprising. The PhysX raycaster returns the same structure, so this is consistent—just worth a docstring note.
4. _export_env0_only_stage Layer Export Robustness (Low)
In ovphysx_manager.py (line 467), if the export fails silently (e.g., disk full, permissions), Sdf.Layer.FindOrOpen could return an older cached layer. The subsequent check handles the None case but consider adding validation for robustness in CI environments.
5. Test Coverage Gap: Multi-Mesh Dynamic Targets (Medium)
The test file covers the FrameView contract suite but does not exercise _create_tracked_target_view or _update_mesh_transforms for multi-mesh raycasters with track_mesh_transforms=True. Consider adding a test that verifies dynamic target tracking with moving meshes.
📋 Summary
This is a well-structured implementation that correctly mirrors the PhysX/Newton patterns while handling OVPhysX-specific constraints around tensor bindings and USD cloning. The main areas to consider are:
- Deferred init for early-constructed sensors
- Additional test coverage for multi-mesh dynamic targets
- Minor documentation improvements for return types
The env_0 USD export stripping approach is a clever workaround for the physx.clone() fast-path constraint, and the limitations are well-documented.
Recommendation: Approve with suggestions for the test coverage improvement.
Update (027d979): The latest commits address important integration concerns:
-
Factory Dispatch Registration ✅: All four factory classes (
RayCaster,RayCasterCamera,MultiMeshRayCaster,MultiMeshRayCasterCamera) now include"ovphysx"in their_backend_class_namesdictionaries, which was required for the factory dispatch pattern to route to the newisaaclab_ovphysx.sensors.ray_castermodule. This fixes theModuleNotFoundErrorthat would have occurred when using these sensors with OVPhysX. -
CI Test Robustness ✅: Added
pytest.importorskip("ovphysx.types")guard totest_views_xform_prim_ovphysx.pyto gracefully skip when the ovphysx wheel is not installed. This prevents theisaaclab_ov*CI pattern from blocking on unrelated dependency failures. -
Velocity Env Config ✅: Added
ovphysx = OvPhysxCfg()toRoughPhysicsCfg, enabling the velocity-based locomotion environments (e.g.,Isaac-Velocity-Rough-Anymal-D-v0) to dispatch under OVPhysX—this is the primary use case for the height_scanner raycaster. -
Documentation Fix: Updated
FrameViewdocstring to clarify OVPhysX usesRIGID_BODY_POSEtensor binding rather than the previousbody_qreference via scene data provider. -
Changelog Entries: Proper changelogs added for both the fix (dispatch routing) and the feature (ray_caster module).
These updates complete the integration—the sensors are now properly discoverable and usable under OVPhysX. The implementation looks ready for final review.
Update (b8dffa2): The additional commits (909a8d7 → b8dffa2) complete the implementation:
- ✅ Velocity Env Config:
RoughPhysicsCfg.ovphysx = OvPhysxCfg()is now wired in. - ✅ Factory Dispatch Routing: All four raycaster classes (
RayCaster,RayCasterCamera,MultiMeshRayCaster,MultiMeshRayCasterCamera) now include"ovphysx"in_backend_class_names. - ✅ New Sensor Package:
isaaclab_ovphysx.sensors.ray_castermodule is added with the mixin-based implementation. - ✅ FrameView:
OvPhysxFrameViewis added with the completeRIGID_BODY_POSEtensor binding implementation. - ✅ Tests: Contract-suite tests added for FrameView with proper
pytest.importorskipguards.
No new issues introduced. Previous suggestions (deferred init for early-constructed sensors, multi-mesh test coverage) remain as future enhancements but are not blockers.
AntoineRichard
force-pushed
the
antoiner/feat/ovphysx_raycaster
branch
from
1550bd8 to
027d979
Compare
Greptile SummaryThis PR adds OVPhysX backend support for the four ray-caster sensor variants (
Confidence Score: 4/5Safe to merge with minor cleanup; the core pose-tracking and env-0-only export logic is sound and locally validated at 4096 envs. The mixin and frame view follow established PhysX backend patterns. Three findings: a stale comment (harmless), an unguarded floor-division in multi-mesh transform update (silently wrong mesh count on misconfiguration), and an inconsistent wp.array.view() call on a 2D tensor. None affect the verified Anymal-D use case. sensors/ray_caster/ray_caster.py for the mesh-count division and static-path warp reinterpretation; interactive_scene.py for the hedge comment. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Under OVPhysX ``clone_usd=True`` only env_0 carries USD prims (the | ||
| # env_1..N bodies are physics-layer clones created by physx.clone()), | ||
| # so a single discovered prim must be broadcast to ``self._view_count`` | ||
| # offset rows. Same broadcast PhysX applies. |
There was a problem hiding this comment.
clone_usd=True change: the comment says "only env_0 carries USD prims (the env_1..N bodies are physics-layer clones)" but the interactive_scene.py change makes clone_usd=True unconditional, so all envs now have authored USD prims in the live stage. find_matching_prims will return one prim per env, and the broadcast branch (len == 1) will not fire. The comment documents a scenario that no longer applies, which would mislead future readers.
| # Under OVPhysX ``clone_usd=True`` only env_0 carries USD prims (the | |
| # env_1..N bodies are physics-layer clones created by physx.clone()), | |
| # so a single discovered prim must be broadcast to ``self._view_count`` | |
| # offset rows. Same broadcast PhysX applies. | |
| # With ``clone_usd=True`` every env has its own authored USD prim, so | |
| # ``find_matching_prims`` returns one entry per env and the broadcast | |
| # branch below (``len == 1``) is only hit in edge cases where a single | |
| # sensor prim covers all envs (e.g. a global terrain marker). |
| view_count = int(view.shape[0]) | ||
| meshes_per_env = view_count | ||
| if view_count != 1: | ||
| meshes_per_env = view_count // self._num_envs |
There was a problem hiding this comment.
view_count is not an exact multiple of self._num_envs, meshes_per_env is silently truncated and the wp.launch processes fewer transforms than expected — remaining mesh slots stay at stale values with no warning. An assertion here would surface misconfiguration immediately.
| view_count = int(view.shape[0]) | |
| meshes_per_env = view_count | |
| if view_count != 1: | |
| meshes_per_env = view_count // self._num_envs | |
| view_count = int(view.shape[0]) | |
| meshes_per_env = view_count | |
| if view_count != 1: | |
| assert view_count % self._num_envs == 0, ( | |
| f"OvPhysxRayCaster: target binding shape {view_count} is not divisible " | |
| f"by num_envs={self._num_envs}; mesh-transform kernel will process wrong count." | |
| ) | |
| meshes_per_env = view_count // self._num_envs |
| self._static_view_transforms_torch = torch.tensor(poses, dtype=torch.float32, device=self._device).contiguous() | ||
| self._static_view_transforms_wp = wp.from_torch(self._static_view_transforms_torch).view(wp.transformf) |
There was a problem hiding this comment.
.view(wp.transformf) on a 2D (N, 7) float32 array, while the dynamic path uses the explicit wp.array(ptr=..., shape=(N,), dtype=wp.transformf) idiom. Aligning them avoids ambiguity in view() semantics on multi-dimensional arrays across Warp versions.
| self._static_view_transforms_torch = torch.tensor(poses, dtype=torch.float32, device=self._device).contiguous() | |
| self._static_view_transforms_wp = wp.from_torch(self._static_view_transforms_torch).view(wp.transformf) | |
| self._static_view_transforms_torch = torch.tensor(poses, dtype=torch.float32, device=self._device).contiguous() | |
| _static_buf = wp.from_torch(self._static_view_transforms_torch) | |
| self._static_view_transforms_wp = wp.array( | |
| ptr=_static_buf.ptr, | |
| shape=(len(prims),), | |
| dtype=wp.transformf, | |
| device=str(_static_buf.device), | |
| copy=False, | |
| ) |
AntoineRichard
force-pushed
the
antoiner/feat/ovphysx_raycaster
branch
from
027d979 to
a59b06d
Compare
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 four core ``isaaclab.sensors.ray_caster.*`` factory classes so calls to ``RayCaster(cfg)`` (and the three siblings) under an OVPhysX ``SimulationContext`` return the corresponding class in ``isaaclab_ovphysx.sensors.ray_caster`` rather than raising ``ModuleNotFoundError``.
Mirror the per-backend RayCaster structure introduced by PR isaac-sim#5510 for the OVPhysX backend: a single ``_OvPhysxRayCasterMixin`` (~200 lines) in ``ray_caster.py`` carries the backend-specific pose-tracking surface, reading live body poses through the ovphysx wheel's ``create_tensor_binding(pattern=..., tensor_type=RIGID_BODY_POSE)`` API. Three 14-line sibling modules compose the mixin with the matching ``Base*`` class (RayCasterCamera, MultiMeshRayCaster, MultiMeshRayCasterCamera). Static (non-physics) sensor frames fall back to a one-time USD pose snapshot, matching PhysX's ``_initialize_static_pose_tracking``. Under OVPhysX ``clone_usd=True`` scenes where only env_0 carries USD prims, the env_0 offset is broadcast across the binding's row count (same broadcast PhysX applies). Multi-mesh dynamic targets create a second binding via the same ``create_tensor_binding`` call. v1 limitation: target paths must dedup to a single env-wildcard pattern; multi-pattern targets raise ``NotImplementedError`` (mirrors the OVPhysX ContactSensor ``track_pose`` single-body constraint). Unblocks ``Isaac-Velocity-Rough-Anymal-D-v0 presets=ovphysx`` (the height_scanner is a RayCaster).
AntoineRichard
force-pushed
the
antoiner/feat/ovphysx_raycaster
branch
from
a59b06d to
b8dffa2
Compare
1 participant
Description
Adds
isaaclab_ovphysx.sensors.ray_castersoRayCaster(cfg),RayCasterCamera,MultiMeshRayCaster, andMultiMeshRayCasterCameradispatch correctly under an OVPhysX
SimulationContext. UnblocksIsaac-Velocity-Rough-Anymal-D-v0underpresets=ovphysx(theheight_scanner is a
RayCaster).Mirrors
isaaclab_physx.sensors.ray_casterstructure: a single_OvPhysxRayCasterMixin(~200 lines inray_caster.py) carries thebackend-specific pose-tracking surface, reading live body poses via the
ovphysx wheel's
create_tensor_binding(pattern=..., tensor_type=RIGID_BODY_POSE)API.Three 14-line sibling modules compose the mixin with the matching
Base*class. Static (non-physics) sensor frames fall back to aone-time USD pose snapshot, matching PhysX's
_initialize_static_pose_tracking.Multi-mesh dynamic targets create a second binding via the same
create_tensor_bindingcall. v1 limitation: target paths must dedupto a single env-wildcard pattern; multi-pattern targets raise
NotImplementedError(mirrors the OVPhysXContactSensortrack_posesingle-body constraint).Also wires
isaaclab_ovphysx.sensors.ContactSensorCfgandisaaclab_ovphysx.physics.OvPhysxCfginto the sharedLocomotionVelocityRoughEnvCfg.RoughPhysicsCfgsoIsaac-Velocity-Rough-Anymal-D-v0can pick the right preset underpresets=ovphysx.Built on top of #5678 (FrameView, now merged). Anymal-D Rough at
4096 envs needs FrameView's
clone_usd=True+ env_0 USD stripping toavoid a wheel-side hang during init; with #5678 in develop, this PR is
now a standalone three-commit diff:
Wire OvPhysX preset into shared RoughPhysicsCfgRoute RayCaster dispatch to ovphysx backendAdd isaaclab_ovphysx.sensors.ray_caster packageVerified locally:
./isaaclab.sh -p scripts/environments/random_agent.py --task Isaac-Velocity-Rough-Anymal-D-v0 --num_envs 64 --headless presets=ovphysxreaches the agent loop cleanly../isaaclab.sh -p scripts/reinforcement_learning/rsl_rl/train.py --task Isaac-Velocity-Rough-Anymal-D-v0 --num_envs 4096 --max_iterations 100 --headless presets=ovphysxcompletes 100 iterations with non-degenerate mean reward.Fixes # (issue)
Type of change
Screenshots
N/A — backend dispatch + sensor implementation, no UI surface.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there