[OvPhysX] Adds FrameView for the OVPhysX backend

[AntoineRichard]

Description

Adds OvPhysxFrameView, the OVPhysX-backend implementation of
:class:isaaclab.sim.views.FrameView. The factory in
isaaclab.sim.views.frame_view now dispatches to it when the active
backend is "ovphysx", mirroring the FabricFrameView (PhysX) and
NewtonSiteFrameView (Newton) entries.

OvPhysxFrameView reads live body poses via the OVPhysX wheel's
create_tensor_binding(pattern, tensor_type=RIGID_BODY_POSE) API and
exposes them as wp.transformf arrays without going through the
SceneDataProvider. Sensors built on top of FrameView (e.g. RayCaster,
the ContactSensor pose-tracking path) can now read body transforms
under OVPhysX with the same API surface they use under PhysX and
Newton.

Two scaling-related changes in InteractiveScene and OvPhysxManager
make OVPhysX fan-out behave like the other backends:

• InteractiveScene now USD-replicates the per-env asset subtree for
  every backend (clone_usd=True), including OVPhysX. The
  find_matching_prims-based _num_envs discovery used by many
  sensors now sees N prims under OVPhysX, matching PhysX/Newton.
• OvPhysxManager strips env_1..N from the in-memory USD before
  handing it to physx.add_usd. Without this, the wheel's per-USD-path
  enumeration scales O(num_envs * bodies) and hangs at 4k+ envs — even
  though physics still uses a single replicated template. The env_0
  USD is what the wheel ingests; physics replication happens via
  physx.clone, identical to the prior clone_usd=False path. A short
  comment on the helper documents the assumption that the per-env
  subtrees are structurally homogeneous.

Tested manually with Isaac-Velocity-Rough-Anymal-D-v0 at 4096 envs
under the OVPhysX preset; iter time matches the prior clone_usd=False
baseline.

Fixes #N/A

Type of change

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

Screenshots

N/A — backend infrastructure change with 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
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump extension.toml — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[isaaclab-review-bot]

Code Review: Add OvPhysxFrameView for OVPhysX backend

Thanks for this contribution, @AntoineRichard! This PR adds important OVPhysX backend support to the FrameView abstraction. The implementation is well-structured and follows the established patterns from NewtonSiteFrameView.

✅ Strengths

• Clean architecture: The site-based approach with Warp kernels mirrors the Newton implementation appropriately
• Thorough documentation: Docstrings and limitation sections are comprehensive
• Deferred initialization: Properly handles the async physics setup via PHYSICS_READY callback
• Test coverage: Good use of the shared contract test suite
• USD/Physics separation: The env_0-scoped export workaround is well-documented with clear limitations

📝 Suggestions

Documentation

1. Changelog accuracy (changelog.d/ovphysx-frameview.minor.rst:6): The changelog mentions reading from "scene data provider's body_q array", but the implementation uses a direct RIGID_BODY_POSE tensor binding. Consider updating for consistency.

2. Scale getter behavior (ovphysx_frame_view.py:785): The get_scales/set_scales docstrings could clarify behavior for clone_usd=False scenes where env_1..N have no USD prims.

Testing

3. World-attached sites: Consider adding a test for prims with no rigid body ancestor (the WORLD_BODY_INDEX == -1 branch).

4. Rigid body rejection: The guard at lines 507-513 that rejects prims with RigidBodyAPI deserves explicit test coverage.

Minor Code Quality

5. Indexed query allocation (ovphysx_frame_view.py:650): The per-call wp.zeros allocation in indexed queries is fine for correctness but worth noting in docs if these become hot paths.

6. Synthetic path validation (ovphysx_frame_view.py:560): The clone expansion assumes self._prims[0] is an env_0 prim. Consider validating this assumption when the first prim doesn't contain env_0.

🔍 Overall

The implementation is solid and production-ready. The suggestions above are minor improvements that don't block merging.

---

_{Review generated with assistance from Isaac Lab Review Bot}

[greptile-apps]

Greptile Summary

This PR adds OvPhysxFrameView, the OVPhysX-backend implementation of FrameView, and wires up the factory dispatch in frame_view.py. Two supporting changes make the new view work correctly at scale: InteractiveScene now always passes clone_usd=True, and OvPhysxManager._export_env0_only_stage strips env_1..N from the USD handed to the physics wheel so that physx.clone() remains the authoritative replication mechanism.

• OvPhysxFrameView reads body poses via an OVPhysX RIGID_BODY_POSE tensor binding and computes site world/local transforms on GPU using Warp kernels; the site-resolution logic, kernel math, and deferred-init path all look correct.
• _export_env0_only_stage uses Sdf.Layer.FindOrOpen where the comment says OpenAsAnonymous, leaving the stripped layer in the global USD registry; the batch prim deletions also lack Sdf.ChangeBlock, which can add noticeable warmup overhead at thousands of envs.
• The test fixture mutates the module-level OVPHYSX_SIM_CFG object across test functions, introducing test order-dependency on the device field.

Confidence Score: 4/5

Safe to merge with minor follow-up; no data-loss or crash paths identified.

The core Warp kernels and tensor-binding data path are logically sound, and the 4096-env manual test gives confidence in the happy path. The _export_env0_only_stage helper has a comment/code mismatch and missing Sdf.ChangeBlock that add memory and warmup overhead but do not break correctness. The shared-config mutation in tests is a test-hygiene concern, not a runtime risk.

source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py deserves a second look for the layer-registry and ChangeBlock issues; source/isaaclab_ovphysx/test/sim/test_views_xform_prim_ovphysx.py for the shared-config mutation.

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_frame_view.py	New 890-line Warp-native FrameView backed by a RIGID_BODY_POSE tensor binding; kernels and site-resolution logic look correct, but set_world_poses/set_local_poses perform a redundant double body_q read on partial-update paths due to in-place buffer sharing.
source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py	New _export_env0_only_stage helper strips env_1..N before physics ingestion; comment misidentifies FindOrOpen as OpenAsAnonymous (leaving the layer in the global registry), and batch prim deletions lack Sdf.ChangeBlock for efficiency at large env counts.
source/isaaclab/isaaclab/sim/views/frame_view.py	Adds ovphysx entry to _backend_class_names and _get_backend; dispatch order is intentional and safe, but the docstring incorrectly attributes reads to the scene data provider rather than the tensor binding.
source/isaaclab/isaaclab/scene/interactive_scene.py	Changes clone_usd from conditional (False for OVPhysX) to unconditional True; the inline comment honestly flags this as experimental and describes the revert path, and manual testing at 4096 envs passed.
source/isaaclab_ovphysx/test/sim/test_views_xform_prim_ovphysx.py	New test file covering factory dispatch and deferred-init guard; the module-level OVPHYSX_SIM_CFG object is mutated in multiple test functions and the fixture, creating test order-dependency on the device setting.

Sequence Diagram

sequenceDiagram
    participant IS as InteractiveScene
    participant FV as FrameView (factory)
    participant OFV as OvPhysxFrameView
    participant OM as OvPhysxManager
    participant PX as physx wheel

    IS->>IS: "clone_usd=True (all backends)"
    IS->>OM: _warmup_and_load()
    OM->>OM: _export_env0_only_stage(stage, scene.usda)
    Note over OM: Export full stage to disk,<br/>open via FindOrOpen,<br/>delete env_1..N specs,<br/>re-export to same file
    OM->>PX: physx.add_usd(scene.usda)
    PX->>PX: physx.clone() replicates env_1..N at physics layer

    FV->>FV: _get_backend() returns ovphysx
    FV->>OFV: __new__() OvPhysxFrameView(prim_path)
    OFV->>OFV: find_matching_prims(prim_path)
    alt PhysX ready
        OFV->>OFV: _initialize_impl(physx)
    else deferred
        OFV->>OM: register_callback(PHYSICS_READY)
        OM-->>OFV: _on_physics_ready() calls _initialize_impl(physx)
    end
    OFV->>PX: create_tensor_binding(pattern, RIGID_BODY_POSE)
    PX-->>OFV: pose_binding num_bodies x 7

    OFV->>OFV: get_world_poses()
    OFV->>PX: pose_binding.read(_pose_buf)
    OFV->>OFV: wp.launch _compute_site_world_transforms
    OFV-->>FV: positions and orientations as ProxyArray

Loading

Comments Outside Diff (2)

1. source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py, line 136-168 (link)

   FindOrOpen contradicts comment and leaves a registered layer in memory

   The step-1 comment explicitly says "We pass the full file to Sdf.Layer.OpenAsAnonymous so the edits below don't write back to the source layer on disk," but the code uses Sdf.Layer.FindOrOpen instead. Beyond the misleading comment, FindOrOpen registers the layer in the global USD layer registry under target_file as its identifier. That registration persists for the process lifetime, so for a 4096-env stage the stripped layer stays in memory even after physx.add_usd has consumed the file. Using Sdf.Layer.OpenAsAnonymous(target_file) (and then calling anonymous_layer.Export(target_file)) would match the documented intent, avoid the registry entry, and release memory when the local reference drops.

2. source/isaaclab_ovphysx/test/sim/test_views_xform_prim_ovphysx.py, line 1198-1207 (link)

   Shared OVPHYSX_SIM_CFG mutated across tests causes order-dependency

   OVPHYSX_SIM_CFG is a module-level singleton and multiple test functions (and the view_factory fixture) all write OVPHYSX_SIM_CFG.device = device. If pytest runs test_factory_dispatches_to_ovphysx_frame_view on cuda:0 and then test_view_raises_before_physics_ready runs before the attribute is reset, the second test inherits the first test's device. Using a fresh SimulationCfg(physics=OvPhysxCfg(), device=device) per call-site avoids the shared-state coupling.

_{Reviews (1): Last reviewed commit: "Use isaaclab.utils.configclass.configcla..." | Re-trigger Greptile}

[greptile-apps]

Batch prim deletions without Sdf.ChangeBlock fires N individual change notifications

Each del envs_spec.nameChildren[child_name] fires a separate USD change notification. At 4096 envs this emits ~4095 notifications before the layer is exported. Wrapping the deletion loop in Sdf.ChangeBlock() batches all spec removals into a single notification burst, which can meaningfully reduce warmup time at large env counts.

Suggested change

	env_name_re = re.compile(r"^env_(\d+)$")
	names_to_remove = [
	child_name
	for child_name in list(envs_spec.nameChildren.keys())
	if (match := env_name_re.match(child_name)) and match.group(1) != "0"
	]
	for child_name in names_to_remove:
	del envs_spec.nameChildren[child_name]
	env_name_re = re.compile(r"^env_(\d+)$")
	names_to_remove = [
	child_name
	for child_name in list(envs_spec.nameChildren.keys())
	if (match := env_name_re.match(child_name)) and match.group(1) != "0"
	]
	with Sdf.ChangeBlock():
	for child_name in names_to_remove:
	del envs_spec.nameChildren[child_name]

[greptile-apps]

Docstring says "scene data provider" but the implementation uses a tensor binding

OvPhysxFrameView reads body poses via physx.create_tensor_binding(…, tensor_type=RIGID_BODY_POSE), not through the OVPhysX scene data provider (SDP). The current text implies an SDP dependency that would make users think requires_newton_model=True is needed, which the implementation explicitly avoids.

Suggested change

	- **OVPhysX**: :class:`~isaaclab_ovphysx.sim.views.OvPhysxFrameView`
	(Warp-native, reads ``body_q`` via the OVPhysX scene data provider).
	- **OVPhysX**: :class:`~isaaclab_ovphysx.sim.views.OvPhysxFrameView`
	(Warp-native, reads body poses via an OVPhysX ``RIGID_BODY_POSE`` tensor binding).

[greptile-apps]

Double _current_body_q() read on partial-update path obscures intent

When only one of positions/orientations is None, _current_body_q() is called at line 684 and again inside get_world_poses() at line 687. Because _current_body_q() writes the physics data in-place into _pose_buf and then returns a view of it, the local body_q variable from line 684 silently reflects the second read's data. The kernel at line 697 therefore always uses the second read's body_q, making the first call at line 684 redundant and misleading. The same pattern repeats in set_local_poses.

[isaaclab-review-bot]

Code Review: OvPhysxFrameView for OVPhysX Backend

Thank you for this comprehensive PR, @AntoineRichard! The implementation brings full FrameView API parity to the OVPhysX backend with a clean site-based architecture. The USD cloning changes in OvPhysxManager solve a real scaling problem elegantly.

✅ Strengths

• Clean Warp kernel design: The site-based transform computation mirrors NewtonSiteFrameView appropriately, with proper handling of world-attached sites via WORLD_BODY_INDEX = -1
• Deferred initialization: Correctly handles async physics setup via PHYSICS_READY callback with clear error messages
• Well-documented limitations: The _export_env0_only_stage docstring clearly explains the homogeneous-env assumption and its implications
• Contract test coverage: Good reuse of frame_view_contract_utils for shared FrameView semantics

📋 Findings

1. Quaternion Convention Clarity (Low Priority)

File: ovphysx_frame_view.py:53-55

The kernel docstring states output is (qx, qy, qz, qw), matching the code:

out_quat[i] = wp.vec4f(q[0], q[1], q[2], q[3])  # q is wp.quatf

However, wp.quatf storage order is (x, y, z, w) while some Warp APIs expect (w, x, y, z). The code appears correct but consider adding a brief comment confirming the convention matches FabricFrameView and NewtonSiteFrameView for maintainability.

2. USD Layer Export Pattern (Low Priority)

File: ovphysx_manager.py:469-470

del envs_spec.nameChildren[child_name]
...
layer.Export(target_file)

The pattern of modifying an Sdf.Layer in-place then re-exporting is correct, but consider calling layer.Clear() or letting the layer go out of scope before the export to ensure the in-memory state does not interfere with subsequent operations in edge cases.

3. Test Coverage: World-Attached Sites (Medium Priority)

File: test_views_xform_prim_ovphysx.py

The test fixture always creates prims under a rigid body (/Cube/CameraMount). Consider adding a test case for prims with no rigid body ancestor to exercise the WORLD_BODY_INDEX == -1 branch:

def test_world_attached_site():
    """Sites without rigid body ancestors use identity body transform."""
    # Create an Xform directly under /World (no physics ancestor)
    prim = stage.DefinePrim("/World/FreeMarker", "Xform")
    view = OvPhysxFrameView("/World/FreeMarker", device=device)
    # Verify get_world_poses returns the USD spawn transform

4. Rigid Body Rejection Test (Low Priority)

File: ovphysx_frame_view.py:507-513

The guard that rejects prims with RigidBodyAPI is good defensive code:

if self._prim_or_template_has_rigid_body_api(prim):
    raise ValueError(...)

An explicit test verifying this error path would strengthen coverage.

5. Indexed Query Allocation (Informational)

File: ovphysx_frame_view.py:650-651

pos_buf = wp.zeros(n, dtype=wp.vec3f, device=self._device)
quat_buf = wp.zeros(n, dtype=wp.vec4f, device=self._device)

Per-call allocation in indexed queries is fine for correctness. If these become hot paths in tight loops, consider pre-allocating based on max expected index count or documenting the allocation cost.

6. Synthetic Path Assumption (Low Priority)

File: ovphysx_frame_view.py:560

The clone expansion uses self._prims[0] as the template. If the first matched prim happens to not contain env_0 in its path (unlikely but possible with certain regex patterns), the substitution logic would produce incorrect paths. Consider validating that the template prim path contains /env_0/ or logging a warning.

🔍 Architecture Notes

The decision to use a direct RIGID_BODY_POSE tensor binding rather than the scene data provider's Newton model is a good one — it removes the requires_newton_model dependency and makes the view usable in headless training scenarios.

The clone_usd=True change in InteractiveScene with compensating env_0 scoping in OvPhysxManager is a clean solution that maintains sensor discovery parity across backends.

Summary

Solid implementation ready for merge. The suggestions above are minor improvements that do not block approval.

---

_{Isaac Lab Review Bot}

---

Update (2cb93be): Reviewed incremental changes:

1. Docstring clarification (frame_view.py): Updated OvPhysX backend description to reference the RIGID_BODY_POSE tensor binding directly instead of scene data provider — this aligns documentation with the actual implementation. ✅

2. CI compatibility fix (test_views_xform_prim_ovphysx.py): Added pytest.importorskip("ovphysx.types") guard with clear reasoning that the isaaclab_ov* CI pattern unintentionally collects these tests. The noqa comments for E402 import-order are appropriate since imports must follow the skip check. This is a pragmatic fix that unblocks CI without masking real issues. ✅

Both changes are minor housekeeping — no functional changes to review. Previous findings remain applicable.