Address remaining SDP code-review feedback

Folds in the polish items the post-implementation code review surfaced:

* Cache the per-binding wp.transformf view at setup time (entry
  pose_buf_transformf) instead of reconstructing on every transforms
  access. Removes Python allocation churn on hot rendering paths.
* Widen _rigid_bindings annotation from list[dict[str, object]] to
  list[dict[str, Any]]. Adds an inline comment documenting the
  per-entry shape so future readers don't have to dig.
* SI-unit note in the transforms property docstring per AGENTS.md:
  position [m], quaternion (xyzw, unit).
* Two new tests covering the previously unverified defensive paths:
  test_setup_continues_when_create_tensor_binding_raises and
  test_transforms_logs_warning_when_a_binding_read_fails. Both assert
  the surviving bindings still produce correct output, so the
  fault-isolation isn't just decorative.

Tests: 11/11 pass (was 9). Pre-commit clean.

Author: AntoineRichard

source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py

@@ -54,7 +54,14 @@ class OvPhysxSceneDataBackend(SceneDataBackend):
 
     def __init__(self):
         self._physx = None
-        self._rigid_bindings: list[dict[str, object]] = []
+        # Each entry: ``{"pattern": str, "pose": TensorBinding,
+        # "pose_buf": wp.array (float32, (N, 7)),
+        # "pose_buf_transformf": wp.array (transformf, (N,)),
+        # "row_offset": int, "row_count": int}``.
+        # The ``pose_buf_transformf`` view aliases ``pose_buf`` via zero-copy
+        # ``wp.array(ptr=...)``; cached at setup time so per-step reads in
+        # :attr:`transforms` don't churn Python allocations.
+        self._rigid_bindings: list[dict[str, Any]] = []
         self._merged_transforms: wp.array | None = None
         self._scene_data = SceneDataFormat.Transform()
         self._device: str = "cpu"
@@ -116,11 +123,23 @@ def setup(self, physx, stage) -> None:
                     pattern,
                 )
                 continue
+            pose_buf = wp.zeros(pose_binding.shape, dtype=wp.float32, device=self._device)
+            # Zero-copy reinterpret of the (N, 7) float32 staging buffer as (N,) wp.transformf.
+            # Same pointer + layout; transformf is 7 float32s (pos.xyz + quat.xyzw). Cached
+            # so per-step ``transforms`` reads don't reallocate the view object.
+            pose_buf_transformf = wp.array(
+                ptr=pose_buf.ptr,
+                shape=(row_count,),
+                dtype=wp.transformf,
+                device=str(pose_buf.device),
+                copy=False,
+            )
             self._rigid_bindings.append(
                 {
                     "pattern": pattern,
                     "pose": pose_binding,
-                    "pose_buf": wp.zeros(pose_binding.shape, dtype=wp.float32, device=self._device),
+                    "pose_buf": pose_buf,
+                    "pose_buf_transformf": pose_buf_transformf,
                     "row_offset": total_count,
                     "row_count": row_count,
                 }
@@ -135,45 +154,37 @@ def transforms(self) -> SceneDataFormat.Transform:
         """Read all bindings into the merged buffer; return as ``SceneDataFormat.Transform``.
 
         Each binding's float32 ``(N, 7)`` read buffer is reinterpreted as ``(N,)`` of
-        ``wp.transformf`` (zero-copy via ``wp.array(ptr=..., dtype=wp.transformf)``)
-        and copied into the merged buffer at the binding's ``row_offset``.
+        ``wp.transformf`` (zero-copy via ``wp.array(ptr=..., dtype=wp.transformf)``,
+        cached on the entry at setup time) and copied into the merged buffer at the
+        binding's ``row_offset``.
 
         Returns:
-            ``SceneDataFormat.Transform`` with ``transforms`` set to the merged
-            ``wp.array(dtype=wp.transformf)``. ``transforms`` is ``None`` when no
+            ``SceneDataFormat.Transform`` whose ``transforms`` field is a
+            ``wp.array(dtype=wp.transformf)`` of length :attr:`transform_count`.
+            Each ``wp.transformf`` row carries position [m] followed by
+            quaternion (xyzw, unit). ``transforms`` is ``None`` when no
             bindings are wired.
         """
         if self._merged_transforms is None or not self._rigid_bindings:
             self._scene_data.transforms = self._merged_transforms
             return self._scene_data
 
         for entry in self._rigid_bindings:
-            buf: wp.array = entry["pose_buf"]
             try:
-                entry["pose"].read(buf)
+                entry["pose"].read(entry["pose_buf"])
             except Exception as exc:
                 logger.warning(
                     "[OvPhysxSceneDataBackend] RIGID_BODY_POSE read failed for %s: %s",
                     entry["pattern"],
                     exc,
                 )
                 continue
-            # Reinterpret the (N, 7) float32 buffer as (N,) wp.transformf without a copy.
-            n = int(entry["row_count"])
-            transformf_view = wp.array(
-                ptr=buf.ptr,
-                shape=(n,),
-                dtype=wp.transformf,
-                device=str(buf.device),
-                copy=False,
-            )
-            offset = int(entry["row_offset"])
             wp.copy(
                 self._merged_transforms,
-                transformf_view,
-                dest_offset=offset,
+                entry["pose_buf_transformf"],
+                dest_offset=int(entry["row_offset"]),
                 src_offset=0,
-                count=n,
+                count=int(entry["row_count"]),
             )
 
         self._scene_data.transforms = self._merged_transforms

source/isaaclab_ovphysx/test/physics/test_ovphysx_scene_data_backend.py

@@ -174,16 +174,24 @@ def fake_read_b(dst):
         host = np.array([[3.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0]], dtype=np.float32)
         _wp.copy(dst, _wp.from_numpy(host, dtype=_wp.float32, device="cpu").reshape((1, 7)))
 
+    # ``pose_buf_transformf`` is the zero-copy transformf view over the float32 staging
+    # buffer; production code caches it at setup time. Tests mirror that shape here.
+    buf_a_tf = _wp.array(ptr=buf_a.ptr, shape=(2,), dtype=_wp.transformf, device="cpu", copy=False)
+    buf_b_tf = _wp.array(ptr=buf_b.ptr, shape=(1,), dtype=_wp.transformf, device="cpu", copy=False)
     b._rigid_bindings = [
         {
+            "pattern": "/World/envs/env_*/Cube",
             "pose": SimpleNamespace(read=fake_read_a, prim_paths=["/Cube0", "/Cube1"]),
             "pose_buf": buf_a,
+            "pose_buf_transformf": buf_a_tf,
             "row_offset": 0,
             "row_count": 2,
         },
         {
+            "pattern": "/World/envs/env_*/Pole",
             "pose": SimpleNamespace(read=fake_read_b, prim_paths=["/Pole"]),
             "pose_buf": buf_b,
+            "pose_buf_transformf": buf_b_tf,
             "row_offset": 2,
             "row_count": 1,
         },
@@ -236,3 +244,106 @@ def test_manager_returns_none_when_backend_uninitialized():
         assert OvPhysxManager.get_scene_data_backend() is None
     finally:
         OvPhysxManager._scene_data_backend = saved
+
+
+def test_setup_continues_when_create_tensor_binding_raises(monkeypatch, caplog):
+    """A single failed binding-creation logs a warning and skips that pattern; others proceed."""
+    import logging
+
+    from isaaclab_ovphysx.physics.ovphysx_manager import OvPhysxSceneDataBackend
+
+    b = OvPhysxSceneDataBackend()
+    b._device = "cpu"
+
+    paths = [
+        "/World/envs/env_0/Robot/cart",
+        "/World/envs/env_0/Robot/pole",
+    ]
+
+    def fake_traverse():
+        for p in paths:
+            yield SimpleNamespace(
+                HasAPI=lambda api: True,
+                GetPath=lambda p=p: SimpleNamespace(pathString=p),
+            )
+
+    stage = SimpleNamespace(Traverse=fake_traverse)
+
+    class FlakyPhysX:
+        def create_tensor_binding(self, pattern, tensor_type):
+            if pattern.endswith("/cart"):
+                raise RuntimeError("simulated wheel-side failure")
+            return SimpleNamespace(
+                pattern=pattern, tensor_type=tensor_type, shape=(1, 7), count=1, prim_paths=[], read=lambda dst: None
+            )
+
+    import isaaclab_ovphysx.physics.ovphysx_manager as om_mod
+
+    monkeypatch.setattr(om_mod, "UsdPhysics", SimpleNamespace(RigidBodyAPI=object()))
+
+    with caplog.at_level(logging.WARNING, logger=om_mod.logger.name):
+        b.setup(FlakyPhysX(), stage)
+
+    # The pole pattern survived; the cart pattern was logged and skipped.
+    assert len(b._rigid_bindings) == 1
+    assert b._rigid_bindings[0]["pattern"].endswith("/pole")
+    assert any("simulated wheel-side failure" in record.message for record in caplog.records)
+
+
+def test_transforms_logs_warning_when_a_binding_read_fails(caplog):
+    """A failed ``binding.read(dst)`` logs and skips that binding; other bindings still merge."""
+    import logging
+
+    import warp as _wp
+
+    _wp.init()
+
+    from isaaclab_ovphysx.physics.ovphysx_manager import OvPhysxSceneDataBackend
+
+    b = OvPhysxSceneDataBackend()
+    b._device = "cpu"
+    b._merged_transforms = _wp.zeros((2,), dtype=_wp.transformf, device="cpu")
+
+    buf_good = _wp.zeros((1, 7), dtype=_wp.float32, device="cpu")
+    buf_bad = _wp.zeros((1, 7), dtype=_wp.float32, device="cpu")
+    buf_good_tf = _wp.array(ptr=buf_good.ptr, shape=(1,), dtype=_wp.transformf, device="cpu", copy=False)
+    buf_bad_tf = _wp.array(ptr=buf_bad.ptr, shape=(1,), dtype=_wp.transformf, device="cpu", copy=False)
+
+    def good_read(dst):
+        import numpy as np
+
+        host = np.array([[7.0, 0.0, 0.0, 0.0, 0.0, 0.0, 1.0]], dtype=np.float32)
+        _wp.copy(dst, _wp.from_numpy(host, dtype=_wp.float32, device="cpu").reshape((1, 7)))
+
+    def bad_read(dst):
+        raise RuntimeError("simulated read failure")
+
+    b._rigid_bindings = [
+        {
+            "pattern": "/World/envs/env_*/Good",
+            "pose": SimpleNamespace(read=good_read, prim_paths=["/Good"]),
+            "pose_buf": buf_good,
+            "pose_buf_transformf": buf_good_tf,
+            "row_offset": 0,
+            "row_count": 1,
+        },
+        {
+            "pattern": "/World/envs/env_*/Bad",
+            "pose": SimpleNamespace(read=bad_read, prim_paths=["/Bad"]),
+            "pose_buf": buf_bad,
+            "pose_buf_transformf": buf_bad_tf,
+            "row_offset": 1,
+            "row_count": 1,
+        },
+    ]
+
+    import isaaclab_ovphysx.physics.ovphysx_manager as om_mod
+
+    with caplog.at_level(logging.WARNING, logger=om_mod.logger.name):
+        out = b.transforms
+
+    assert out is b._scene_data
+    assert any("simulated read failure" in record.message for record in caplog.records)
+    # Good row was still written; bad row is left at the merged buffer's prior contents (zeros).
+    flat = out.transforms.numpy().view("<f4").reshape((2, 7))
+    assert flat[0, 0] == 7.0