Skip to content

Commit 8ef6471

Browse files
Adapt test_articulation.py imports and kitless harness
Only the imports and the session-scoped autouse fixtures change vs the verbatim PhysX copy in the previous commit. Test bodies are untouched.
1 parent 0b6e6d1 commit 8ef6471

1 file changed

Lines changed: 237 additions & 13 deletions

File tree

source/isaaclab_ovphysx/test/assets/test_articulation.py

Lines changed: 237 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,79 @@
66
# ignore private usage of variables warning
77
# pyright: reportPrivateUsage=none
88

9-
"""Launch Isaac Sim Simulator first."""
10-
11-
from isaaclab.app import AppLauncher
12-
13-
HEADLESS = True
14-
15-
# launch omniverse app
16-
simulation_app = AppLauncher(headless=True).app
17-
18-
"""Rest everything follows."""
199

10+
"""Real-backend tests for the OVPhysX Articulation.
11+
12+
Mirrors :mod:`isaaclab_physx.test.assets.test_articulation` 1-to-1: same set
13+
of test functions, names, parametrizations, and assertions.
14+
15+
OVPhysX runs kitless under ``./scripts/run_ovphysx.sh`` so there is no
16+
``AppLauncher`` boot — :class:`~isaaclab.sim.SimulationContext` is driven
17+
directly via ``build_simulation_context(sim_cfg=SimulationCfg(physics=OvPhysxCfg(), ...))``
18+
which works because :func:`isaaclab.app.has_kit` returns False in this
19+
environment.
20+
21+
PhysX-specific ``cube_object.root_view.set_X(...)`` / ``get_X(...)`` calls are
22+
adapted to OVPhysX by going through the backend's per-tensor-type binding
23+
dictionary (``cube_object._bindings`` / :meth:`~isaaclab_ovphysx.assets.Articulation._get_binding`)
24+
and the public setters (:meth:`set_masses_index`, :meth:`set_coms_index`,
25+
:meth:`set_inertias_index`). Reads use the data-class properties
26+
(``cube_object.data.body_mass``, ``body_inertia``, ``body_com_pose_b``).
27+
28+
Single-PhysX-instance lifecycle
29+
-------------------------------
30+
31+
PhysX's ``test_articulation.py`` builds a fresh :func:`build_simulation_context`
32+
per test, but on the OVPhysX side that pattern segfaults: ``ovphysx<=0.3.7``'s
33+
:class:`ovphysx.PhysX` destructor races on dual-Carbonite static state when
34+
garbage-collected mid-process (see
35+
:meth:`~isaaclab_ovphysx.physics.OvPhysxManager._release_physx`). The intended
36+
pattern is "never explicitly release; let ``os._exit()`` at process exit handle
37+
teardown", which means at most one :class:`ovphysx.PhysX` may exist in a single
38+
pytest session.
39+
40+
A second wheel-side constraint compounds this: ``ovphysx<=0.3.7`` locks the
41+
process-global device mode (CPU vs GPU) on the first
42+
``ovphysx.PhysX(device=...)`` call. A second instance with a different device
43+
raises :exc:`ovphysx.types.PhysXDeviceError`. As a result, only one device's
44+
tests can run per pytest invocation -- the ``_ovphysx_skip_other_device``
45+
autouse fixture pins the session to whichever device is requested first and
46+
skips any subsequent test parametrized to a different device. To run both CPU
47+
and GPU coverage, invoke pytest twice (once per ``-k`` filter, or two separate
48+
processes).
49+
50+
CI note
51+
-------
52+
Because the device-mode lock is process-global, full coverage requires **two
53+
separate ``./scripts/run_ovphysx.sh -m pytest`` invocations** in CI -- one with
54+
``-k 'cpu'`` and one with ``-k 'cuda:0'``. Tracked as gap G5 in
55+
``docs/superpowers/specs/2026-04-28-ovphysx-wheel-gaps-for-marco.md``; until
56+
the wheel exposes a way to reset Carbonite device state, this is the supported
57+
pattern.
58+
59+
The ``_ovphysx_session_patches`` autouse fixture installs class-level monkey
60+
patches on :class:`~isaaclab_ovphysx.physics.OvPhysxManager` that:
61+
62+
* Keep the cached ``_physx`` instance alive across :meth:`SimulationContext.clear_instance`
63+
calls (instead of dropping it, which would trigger GC and the destructor race).
64+
* Reuse the cached ``_physx`` on the next test's :meth:`sim.reset`, calling
65+
``physx.reset()`` to clear the stage and ``physx.add_usd()`` to load the
66+
freshly-exported USD -- bypassing :meth:`OvPhysxManager._warmup_and_load`'s
67+
fresh-instance creation path.
68+
"""
69+
70+
from __future__ import annotations
71+
72+
import logging
73+
import os
2074
import sys
75+
import tempfile
2176

2277
import pytest
2378
import torch
2479
import warp as wp
25-
from isaaclab_physx.assets import Articulation
80+
from isaaclab_ovphysx.assets import Articulation
81+
from isaaclab_ovphysx.physics import OvPhysxCfg, OvPhysxManager
2682

2783
import isaaclab.sim as sim_utils
2884
import isaaclab.utils.math as math_utils
@@ -31,7 +87,8 @@
3187
from isaaclab.assets import ArticulationCfg
3288
from isaaclab.envs.mdp.terminations import joint_effort_out_of_limit
3389
from isaaclab.managers import SceneEntityCfg
34-
from isaaclab.sim import build_simulation_context
90+
from isaaclab.physics import PhysicsEvent, PhysicsManager
91+
from isaaclab.sim import SimulationCfg, build_simulation_context
3592
from isaaclab.utils.assets import ISAAC_NUCLEUS_DIR
3693
from isaaclab.utils.version import get_isaac_sim_version, has_kit
3794

@@ -40,6 +97,173 @@
4097
##
4198
from isaaclab_assets import ANYMAL_C_CFG, FRANKA_PANDA_CFG, SHADOW_HAND_CFG # isort:skip
4299

100+
wp.init()
101+
102+
_logger = logging.getLogger(__name__)
103+
104+
105+
# ---------------------------------------------------------------------------
106+
# Session-level OvPhysxManager patches
107+
# ---------------------------------------------------------------------------
108+
#
109+
# Cached ovphysx.PhysX instances keyed by device string. Populated lazily by
110+
# the patched _warmup_and_load on first use per device. Never cleared
111+
# mid-process: ovphysx<=0.3.7's destructor races on dual-Carbonite static state
112+
# when an instance is garbage-collected, so we keep references alive until
113+
# os._exit() (registered via OvPhysxManager._warmup_and_load's atexit handler)
114+
# tears the process down.
115+
_PHYSX_BY_DEVICE: dict[str, object] = {}
116+
117+
118+
def _patched_release_physx() -> None:
119+
"""Replacement for :meth:`OvPhysxManager._release_physx` used during tests.
120+
121+
The original drops ``cls._physx`` to None which lets the C++ destructor run
122+
on GC and crashes due to the dual-Carbonite race. Instead, we leave the
123+
instance live (cached in :data:`_PHYSX_BY_DEVICE`) and just reset the
124+
runtime stage so the next test can load a fresh USD.
125+
"""
126+
if OvPhysxManager._physx is not None:
127+
try:
128+
op = OvPhysxManager._physx.reset()
129+
OvPhysxManager._physx.wait_op(op)
130+
except Exception as exc: # pragma: no cover - defensive
131+
_logger.warning("ovphysx.reset() raised during test teardown: %s", exc)
132+
# Detach from OvPhysxManager class state but DO NOT release the underlying
133+
# ovphysx.PhysX -- _PHYSX_BY_DEVICE still holds a strong reference.
134+
OvPhysxManager._physx = None
135+
136+
137+
def _patched_warmup_and_load() -> None:
138+
"""Replacement for :meth:`OvPhysxManager._warmup_and_load` used during tests.
139+
140+
On the first use per device, delegate to the original implementation (which
141+
creates the :class:`ovphysx.PhysX`, registers the ``atexit`` handler, and
142+
loads the USD), then cache the freshly-built instance. On subsequent
143+
uses, reuse the cached instance: re-export the USD stage, re-add it to the
144+
already-running PhysX, and replay any pending clones -- skipping the
145+
constructor call that would otherwise trigger the dual-Carbonite race.
146+
"""
147+
device_str = PhysicsManager._device
148+
cache_key = device_str
149+
physx = _PHYSX_BY_DEVICE.get(cache_key)
150+
if physx is None:
151+
_orig_warmup_and_load(OvPhysxManager)
152+
_PHYSX_BY_DEVICE[cache_key] = OvPhysxManager._physx
153+
return
154+
155+
sim = PhysicsManager._sim
156+
if sim is None:
157+
raise RuntimeError("OvPhysxManager: SimulationContext is not set.")
158+
159+
if "cuda" in device_str:
160+
ovphysx_device = "gpu"
161+
else:
162+
ovphysx_device = "cpu"
163+
164+
scene_prim = sim.stage.GetPrimAtPath(sim.cfg.physics_prim_path)
165+
if scene_prim.IsValid():
166+
OvPhysxManager._configure_physx_scene_prim(scene_prim, PhysicsManager._cfg, ovphysx_device)
167+
168+
OvPhysxManager._tmp_dir = tempfile.TemporaryDirectory(prefix="isaaclab_ovphysx_")
169+
stage_file = os.path.join(OvPhysxManager._tmp_dir.name, "scene.usda")
170+
sim.stage.Export(stage_file)
171+
OvPhysxManager._stage_path = stage_file
172+
173+
# Reuse the cached instance. The previous test's _patched_release_physx
174+
# already called physx.reset() and detached the class-level reference;
175+
# re-attach and load the new USD.
176+
OvPhysxManager._physx = physx
177+
178+
usd_handle, op_idx = physx.add_usd(stage_file)
179+
physx.wait_op(op_idx)
180+
OvPhysxManager._usd_handle = usd_handle
181+
182+
if OvPhysxManager._pending_clones:
183+
for source, targets, parent_positions in OvPhysxManager._pending_clones:
184+
transforms = [(x, y, z, 0.0, 0.0, 0.0, 1.0) for x, y, z in parent_positions] if parent_positions else None
185+
op_idx = physx.clone(source, targets, transforms)
186+
physx.wait_op(op_idx)
187+
OvPhysxManager._pending_clones = []
188+
189+
OvPhysxManager.dispatch_event(PhysicsEvent.MODEL_INIT, payload={})
190+
OvPhysxManager._warmup_done = True
191+
192+
193+
# Reference to the unmodified class methods so the patched versions can fall
194+
# back to them on the first warmup of each device.
195+
_orig_release_physx = OvPhysxManager._release_physx.__func__
196+
_orig_warmup_and_load = OvPhysxManager._warmup_and_load.__func__
197+
198+
199+
@pytest.fixture(scope="session", autouse=True)
200+
def _ovphysx_session_patches():
201+
"""Install module-level monkey patches on :class:`OvPhysxManager`.
202+
203+
Active for the entire pytest session (autouse). Restored at session
204+
teardown -- though by that point the process is exiting via the manager's
205+
``atexit`` ``os._exit(0)`` so any post-yield work here is best-effort.
206+
"""
207+
OvPhysxManager._release_physx = classmethod(lambda cls: _patched_release_physx())
208+
OvPhysxManager._warmup_and_load = classmethod(lambda cls: _patched_warmup_and_load())
209+
try:
210+
yield
211+
finally:
212+
OvPhysxManager._release_physx = classmethod(_orig_release_physx)
213+
OvPhysxManager._warmup_and_load = classmethod(_orig_warmup_and_load)
214+
215+
216+
# Session-locked device. Set on the first parametrized test that runs and
217+
# never reassigned -- ovphysx's process-global device lock means subsequent
218+
# tests on the other device must skip.
219+
_LOCKED_DEVICE: list[str | None] = [None]
220+
221+
222+
@pytest.fixture(autouse=True)
223+
def _ovphysx_skip_other_device(request):
224+
"""Skip tests whose ``device`` parameter mismatches the session-locked device.
225+
226+
``ovphysx<=0.3.7`` locks the process-global device mode on the first
227+
``ovphysx.PhysX(device=...)`` call, so any test parametrized to a different
228+
device after the first ``sim.reset()`` would hit
229+
:exc:`ovphysx.types.PhysXDeviceError`. We detect the locked device on the
230+
first encounter and skip subsequent tests on the other device with a clear
231+
message so the run finishes cleanly rather than producing spurious failures.
232+
"""
233+
callspec = getattr(request.node, "callspec", None)
234+
device = callspec.params.get("device") if callspec is not None else None
235+
if device is None:
236+
# Test does not parametrize on device (e.g. test_warmup_attach_stage_not_called_for_cpu).
237+
return
238+
locked = _LOCKED_DEVICE[0]
239+
if locked is None:
240+
_LOCKED_DEVICE[0] = device
241+
return
242+
if device != locked:
243+
pytest.skip(
244+
f"ovphysx process-global device lock is held by '{locked}'; cannot run '{device}' "
245+
"tests in the same session. Run pytest twice (once per device) for full coverage."
246+
)
247+
248+
249+
def _ovphysx_sim_context(device: str, **kwargs):
250+
"""Wrapper around :func:`build_simulation_context` that injects OVPhysX cfg.
251+
252+
PhysX tests pass ``device=device`` directly and let
253+
:func:`build_simulation_context` build a default :class:`SimulationCfg`.
254+
OVPhysX needs ``physics=OvPhysxCfg()`` set on the cfg so the manager
255+
dispatches to OVPhysX rather than PhysX, so we build the cfg here and
256+
pass it through. ``gravity_enabled`` is consumed locally (it is ignored
257+
by ``build_simulation_context`` once a ``sim_cfg`` is provided).
258+
``add_ground_plane``, ``auto_add_lighting``, and other kwargs continue
259+
to flow through ``build_simulation_context`` as before.
260+
"""
261+
dt = kwargs.pop("dt", 1.0 / 60.0)
262+
gravity_enabled = kwargs.pop("gravity_enabled", True)
263+
gravity = (0.0, 0.0, -9.81) if gravity_enabled else (0.0, 0.0, 0.0)
264+
sim_cfg = SimulationCfg(physics=OvPhysxCfg(), device=device, dt=dt, gravity=gravity)
265+
return build_simulation_context(device=device, sim_cfg=sim_cfg, **kwargs)
266+
43267

44268
def generate_articulation_cfg(
45269
articulation_type: str,
@@ -194,7 +418,7 @@ def sim(request):
194418
add_ground_plane = request.getfixturevalue("add_ground_plane")
195419
else:
196420
add_ground_plane = False # default to no ground plane
197-
with build_simulation_context(
421+
with _ovphysx_sim_context(
198422
device=device, auto_add_lighting=True, gravity_enabled=gravity_enabled, add_ground_plane=add_ground_plane
199423
) as sim:
200424
sim._app_control_on_stop_handle = None

0 commit comments

Comments
 (0)