Migrate OVPhysX frame-transformer sensor onto OvPhysxView (view series, part 5)

[AntoineRichard]

🔗 Stacked on top of #6224 (Part 1 — OvPhysxView)

Branched off #6224 as a sibling of the asset migrations (#6225/#6226/#6227). Targets develop; until #6224 merges, the diff below includes the Part 1 OvPhysxView commits. Please review #6224 first.

Description

Part 5 of the OVPhysX view-migration series — route the frame-transformer sensor's tensor-binding management through OvPhysxView. Internal refactor; no public API change (sensors do not expose a root_view).

• _initialize_impl builds one OvPhysxView instead of separate create_tensor_binding calls; binding handles come from binding_for.
• Pose/state reads route through read_into (cached float32 reinterpret, typed off the binding shape), replacing the manual wp.array(ptr=...) reinterpret views.
• All pose bindings are GPU-resident state; no CPU staging. One view per tracked body.
• Numeric behavior unchanged.

Type of change

• This change requires no public API change (internal refactor)

Checklist

• I have run the pre-commit checks with ./isaaclab.sh --format
• My changes generate no new warnings
• I have added a changelog fragment (.skip — internal refactor, no user-facing change / no bump)

Testing

test_frame_transformer.py — cpu: 12 passed, cuda: 12 passed.

[greptile-apps]

Greptile Summary

This PR introduces OvPhysxView, a string-keyed binding manager over OVPhysX TensorBinding handles, and migrates the frame-transformer sensor to use it — replacing per-body create_tensor_binding calls with a single OvPhysxView per tracked body whose read_into fills a structured wp.transformf buffer directly.

• OvPhysxView adds lazy/eager binding creation, structured-dtype reinterpret (wp.transformf / wp.spatial_vectorf), a float32 reinterpret cache (_read_view) for wheel object-identity read-cache warmth, and explicit CPU↔GPU device guards for CPU-only property types.
• frame_transformer.py replaces _body_bindings: list[Any] with _body_views: list[OvPhysxView]; read_buf is now (num_envs,) wp.transformf instead of (num_envs, 7) float32, eliminating the manual .view(wp.transformf) call before the kernel launch.
• A 30-test suite (test_ovphysx_view.py) covers construction, get/set/read-into dispatch, device policy, dtype safety, and adversarial construction — but does not exercise cross-attribute reuse of the same destination buffer, leaving the _read_view cache's missing shape-validation gap untested.

Confidence Score: 4/5

The frame-transformer migration is correct and the test suite passes. The new OvPhysxView class has a gap in its read-view cache that does not affect the current frame-transformer usage but could silently corrupt data for other assets that reuse a destination buffer across attributes with different shapes.

Each read_buf in frame_transformer.py is dedicated to a single attribute (RIGID_BODY_POSE), so the cross-attribute cache issue never triggers there. The concern is in the general OvPhysxView surface being adopted by other assets. The missing binding-shape check on cache hit in _read_view means that a caller passing the same wp.array to read_into for two different attributes gets back a stale view sized for the first attribute, causing the wheel to write the wrong number of floats. The fix is a one-line shape guard on the cache-hit path.

source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py (_read_view cache, lines 839–855) and a corresponding missing test case in test_ovphysx_view.py for cross-attribute buffer reuse.

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py	New OvPhysxView class: well-documented binding manager with lazy/eager creation, structured dtype reinterpret, and device guards. The _read_view cache keyed by id(dst)+ptr is missing a binding-shape check, risking stale views for cross-attribute reuse of the same buffer.
source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/frame_transformer/frame_transformer.py	Migrated from raw create_tensor_binding to OvPhysxView; read_buf changed from (N,7) float32 to (N,) transformf. Logic is correct for the single-attribute-per-buffer usage; error message for AttributeUnavailable is slightly misleading.
source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py	Comprehensive unit test suite with 30+ tests covering construction, get/set/read_into dispatch, device policy, dtype safety, discoverability, and edge cases against fake bindings. No test for cross-attribute buffer reuse.
source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/frame_transformer/kernels.py	Doc-comment-only update to reflect that pose_buffer is now a structured (num_envs,) transformf array filled by OvPhysxView.read_into. No logic changes.
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/init.pyi	Type stub updated to export OvPhysxView alongside OvPhysxFrameView; consistent with the lazy_export() in init.py.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant FT as FrameTransformer._update_buffers
    participant OV as OvPhysxView
    participant BC as _bindings cache
    participant RV as _read_views cache
    participant PX as TensorBinding (wheel)
    participant K as gather_body_pose_kernel

    Note over FT,K: Per-step update loop (one iteration per tracked body)

    FT->>OV: read_into(TT.RIGID_BODY_POSE, read_buf)
    OV->>BC: _binding(RIGID_BODY_POSE)
    BC-->>OV: cached TensorBinding (created on first access)
    OV->>OV: _check_device(read_buf, native_device)
    OV->>RV: _read_view(read_buf, binding)
    alt first call or ptr changed or shape mismatch
        RV->>RV: _as_binding_view → float32 reinterpret of transformf buf
        RV-->>OV: float32 view (cached by id(read_buf))
    else subsequent calls (same ptr and shape)
        RV-->>OV: same float32 view object (cache hit)
    end
    OV->>PX: binding.read(float32_view)
    PX-->>OV: fills read_buf in place
    FT->>K: "wp.launch(gather_body_pose_kernel, inputs=[env_mask, read_buf, dst_indices, raw_transforms])"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant FT as FrameTransformer._update_buffers
    participant OV as OvPhysxView
    participant BC as _bindings cache
    participant RV as _read_views cache
    participant PX as TensorBinding (wheel)
    participant K as gather_body_pose_kernel

    Note over FT,K: Per-step update loop (one iteration per tracked body)

    FT->>OV: read_into(TT.RIGID_BODY_POSE, read_buf)
    OV->>BC: _binding(RIGID_BODY_POSE)
    BC-->>OV: cached TensorBinding (created on first access)
    OV->>OV: _check_device(read_buf, native_device)
    OV->>RV: _read_view(read_buf, binding)
    alt first call or ptr changed or shape mismatch
        RV->>RV: _as_binding_view → float32 reinterpret of transformf buf
        RV-->>OV: float32 view (cached by id(read_buf))
    else subsequent calls (same ptr and shape)
        RV-->>OV: same float32 view object (cache hit)
    end
    OV->>PX: binding.read(float32_view)
    PX-->>OV: fills read_buf in place
    FT->>K: "wp.launch(gather_body_pose_kernel, inputs=[env_mask, read_buf, dst_indices, raw_transforms])"

Loading

Comments Outside Diff (1)

1. source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py, line 839-855 (link)

   Stale cached view on cross-attribute reuse

   _read_view is keyed by id(dst) + ptr but not by binding.shape. If the same wp.array object is passed to read_into for two different attributes with different shapes (e.g. rigid_body_pose with shape (N, 7) and then rigid_body_velocity with shape (N, 6)), the cache hits and returns the stale (N, 7) float32 view while the velocity binding expects an (N, 6) view. The wheel then either silently reads the wrong number of floats into the buffer (stale trailing element) or raises an unexpected shape error — neither is correct.

   The frame_transformer.py usage is safe (each read_buf is dedicated to one attribute), but OvPhysxView is a general class. The fix is to add a shape check on cache hit so a mismatched binding rebuilds the view:

   if cached is not None and cached.ptr == dst.ptr and tuple(cached.shape) == tuple(binding.shape): return cached

_{Reviews (1): Last reviewed commit: "Merge branch 'develop' into antoiner/fea..." | Re-trigger Greptile}

[greptile-apps]

The error message hard-codes "matched zero bodies" but OvPhysxView.AttributeUnavailable is also raised when create_tensor_binding itself throws (e.g. ABI / OOM / init failure), so the message misleads debugging in those cases. The chained exception exposes the real cause, but the primary message is incorrect. Consider making the message more general.

Suggested change

	raise RuntimeError(
	f"FrameTransformer: TT.RIGID_BODY_POSE binding for pattern {pattern!r} matched zero bodies."
	" Verify the prim has UsdPhysics.RigidBodyAPI."
	)
	) from exc
	raise RuntimeError(
	f"FrameTransformer: TT.RIGID_BODY_POSE binding for pattern {pattern!r} is not available"
	" (no matching prims or binding creation failed)."
	" Verify the prim has UsdPhysics.RigidBodyAPI."
	) from exc

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[AntoineRichard]

Re the read-view cache's missing shape re-check (and the untested cross-attribute reuse): verified latent, not a live bug. The wheel validates tensor shape in native code before copying and raises RuntimeError on a mismatch (not silent), and no shipped caller reuses one buffer across two attributes — each buffer owns a single attribute, and a binding's flat shape is fixed for a session (it only changes on a full reset, which rebuilds the view). We opted not to add a per-hit guard (hot-path cost for an unreachable case); dtype-aware narrowing is a wheel-metadata follow-up. We did add reset-boundary tidiness (clearing the view on _invalidate_initialize_callback) across the consumers — frame_transformer already clears _body_views.