Skip to content

Commit 1bbcca3

Browse files
Align OvPhysxSceneDataBackend parity with PhysX/Newton
Address review feedback on PR isaac-sim#5589: - Drop the :class: cross-ref to PhysxSceneDataBackend; the symbol is not re-exported from isaaclab_physx.physics, so the Sphinx target violated AGENTS.md's public-API-path rule. Replaced with a plain literal name. - Document explicitly why this backend uses an explicit setup(physx, stage, device) call instead of PhysX's simulation_view property setter. The ovphysx wheel exposes a physx + stage pair rather than a single SimulationView, so a property-setter shape would have had to bundle the two or fire on the second assignment. - Plumb device through setup() instead of mutating the backend's private _device attribute from _warmup_and_load. The field is only used during binding allocation, so it doesn't need to persist on the instance. - Strip "[OvPhysxSceneDataBackend]" prefixes from logger.warning / logger.debug calls to match PhysX/Newton convention; the logger name already disambiguates. Tests updated to pass "cpu" as the new setup() device argument and drop the now-dead ``b._device = "cpu"`` lines.
1 parent d8f04d8 commit 1bbcca3

2 files changed

Lines changed: 25 additions & 33 deletions

File tree

source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,25 @@
3939
class OvPhysxSceneDataBackend(SceneDataBackend):
4040
"""Scene-data backend for the OVPhysX physics manager.
4141
42-
Mirrors :class:`~isaaclab_physx.physics.PhysxSceneDataBackend`'s contract
43-
but adapts to the ovphysx wheel's one-pattern-per-binding API: each
44-
distinct env-wildcard rigid-body prim path produces its own
45-
``TT.RIGID_BODY_POSE`` binding. ``transforms`` reads each binding into
46-
its pre-allocated float32 staging buffer and concatenates them into a
47-
single ``wp.transformf`` array.
42+
Mirrors the contract of ``PhysxSceneDataBackend`` but adapts to the
43+
ovphysx wheel's one-pattern-per-binding API: each distinct env-wildcard
44+
rigid-body prim path produces its own ``TT.RIGID_BODY_POSE`` binding.
45+
:attr:`transforms` reads each binding into its pre-allocated float32
46+
staging buffer and concatenates them into a single ``wp.transformf``
47+
array.
4848
4949
The merged-buffer + staging-buffer separation is required because the
5050
wheel's ``TensorBinding.read(dst)`` writes into ``dst`` only when
5151
``dst.shape == binding.shape``, so we cannot read directly into a slice
5252
of the merged buffer.
53+
54+
Unlike PhysX -- which receives a live :class:`omni.physics.tensors.SimulationView`
55+
via a ``simulation_view`` property setter and discovers prims lazily --
56+
OVPhysX wires bindings through an explicit :meth:`setup` call that
57+
takes the live ``ovphysx.PhysX`` handle and the USD stage. The wheel
58+
exposes a ``physx + stage`` pair rather than a single ``SimulationView``,
59+
so a property setter would have to either bundle the two or fire on the
60+
second assignment; the explicit call keeps the lifecycle obvious.
5361
"""
5462

5563
def __init__(self):
@@ -64,7 +72,6 @@ def __init__(self):
6472
self._rigid_bindings: list[dict[str, Any]] = []
6573
self._merged_transforms: wp.array | None = None
6674
self._scene_data = SceneDataFormat.Transform()
67-
self._device: str = "cpu"
6875

6976
@property
7077
def transform_count(self) -> int:
@@ -79,12 +86,13 @@ def transform_paths(self) -> list[str]:
7986
paths.extend(list(entry["pose"].prim_paths))
8087
return paths
8188

82-
def setup(self, physx, stage) -> None:
89+
def setup(self, physx, stage, device: str) -> None:
8390
"""Discover RigidBodyAPI prims, dedup by env-wildcard form, create one binding per pattern.
8491
8592
Args:
8693
physx: Live ``ovphysx.PhysX`` instance (the wheel handle).
8794
stage: USD stage to traverse for RigidBodyAPI prims.
95+
device: Warp device string used to allocate the staging and merged buffers.
8896
"""
8997
from isaaclab_ovphysx import tensor_types as TT # local: keep heavy ovphysx out of module load
9098

@@ -110,20 +118,13 @@ def setup(self, physx, stage) -> None:
110118
try:
111119
pose_binding = physx.create_tensor_binding(pattern=pattern, tensor_type=TT.RIGID_BODY_POSE)
112120
except Exception as exc:
113-
logger.warning(
114-
"[OvPhysxSceneDataBackend] Failed to create RIGID_BODY_POSE binding for %s: %s",
115-
pattern,
116-
exc,
117-
)
121+
logger.warning("Failed to create RIGID_BODY_POSE binding for %s: %s", pattern, exc)
118122
continue
119123
row_count = int(pose_binding.shape[0])
120124
if row_count == 0:
121-
logger.debug(
122-
"[OvPhysxSceneDataBackend] Pattern %s matched 0 rigid bodies; skipping.",
123-
pattern,
124-
)
125+
logger.debug("Pattern %s matched 0 rigid bodies; skipping.", pattern)
125126
continue
126-
pose_buf = wp.zeros(pose_binding.shape, dtype=wp.float32, device=self._device)
127+
pose_buf = wp.zeros(pose_binding.shape, dtype=wp.float32, device=device)
127128
# Zero-copy reinterpret of the (N, 7) float32 staging buffer as (N,) wp.transformf.
128129
# Same pointer + layout; transformf is 7 float32s (pos.xyz + quat.xyzw). Cached
129130
# so per-step ``transforms`` reads don't reallocate the view object.
@@ -147,7 +148,7 @@ def setup(self, physx, stage) -> None:
147148
total_count += row_count
148149

149150
if total_count > 0:
150-
self._merged_transforms = wp.zeros((total_count,), dtype=wp.transformf, device=self._device)
151+
self._merged_transforms = wp.zeros((total_count,), dtype=wp.transformf, device=device)
151152

152153
@property
153154
def transforms(self) -> SceneDataFormat.Transform:
@@ -173,11 +174,7 @@ def transforms(self) -> SceneDataFormat.Transform:
173174
try:
174175
entry["pose"].read(entry["pose_buf"])
175176
except Exception as exc:
176-
logger.warning(
177-
"[OvPhysxSceneDataBackend] RIGID_BODY_POSE read failed for %s: %s",
178-
entry["pattern"],
179-
exc,
180-
)
177+
logger.warning("RIGID_BODY_POSE read failed for %s: %s", entry["pattern"], exc)
181178
continue
182179
wp.copy(
183180
self._merged_transforms,
@@ -480,8 +477,7 @@ def _warmup_and_load(cls) -> None:
480477
# via :meth:`get_scene_data_backend`.
481478
if cls._scene_data_backend is None:
482479
cls._scene_data_backend = OvPhysxSceneDataBackend()
483-
cls._scene_data_backend._device = PhysicsManager._device
484-
cls._scene_data_backend.setup(cls._physx, sim.stage)
480+
cls._scene_data_backend.setup(cls._physx, sim.stage, PhysicsManager._device)
485481

486482
cls.dispatch_event(PhysicsEvent.MODEL_INIT, payload={})
487483
cls._warmup_done = True

source/isaaclab_ovphysx/test/physics/test_ovphysx_scene_data_backend.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,15 +88,14 @@ def test_transform_paths_empty_when_no_bindings():
8888

8989

9090
def test_setup_creates_one_binding_per_distinct_pattern(monkeypatch):
91-
"""``setup(physx, stage)`` buckets RigidBodyAPI prims by env-wildcard form.
91+
"""``setup(physx, stage, device)`` buckets RigidBodyAPI prims by env-wildcard form.
9292
9393
For cartpole-shaped scenes (``cart``, ``pole``), expect 2 bindings — one
9494
per distinct env-relative prim path.
9595
"""
9696
from isaaclab_ovphysx.physics.ovphysx_manager import OvPhysxSceneDataBackend
9797

9898
b = OvPhysxSceneDataBackend()
99-
b._device = "cpu"
10099

101100
# Stage stub: traversal yields four RigidBodyAPI prims (cart/pole across two envs).
102101
paths = [
@@ -136,7 +135,7 @@ def create_tensor_binding(self, pattern, tensor_type):
136135

137136
monkeypatch.setattr(om_mod, "UsdPhysics", SimpleNamespace(RigidBodyAPI=object()))
138137

139-
b.setup(FakePhysX(), stage)
138+
b.setup(FakePhysX(), stage, "cpu")
140139

141140
# Cartpole = 2 distinct env-wildcard patterns -> 2 bindings.
142141
assert len(created) == 2
@@ -161,7 +160,6 @@ def test_transforms_reads_each_binding_and_returns_transform_format():
161160
from isaaclab_ovphysx.physics.ovphysx_manager import OvPhysxSceneDataBackend
162161

163162
b = OvPhysxSceneDataBackend()
164-
b._device = "cpu"
165163
b._merged_transforms = _wp.zeros((3,), dtype=_wp.transformf, device="cpu")
166164

167165
# Two bindings: first with 2 rows, second with 1 row.
@@ -260,7 +258,6 @@ def test_setup_continues_when_create_tensor_binding_raises(monkeypatch, caplog):
260258
from isaaclab_ovphysx.physics.ovphysx_manager import OvPhysxSceneDataBackend
261259

262260
b = OvPhysxSceneDataBackend()
263-
b._device = "cpu"
264261

265262
paths = [
266263
"/World/envs/env_0/Robot/cart",
@@ -289,7 +286,7 @@ def create_tensor_binding(self, pattern, tensor_type):
289286
monkeypatch.setattr(om_mod, "UsdPhysics", SimpleNamespace(RigidBodyAPI=object()))
290287

291288
with caplog.at_level(logging.WARNING, logger=om_mod.logger.name):
292-
b.setup(FlakyPhysX(), stage)
289+
b.setup(FlakyPhysX(), stage, "cpu")
293290

294291
# The pole pattern survived; the cart pattern was logged and skipped.
295292
assert len(b._rigid_bindings) == 1
@@ -308,7 +305,6 @@ def test_transforms_logs_warning_when_a_binding_read_fails(caplog):
308305
from isaaclab_ovphysx.physics.ovphysx_manager import OvPhysxSceneDataBackend
309306

310307
b = OvPhysxSceneDataBackend()
311-
b._device = "cpu"
312308
b._merged_transforms = _wp.zeros((2,), dtype=_wp.transformf, device="cpu")
313309

314310
buf_good = _wp.zeros((1, 7), dtype=_wp.float32, device="cpu")

0 commit comments

Comments
 (0)