From 1afba8b1128351248e38481f4198598c531ebb1b Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 12:37:32 +0200 Subject: [PATCH 01/12] Add OvPhysxView string-keyed tensor-binding view OVPhysX exposes physics attributes as a loose dict of TensorType -> TensorBinding with no view object, unlike Newton's selection.ArticulationView and PhysX's typed tensor views. OvPhysxView wraps the bindings for one prim pattern behind a string-keyed get_attribute / set_attribute surface, addressing attributes by the lowercased TensorType enum name (e.g. "articulation_dof_stiffness"). It needs no Model/State/Control source object because the TensorType already implies where the data lives. Prototype per docs/superpowers/specs/2026-06-17-ovphysx-view-design.md. Adds unit tests covering name<->enum resolution, the read-only guard, discoverability, and get/set dispatch against a fake binding (no native simulation required). --- .../changelog.d/ovphysx-view.minor.rst | 9 + .../isaaclab_ovphysx/sim/views/__init__.pyi | 2 + .../sim/views/ovphysx_view.py | 360 ++++++++++++++++++ .../test/sim/test_ovphysx_view.py | 200 ++++++++++ 4 files changed, 571 insertions(+) create mode 100644 source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst create mode 100644 source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py create mode 100644 source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py diff --git a/source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst new file mode 100644 index 000000000000..4543e9329b5e --- /dev/null +++ b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst @@ -0,0 +1,9 @@ +Added +^^^^^ + +* Added :class:`~isaaclab_ovphysx.sim.views.OvPhysxView`, a string-keyed view over the + OVPhysX tensor bindings. Attributes are addressed by the lowercased ``TensorType`` + name (e.g. ``view.get_attribute("articulation_dof_stiffness")`` / + ``view.set_attribute("rigid_body_pose", values, mask=...)``), bringing the OVPhysX + binding surface closer to the Newton selection API. Prototype tracked by the design + note at ``docs/superpowers/specs/2026-06-17-ovphysx-view-design.md``. diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/__init__.pyi b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/__init__.pyi index cbb07417da49..b771ab472dc4 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/__init__.pyi +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/__init__.pyi @@ -5,6 +5,8 @@ __all__ = [ "OvPhysxFrameView", + "OvPhysxView", ] from .ovphysx_frame_view import OvPhysxFrameView +from .ovphysx_view import OvPhysxView diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py new file mode 100644 index 000000000000..a1e91ee59b60 --- /dev/null +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -0,0 +1,360 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""String-keyed view over OVPhysX ``TensorBinding`` handles. + +PROTOTYPE for the design at +``docs/superpowers/specs/2026-06-17-ovphysx-view-design.md``. + +OVPhysX exposes physics attributes as a loose ``dict[TensorType, TensorBinding]`` +with no view object -- unlike Newton's ``selection.ArticulationView`` or PhysX's +typed tensor views. :class:`OvPhysxView` wraps those bindings for a single prim +pattern behind a string-keyed ``get_attribute`` / ``set_attribute`` surface that +mirrors Newton's selection ergonomics, but is simpler: there is no +``Model``/``State``/``Control`` source object because the :class:`TensorType` +already implies where the data lives. + +Attribute names are the **lowercased** ``TensorType`` enum members, derived +directly from the wheel enum (no hand-maintained table):: + + view.get_attribute("articulation_dof_stiffness") + view.set_attribute("rigid_body_pose", values, mask=env_mask) + +Known follow-ups (see design doc §10 and §7): + +* Buffers are :class:`warp.array` for consistency with the existing + ``_binding_read`` / ``_binding_write`` helpers; a torch-returning convenience + can wrap this (``wp.to_torch(view.get_attribute(...))``). +* ``_CPU_ONLY_NAMES`` mirrors ``isaaclab_ovphysx.tensor_types._CPU_ONLY_TYPES``; + these should be consolidated, ideally sourced from wheel writability/placement + metadata once it is exposed. +* The read-only set is hard-coded pending a wheel ``is_writable`` query. +""" + +from __future__ import annotations + +import logging +from typing import Any + +import warp as wp + +from isaaclab_ovphysx._runtime import import_ovphysx + +logger = logging.getLogger(__name__) + +# Pure-Python enum (no native dependency); safe to import regardless of USD state. +TensorType = import_ovphysx("ovphysx.types").TensorType + +# Tensor types that are read-only on the wheel today. The first group is marked +# read-only in the enum; the computed-dynamics group is read-only in practice. +# TODO(ovphysx): source this from a wheel writability query (design doc §7 ask 3). +_READ_ONLY_NAMES: frozenset[str] = frozenset( + { + "rigid_body_acceleration", + "rigid_body_inv_mass", + "rigid_body_inv_inertia", + "articulation_link_acceleration", + "articulation_body_inv_mass", + "articulation_body_inv_inertia", + "articulation_dof_projected_joint_force", + # computed dynamics -- read-only in practice (confirm via wheel metadata) + "articulation_jacobian", + "articulation_mass_matrix", + "articulation_coriolis_and_centrifugal_force", + "articulation_gravity_force", + } +) + +# DOF/body property tensor types are CPU-resident even on GPU sims, so their +# bindings must be read/written through a CPU buffer. Mirrors +# ``isaaclab_ovphysx.tensor_types._CPU_ONLY_TYPES`` (consolidate later). +_CPU_ONLY_NAMES: frozenset[str] = frozenset( + { + "articulation_dof_stiffness", + "articulation_dof_damping", + "articulation_dof_limit", + "articulation_dof_max_velocity", + "articulation_dof_max_force", + "articulation_dof_armature", + "articulation_dof_friction_properties", + "articulation_body_mass", + "articulation_body_com_pose", + "articulation_body_inertia", + "articulation_body_inv_mass", + "articulation_body_inv_inertia", + "rigid_body_mass", + "rigid_body_com_pose", + "rigid_body_inertia", + "rigid_body_inv_mass", + "rigid_body_inv_inertia", + } +) + + +class OvPhysxViewError(RuntimeError): + """Raised for unknown attribute names, read-only writes, or prims a name cannot bind to.""" + + +# ----------------------------------------------------------------------------- +# Pure helpers (no native simulation required; testable against ``ovphysx.types``) +# ----------------------------------------------------------------------------- + + +def attribute_vocabulary() -> list[str]: + """Return every valid attribute name (sorted lowercased ``TensorType`` members).""" + return sorted(t.name.lower() for t in TensorType if t.name != "INVALID") + + +def resolve_tensor_type(name: str) -> Any: + """Resolve a lowercased attribute name to its :class:`TensorType` member. + + Args: + name: Lowercased enum name, e.g. ``"articulation_dof_stiffness"``. + + Returns: + The matching :class:`TensorType` member. + + Raises: + OvPhysxViewError: If the name is not a valid ``TensorType`` member. + """ + try: + tt = TensorType[name.upper()] + except KeyError: + raise OvPhysxViewError( + f"Unknown attribute {name!r}. Valid names are the lowercased TensorType members, " + f"e.g. {attribute_vocabulary()[:4]} ... ({len(attribute_vocabulary())} total)." + ) from None + if tt.name == "INVALID": + raise OvPhysxViewError(f"{name!r} is not an addressable attribute.") + return tt + + +def tensor_type_name(tensor_type: Any) -> str: + """Return the canonical lowercased name of a :class:`TensorType` member.""" + return tensor_type.name.lower() + + +def is_read_only(name: str) -> bool: + """Return whether an attribute name is read-only (cannot be written).""" + return name.lower() in _READ_ONLY_NAMES + + +# ----------------------------------------------------------------------------- +# The view +# ----------------------------------------------------------------------------- + + +class OvPhysxView: + """A string-keyed, generic view over OVPhysX ``TensorBinding`` handles for one prim pattern. + + Args: + physx: The OVPhysX ``PhysX`` instance exposing ``create_tensor_binding``. + pattern: An fnmatch glob selecting the prims this view addresses. + device: Simulation device (e.g. ``"cuda:0"`` or ``"cpu"``) used to size buffers. + tensor_types: Optional explicit set of :class:`TensorType` members to instantiate + eagerly. Ignored unless ``eager`` is set; defaults to "all applicable" when eager. + eager: If ``True``, create bindings up front (see design doc §4/§8). Defaults to ``False`` + (lazy: bindings are created on first access). + """ + + def __init__( + self, + physx: Any, + pattern: str, + device: str, + *, + tensor_types: list | None = None, + eager: bool = False, + ) -> None: + self._physx = physx + self._pattern = pattern + self._device = device + self._bindings: dict[Any, Any] = {} + self._read_buffers: dict[Any, wp.array] = {} + self._staging_buffers: dict[Any, wp.array] = {} + + if eager: + requested = tensor_types if tensor_types is not None else [t for t in TensorType if t.name != "INVALID"] + for tt in requested: + try: + self._binding(tt) + except OvPhysxViewError: + # Not applicable to these prims; skip silently (matches asset eager-create). + logger.debug("eager binding skipped for %s on %s", tt, pattern) + + # -- core: string-keyed get / set ---------------------------------------- + + def get_attribute(self, name: str, *, out: wp.array | None = None) -> wp.array: + """Read the full attribute tensor. + + Reads are full-array (the wheel exposes no selective read); index into the + returned tensor for a subset. + + Args: + name: Lowercased ``TensorType`` name. + out: Optional destination buffer (shape ``binding.shape``); receives a copy. + If omitted, a view-owned buffer is returned that is **overwritten on the + next read of the same attribute**. + + Returns: + A :class:`warp.array` holding the attribute values. + """ + tt = resolve_tensor_type(name) + binding = self._binding(tt) + buf = self._read_buffers.get(tt) + if buf is None: + buf = wp.zeros(tuple(binding.shape), dtype=wp.float32, device=self._read_device(name)) + self._read_buffers[tt] = buf + binding.read(buf) + if out is not None: + wp.copy(out, buf) + return out + return buf + + def set_attribute( + self, + name: str, + values: wp.array, + *, + indices: wp.array | None = None, + mask: wp.array | None = None, + ) -> None: + """Write a full-shaped attribute tensor; ``indices``/``mask`` select which rows apply. + + If both ``indices`` and ``mask`` are given, ``mask`` wins (and the wheel warns), + matching ``TensorBinding.write``. + + Args: + name: Lowercased ``TensorType`` name. + values: Source buffer with shape ``binding.shape``. + indices: Optional integer row indices to write. + mask: Optional boolean row mask to write. + + Raises: + OvPhysxViewError: If the attribute is read-only or the shape mismatches. + """ + if is_read_only(name): + raise OvPhysxViewError(f"Attribute {name!r} is read-only and cannot be written.") + tt = resolve_tensor_type(name) + binding = self._binding(tt) + src = self._as_wp(values) + if tuple(src.shape) != tuple(binding.shape): + raise OvPhysxViewError( + f"Shape mismatch writing {name!r}: got {tuple(src.shape)}, expected {tuple(binding.shape)}." + ) + if self._needs_cpu_staging(name): + staging = self._staging_buffers.get(tt) + if staging is None: + staging = wp.zeros(tuple(binding.shape), dtype=wp.float32, device="cpu", pinned=True) + self._staging_buffers[tt] = staging + wp.copy(staging, src) + binding.write(staging, indices=indices, mask=mask) + else: + binding.write(src, indices=indices, mask=mask) + + # -- discoverability ------------------------------------------------------- + + @property + def attribute_names(self) -> list[str]: + """All valid attribute names addressable by this view (the full vocabulary).""" + return attribute_vocabulary() + + @property + def available_attributes(self) -> list[str]: + """Names with a live binding instantiated for this view's prims.""" + return sorted(tensor_type_name(tt) for tt in self._bindings) + + def has_attribute(self, name: str) -> bool: + """Return whether ``name`` is a valid attribute name (resolvable to a ``TensorType``).""" + try: + resolve_tensor_type(name) + except OvPhysxViewError: + return False + return True + + def __contains__(self, name: str) -> bool: + return self.has_attribute(name) + + # -- metadata passthrough (from a sample binding) -------------------------- + + @property + def count(self) -> int: + """Number of prims matched by this view's pattern.""" + return self._sample().count + + @property + def prim_paths(self) -> list[str]: + """USD paths of the prims matched by this view's pattern.""" + return list(self._sample().prim_paths) + + @property + def dof_names(self) -> list[str]: + """Per-articulation DOF names (articulation views only).""" + return list(self._sample().dof_names) + + @property + def body_names(self) -> list[str]: + """Per-articulation body (link) names (articulation views only).""" + return list(self._sample().body_names) + + @property + def dof_count(self) -> int: + return self._sample().dof_count + + @property + def body_count(self) -> int: + return self._sample().body_count + + # -- internals ------------------------------------------------------------- + + def _binding(self, tensor_type: Any) -> Any: + """Return the cached :class:`TensorBinding` for ``tensor_type``, creating it on first use.""" + binding = self._bindings.get(tensor_type) + if binding is not None: + return binding + try: + binding = self._physx.create_tensor_binding(pattern=self._pattern, tensor_type=tensor_type) + except Exception as exc: # noqa: BLE001 -- wheel raises bare exceptions; re-wrapped below + raise OvPhysxViewError( + f"Attribute {tensor_type_name(tensor_type)!r} is not available for prims matching {self._pattern!r}." + ) from exc + # The wheel returns a 0-count binding when the pattern matches nothing applicable. + if binding is None or getattr(binding, "count", 0) == 0: + raise OvPhysxViewError( + f"Attribute {tensor_type_name(tensor_type)!r} is not available for prims matching " + f"{self._pattern!r} (no matching prims)." + ) + self._bindings[tensor_type] = binding + return binding + + def _sample(self) -> Any: + """Return any instantiated binding to read view-level metadata from.""" + if not self._bindings: + raise OvPhysxViewError( + "No bindings instantiated yet; access an attribute (or construct with eager=True) " + "before reading view metadata." + ) + return next(iter(self._bindings.values())) + + def _read_device(self, name: str) -> str: + """Device a read buffer for ``name`` should live on (CPU for CPU-only types on GPU sims).""" + if name.lower() in _CPU_ONLY_NAMES and self._device != "cpu": + return "cpu" + return self._device + + def _needs_cpu_staging(self, name: str) -> bool: + return name.lower() in _CPU_ONLY_NAMES and self._device != "cpu" + + def _as_wp(self, values: Any) -> wp.array: + """Coerce ``values`` to a :class:`warp.array` (accepts warp or torch input).""" + if isinstance(values, wp.array): + return values + # torch tensor -> warp (zero-copy view) without importing torch eagerly + if hasattr(values, "__dlpack__") or values.__class__.__module__.startswith("torch"): + return wp.from_torch(values) + return wp.array(values) + + def __repr__(self) -> str: + return f"OvPhysxView(pattern={self._pattern!r}, device={self._device!r}, instantiated={len(self._bindings)})" diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py new file mode 100644 index 000000000000..68c3a4a14ae3 --- /dev/null +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -0,0 +1,200 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Unit tests for the prototype :class:`OvPhysxView` string-keyed binding wrapper. + +These exercise the pure-Python name<->enum logic and the view's get/set dispatch +against a fake ``PhysX`` + fake ``TensorBinding`` -- no native simulation required. +Full read/write round-trips on a live sim are covered by the asset integration tests. +""" + +from __future__ import annotations + +import pytest + +# The OVPhysX runtime wheel is optional. ``ovphysx.types`` is pure Python (no native +# dependency), so the import-skip guards only the wheel's presence. +pytest.importorskip("ovphysx.types", reason="ovphysx wheel not installed") + +import warp as wp # noqa: E402 +from isaaclab_ovphysx.sim.views.ovphysx_view import ( # noqa: E402 + OvPhysxView, + OvPhysxViewError, + attribute_vocabulary, + is_read_only, + resolve_tensor_type, + tensor_type_name, +) +from ovphysx.types import TensorType # noqa: E402 + +wp.init() + +# Per-type shapes used by the fakes (only the types touched by the tests). +_SHAPES = { + TensorType.RIGID_BODY_POSE: lambda n: (n, 7), + TensorType.RIGID_BODY_VELOCITY: lambda n: (n, 6), + TensorType.RIGID_BODY_MASS: lambda n: (n,), + TensorType.RIGID_BODY_ACCELERATION: lambda n: (n, 6), +} + + +class _FakeBinding: + """Minimal stand-in for an ovphysx ``TensorBinding``.""" + + def __init__(self, tensor_type, n: int): + self.tensor_type = tensor_type + self.shape = _SHAPES.get(tensor_type, lambda k: (k, 1))(n) + self.count = n + self.prim_paths = [f"/World/env_{i}/body" for i in range(n)] + self.dof_names: list[str] = [] + self.body_names = ["body"] + self.dof_count = 0 + self.body_count = 1 + self.read_calls = 0 + self.write_calls: list[tuple] = [] + + def read(self, dst) -> None: + assert tuple(dst.shape) == tuple(self.shape) + self.read_calls += 1 + + def write(self, tensor, indices=None, mask=None) -> None: + assert tuple(tensor.shape) == tuple(self.shape) + self.write_calls.append((indices, mask)) + + +class _FakePhysX: + """Fake ``PhysX`` whose ``create_tensor_binding`` hands back ``_FakeBinding`` instances.""" + + def __init__(self, n: int = 3, unavailable: set | None = None): + self.n = n + self._unavailable = unavailable or set() + self.created: list = [] + + def create_tensor_binding(self, *, pattern, tensor_type): + self.created.append(tensor_type) + if tensor_type in self._unavailable: + # The wheel returns a 0-count binding when nothing matches. + b = _FakeBinding(tensor_type, 0) + return b + return _FakeBinding(tensor_type, self.n) + + +def _make_view(n: int = 3, unavailable: set | None = None) -> OvPhysxView: + return OvPhysxView(_FakePhysX(n=n, unavailable=unavailable), pattern="/World/env_*/body", device="cpu") + + +# ----------------------------------------------------------------------------- +# Pure helpers +# ----------------------------------------------------------------------------- + + +def test_vocabulary_is_lowercased_enum_without_invalid(): + vocab = attribute_vocabulary() + assert "articulation_dof_stiffness" in vocab + assert "rigid_body_pose" in vocab + assert "invalid" not in vocab + assert vocab == sorted(vocab) + + +def test_resolve_roundtrips_name_and_enum(): + tt = resolve_tensor_type("articulation_dof_stiffness") + assert tt is TensorType.ARTICULATION_DOF_STIFFNESS + assert tensor_type_name(tt) == "articulation_dof_stiffness" + # case-insensitive + assert resolve_tensor_type("RIGID_BODY_POSE") is TensorType.RIGID_BODY_POSE + + +def test_resolve_unknown_name_raises(): + with pytest.raises(OvPhysxViewError): + resolve_tensor_type("not_a_real_attribute") + + +def test_read_only_classification(): + assert is_read_only("articulation_jacobian") + assert is_read_only("rigid_body_acceleration") + assert not is_read_only("articulation_dof_stiffness") + assert not is_read_only("rigid_body_pose") + + +# ----------------------------------------------------------------------------- +# View dispatch (fake physx) +# ----------------------------------------------------------------------------- + + +def test_get_attribute_reads_into_sized_buffer_and_reuses_it(): + view = _make_view(n=4) + buf = view.get_attribute("rigid_body_pose") + assert tuple(buf.shape) == (4, 7) + binding = view._bindings[TensorType.RIGID_BODY_POSE] + assert binding.read_calls == 1 + # second read reuses the same cached buffer object + buf2 = view.get_attribute("rigid_body_pose") + assert buf2 is buf + assert binding.read_calls == 2 + + +def test_get_attribute_out_param_receives_copy(): + view = _make_view(n=2) + out = wp.zeros((2, 7), dtype=wp.float32, device="cpu") + ret = view.get_attribute("rigid_body_pose", out=out) + assert ret is out + + +def test_set_attribute_forwards_indices_and_mask(): + view = _make_view(n=3) + values = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + idx = wp.array([0, 2], dtype=wp.int32, device="cpu") + view.set_attribute("rigid_body_pose", values, indices=idx) + binding = view._bindings[TensorType.RIGID_BODY_POSE] + assert binding.write_calls == [(idx, None)] + + +def test_set_attribute_read_only_raises_and_does_not_write(): + view = _make_view(n=3) + values = wp.zeros((3, 6), dtype=wp.float32, device="cpu") + with pytest.raises(OvPhysxViewError, match="read-only"): + view.set_attribute("rigid_body_acceleration", values) + assert TensorType.RIGID_BODY_ACCELERATION not in view._bindings + + +def test_set_attribute_shape_mismatch_raises(): + view = _make_view(n=3) + wrong = wp.zeros((3, 6), dtype=wp.float32, device="cpu") + with pytest.raises(OvPhysxViewError, match="Shape mismatch"): + view.set_attribute("rigid_body_pose", wrong) + + +def test_unknown_attribute_raises_on_access(): + view = _make_view() + with pytest.raises(OvPhysxViewError): + view.get_attribute("totally_made_up") + + +def test_unavailable_binding_reports_clear_error(): + view = _make_view(n=3, unavailable={TensorType.RIGID_BODY_VELOCITY}) + with pytest.raises(OvPhysxViewError, match="not available"): + view.get_attribute("rigid_body_velocity") + + +def test_discoverability_surface(): + view = _make_view() + assert "rigid_body_pose" in view + assert view.has_attribute("articulation_dof_stiffness") + assert not view.has_attribute("nope") + assert "rigid_body_pose" in view.attribute_names + # available_attributes only lists instantiated bindings + assert view.available_attributes == [] + view.get_attribute("rigid_body_pose") + assert view.available_attributes == ["rigid_body_pose"] + + +def test_metadata_passthrough_from_sample_binding(): + view = _make_view(n=5) + # metadata before any access raises a clear error + with pytest.raises(OvPhysxViewError): + _ = view.count + view.get_attribute("rigid_body_pose") + assert view.count == 5 + assert len(view.prim_paths) == 5 From 3a9cf1eb1ed2047861129c36b4cad2ab9a9c1aaf Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 13:09:37 +0200 Subject: [PATCH 02/12] Make OvPhysxView a binding manager; raise on device mismatch Reworks the view from a convenience wrapper into a layer that can back the OVPhysX asset/data classes, per the PR review of #6224: - read_into(name, dst): zero-copy fill of a caller-owned, possibly structured-dtype buffer (e.g. wp.transformf) via a float32 reinterpret view -- the mechanism the data containers use today. - set_attribute: accepts structured-dtype sources via the same reinterpret; non-float32-width buffers are rejected rather than silently bit-cast. - prim_paths + key_aliases: support the fused multi-prim binding form (create_tensor_binding(prim_paths=[...])) and storing a binding under a different TensorType key, as RigidObjectCollection needs. - binding_for(): raw TensorBinding accessor for adoption. - _CPU_ONLY_NAMES is now derived from tensor_types._CPU_ONLY_TYPES (no drift). - Added joint/tendon/is_fixed_base metadata passthrough; eager construction raises if it creates zero bindings; get_attribute allocates a fresh buffer (no aliasing); nested error hierarchy; PhysX/binding Protocols. Device policy: no implicit CPU<->GPU conversion. CPU-resident property types are read/written on CPU; a buffer on the wrong device raises DeviceMismatch instead of being staged. Device-less host data (numpy/list) is materialized on the binding's native device. --- .../sim/views/ovphysx_view.py | 418 ++++++++++++------ .../test/sim/test_ovphysx_view.py | 229 ++++++++-- 2 files changed, 470 insertions(+), 177 deletions(-) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index a1e91ee59b60..6eb52e031225 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -5,50 +5,57 @@ """String-keyed view over OVPhysX ``TensorBinding`` handles. -PROTOTYPE for the design at -``docs/superpowers/specs/2026-06-17-ovphysx-view-design.md``. - OVPhysX exposes physics attributes as a loose ``dict[TensorType, TensorBinding]`` with no view object -- unlike Newton's ``selection.ArticulationView`` or PhysX's -typed tensor views. :class:`OvPhysxView` wraps those bindings for a single prim -pattern behind a string-keyed ``get_attribute`` / ``set_attribute`` surface that -mirrors Newton's selection ergonomics, but is simpler: there is no -``Model``/``State``/``Control`` source object because the :class:`TensorType` -already implies where the data lives. - -Attribute names are the **lowercased** ``TensorType`` enum members, derived -directly from the wheel enum (no hand-maintained table):: - - view.get_attribute("articulation_dof_stiffness") +typed tensor views. :class:`OvPhysxView` wraps those bindings for one prim pattern +(or an explicit ``prim_paths`` list) behind a string-keyed surface that mirrors +Newton's selection ergonomics, but is simpler: there is no ``Model``/``State``/ +``Control`` source object because the :class:`TensorType` already implies where the +data lives. + +Attributes are addressed by the lowercased ``TensorType`` enum member name (derived +directly from the wheel enum -- no hand-maintained table) or by the enum member +itself:: + + view.get_attribute("articulation_dof_stiffness") # allocates and returns + view.read_into("articulation_root_pose", root_pose_buf) # zero-copy into a caller buffer view.set_attribute("rigid_body_pose", values, mask=env_mask) -Known follow-ups (see design doc §10 and §7): - -* Buffers are :class:`warp.array` for consistency with the existing - ``_binding_read`` / ``_binding_write`` helpers; a torch-returning convenience - can wrap this (``wp.to_torch(view.get_attribute(...))``). -* ``_CPU_ONLY_NAMES`` mirrors ``isaaclab_ovphysx.tensor_types._CPU_ONLY_TYPES``; - these should be consolidated, ideally sourced from wheel writability/placement - metadata once it is exposed. -* The read-only set is hard-coded pending a wheel ``is_writable`` query. +Design intent: be usable as the binding-management layer *inside* the OVPhysX asset +classes (see ``docs/superpowers/specs/2026-06-17-ovphysx-view-design.md`` §6), so it +exposes a raw :meth:`binding_for` accessor and a zero-copy :meth:`read_into` that +fills a caller-owned, possibly structured-dtype buffer via a ``float32`` reinterpret +view -- the same mechanism the data containers use today. + +**Device policy: no implicit CPU<->GPU conversion.** OVPhysX serves DOF/body +*property* tensor types from CPU memory even on a GPU sim (see :data:`_CPU_ONLY_NAMES`), +while *state* tensor types are device-resident. This view reads/writes each binding on +its native device and **raises** :class:`OvPhysxView.DeviceMismatch` if a caller hands +it a buffer on the wrong device. Staging a CPU property to/from the simulation device +is the caller's explicit responsibility, never hidden here. """ from __future__ import annotations import logging -from typing import Any +import math +from typing import Any, Protocol, runtime_checkable import warp as wp from isaaclab_ovphysx._runtime import import_ovphysx +from isaaclab_ovphysx.tensor_types import _CPU_ONLY_TYPES logger = logging.getLogger(__name__) # Pure-Python enum (no native dependency); safe to import regardless of USD state. TensorType = import_ovphysx("ovphysx.types").TensorType -# Tensor types that are read-only on the wheel today. The first group is marked -# read-only in the enum; the computed-dynamics group is read-only in practice. +# Tensor types that cannot be written. The first group is read-only by PhysX +# convention (accelerations, inverse mass/inertia, projected joint force); the +# computed-dynamics group (jacobian, mass matrix, coriolis, gravity) is read-only +# in practice. The wheel enum exposes no writability flag today, so this set is +# hand-maintained. # TODO(ovphysx): source this from a wheel writability query (design doc §7 ask 3). _READ_ONLY_NAMES: frozenset[str] = frozenset( { @@ -59,7 +66,6 @@ "articulation_body_inv_mass", "articulation_body_inv_inertia", "articulation_dof_projected_joint_force", - # computed dynamics -- read-only in practice (confirm via wheel metadata) "articulation_jacobian", "articulation_mass_matrix", "articulation_coriolis_and_centrifugal_force", @@ -67,34 +73,38 @@ } ) -# DOF/body property tensor types are CPU-resident even on GPU sims, so their -# bindings must be read/written through a CPU buffer. Mirrors -# ``isaaclab_ovphysx.tensor_types._CPU_ONLY_TYPES`` (consolidate later). -_CPU_ONLY_NAMES: frozenset[str] = frozenset( - { - "articulation_dof_stiffness", - "articulation_dof_damping", - "articulation_dof_limit", - "articulation_dof_max_velocity", - "articulation_dof_max_force", - "articulation_dof_armature", - "articulation_dof_friction_properties", - "articulation_body_mass", - "articulation_body_com_pose", - "articulation_body_inertia", - "articulation_body_inv_mass", - "articulation_body_inv_inertia", - "rigid_body_mass", - "rigid_body_com_pose", - "rigid_body_inertia", - "rigid_body_inv_mass", - "rigid_body_inv_inertia", - } -) +# DOF/body property tensor types that are CPU-resident even on a GPU sim. Derived +# from the canonical, wheel-availability-gated set in ``isaaclab_ovphysx.tensor_types`` +# so the two never drift. +_CPU_ONLY_NAMES: frozenset[str] = frozenset(tt.name.lower() for tt in _CPU_ONLY_TYPES) + + +@runtime_checkable +class _BindingLike(Protocol): + """Structural type of an ovphysx ``TensorBinding`` as used by this view.""" + + shape: tuple[int, ...] + count: int + prim_paths: list[str] + dof_names: list[str] + body_names: list[str] + joint_names: list[str] + dof_count: int + body_count: int + joint_count: int + is_fixed_base: bool + fixed_tendon_count: int + spatial_tendon_count: int + + def read(self, tensor: wp.array) -> None: ... + + def write(self, tensor: wp.array, indices: wp.array | None = None, mask: wp.array | None = None) -> None: ... -class OvPhysxViewError(RuntimeError): - """Raised for unknown attribute names, read-only writes, or prims a name cannot bind to.""" +class _PhysXLike(Protocol): + """Structural type of the ovphysx ``PhysX`` instance this view depends on.""" + + def create_tensor_binding(self, *, tensor_type: Any, pattern: str = ..., prim_paths: list[str] = ...) -> Any: ... # ----------------------------------------------------------------------------- @@ -117,17 +127,17 @@ def resolve_tensor_type(name: str) -> Any: The matching :class:`TensorType` member. Raises: - OvPhysxViewError: If the name is not a valid ``TensorType`` member. + OvPhysxView.UnknownAttribute: If the name is not an addressable ``TensorType``. """ try: tt = TensorType[name.upper()] except KeyError: - raise OvPhysxViewError( + raise OvPhysxView.UnknownAttribute( f"Unknown attribute {name!r}. Valid names are the lowercased TensorType members, " f"e.g. {attribute_vocabulary()[:4]} ... ({len(attribute_vocabulary())} total)." ) from None if tt.name == "INVALID": - raise OvPhysxViewError(f"{name!r} is not an addressable attribute.") + raise OvPhysxView.UnknownAttribute(f"{name!r} is not an addressable attribute.") return tt @@ -141,118 +151,181 @@ def is_read_only(name: str) -> bool: return name.lower() in _READ_ONLY_NAMES +def is_cpu_only(name: str) -> bool: + """Return whether an attribute is CPU-resident even on a GPU simulation.""" + return name.lower() in _CPU_ONLY_NAMES + + # ----------------------------------------------------------------------------- # The view # ----------------------------------------------------------------------------- class OvPhysxView: - """A string-keyed, generic view over OVPhysX ``TensorBinding`` handles for one prim pattern. + """A string-keyed, generic view over OVPhysX ``TensorBinding`` handles for one prim set. Args: physx: The OVPhysX ``PhysX`` instance exposing ``create_tensor_binding``. - pattern: An fnmatch glob selecting the prims this view addresses. - device: Simulation device (e.g. ``"cuda:0"`` or ``"cpu"``) used to size buffers. - tensor_types: Optional explicit set of :class:`TensorType` members to instantiate - eagerly. Ignored unless ``eager`` is set; defaults to "all applicable" when eager. - eager: If ``True``, create bindings up front (see design doc §4/§8). Defaults to ``False`` - (lazy: bindings are created on first access). + pattern: An fnmatch glob selecting the prims this view addresses. Mutually + exclusive with ``prim_paths``. + device: Simulation device (e.g. ``"cuda:0"`` or ``"cpu"``). State bindings are + read/written on this device; CPU-only property bindings always use ``"cpu"``. + prim_paths: An explicit list of fnmatch globs for the fused multi-prim binding + form (``create_tensor_binding(prim_paths=[...])``). Mutually exclusive with + ``pattern``. + key_aliases: Optional mapping ``requested_type -> created_type`` so a binding can + be stored under a different :class:`TensorType` key than the one created + (e.g. a ``RigidObjectCollection`` stores ``rigid_body_pose`` under ``link_pose``). + tensor_types: Explicit set of :class:`TensorType` members to instantiate eagerly. + Used only when ``eager`` is set; defaults to every applicable type. + eager: If ``True``, create bindings up front and raise if none could be created. + Defaults to ``False`` (lazy: bindings are created on first access). """ + class OvPhysxViewError(RuntimeError): + """Base class for all errors raised by :class:`OvPhysxView`.""" + + class UnknownAttribute(OvPhysxViewError): + """The attribute name does not resolve to an addressable ``TensorType``.""" + + class ReadOnlyAttribute(OvPhysxViewError): + """A write was attempted on a read-only attribute.""" + + class AttributeUnavailable(OvPhysxViewError): + """No binding could be created for the attribute on this view's prims.""" + + class ShapeMismatch(OvPhysxViewError): + """A supplied buffer does not match the binding's element count.""" + + class DeviceMismatch(OvPhysxViewError): + """A supplied buffer is on a different device than the binding requires.""" + def __init__( self, - physx: Any, - pattern: str, - device: str, + physx: _PhysXLike, + pattern: str | None = None, + device: str = "cpu", *, - tensor_types: list | None = None, + prim_paths: list[str] | None = None, + key_aliases: dict[Any, Any] | None = None, + tensor_types: list[Any] | None = None, eager: bool = False, ) -> None: + if (pattern is None) == (prim_paths is None): + raise ValueError("Provide exactly one of 'pattern' or 'prim_paths'.") self._physx = physx self._pattern = pattern + self._prim_paths = prim_paths self._device = device + self._key_aliases: dict[Any, Any] = dict(key_aliases or {}) self._bindings: dict[Any, Any] = {} - self._read_buffers: dict[Any, wp.array] = {} - self._staging_buffers: dict[Any, wp.array] = {} if eager: requested = tensor_types if tensor_types is not None else [t for t in TensorType if t.name != "INVALID"] for tt in requested: try: self._binding(tt) - except OvPhysxViewError: - # Not applicable to these prims; skip silently (matches asset eager-create). - logger.debug("eager binding skipped for %s on %s", tt, pattern) - - # -- core: string-keyed get / set ---------------------------------------- - - def get_attribute(self, name: str, *, out: wp.array | None = None) -> wp.array: + except OvPhysxView.AttributeUnavailable: + # Not applicable to these prims; skip and keep going. + logger.debug("eager binding skipped for %s", tt) + if not self._bindings: + raise OvPhysxView.AttributeUnavailable( + f"Could not create any bindings for {self._target_repr()}; " + "the pattern/prim_paths likely match no prims." + ) + + # -- core: string-keyed get / set / read-into ------------------------------ + + def get_attribute(self, name: str | Any, *, out: wp.array | None = None) -> wp.array: """Read the full attribute tensor. Reads are full-array (the wheel exposes no selective read); index into the returned tensor for a subset. Args: - name: Lowercased ``TensorType`` name. - out: Optional destination buffer (shape ``binding.shape``); receives a copy. - If omitted, a view-owned buffer is returned that is **overwritten on the - next read of the same attribute**. + name: Lowercased ``TensorType`` name or the member itself. + out: Optional destination buffer to fill (must be on the binding's native + device and match its element count). If omitted, a freshly allocated + ``float32`` :class:`warp.array` on the native device is returned. Returns: - A :class:`warp.array` holding the attribute values. + A :class:`warp.array` holding the attribute values. When ``out`` is omitted + this is a fresh, caller-owned array (no aliasing of view state). """ - tt = resolve_tensor_type(name) + tt = self._resolve(name) binding = self._binding(tt) - buf = self._read_buffers.get(tt) - if buf is None: - buf = wp.zeros(tuple(binding.shape), dtype=wp.float32, device=self._read_device(name)) - self._read_buffers[tt] = buf - binding.read(buf) + device = self._native_device(tt) if out is not None: - wp.copy(out, buf) + self._check_device(out, device, tensor_type_name(tt), "destination") + binding.read(self._as_binding_view(out, binding, "destination")) return out + buf = wp.zeros(tuple(binding.shape), dtype=wp.float32, device=device) + binding.read(buf) return buf + def read_into(self, name: str | Any, dst: wp.array) -> None: + """Fill ``dst`` in place from the attribute binding (zero-copy). + + ``dst`` may be a structured-dtype buffer (e.g. ``wp.transformf``); it is read + through a ``float32`` reinterpret view that matches the binding's flat shape, so + the structured GPU/CPU buffer is filled directly with no extra copy. This is the + path the asset data containers use. + + Args: + name: Lowercased ``TensorType`` name or the member itself. + dst: Caller-owned buffer on the binding's native device whose element count + matches the binding. + + Raises: + OvPhysxView.DeviceMismatch: If ``dst`` is not on the binding's native device. + OvPhysxView.ShapeMismatch: If ``dst``'s element count does not match. + """ + tt = self._resolve(name) + binding = self._binding(tt) + self._check_device(dst, self._native_device(tt), tensor_type_name(tt), "destination") + binding.read(self._as_binding_view(dst, binding, "destination")) + def set_attribute( self, - name: str, + name: str | Any, values: wp.array, *, indices: wp.array | None = None, mask: wp.array | None = None, ) -> None: - """Write a full-shaped attribute tensor; ``indices``/``mask`` select which rows apply. + """Write a full attribute tensor; ``indices``/``mask`` select which rows apply. - If both ``indices`` and ``mask`` are given, ``mask`` wins (and the wheel warns), - matching ``TensorBinding.write``. + ``values`` may be a structured-dtype buffer (read through a ``float32`` + reinterpret view). If both ``indices`` and ``mask`` are given, ``mask`` wins and + the wheel emits a ``UserWarning`` -- this view forwards both verbatim to + ``TensorBinding.write`` and does not implement the precedence itself. Args: - name: Lowercased ``TensorType`` name. - values: Source buffer with shape ``binding.shape``. + name: Lowercased ``TensorType`` name or the member itself. + values: Source buffer on the binding's native device, matching its element count. indices: Optional integer row indices to write. mask: Optional boolean row mask to write. Raises: - OvPhysxViewError: If the attribute is read-only or the shape mismatches. + OvPhysxView.ReadOnlyAttribute: If the attribute is read-only. + OvPhysxView.DeviceMismatch: If ``values`` is not on the binding's native device. + OvPhysxView.ShapeMismatch: If ``values``' element count does not match. """ - if is_read_only(name): - raise OvPhysxViewError(f"Attribute {name!r} is read-only and cannot be written.") - tt = resolve_tensor_type(name) + tt = self._resolve(name) + attr = tensor_type_name(tt) + if attr in _READ_ONLY_NAMES: + raise OvPhysxView.ReadOnlyAttribute(f"Attribute {attr!r} is read-only and cannot be written.") binding = self._binding(tt) - src = self._as_wp(values) - if tuple(src.shape) != tuple(binding.shape): - raise OvPhysxViewError( - f"Shape mismatch writing {name!r}: got {tuple(src.shape)}, expected {tuple(binding.shape)}." - ) - if self._needs_cpu_staging(name): - staging = self._staging_buffers.get(tt) - if staging is None: - staging = wp.zeros(tuple(binding.shape), dtype=wp.float32, device="cpu", pinned=True) - self._staging_buffers[tt] = staging - wp.copy(staging, src) - binding.write(staging, indices=indices, mask=mask) - else: - binding.write(src, indices=indices, mask=mask) + device = self._native_device(tt) + src = self._as_wp(values, device) + self._check_device(src, device, attr, "source") + binding.write(self._as_binding_view(src, binding, "source"), indices=indices, mask=mask) + + # -- raw binding access (for asset/data-container adoption) ---------------- + + def binding_for(self, name: str | Any) -> _BindingLike: + """Return the underlying ``TensorBinding`` for an attribute, creating it on first use.""" + return self._binding(self._resolve(name)) # -- discoverability ------------------------------------------------------- @@ -266,27 +339,27 @@ def available_attributes(self) -> list[str]: """Names with a live binding instantiated for this view's prims.""" return sorted(tensor_type_name(tt) for tt in self._bindings) - def has_attribute(self, name: str) -> bool: - """Return whether ``name`` is a valid attribute name (resolvable to a ``TensorType``).""" + def has_attribute(self, name: str | Any) -> bool: + """Return whether ``name`` resolves to a valid ``TensorType``.""" try: - resolve_tensor_type(name) - except OvPhysxViewError: + self._resolve(name) + except OvPhysxView.UnknownAttribute: return False return True - def __contains__(self, name: str) -> bool: + def __contains__(self, name: str | Any) -> bool: return self.has_attribute(name) # -- metadata passthrough (from a sample binding) -------------------------- @property def count(self) -> int: - """Number of prims matched by this view's pattern.""" + """Number of prims matched by this view.""" return self._sample().count @property def prim_paths(self) -> list[str]: - """USD paths of the prims matched by this view's pattern.""" + """USD paths of the prims matched by this view.""" return list(self._sample().prim_paths) @property @@ -299,6 +372,11 @@ def body_names(self) -> list[str]: """Per-articulation body (link) names (articulation views only).""" return list(self._sample().body_names) + @property + def joint_names(self) -> list[str]: + """Per-articulation joint names (articulation views only).""" + return list(self._sample().joint_names) + @property def dof_count(self) -> int: return self._sample().dof_count @@ -307,24 +385,52 @@ def dof_count(self) -> int: def body_count(self) -> int: return self._sample().body_count + @property + def joint_count(self) -> int: + return self._sample().joint_count + + @property + def is_fixed_base(self) -> bool: + return self._sample().is_fixed_base + + @property + def fixed_tendon_count(self) -> int: + return self._sample().fixed_tendon_count + + @property + def spatial_tendon_count(self) -> int: + return self._sample().spatial_tendon_count + # -- internals ------------------------------------------------------------- + def _resolve(self, name: str | Any) -> Any: + """Resolve a string name or a ``TensorType`` member to a ``TensorType``.""" + if isinstance(name, str): + return resolve_tensor_type(name) + return name + def _binding(self, tensor_type: Any) -> Any: - """Return the cached :class:`TensorBinding` for ``tensor_type``, creating it on first use.""" + """Return the cached ``TensorBinding`` for ``tensor_type``, creating it on first use.""" binding = self._bindings.get(tensor_type) if binding is not None: return binding + create_type = self._key_aliases.get(tensor_type, tensor_type) + kwargs: dict[str, Any] = {"tensor_type": create_type} + if self._prim_paths is not None: + kwargs["prim_paths"] = self._prim_paths + else: + kwargs["pattern"] = self._pattern try: - binding = self._physx.create_tensor_binding(pattern=self._pattern, tensor_type=tensor_type) + binding = self._physx.create_tensor_binding(**kwargs) except Exception as exc: # noqa: BLE001 -- wheel raises bare exceptions; re-wrapped below - raise OvPhysxViewError( - f"Attribute {tensor_type_name(tensor_type)!r} is not available for prims matching {self._pattern!r}." + raise OvPhysxView.AttributeUnavailable( + f"Attribute {tensor_type_name(tensor_type)!r} is not available for {self._target_repr()}." ) from exc - # The wheel returns a 0-count binding when the pattern matches nothing applicable. + # The wheel returns a 0-count binding when nothing matches. if binding is None or getattr(binding, "count", 0) == 0: - raise OvPhysxViewError( - f"Attribute {tensor_type_name(tensor_type)!r} is not available for prims matching " - f"{self._pattern!r} (no matching prims)." + raise OvPhysxView.AttributeUnavailable( + f"Attribute {tensor_type_name(tensor_type)!r} is not available for {self._target_repr()} " + "(no matching prims)." ) self._bindings[tensor_type] = binding return binding @@ -332,29 +438,61 @@ def _binding(self, tensor_type: Any) -> Any: def _sample(self) -> Any: """Return any instantiated binding to read view-level metadata from.""" if not self._bindings: - raise OvPhysxViewError( + raise OvPhysxView.AttributeUnavailable( "No bindings instantiated yet; access an attribute (or construct with eager=True) " "before reading view metadata." ) return next(iter(self._bindings.values())) - def _read_device(self, name: str) -> str: - """Device a read buffer for ``name`` should live on (CPU for CPU-only types on GPU sims).""" - if name.lower() in _CPU_ONLY_NAMES and self._device != "cpu": - return "cpu" - return self._device + def _native_device(self, tensor_type: Any) -> str: + """Device a buffer for ``tensor_type`` must live on (CPU for CPU-only types).""" + return "cpu" if tensor_type in _CPU_ONLY_TYPES else self._device + + def _check_device(self, arr: wp.array, device: str, attr: str, role: str) -> None: + """Raise if ``arr`` is not on the binding's native device (no implicit conversion).""" + if str(arr.device) != device: + raise OvPhysxView.DeviceMismatch( + f"{role} for {attr!r} must be on device {device!r}, got {str(arr.device)!r}. " + "OvPhysxView does not stage between CPU and GPU; move the buffer yourself." + ) - def _needs_cpu_staging(self, name: str) -> bool: - return name.lower() in _CPU_ONLY_NAMES and self._device != "cpu" + def _as_binding_view(self, arr: wp.array, binding: Any, role: str) -> wp.array: + """Return a ``float32`` view of ``arr`` matching the binding's flat shape. - def _as_wp(self, values: Any) -> wp.array: - """Coerce ``values`` to a :class:`warp.array` (accepts warp or torch input).""" + Validates that ``arr``'s element count matches the binding, then returns ``arr`` + directly if it is already ``float32`` with the binding's shape, or a zero-copy + reinterpret view otherwise (for structured dtypes such as ``wp.transformf``). + """ + expected = math.prod(tuple(binding.shape)) + actual = arr.size * (wp.types.type_size_in_bytes(arr.dtype) // 4) + if actual != expected: + raise OvPhysxView.ShapeMismatch( + f"Shape mismatch for {role}: {actual} float32-equivalent elements, " + f"binding expects {expected} (shape {tuple(binding.shape)})." + ) + if arr.dtype == wp.float32 and tuple(arr.shape) == tuple(binding.shape): + return arr + return wp.array(ptr=arr.ptr, shape=tuple(binding.shape), dtype=wp.float32, device=str(arr.device), copy=False) + + def _as_wp(self, values: Any, device: str) -> wp.array: + """Coerce ``values`` to a :class:`warp.array`. + + Device-bearing inputs (warp / torch) keep their own device and are validated by + the caller (a mismatch raises -- never staged). Device-less host data (numpy + arrays, lists) carries no device, so it is materialized directly on ``device``. + """ if isinstance(values, wp.array): return values - # torch tensor -> warp (zero-copy view) without importing torch eagerly - if hasattr(values, "__dlpack__") or values.__class__.__module__.startswith("torch"): + if type(values).__module__.split(".")[0] == "torch": return wp.from_torch(values) - return wp.array(values) + return wp.array(values, device=device) + + def _target_repr(self) -> str: + return f"prim_paths={self._prim_paths!r}" if self._prim_paths is not None else f"pattern={self._pattern!r}" def __repr__(self) -> str: - return f"OvPhysxView(pattern={self._pattern!r}, device={self._device!r}, instantiated={len(self._bindings)})" + return f"OvPhysxView({self._target_repr()}, device={self._device!r}, instantiated={len(self._bindings)})" + + +# Backward-compatible module-level alias for the error base class. +OvPhysxViewError = OvPhysxView.OvPhysxViewError diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 68c3a4a14ae3..00912808ab49 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -3,15 +3,17 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Unit tests for the prototype :class:`OvPhysxView` string-keyed binding wrapper. +"""Unit tests for the :class:`OvPhysxView` string-keyed binding manager. -These exercise the pure-Python name<->enum logic and the view's get/set dispatch -against a fake ``PhysX`` + fake ``TensorBinding`` -- no native simulation required. +These exercise the pure-Python name<->enum logic and the view's get/set/read-into +dispatch (including the float32 reinterpret of structured buffers and the +no-implicit-conversion device policy) against a fake ``PhysX`` + fake ``TensorBinding``. Full read/write round-trips on a live sim are covered by the asset integration tests. """ from __future__ import annotations +import numpy as np import pytest # The OVPhysX runtime wheel is optional. ``ovphysx.types`` is pure Python (no native @@ -23,6 +25,7 @@ OvPhysxView, OvPhysxViewError, attribute_vocabulary, + is_cpu_only, is_read_only, resolve_tensor_type, tensor_type_name, @@ -30,13 +33,14 @@ from ovphysx.types import TensorType # noqa: E402 wp.init() +_HAS_CUDA = wp.get_cuda_device_count() > 0 # Per-type shapes used by the fakes (only the types touched by the tests). _SHAPES = { TensorType.RIGID_BODY_POSE: lambda n: (n, 7), TensorType.RIGID_BODY_VELOCITY: lambda n: (n, 6), - TensorType.RIGID_BODY_MASS: lambda n: (n,), - TensorType.RIGID_BODY_ACCELERATION: lambda n: (n, 6), + TensorType.RIGID_BODY_MASS: lambda n: (n,), # CPU-only + TensorType.RIGID_BODY_ACCELERATION: lambda n: (n, 6), # read-only } @@ -50,18 +54,23 @@ def __init__(self, tensor_type, n: int): self.prim_paths = [f"/World/env_{i}/body" for i in range(n)] self.dof_names: list[str] = [] self.body_names = ["body"] + self.joint_names: list[str] = [] self.dof_count = 0 self.body_count = 1 + self.joint_count = 0 + self.is_fixed_base = True + self.fixed_tendon_count = 0 + self.spatial_tendon_count = 0 self.read_calls = 0 + self.last_read: tuple | None = None self.write_calls: list[tuple] = [] def read(self, dst) -> None: - assert tuple(dst.shape) == tuple(self.shape) self.read_calls += 1 + self.last_read = (dst.dtype, tuple(dst.shape), str(dst.device)) def write(self, tensor, indices=None, mask=None) -> None: - assert tuple(tensor.shape) == tuple(self.shape) - self.write_calls.append((indices, mask)) + self.write_calls.append((tensor.dtype, tuple(tensor.shape), indices, mask)) class _FakePhysX: @@ -70,19 +79,17 @@ class _FakePhysX: def __init__(self, n: int = 3, unavailable: set | None = None): self.n = n self._unavailable = unavailable or set() - self.created: list = [] + self.created: list[tuple] = [] - def create_tensor_binding(self, *, pattern, tensor_type): - self.created.append(tensor_type) + def create_tensor_binding(self, *, tensor_type, pattern=None, prim_paths=None): + self.created.append((tensor_type, pattern, prim_paths)) if tensor_type in self._unavailable: - # The wheel returns a 0-count binding when nothing matches. - b = _FakeBinding(tensor_type, 0) - return b + return _FakeBinding(tensor_type, 0) # wheel returns a 0-count binding on no match return _FakeBinding(tensor_type, self.n) -def _make_view(n: int = 3, unavailable: set | None = None) -> OvPhysxView: - return OvPhysxView(_FakePhysX(n=n, unavailable=unavailable), pattern="/World/env_*/body", device="cpu") +def _make_view(n: int = 3, unavailable: set | None = None, device: str = "cpu") -> OvPhysxView: + return OvPhysxView(_FakePhysX(n=n, unavailable=unavailable), pattern="/World/env_*/body", device=device) # ----------------------------------------------------------------------------- @@ -102,8 +109,7 @@ def test_resolve_roundtrips_name_and_enum(): tt = resolve_tensor_type("articulation_dof_stiffness") assert tt is TensorType.ARTICULATION_DOF_STIFFNESS assert tensor_type_name(tt) == "articulation_dof_stiffness" - # case-insensitive - assert resolve_tensor_type("RIGID_BODY_POSE") is TensorType.RIGID_BODY_POSE + assert resolve_tensor_type("RIGID_BODY_POSE") is TensorType.RIGID_BODY_POSE # case-insensitive def test_resolve_unknown_name_raises(): @@ -111,35 +117,132 @@ def test_resolve_unknown_name_raises(): resolve_tensor_type("not_a_real_attribute") -def test_read_only_classification(): +def test_read_only_and_cpu_only_classification(): assert is_read_only("articulation_jacobian") assert is_read_only("rigid_body_acceleration") assert not is_read_only("articulation_dof_stiffness") - assert not is_read_only("rigid_body_pose") + assert is_cpu_only("articulation_dof_stiffness") + assert is_cpu_only("rigid_body_mass") + assert not is_cpu_only("rigid_body_pose") + + +def test_cpu_only_names_match_canonical_set(): + # The view derives its CPU-only set from tensor_types so the two cannot drift. + from isaaclab_ovphysx.sim.views import ovphysx_view as mod + from isaaclab_ovphysx.tensor_types import _CPU_ONLY_TYPES + + assert frozenset(tt.name.lower() for tt in _CPU_ONLY_TYPES) == mod._CPU_ONLY_NAMES # ----------------------------------------------------------------------------- -# View dispatch (fake physx) +# Construction # ----------------------------------------------------------------------------- -def test_get_attribute_reads_into_sized_buffer_and_reuses_it(): +def test_requires_exactly_one_of_pattern_or_prim_paths(): + with pytest.raises(ValueError): + OvPhysxView(_FakePhysX(), pattern="/p", prim_paths=["/p"], device="cpu") + with pytest.raises(ValueError): + OvPhysxView(_FakePhysX(), device="cpu") + + +def test_eager_creates_requested_and_exposes_metadata(): + view = OvPhysxView( + _FakePhysX(n=5), + pattern="/World/env_*/body", + device="cpu", + tensor_types=[TensorType.RIGID_BODY_POSE, TensorType.RIGID_BODY_MASS], + eager=True, + ) + assert view.available_attributes == ["rigid_body_mass", "rigid_body_pose"] + assert view.count == 5 # metadata works without an explicit get_attribute call + + +def test_eager_empty_view_raises(): + physx = _FakePhysX(n=3, unavailable={TensorType.RIGID_BODY_POSE}) + with pytest.raises(OvPhysxViewError, match="Could not create any bindings"): + OvPhysxView(physx, pattern="/no/match", device="cpu", tensor_types=[TensorType.RIGID_BODY_POSE], eager=True) + + +# ----------------------------------------------------------------------------- +# get_attribute / read_into +# ----------------------------------------------------------------------------- + + +def test_get_attribute_allocates_fresh_buffer_each_call(): view = _make_view(n=4) buf = view.get_attribute("rigid_body_pose") - assert tuple(buf.shape) == (4, 7) + assert tuple(buf.shape) == (4, 7) and buf.dtype == wp.float32 binding = view._bindings[TensorType.RIGID_BODY_POSE] assert binding.read_calls == 1 - # second read reuses the same cached buffer object buf2 = view.get_attribute("rigid_body_pose") - assert buf2 is buf + assert buf2 is not buf # no aliasing of view state assert binding.read_calls == 2 -def test_get_attribute_out_param_receives_copy(): +def test_get_attribute_out_param_is_filled_and_returned(): view = _make_view(n=2) out = wp.zeros((2, 7), dtype=wp.float32, device="cpu") ret = view.get_attribute("rigid_body_pose", out=out) assert ret is out + assert view._bindings[TensorType.RIGID_BODY_POSE].read_calls == 1 + + +def test_read_into_reinterprets_structured_buffer(): + view = _make_view(n=3) + dst = wp.zeros((3,), dtype=wp.transformf, device="cpu") # [N] transformf == [N,7] float32 + view.read_into("rigid_body_pose", dst) + binding = view._bindings[TensorType.RIGID_BODY_POSE] + # The binding was handed a float32 view matching its flat shape, not the transformf buffer. + assert binding.last_read == (wp.float32, (3, 7), "cpu") + + +def test_read_into_passthrough_when_already_float32(): + view = _make_view(n=3) + dst = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + view.read_into("rigid_body_pose", dst) + assert view._bindings[TensorType.RIGID_BODY_POSE].last_read == (wp.float32, (3, 7), "cpu") + + +def test_read_into_shape_mismatch_raises(): + view = _make_view(n=3) + wrong = wp.zeros((3, 6), dtype=wp.float32, device="cpu") + with pytest.raises(OvPhysxView.ShapeMismatch): + view.read_into("rigid_body_pose", wrong) + + +# ----------------------------------------------------------------------------- +# Device policy (no implicit CPU<->GPU conversion) +# ----------------------------------------------------------------------------- + + +def test_cpu_array_for_device_state_on_gpu_sim_raises(): + # GPU sim, but a CPU buffer is supplied for a device-resident state attribute. + view = _make_view(n=3, device="cuda:0") + cpu_buf = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + with pytest.raises(OvPhysxView.DeviceMismatch, match="cuda:0"): + view.read_into("rigid_body_pose", cpu_buf) + + +def test_cpu_only_property_accepts_cpu_buffer_on_gpu_sim(): + # GPU sim, CPU-only property (mass): a CPU buffer is correct and must NOT raise. + view = _make_view(n=3, device="cuda:0") + cpu_buf = wp.zeros((3,), dtype=wp.float32, device="cpu") + view.read_into("rigid_body_mass", cpu_buf) # no raise + assert view._bindings[TensorType.RIGID_BODY_MASS].read_calls == 1 + + +@pytest.mark.skipif(not _HAS_CUDA, reason="needs a CUDA device to allocate a GPU buffer") +def test_gpu_array_for_cpu_only_property_raises(): + view = _make_view(n=3, device="cuda:0") + gpu_buf = wp.zeros((3,), dtype=wp.float32, device="cuda:0") + with pytest.raises(OvPhysxView.DeviceMismatch, match="cpu"): + view.read_into("rigid_body_mass", gpu_buf) + + +# ----------------------------------------------------------------------------- +# set_attribute +# ----------------------------------------------------------------------------- def test_set_attribute_forwards_indices_and_mask(): @@ -147,14 +250,22 @@ def test_set_attribute_forwards_indices_and_mask(): values = wp.zeros((3, 7), dtype=wp.float32, device="cpu") idx = wp.array([0, 2], dtype=wp.int32, device="cpu") view.set_attribute("rigid_body_pose", values, indices=idx) - binding = view._bindings[TensorType.RIGID_BODY_POSE] - assert binding.write_calls == [(idx, None)] + dtype, shape, indices, mask = view._bindings[TensorType.RIGID_BODY_POSE].write_calls[0] + assert (dtype, shape, indices, mask) == (wp.float32, (3, 7), idx, None) + + +def test_set_attribute_reinterprets_structured_source(): + view = _make_view(n=3) + values = wp.zeros((3,), dtype=wp.transformf, device="cpu") + view.set_attribute("rigid_body_pose", values) + dtype, shape, _, _ = view._bindings[TensorType.RIGID_BODY_POSE].write_calls[0] + assert (dtype, shape) == (wp.float32, (3, 7)) -def test_set_attribute_read_only_raises_and_does_not_write(): +def test_set_attribute_read_only_raises_and_does_not_bind(): view = _make_view(n=3) values = wp.zeros((3, 6), dtype=wp.float32, device="cpu") - with pytest.raises(OvPhysxViewError, match="read-only"): + with pytest.raises(OvPhysxView.ReadOnlyAttribute, match="read-only"): view.set_attribute("rigid_body_acceleration", values) assert TensorType.RIGID_BODY_ACCELERATION not in view._bindings @@ -162,19 +273,62 @@ def test_set_attribute_read_only_raises_and_does_not_write(): def test_set_attribute_shape_mismatch_raises(): view = _make_view(n=3) wrong = wp.zeros((3, 6), dtype=wp.float32, device="cpu") - with pytest.raises(OvPhysxViewError, match="Shape mismatch"): + with pytest.raises(OvPhysxView.ShapeMismatch, match="Shape mismatch"): view.set_attribute("rigid_body_pose", wrong) +def test_set_attribute_cpu_array_for_state_on_gpu_sim_raises(): + view = _make_view(n=3, device="cuda:0") + values = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + with pytest.raises(OvPhysxView.DeviceMismatch): + view.set_attribute("rigid_body_pose", values) + + +def test_as_wp_accepts_numpy_float32_and_rejects_float64(): + view = _make_view(n=2) + # float32 host data is materialized on the native device and written. + view.set_attribute("rigid_body_pose", np.zeros((2, 7), dtype=np.float32)) + assert len(view._bindings[TensorType.RIGID_BODY_POSE].write_calls) == 1 + # float64 is not float32-bit-equivalent; reject rather than silently reinterpret. + with pytest.raises(OvPhysxView.ShapeMismatch): + view.set_attribute("rigid_body_pose", np.zeros((2, 7), dtype=np.float64)) + + +# ----------------------------------------------------------------------------- +# prim_paths + key aliases (RigidObjectCollection fused-binding shape) +# ----------------------------------------------------------------------------- + + +def test_prim_paths_with_key_alias_creates_remapped_type(): + physx = _FakePhysX(n=6) + view = OvPhysxView( + physx, + prim_paths=["/World/env_*/cube", "/World/env_*/sphere"], + device="cpu", + key_aliases={TensorType.ARTICULATION_LINK_POSE: TensorType.RIGID_BODY_POSE}, + ) + binding = view.binding_for("articulation_link_pose") + # Created as RIGID_BODY_POSE via prim_paths, cached under the requested LINK_POSE key. + created_type, pattern, prim_paths = physx.created[0] + assert created_type is TensorType.RIGID_BODY_POSE + assert pattern is None and prim_paths == ["/World/env_*/cube", "/World/env_*/sphere"] + assert view._bindings[TensorType.ARTICULATION_LINK_POSE] is binding + + +# ----------------------------------------------------------------------------- +# Errors / discoverability / metadata +# ----------------------------------------------------------------------------- + + def test_unknown_attribute_raises_on_access(): view = _make_view() - with pytest.raises(OvPhysxViewError): + with pytest.raises(OvPhysxView.UnknownAttribute): view.get_attribute("totally_made_up") def test_unavailable_binding_reports_clear_error(): view = _make_view(n=3, unavailable={TensorType.RIGID_BODY_VELOCITY}) - with pytest.raises(OvPhysxViewError, match="not available"): + with pytest.raises(OvPhysxView.AttributeUnavailable, match="not available"): view.get_attribute("rigid_body_velocity") @@ -184,7 +338,6 @@ def test_discoverability_surface(): assert view.has_attribute("articulation_dof_stiffness") assert not view.has_attribute("nope") assert "rigid_body_pose" in view.attribute_names - # available_attributes only lists instantiated bindings assert view.available_attributes == [] view.get_attribute("rigid_body_pose") assert view.available_attributes == ["rigid_body_pose"] @@ -192,9 +345,11 @@ def test_discoverability_surface(): def test_metadata_passthrough_from_sample_binding(): view = _make_view(n=5) - # metadata before any access raises a clear error - with pytest.raises(OvPhysxViewError): - _ = view.count + with pytest.raises(OvPhysxView.AttributeUnavailable): + _ = view.count # metadata before any access raises a clear error view.get_attribute("rigid_body_pose") assert view.count == 5 assert len(view.prim_paths) == 5 + assert view.body_names == ["body"] + assert view.is_fixed_base is True + assert view.joint_count == 0 and view.fixed_tendon_count == 0 From 42dcd5b1243ef75c04c9163ec165a240db400d27 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 13:48:07 +0200 Subject: [PATCH 03/12] Bump OvPhysxView changelog to major; refine entry The OvPhysxView addition is a significant new public surface for the OVPhysX backend, so promote the changelog fragment from a minor to a major bump. Reword the entry to describe the binding-manager surface (read_into, the no-device-conversion policy) and drop the internal design-note path from the user-facing changelog. --- .../changelog.d/ovphysx-view.major.rst | 11 +++++++++++ .../changelog.d/ovphysx-view.minor.rst | 9 --------- 2 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst delete mode 100644 source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst diff --git a/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst new file mode 100644 index 000000000000..d276cbc65b38 --- /dev/null +++ b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst @@ -0,0 +1,11 @@ +Added +^^^^^ + +* Added :class:`~isaaclab_ovphysx.sim.views.OvPhysxView`, a string-keyed binding manager + over the OVPhysX tensor bindings. Attributes are addressed by the lowercased + ``TensorType`` name (e.g. ``view.get_attribute("articulation_dof_stiffness")``, + ``view.read_into("articulation_root_pose", buf)``, + ``view.set_attribute("rigid_body_pose", values, mask=...)``), bringing the OVPhysX + binding surface closer to the Newton selection API. The view reads/writes each binding + on its native device and raises on a device mismatch rather than staging between CPU + and GPU. diff --git a/source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst deleted file mode 100644 index 4543e9329b5e..000000000000 --- a/source/isaaclab_ovphysx/changelog.d/ovphysx-view.minor.rst +++ /dev/null @@ -1,9 +0,0 @@ -Added -^^^^^ - -* Added :class:`~isaaclab_ovphysx.sim.views.OvPhysxView`, a string-keyed view over the - OVPhysX tensor bindings. Attributes are addressed by the lowercased ``TensorType`` - name (e.g. ``view.get_attribute("articulation_dof_stiffness")`` / - ``view.set_attribute("rigid_body_pose", values, mask=...)``), bringing the OVPhysX - binding surface closer to the Newton selection API. Prototype tracked by the design - note at ``docs/superpowers/specs/2026-06-17-ovphysx-view-design.md``. From 7d3d3056bcc73ed0e8eb1175ab9d71a2c1efee19 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 14:00:56 +0200 Subject: [PATCH 04/12] Address review: reject non-float32 buffers; harden errors From the second PR review of #6224: - Critical: _as_binding_view now requires a float32 scalar dtype before the zero-copy reinterpret. A same-byte-width wrong dtype (int32) previously passed the count-only guard and was bit-reinterpreted into garbage on the write path; sub-4-byte dtypes (float16) produced a misleading "0 elements" error. Both are now rejected with a clear message. Regression tests added (verified failing without the guard). - _resolve enforces the str | TensorType union and raises UnknownAttribute on anything else, instead of letting a bogus key reach the wheel. - _binding accesses binding.count directly (a malformed binding surfaces instead of being masked as a phantom no-match) and surfaces the underlying create_tensor_binding exception in the AttributeUnavailable message. - Added docstrings to the six metadata properties; dropped the unused runtime_checkable decorator. - Tests: same-byte/sub-4-byte dtype rejection, get_attribute(out=) wrong device, both indices+mask forwarded, read/write through a prim_paths+key_aliases view, non-str/non-TensorType key, and a read-only-names-are-valid-vocabulary check. --- .../sim/views/ovphysx_view.py | 57 ++++++++++++++---- .../test/sim/test_ovphysx_view.py | 60 ++++++++++++++++++- 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index 6eb52e031225..c95a08fd117b 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -39,7 +39,7 @@ import logging import math -from typing import Any, Protocol, runtime_checkable +from typing import Any, Protocol import warp as wp @@ -79,9 +79,13 @@ _CPU_ONLY_NAMES: frozenset[str] = frozenset(tt.name.lower() for tt in _CPU_ONLY_TYPES) -@runtime_checkable class _BindingLike(Protocol): - """Structural type of an ovphysx ``TensorBinding`` as used by this view.""" + """Structural type of an ovphysx ``TensorBinding`` as used by this view. + + ``read`` fills the passed array in place; ``write`` consumes ``indices``/``mask`` + for partial writes. ``shape`` is the binding's flat tensor shape; ``count`` is the + number of matched prims. + """ shape: tuple[int, ...] count: int @@ -379,26 +383,32 @@ def joint_names(self) -> list[str]: @property def dof_count(self) -> int: + """Number of DOFs per articulation (articulation views only).""" return self._sample().dof_count @property def body_count(self) -> int: + """Number of bodies (links) per articulation (articulation views only).""" return self._sample().body_count @property def joint_count(self) -> int: + """Number of joints per articulation (articulation views only).""" return self._sample().joint_count @property def is_fixed_base(self) -> bool: + """Whether the articulation has a fixed base (articulation views only).""" return self._sample().is_fixed_base @property def fixed_tendon_count(self) -> int: + """Number of fixed tendons per articulation (articulation views only).""" return self._sample().fixed_tendon_count @property def spatial_tendon_count(self) -> int: + """Number of spatial tendons per articulation (articulation views only).""" return self._sample().spatial_tendon_count # -- internals ------------------------------------------------------------- @@ -407,7 +417,11 @@ def _resolve(self, name: str | Any) -> Any: """Resolve a string name or a ``TensorType`` member to a ``TensorType``.""" if isinstance(name, str): return resolve_tensor_type(name) - return name + if isinstance(name, TensorType): + return name + raise OvPhysxView.UnknownAttribute( + f"Attribute key must be a str name or a TensorType member, got {type(name).__name__}." + ) def _binding(self, tensor_type: Any) -> Any: """Return the cached ``TensorBinding`` for ``tensor_type``, creating it on first use.""" @@ -422,12 +436,18 @@ def _binding(self, tensor_type: Any) -> Any: kwargs["pattern"] = self._pattern try: binding = self._physx.create_tensor_binding(**kwargs) - except Exception as exc: # noqa: BLE001 -- wheel raises bare exceptions; re-wrapped below + except Exception as exc: # noqa: BLE001 -- wheel raises bare exceptions; surface the cause below + # The wheel raises both for "type not applicable to these prims" and for genuine + # failures (init/ABI/OOM); we can't tell them apart without a wheel-side exception + # type, so the underlying error is surfaced in the message (and chained) rather than + # hidden behind a generic "not available". TODO(ovphysx): a typed no-match error. raise OvPhysxView.AttributeUnavailable( - f"Attribute {tensor_type_name(tensor_type)!r} is not available for {self._target_repr()}." + f"Could not create the {tensor_type_name(tensor_type)!r} binding for " + f"{self._target_repr()}: create_tensor_binding raised {type(exc).__name__}: {exc}" ) from exc - # The wheel returns a 0-count binding when nothing matches. - if binding is None or getattr(binding, "count", 0) == 0: + # The wheel returns a 0-count binding when nothing matches. Access ``count`` directly so a + # malformed binding (missing ``count``) surfaces as an error rather than a phantom no-match. + if binding is None or binding.count == 0: raise OvPhysxView.AttributeUnavailable( f"Attribute {tensor_type_name(tensor_type)!r} is not available for {self._target_repr()} " "(no matching prims)." @@ -459,15 +479,26 @@ def _check_device(self, arr: wp.array, device: str, attr: str, role: str) -> Non def _as_binding_view(self, arr: wp.array, binding: Any, role: str) -> wp.array: """Return a ``float32`` view of ``arr`` matching the binding's flat shape. - Validates that ``arr``'s element count matches the binding, then returns ``arr`` - directly if it is already ``float32`` with the binding's shape, or a zero-copy - reinterpret view otherwise (for structured dtypes such as ``wp.transformf``). + ``arr`` must have a ``float32`` scalar element type (``float32`` itself or a + composite built on it, e.g. ``wp.transformf``/``wp.vec3f``): the view + **reinterprets bits, not values**, so a non-``float32`` dtype (``int32``, + ``float64``, ``float16``) would corrupt the data and is rejected. Given a matching + scalar, validates the flat ``float32`` element count and returns ``arr`` directly + when it is already ``float32`` with the binding's shape, else a zero-copy + reinterpret view. """ + scalar = getattr(arr.dtype, "_wp_scalar_type_", arr.dtype) + if scalar is not wp.float32: + raise OvPhysxView.ShapeMismatch( + f"{role} must have float32 scalar elements (got dtype " + f"{getattr(arr.dtype, '__name__', arr.dtype)}); the view reinterprets bits, " + "not values, so a non-float32 dtype would silently corrupt the buffer." + ) expected = math.prod(tuple(binding.shape)) - actual = arr.size * (wp.types.type_size_in_bytes(arr.dtype) // 4) + actual = arr.size * (wp.types.type_size_in_bytes(arr.dtype) // 4) # scalar is float32 -> exact if actual != expected: raise OvPhysxView.ShapeMismatch( - f"Shape mismatch for {role}: {actual} float32-equivalent elements, " + f"Shape mismatch for {role}: {actual} float32 elements, " f"binding expects {expected} (shape {tuple(binding.shape)})." ) if arr.dtype == wp.float32 and tuple(arr.shape) == tuple(binding.shape): diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 00912808ab49..7779911b4dde 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -113,10 +113,18 @@ def test_resolve_roundtrips_name_and_enum(): def test_resolve_unknown_name_raises(): - with pytest.raises(OvPhysxViewError): + with pytest.raises(OvPhysxView.UnknownAttribute): resolve_tensor_type("not_a_real_attribute") +def test_read_only_names_are_valid_vocabulary(): + # Every read-only name must resolve to a real TensorType, so the hand-maintained + # set stays coupled to the wheel enum (no dead names). + from isaaclab_ovphysx.sim.views import ovphysx_view as mod + + assert set(attribute_vocabulary()) >= mod._READ_ONLY_NAMES + + def test_read_only_and_cpu_only_classification(): assert is_read_only("articulation_jacobian") assert is_read_only("rigid_body_acceleration") @@ -313,6 +321,9 @@ def test_prim_paths_with_key_alias_creates_remapped_type(): assert created_type is TensorType.RIGID_BODY_POSE assert pattern is None and prim_paths == ["/World/env_*/cube", "/World/env_*/sphere"] assert view._bindings[TensorType.ARTICULATION_LINK_POSE] is binding + # And a write through the aliased key resolves to that same cached binding. + view.set_attribute("articulation_link_pose", wp.zeros((6, 7), dtype=wp.float32, device="cpu")) + assert len(binding.write_calls) == 1 and binding is view._bindings[TensorType.ARTICULATION_LINK_POSE] # ----------------------------------------------------------------------------- @@ -353,3 +364,50 @@ def test_metadata_passthrough_from_sample_binding(): assert view.body_names == ["body"] assert view.is_fixed_base is True assert view.joint_count == 0 and view.fixed_tendon_count == 0 + + +# ----------------------------------------------------------------------------- +# dtype safety — the view reinterprets bits, so non-float32 scalars must be rejected +# ----------------------------------------------------------------------------- + + +def test_set_attribute_rejects_same_byte_size_wrong_dtype(): + # int32 has the same 4-byte width as float32: it would pass a byte-count-only guard + # and get bit-reinterpreted into garbage. It must be rejected, not silently written. + view = _make_view(n=3) + int_buf = wp.zeros((3, 7), dtype=wp.int32, device="cpu") + with pytest.raises(OvPhysxView.ShapeMismatch, match="float32 scalar"): + view.set_attribute("rigid_body_pose", int_buf) + assert view._bindings[TensorType.RIGID_BODY_POSE].write_calls == [] + + +def test_set_attribute_rejects_sub_4byte_dtype(): + view = _make_view(n=3) + half_buf = wp.zeros((3, 7), dtype=wp.float16, device="cpu") + with pytest.raises(OvPhysxView.ShapeMismatch, match="float32 scalar"): + view.set_attribute("rigid_body_pose", half_buf) + + +def test_set_attribute_forwards_both_indices_and_mask(): + # The view forwards both verbatim; the wheel resolves precedence (mask wins). + view = _make_view(n=3) + values = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + idx = wp.array([0, 2], dtype=wp.int32, device="cpu") + mask = wp.array([True, False, True], dtype=wp.bool, device="cpu") + view.set_attribute("rigid_body_pose", values, indices=idx, mask=mask) + _, _, fwd_idx, fwd_mask = view._bindings[TensorType.RIGID_BODY_POSE].write_calls[0] + assert fwd_idx is idx and fwd_mask is mask + + +def test_get_attribute_out_on_wrong_device_raises(): + # `get_attribute(out=)` has its own device check distinct from read_into's. + view = _make_view(n=3, device="cuda:0") + cpu_out = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + with pytest.raises(OvPhysxView.DeviceMismatch): + view.get_attribute("rigid_body_pose", out=cpu_out) + + +def test_resolve_rejects_non_str_non_tensortype(): + view = _make_view() + with pytest.raises(OvPhysxView.UnknownAttribute): + view.get_attribute(123) From e74cc82f234cf9b30290527ba85daca1727ca06c Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 14:14:25 +0200 Subject: [PATCH 05/12] Harden OvPhysxView API against adversarial inputs From the API-hardening review of #6224. Validate at the boundary and fail loud instead of silently corrupting, mis-binding, or no-op'ing: - Reject non-contiguous buffers in _as_binding_view (a strided/sliced source would be reinterpreted as contiguous and read/write the wrong memory). - Canonicalize the device (wp.get_device) so a "cuda" view accepts a "cuda:0" buffer instead of raising a spurious, unsatisfiable DeviceMismatch; falls back to the raw string when the device can't be resolved locally. - Reject TensorType.INVALID via the member path too (string path already did). - Normalize key_aliases to TensorType members so string keys are honored rather than silently dropped, and reject aliases that cross the CPU/GPU residency or read-only boundary (the device/read-only guards key on the requested type). - Reject empty pattern/prim_paths and tensor_types-without-eager at construction. - Eager construction with an explicit tensor_types list now surfaces a failing type instead of swallowing it at debug level (default sweep still skips inapplicable types). - Document binding_for as an unguarded escape hatch, get_attribute's native-device return, and the has_attribute name-validity-vs-availability split. Adds regression tests for each (contiguity and INVALID verified failing without the guard). --- .../sim/views/ovphysx_view.py | 78 +++++++++++-- .../test/sim/test_ovphysx_view.py | 109 +++++++++++++++++- 2 files changed, 171 insertions(+), 16 deletions(-) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index c95a08fd117b..43c29c94f48a 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -217,21 +217,52 @@ def __init__( ) -> None: if (pattern is None) == (prim_paths is None): raise ValueError("Provide exactly one of 'pattern' or 'prim_paths'.") + if pattern is not None and not pattern: + raise ValueError("'pattern' must be a non-empty glob string.") + if prim_paths is not None and not prim_paths: + raise ValueError("'prim_paths' must contain at least one glob.") + if tensor_types is not None and not eager: + raise ValueError("'tensor_types' is only honored with eager=True; pass eager=True or omit it.") self._physx = physx self._pattern = pattern self._prim_paths = prim_paths - self._device = device - self._key_aliases: dict[Any, Any] = dict(key_aliases or {}) + # Canonicalize the device so a "cuda" alias compares equal to a buffer's "cuda:0" + # (warp canonicalizes buffer devices). Fall back to the raw string when the device + # cannot be resolved here (e.g. constructing a cuda view on a CPU-only CI box) -- the + # string is only used for comparison, so construction must not fail on it. + try: + self._device = str(wp.get_device(device)) + except Exception: # noqa: BLE001 -- unresolvable device: keep the raw string for comparison + self._device = device + # Normalize key_aliases to TensorType members (accepts str names too) so string keys are + # honored rather than silently dropped, and reject aliases that cross the CPU/GPU residency + # or read-only boundary -- the device and read-only guards key on the requested type. + self._key_aliases: dict[Any, Any] = {} + for requested_type, created_type in (key_aliases or {}).items(): + req_tt, made_tt = self._resolve(requested_type), self._resolve(created_type) + if (req_tt in _CPU_ONLY_TYPES) != (made_tt in _CPU_ONLY_TYPES): + raise ValueError( + f"key_alias {tensor_type_name(req_tt)!r} -> {tensor_type_name(made_tt)!r} crosses the " + "CPU/GPU residency boundary; the device policy would apply to the wrong type." + ) + if is_read_only(tensor_type_name(req_tt)) != is_read_only(tensor_type_name(made_tt)): + raise ValueError( + f"key_alias {tensor_type_name(req_tt)!r} -> {tensor_type_name(made_tt)!r} mixes a read-only " + "and a writable type." + ) + self._key_aliases[req_tt] = made_tt self._bindings: dict[Any, Any] = {} if eager: - requested = tensor_types if tensor_types is not None else [t for t in TensorType if t.name != "INVALID"] + explicit = tensor_types is not None + requested = tensor_types if explicit else [t for t in TensorType if t.name != "INVALID"] for tt in requested: try: - self._binding(tt) + self._binding(self._resolve(tt)) except OvPhysxView.AttributeUnavailable: - # Not applicable to these prims; skip and keep going. - logger.debug("eager binding skipped for %s", tt) + if explicit: + raise # caller named this exact type; surface the failure rather than drop it + logger.debug("eager binding skipped for %s", tt) # default sweep: skip inapplicable types if not self._bindings: raise OvPhysxView.AttributeUnavailable( f"Could not create any bindings for {self._target_repr()}; " @@ -253,8 +284,10 @@ def get_attribute(self, name: str | Any, *, out: wp.array | None = None) -> wp.a ``float32`` :class:`warp.array` on the native device is returned. Returns: - A :class:`warp.array` holding the attribute values. When ``out`` is omitted - this is a fresh, caller-owned array (no aliasing of view state). + A :class:`warp.array` holding the attribute values, on the attribute's native + device -- ``cpu`` for CPU-only property types even on a GPU sim (see + :func:`is_cpu_only`). When ``out`` is omitted this is a fresh, caller-owned array + (no aliasing of view state). """ tt = self._resolve(name) binding = self._binding(tt) @@ -328,14 +361,25 @@ def set_attribute( # -- raw binding access (for asset/data-container adoption) ---------------- def binding_for(self, name: str | Any) -> _BindingLike: - """Return the underlying ``TensorBinding`` for an attribute, creating it on first use.""" + """Return the underlying ``TensorBinding`` for an attribute, creating it on first use. + + This is a raw escape hatch for asset-internal binding management: the returned + binding's ``read``/``write`` **bypass** the view's device, dtype-reinterpret, shape, + and read-only guards. Prefer :meth:`get_attribute` / :meth:`read_into` / + :meth:`set_attribute` unless you are deliberately managing bindings directly. + """ return self._binding(self._resolve(name)) # -- discoverability ------------------------------------------------------- @property def attribute_names(self) -> list[str]: - """All valid attribute names addressable by this view (the full vocabulary).""" + """Every valid attribute name (the full ``TensorType`` vocabulary). + + This is name *validity*, not availability for this view's prims -- a rigid-body view + still lists ``"articulation_*"`` names. Use :attr:`available_attributes` for what is + actually instantiated. + """ return attribute_vocabulary() @property @@ -344,7 +388,12 @@ def available_attributes(self) -> list[str]: return sorted(tensor_type_name(tt) for tt in self._bindings) def has_attribute(self, name: str | Any) -> bool: - """Return whether ``name`` resolves to a valid ``TensorType``.""" + """Return whether ``name`` is a valid attribute name (resolves to a ``TensorType``). + + This checks name *validity* for any view, not availability for these prims: it can + return ``True`` for a name whose binding does not apply to this view's prims (in which + case :meth:`get_attribute` raises :class:`AttributeUnavailable`). + """ try: self._resolve(name) except OvPhysxView.UnknownAttribute: @@ -418,6 +467,8 @@ def _resolve(self, name: str | Any) -> Any: if isinstance(name, str): return resolve_tensor_type(name) if isinstance(name, TensorType): + if name.name == "INVALID": # mirror the string path's INVALID rejection + raise OvPhysxView.UnknownAttribute(f"{name!r} is not an addressable attribute.") return name raise OvPhysxView.UnknownAttribute( f"Attribute key must be a str name or a TensorType member, got {type(name).__name__}." @@ -494,6 +545,11 @@ def _as_binding_view(self, arr: wp.array, binding: Any, role: str) -> wp.array: f"{getattr(arr.dtype, '__name__', arr.dtype)}); the view reinterprets bits, " "not values, so a non-float32 dtype would silently corrupt the buffer." ) + if not arr.is_contiguous: + raise OvPhysxView.ShapeMismatch( + f"{role} must be a contiguous array; the view reinterprets the buffer's raw memory, " + "so a strided/sliced view would read or write the wrong elements." + ) expected = math.prod(tuple(binding.shape)) actual = arr.size * (wp.types.type_size_in_bytes(arr.dtype) // 4) # scalar is float32 -> exact if actual != expected: diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 7779911b4dde..72f5db450594 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -76,14 +76,15 @@ def write(self, tensor, indices=None, mask=None) -> None: class _FakePhysX: """Fake ``PhysX`` whose ``create_tensor_binding`` hands back ``_FakeBinding`` instances.""" - def __init__(self, n: int = 3, unavailable: set | None = None): + def __init__(self, n: int = 3, unavailable: set | None = None, all_unavailable: bool = False): self.n = n self._unavailable = unavailable or set() + self._all_unavailable = all_unavailable self.created: list[tuple] = [] def create_tensor_binding(self, *, tensor_type, pattern=None, prim_paths=None): self.created.append((tensor_type, pattern, prim_paths)) - if tensor_type in self._unavailable: + if self._all_unavailable or tensor_type in self._unavailable: return _FakeBinding(tensor_type, 0) # wheel returns a 0-count binding on no match return _FakeBinding(tensor_type, self.n) @@ -166,10 +167,11 @@ def test_eager_creates_requested_and_exposes_metadata(): assert view.count == 5 # metadata works without an explicit get_attribute call -def test_eager_empty_view_raises(): - physx = _FakePhysX(n=3, unavailable={TensorType.RIGID_BODY_POSE}) +def test_eager_default_sweep_empty_view_raises(): + # Default sweep (no tensor_types) on a pattern that matches nothing -> aggregate raise. + physx = _FakePhysX(all_unavailable=True) with pytest.raises(OvPhysxViewError, match="Could not create any bindings"): - OvPhysxView(physx, pattern="/no/match", device="cpu", tensor_types=[TensorType.RIGID_BODY_POSE], eager=True) + OvPhysxView(physx, pattern="/no/match", device="cpu", eager=True) # ----------------------------------------------------------------------------- @@ -411,3 +413,100 @@ def test_resolve_rejects_non_str_non_tensortype(): view = _make_view() with pytest.raises(OvPhysxView.UnknownAttribute): view.get_attribute(123) + + +def test_set_attribute_rejects_non_contiguous_source(): + # A strided slice has the right element count but non-contiguous memory; the ptr-based + # reinterpret would read the wrong elements, so it must be rejected. + view = _make_view(n=3) + base = wp.zeros((3, 14), dtype=wp.float32, device="cpu") + strided = base[:, :7] + assert not strided.is_contiguous + with pytest.raises(OvPhysxView.ShapeMismatch, match="contiguous"): + view.set_attribute("rigid_body_pose", strided) + + +# ----------------------------------------------------------------------------- +# API hardening — adversarial construction / resolution +# ----------------------------------------------------------------------------- + + +def test_invalid_tensortype_member_rejected(): + view = _make_view() + with pytest.raises(OvPhysxView.UnknownAttribute): + view.get_attribute(TensorType.INVALID) + + +def test_string_keyed_aliases_are_honored(): + # Passing string alias keys/values must be normalized to TensorType, not silently dropped. + physx = _FakePhysX(n=6) + view = OvPhysxView( + physx, + prim_paths=["/World/env_*/cube"], + device="cpu", + key_aliases={"articulation_link_pose": "rigid_body_pose"}, + ) + view.binding_for("articulation_link_pose") + assert physx.created[0][0] is TensorType.RIGID_BODY_POSE # alias applied + + +def test_key_alias_crossing_residency_is_rejected(): + # LINK_POSE is GPU state; RIGID_BODY_MASS is CPU-only -> the device guard would be wrong. + with pytest.raises(ValueError, match="residency"): + OvPhysxView( + _FakePhysX(), + pattern="/p", + device="cpu", + key_aliases={TensorType.ARTICULATION_LINK_POSE: TensorType.RIGID_BODY_MASS}, + ) + + +def test_tensor_types_without_eager_raises(): + with pytest.raises(ValueError, match="eager"): + OvPhysxView(_FakePhysX(), pattern="/p", device="cpu", tensor_types=[TensorType.RIGID_BODY_POSE]) + + +def test_empty_target_is_rejected(): + with pytest.raises(ValueError): + OvPhysxView(_FakePhysX(), prim_paths=[], device="cpu") + with pytest.raises(ValueError): + OvPhysxView(_FakePhysX(), pattern="", device="cpu") + + +def test_eager_explicit_unavailable_type_raises_loud(): + # When the caller names exact types, a failing one is surfaced (not silently dropped). + physx = _FakePhysX(n=3, unavailable={TensorType.RIGID_BODY_VELOCITY}) + with pytest.raises(OvPhysxView.AttributeUnavailable): + OvPhysxView( + physx, + pattern="/World/env_*/body", + device="cpu", + tensor_types=[TensorType.RIGID_BODY_POSE, TensorType.RIGID_BODY_VELOCITY], + eager=True, + ) + + +def test_get_attribute_cpu_only_property_returns_cpu_buffer_on_gpu_sim(): + # No-out allocation path must use the native device: CPU for a CPU-only property even + # though the sim device is a GPU. (CPU allocation -> runs without a GPU.) + view = _make_view(n=3, device="cuda:0") + buf = view.get_attribute("rigid_body_mass") + assert str(buf.device) == "cpu" + + +def test_binding_for_is_idempotent_and_unguarded(): + view = _make_view(n=3) + # Returns a binding even for a read-only attribute (raw access bypasses the write guard)... + b1 = view.binding_for("rigid_body_acceleration") + b2 = view.binding_for("rigid_body_acceleration") + assert b1 is b2 # cached / created once + assert len(view._physx.created) == 1 + + +@pytest.mark.skipif(not _HAS_CUDA, reason="needs a CUDA device for the cuda:0 buffer") +def test_device_cuda_alias_is_canonicalized(): + # A view built with the bare "cuda" alias must accept a canonical "cuda:0" buffer. + view = _make_view(n=3, device="cuda") + gpu_buf = wp.zeros((3, 7), dtype=wp.float32, device="cuda:0") + view.read_into("rigid_body_pose", gpu_buf) # must not raise + assert view._bindings[TensorType.RIGID_BODY_POSE].read_calls == 1 From ff06224b8de47cb81dd1a9d178f72aa913225fb1 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 14:36:11 +0200 Subject: [PATCH 06/12] Add OvPhysxView.try_binding_for (non-raising availability accessor) Surfaced by dogfooding the view in the articulation migration: the assets branch on a "binding or None" pattern for optional/absent bindings (tendon types on a tendon-less articulation, not-yet-created bindings), which the raising binding_for can't express. try_binding_for returns None when the attribute is valid but not available for the view's prims, while still raising UnknownAttribute for an invalid name (a programming error, not an availability question). --- .../isaaclab_ovphysx/sim/views/ovphysx_view.py | 14 ++++++++++++++ .../isaaclab_ovphysx/test/sim/test_ovphysx_view.py | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index 43c29c94f48a..eaaeb57719b5 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -370,6 +370,20 @@ def binding_for(self, name: str | Any) -> _BindingLike: """ return self._binding(self._resolve(name)) + def try_binding_for(self, name: str | Any) -> _BindingLike | None: + """Like :meth:`binding_for`, but return ``None`` instead of raising when the attribute + is valid yet **not available for this view's prims** (e.g. tendon types on a + tendon-less articulation, or a not-yet-created optional binding). + + An invalid *name* still raises :class:`UnknownAttribute` -- that is a programming + error, not an availability question. Use this for the asset's ``binding or None`` + pattern over optional bindings. + """ + try: + return self._binding(self._resolve(name)) + except OvPhysxView.AttributeUnavailable: + return None + # -- discoverability ------------------------------------------------------- @property diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 72f5db450594..4f98f03a87f0 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -503,6 +503,16 @@ def test_binding_for_is_idempotent_and_unguarded(): assert len(view._physx.created) == 1 +def test_try_binding_for_returns_none_when_unavailable(): + view = _make_view(n=3, unavailable={TensorType.RIGID_BODY_VELOCITY}) + # Available for these prims -> the (cached) binding; unavailable -> None, no raise. + assert view.try_binding_for("rigid_body_pose") is view._bindings[TensorType.RIGID_BODY_POSE] + assert view.try_binding_for("rigid_body_velocity") is None + # An invalid name is still a hard error, not an availability question. + with pytest.raises(OvPhysxView.UnknownAttribute): + view.try_binding_for("not_a_real_attribute") + + @pytest.mark.skipif(not _HAS_CUDA, reason="needs a CUDA device for the cuda:0 buffer") def test_device_cuda_alias_is_canonicalized(): # A view built with the bare "cuda" alias must accept a canonical "cuda:0" buffer. From 5bc49c08640706b15d092499b2293d08b3bb3843 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 16:16:18 +0200 Subject: [PATCH 07/12] feat(ovphysx): cache read reinterprets + type get_attribute in OvPhysxView read_into now reuses the float32 reinterpret of a destination buffer across calls (keyed by buffer id, with a pointer-staleness guard), so the wheel's object-identity read cache stays warm even when callers hand a structured buffer each step -- they no longer need to maintain their own reinterpret cache. get_attribute returns a typed array for attributes with a known structured layout (transformf for poses, spatial_vectorf for velocities, via a hand-maintained _ATTR_DTYPE map) and flat float32 otherwise. This lets the asset data containers drop their bespoke _get_read_view caching and read structured buffers straight through the view. --- .../changelog.d/ovphysx-view.major.rst | 6 +- .../sim/views/ovphysx_view.py | 79 +++++++++++++++++-- .../test/sim/test_ovphysx_view.py | 52 +++++++++++- 3 files changed, 126 insertions(+), 11 deletions(-) diff --git a/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst index d276cbc65b38..a6c0e1be1262 100644 --- a/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst +++ b/source/isaaclab_ovphysx/changelog.d/ovphysx-view.major.rst @@ -8,4 +8,8 @@ Added ``view.set_attribute("rigid_body_pose", values, mask=...)``), bringing the OVPhysX binding surface closer to the Newton selection API. The view reads/writes each binding on its native device and raises on a device mismatch rather than staging between CPU - and GPU. + and GPU. :meth:`~isaaclab_ovphysx.sim.views.OvPhysxView.get_attribute` returns a typed + array for attributes with a structured layout (e.g. ``wp.transformf`` for poses, + ``wp.spatial_vectorf`` for velocities) and flat ``float32`` otherwise, and + :meth:`~isaaclab_ovphysx.sim.views.OvPhysxView.read_into` reuses the ``float32`` + reinterpret of a destination buffer across calls so the wheel's read cache stays warm. diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index eaaeb57719b5..8327dd1b4e05 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -78,6 +78,27 @@ # so the two never drift. _CPU_ONLY_NAMES: frozenset[str] = frozenset(tt.name.lower() for tt in _CPU_ONLY_TYPES) +# Structured Warp dtype for attributes whose flat trailing dimension has a fixed semantic +# layout: 7-float poses -> ``wp.transformf``, 6-float spatial vectors -> ``wp.spatial_vectorf``. +# :meth:`OvPhysxView.get_attribute` returns an array of this dtype, so callers get a typed +# ``[N, ...]`` array rather than a flat ``[N, ..., k]`` float32 one. Attributes absent from this +# map default to flat ``float32``. The wheel exposes only flat float32 shapes, so this map is +# hand-maintained. +# TODO(ovphysx): source structured layouts from a wheel dtype query if one is added. +_ATTR_DTYPE: dict[str, Any] = { + "articulation_root_pose": wp.transformf, + "articulation_link_pose": wp.transformf, + "articulation_body_com_pose": wp.transformf, + "rigid_body_pose": wp.transformf, + "rigid_body_com_pose": wp.transformf, + "articulation_root_velocity": wp.spatial_vectorf, + "articulation_link_velocity": wp.spatial_vectorf, + "articulation_link_acceleration": wp.spatial_vectorf, + "articulation_link_incoming_joint_force": wp.spatial_vectorf, + "rigid_body_velocity": wp.spatial_vectorf, + "rigid_body_acceleration": wp.spatial_vectorf, +} + class _BindingLike(Protocol): """Structural type of an ovphysx ``TensorBinding`` as used by this view. @@ -252,6 +273,10 @@ def __init__( ) self._key_aliases[req_tt] = made_tt self._bindings: dict[Any, Any] = {} + # Cache of float32 reinterpret views for read_into / get_attribute, keyed by the + # destination buffer's id(). Reusing the same reinterpret object across calls keeps the + # wheel's object-identity read cache (the TensorBinding.read fast path) warm. + self._read_views: dict[int, wp.array] = {} if eager: explicit = tensor_types is not None @@ -281,23 +306,26 @@ def get_attribute(self, name: str | Any, *, out: wp.array | None = None) -> wp.a name: Lowercased ``TensorType`` name or the member itself. out: Optional destination buffer to fill (must be on the binding's native device and match its element count). If omitted, a freshly allocated - ``float32`` :class:`warp.array` on the native device is returned. + :class:`warp.array` on the native device is returned. Returns: A :class:`warp.array` holding the attribute values, on the attribute's native device -- ``cpu`` for CPU-only property types even on a GPU sim (see - :func:`is_cpu_only`). When ``out`` is omitted this is a fresh, caller-owned array - (no aliasing of view state). + :func:`is_cpu_only`). When ``out`` is omitted this is a fresh, caller-owned array; + its dtype is the attribute's structured Warp dtype when it has one (e.g. + ``wp.transformf`` for poses, ``wp.spatial_vectorf`` for velocities) and flat + ``float32`` otherwise (see :data:`_ATTR_DTYPE`). """ tt = self._resolve(name) binding = self._binding(tt) device = self._native_device(tt) if out is not None: self._check_device(out, device, tensor_type_name(tt), "destination") - binding.read(self._as_binding_view(out, binding, "destination")) + binding.read(self._read_view(out, binding)) return out - buf = wp.zeros(tuple(binding.shape), dtype=wp.float32, device=device) - binding.read(buf) + alloc_shape, dtype = self._attribute_dtype(tt, binding) + buf = wp.zeros(alloc_shape, dtype=dtype, device=device) + binding.read(self._read_view(buf, binding)) return buf def read_into(self, name: str | Any, dst: wp.array) -> None: @@ -306,7 +334,10 @@ def read_into(self, name: str | Any, dst: wp.array) -> None: ``dst`` may be a structured-dtype buffer (e.g. ``wp.transformf``); it is read through a ``float32`` reinterpret view that matches the binding's flat shape, so the structured GPU/CPU buffer is filled directly with no extra copy. This is the - path the asset data containers use. + path the asset data containers use. The reinterpret view for a given ``dst`` is + built once and reused across calls (see :meth:`_read_view`) so the wheel's + object-identity read cache stays warm -- callers can pass the structured buffer + directly each step without maintaining their own reinterpret cache. Args: name: Lowercased ``TensorType`` name or the member itself. @@ -320,7 +351,7 @@ def read_into(self, name: str | Any, dst: wp.array) -> None: tt = self._resolve(name) binding = self._binding(tt) self._check_device(dst, self._native_device(tt), tensor_type_name(tt), "destination") - binding.read(self._as_binding_view(dst, binding, "destination")) + binding.read(self._read_view(dst, binding)) def set_attribute( self, @@ -575,6 +606,38 @@ def _as_binding_view(self, arr: wp.array, binding: Any, role: str) -> wp.array: return arr return wp.array(ptr=arr.ptr, shape=tuple(binding.shape), dtype=wp.float32, device=str(arr.device), copy=False) + def _read_view(self, dst: wp.array, binding: Any) -> wp.array: + """Return the ``float32`` view of ``dst`` to hand to ``binding.read``, reused across calls. + + The wheel's ``TensorBinding.read`` has an object-identity read cache: it skips DLPack + acquisition and the attribute-chain lookup when handed the *same* tensor object as the + previous read. To keep that cache warm, the ``float32`` reinterpret of a structured + ``dst`` is built once and reused for that destination buffer; a pointer-staleness guard + rebuilds it if the buffer's backing storage moved. A ``dst`` that is already flat + ``float32`` is its own stable identity, so it is returned directly (and not cached). + """ + cached = self._read_views.get(id(dst)) + if cached is not None and cached.ptr == dst.ptr: + return cached + view = self._as_binding_view(dst, binding, "destination") + if view is not dst: # structured dst -> cache the reinterpret; a flat float32 dst caches nothing + self._read_views[id(dst)] = view + return view + + def _attribute_dtype(self, tensor_type: Any, binding: Any) -> tuple[tuple[int, ...], Any]: + """Return ``(alloc_shape, dtype)`` for :meth:`get_attribute`. + + Maps an attribute to its structured Warp dtype (see :data:`_ATTR_DTYPE`) when the + binding's trailing dimension matches that dtype's ``float32`` count, dropping the + trailing dimension from the allocation shape (e.g. ``[N, 7] -> ([N], wp.transformf)``). + Falls back to the flat ``float32`` shape for unmapped attributes or a mismatched layout. + """ + dtype = _ATTR_DTYPE.get(tensor_type_name(tensor_type)) + shape = tuple(binding.shape) + if dtype is not None and shape and shape[-1] == wp.types.type_size_in_bytes(dtype) // 4: + return shape[:-1], dtype + return shape, wp.float32 + def _as_wp(self, values: Any, device: str) -> wp.array: """Coerce ``values`` to a :class:`warp.array`. diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 4f98f03a87f0..9e5d6d768d8f 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -63,11 +63,13 @@ def __init__(self, tensor_type, n: int): self.spatial_tendon_count = 0 self.read_calls = 0 self.last_read: tuple | None = None + self.last_read_obj = None # the actual array object handed to read (for cache-warmth checks) self.write_calls: list[tuple] = [] def read(self, dst) -> None: self.read_calls += 1 self.last_read = (dst.dtype, tuple(dst.shape), str(dst.device)) + self.last_read_obj = dst def write(self, tensor, indices=None, mask=None) -> None: self.write_calls.append((tensor.dtype, tuple(tensor.shape), indices, mask)) @@ -179,10 +181,11 @@ def test_eager_default_sweep_empty_view_raises(): # ----------------------------------------------------------------------------- -def test_get_attribute_allocates_fresh_buffer_each_call(): +def test_get_attribute_allocates_fresh_typed_buffer_each_call(): view = _make_view(n=4) buf = view.get_attribute("rigid_body_pose") - assert tuple(buf.shape) == (4, 7) and buf.dtype == wp.float32 + # Pose maps to a structured dtype: an [N] transformf array (== [N, 7] float32). + assert tuple(buf.shape) == (4,) and buf.dtype == wp.transformf binding = view._bindings[TensorType.RIGID_BODY_POSE] assert binding.read_calls == 1 buf2 = view.get_attribute("rigid_body_pose") @@ -190,6 +193,51 @@ def test_get_attribute_allocates_fresh_buffer_each_call(): assert binding.read_calls == 2 +def test_get_attribute_types_pose_and_velocity_falls_back_to_float32(): + view = _make_view(n=3) + assert view.get_attribute("rigid_body_pose").dtype == wp.transformf + assert view.get_attribute("rigid_body_velocity").dtype == wp.spatial_vectorf + # An attribute absent from the structured-dtype map stays flat float32. + mass = view.get_attribute("rigid_body_mass") + assert mass.dtype == wp.float32 and tuple(mass.shape) == (3,) + + +def test_read_into_reuses_reinterpret_view_across_calls(): + # The float32 reinterpret of a structured dst is built once and reused so the wheel's + # object-identity read cache stays warm across steps. + view = _make_view(n=3) + dst = wp.zeros((3,), dtype=wp.transformf, device="cpu") + view.read_into("rigid_body_pose", dst) + binding = view._bindings[TensorType.RIGID_BODY_POSE] + first = binding.last_read_obj + view.read_into("rigid_body_pose", dst) + assert binding.last_read_obj is first # same object handed to the wheel both times + assert first is not dst # it is the float32 reinterpret, not the transformf buffer + + +def test_read_into_passthrough_reuses_dst_object(): + # A flat float32 dst is its own stable identity -- passed straight through, not cached. + view = _make_view(n=3) + dst = wp.zeros((3, 7), dtype=wp.float32, device="cpu") + view.read_into("rigid_body_pose", dst) + binding = view._bindings[TensorType.RIGID_BODY_POSE] + assert binding.last_read_obj is dst + assert id(dst) not in view._read_views # float32 dst is not cached + + +def test_read_into_caches_per_destination_buffer(): + view = _make_view(n=3) + a = wp.zeros((3,), dtype=wp.transformf, device="cpu") + b = wp.zeros((3,), dtype=wp.transformf, device="cpu") + binding = view._bindings.get(TensorType.RIGID_BODY_POSE) or view.binding_for("rigid_body_pose") + view.read_into("rigid_body_pose", a) + view_a = binding.last_read_obj + view.read_into("rigid_body_pose", b) + view_b = binding.last_read_obj + assert view_a is not view_b # distinct reinterprets per destination buffer + assert view_a.ptr == a.ptr and view_b.ptr == b.ptr + + def test_get_attribute_out_param_is_filled_and_returned(): view = _make_view(n=2) out = wp.zeros((2, 7), dtype=wp.float32, device="cpu") From 43d34b2196cc49df522d79009a46e5ec515ca898 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 19:45:10 +0200 Subject: [PATCH 08/12] docs(ovphysx): add isaaclab_ovphysx.sim.views to the API reference OvPhysxView (and OvPhysxFrameView) live in isaaclab_ovphysx.sim.views, which had no API-docs page. Add the automodule stub and wire it into the isaaclab_ovphysx autosummary so the new binding-manager view shows up in the rendered API reference alongside assets / cloner / physics. --- docs/source/api/index.rst | 1 + docs/source/api/lab_ovphysx/isaaclab_ovphysx.sim.views.rst | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 docs/source/api/lab_ovphysx/isaaclab_ovphysx.sim.views.rst diff --git a/docs/source/api/index.rst b/docs/source/api/index.rst index 2ff0cf6174be..a0bfe388449a 100644 --- a/docs/source/api/index.rst +++ b/docs/source/api/index.rst @@ -214,6 +214,7 @@ The following modules are available in the ``isaaclab_ovphysx`` extension: assets cloner physics + sim.views isaaclab_experimental extension diff --git a/docs/source/api/lab_ovphysx/isaaclab_ovphysx.sim.views.rst b/docs/source/api/lab_ovphysx/isaaclab_ovphysx.sim.views.rst new file mode 100644 index 000000000000..a2a93b474e58 --- /dev/null +++ b/docs/source/api/lab_ovphysx/isaaclab_ovphysx.sim.views.rst @@ -0,0 +1,4 @@ +isaaclab\_ovphysx.sim.views +=========================== + +.. automodule:: isaaclab_ovphysx.sim.views From 8144819af2ad569dffe22fdd6f2535dbcca8d41f Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Tue, 23 Jun 2026 10:02:35 +0200 Subject: [PATCH 09/12] fix(ovphysx): address Greptile review of OvPhysxView Three fixes from the Part 1 review: - get_attribute (no out): route the freshly allocated buffer through _as_binding_view directly instead of the id()-keyed read cache. A fresh buffer can never hit the cache and would leak one entry (keeping the buffer alive) per call in a step loop; the cache only pays off for a reused out/dst buffer. Add a regression test asserting the no-out path leaves _read_views empty. - Raise a dedicated DtypeMismatch instead of ShapeMismatch when a buffer's scalar element type is not float32, so a dtype error no longer reads as a dimensions error. Update the affected tests and the Raises docstrings. - Make the view Warp-native: drop the fragile __module__ string-match that auto-converted Torch tensors on writes. Callers bridge a Torch tensor with wp.from_torch(t), keeping the device policy explicit and avoiding an optional Torch dependency. --- .../sim/views/ovphysx_view.py | 30 +++++++++++++------ .../test/sim/test_ovphysx_view.py | 18 +++++++++-- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index 8327dd1b4e05..9b27b28cf1ad 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -222,6 +222,9 @@ class AttributeUnavailable(OvPhysxViewError): class ShapeMismatch(OvPhysxViewError): """A supplied buffer does not match the binding's element count.""" + class DtypeMismatch(OvPhysxViewError): + """A supplied buffer's scalar element type is not ``float32``.""" + class DeviceMismatch(OvPhysxViewError): """A supplied buffer is on a different device than the binding requires.""" @@ -325,7 +328,11 @@ def get_attribute(self, name: str | Any, *, out: wp.array | None = None) -> wp.a return out alloc_shape, dtype = self._attribute_dtype(tt, binding) buf = wp.zeros(alloc_shape, dtype=dtype, device=device) - binding.read(self._read_view(buf, binding)) + # ``buf`` is freshly allocated here, so it is never a persistent destination: route it + # through ``_as_binding_view`` directly rather than ``_read_view``. Caching by ``id(buf)`` + # could never hit on a later call and would leak one entry (and keep ``buf`` alive) per + # call in a step loop -- the read cache only pays off for a reused ``out``/``dst`` buffer. + binding.read(self._as_binding_view(buf, binding, "destination")) return buf def read_into(self, name: str | Any, dst: wp.array) -> None: @@ -346,7 +353,8 @@ def read_into(self, name: str | Any, dst: wp.array) -> None: Raises: OvPhysxView.DeviceMismatch: If ``dst`` is not on the binding's native device. - OvPhysxView.ShapeMismatch: If ``dst``'s element count does not match. + OvPhysxView.DtypeMismatch: If ``dst``'s scalar element type is not ``float32``. + OvPhysxView.ShapeMismatch: If ``dst`` is non-contiguous or its element count does not match. """ tt = self._resolve(name) binding = self._binding(tt) @@ -377,7 +385,8 @@ def set_attribute( Raises: OvPhysxView.ReadOnlyAttribute: If the attribute is read-only. OvPhysxView.DeviceMismatch: If ``values`` is not on the binding's native device. - OvPhysxView.ShapeMismatch: If ``values``' element count does not match. + OvPhysxView.DtypeMismatch: If ``values``' scalar element type is not ``float32``. + OvPhysxView.ShapeMismatch: If ``values`` is non-contiguous or its element count does not match. """ tt = self._resolve(name) attr = tensor_type_name(tt) @@ -585,7 +594,7 @@ def _as_binding_view(self, arr: wp.array, binding: Any, role: str) -> wp.array: """ scalar = getattr(arr.dtype, "_wp_scalar_type_", arr.dtype) if scalar is not wp.float32: - raise OvPhysxView.ShapeMismatch( + raise OvPhysxView.DtypeMismatch( f"{role} must have float32 scalar elements (got dtype " f"{getattr(arr.dtype, '__name__', arr.dtype)}); the view reinterprets bits, " "not values, so a non-float32 dtype would silently corrupt the buffer." @@ -641,14 +650,17 @@ def _attribute_dtype(self, tensor_type: Any, binding: Any) -> tuple[tuple[int, . def _as_wp(self, values: Any, device: str) -> wp.array: """Coerce ``values`` to a :class:`warp.array`. - Device-bearing inputs (warp / torch) keep their own device and are validated by - the caller (a mismatch raises -- never staged). Device-less host data (numpy - arrays, lists) carries no device, so it is materialized directly on ``device``. + A :class:`warp.array` is used as-is, keeping its own device (validated by the caller; + a mismatch raises and is never staged). Device-less host data (numpy arrays, lists) + carries no device, so it is materialized directly on ``device``. + + This view is Warp-native and does **not** special-case framework tensors: bridge a + Torch tensor on the caller side with ``view.set_attribute(name, wp.from_torch(t))``. + This keeps the device policy explicit and avoids an optional Torch dependency and the + fragile detection a built-in conversion would require. """ if isinstance(values, wp.array): return values - if type(values).__module__.split(".")[0] == "torch": - return wp.from_torch(values) return wp.array(values, device=device) def _target_repr(self) -> str: diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 9e5d6d768d8f..1b1fffe03acf 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -193,6 +193,18 @@ def test_get_attribute_allocates_fresh_typed_buffer_each_call(): assert binding.read_calls == 2 +def test_get_attribute_no_out_does_not_grow_read_cache(): + # A no-`out` get_attribute allocates a fresh buffer each call, so caching its reinterpret by + # id() could never hit on a later call and would leak one entry (keeping the buffer alive) per + # call in a step loop. The structured-dtype path must reinterpret directly, never touching the + # cache -- the read cache only pays off for a reused `out`/`dst`. Pose is the structured case + # (transformf), the one that would have been cached before the fix. + view = _make_view(n=3) + for _ in range(5): + view.get_attribute("rigid_body_pose") + assert view._read_views == {} + + def test_get_attribute_types_pose_and_velocity_falls_back_to_float32(): view = _make_view(n=3) assert view.get_attribute("rigid_body_pose").dtype == wp.transformf @@ -348,7 +360,7 @@ def test_as_wp_accepts_numpy_float32_and_rejects_float64(): view.set_attribute("rigid_body_pose", np.zeros((2, 7), dtype=np.float32)) assert len(view._bindings[TensorType.RIGID_BODY_POSE].write_calls) == 1 # float64 is not float32-bit-equivalent; reject rather than silently reinterpret. - with pytest.raises(OvPhysxView.ShapeMismatch): + with pytest.raises(OvPhysxView.DtypeMismatch, match="float32 scalar"): view.set_attribute("rigid_body_pose", np.zeros((2, 7), dtype=np.float64)) @@ -426,7 +438,7 @@ def test_set_attribute_rejects_same_byte_size_wrong_dtype(): # and get bit-reinterpreted into garbage. It must be rejected, not silently written. view = _make_view(n=3) int_buf = wp.zeros((3, 7), dtype=wp.int32, device="cpu") - with pytest.raises(OvPhysxView.ShapeMismatch, match="float32 scalar"): + with pytest.raises(OvPhysxView.DtypeMismatch, match="float32 scalar"): view.set_attribute("rigid_body_pose", int_buf) assert view._bindings[TensorType.RIGID_BODY_POSE].write_calls == [] @@ -434,7 +446,7 @@ def test_set_attribute_rejects_same_byte_size_wrong_dtype(): def test_set_attribute_rejects_sub_4byte_dtype(): view = _make_view(n=3) half_buf = wp.zeros((3, 7), dtype=wp.float16, device="cpu") - with pytest.raises(OvPhysxView.ShapeMismatch, match="float32 scalar"): + with pytest.raises(OvPhysxView.DtypeMismatch, match="float32 scalar"): view.set_attribute("rigid_body_pose", half_buf) From 0757a7997d6dba0e3ad5a4b5ca7817f3557636b2 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Tue, 23 Jun 2026 11:56:46 +0200 Subject: [PATCH 10/12] docs(ovphysx): address Marco's API-contract review of OvPhysxView Documentation/comment clarifications from the #6224 review (no behavior change): - Narrow the documented contract to float32-only: attribute_names/has_attribute and the module docstring now state that a listed name is name-validity, not a dtype-support promise; non-float dtype handling awaits wheel dtype metadata. - Mark _READ_ONLY_NAMES explicitly temporary; name the three access modes (read/write, read-only, write-only) and the wheel access_mode enum that should replace the table. - Document key_aliases as an internal collection adapter, not general public API, pending descriptor metadata. - Make the view test scope explicit: mock API mechanics here; live CPU-only-on-GPU / read-only+write-only / structured read_into coverage lives in the asset-integration tests. --- .../sim/views/ovphysx_view.py | 38 +++++++++++++++++-- .../test/sim/test_ovphysx_view.py | 7 +++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py index 9b27b28cf1ad..099f4309e617 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py @@ -33,6 +33,16 @@ its native device and **raises** :class:`OvPhysxView.DeviceMismatch` if a caller hands it a buffer on the wrong device. Staging a CPU property to/from the simulation device is the caller's explicit responsibility, never hidden here. + +**Dtype: float32 only (interim).** The wheel's ``TensorBinding.read``/``write`` are +float32-only and expose no dtype metadata, so this view treats every binding as ``float32`` +(the structured dtypes in :data:`_ATTR_DTYPE`, e.g. ``wp.transformf``, are byte-compatible +views over float32, not a different scalar type). Every ``TensorType`` in the current wheel +is float32; a future non-float binding (e.g. a ``uint8``/``bool`` control tensor) could not be +read correctly here until the wheel exposes dtype metadata. Rather than guess a dtype-supported +subset, the public surface (:attr:`~OvPhysxView.attribute_names`) deliberately stays at +name-validity; narrowing it to a dtype-aware subset is deferred to wheel dtype metadata +(design doc §7 ask). """ from __future__ import annotations @@ -54,9 +64,14 @@ # Tensor types that cannot be written. The first group is read-only by PhysX # convention (accelerations, inverse mass/inertia, projected joint force); the # computed-dynamics group (jacobian, mass matrix, coriolis, gravity) is read-only -# in practice. The wheel enum exposes no writability flag today, so this set is -# hand-maintained. -# TODO(ovphysx): source this from a wheel writability query (design doc §7 ask 3). +# in practice. +# +# TEMPORARY: this is a hand-maintained table and WILL drift as ``TensorType`` grows. +# The wheel has at least three access modes in practice -- read/write, read-only, and +# write-only control tensors -- and exposes no access metadata today. Replace this whole +# table once the wheel exposes a per-type ``access_mode`` enum (preferred over a boolean +# ``is_writable`` flag, so write-only control tensors stay distinguishable). +# TODO(ovphysx): source access mode from a wheel ``access_mode`` query (design doc §7 ask 3). _READ_ONLY_NAMES: frozenset[str] = frozenset( { "rigid_body_acceleration", @@ -201,6 +216,13 @@ class OvPhysxView: key_aliases: Optional mapping ``requested_type -> created_type`` so a binding can be stored under a different :class:`TensorType` key than the one created (e.g. a ``RigidObjectCollection`` stores ``rigid_body_pose`` under ``link_pose``). + This is an **internal IsaacLab adapter** for the fused-collection binding path, not + a general public API: the requested key and the created binding type deliberately + differ, so a caller reasoning from the visible key can get different runtime + semantics. A public form would instead carry descriptor metadata (requested key, + source tensor type, shape, native device, access mode); that is deferred to + wheel-exposed metadata (design doc §7 ask). Prefer not to rely on it outside the + collection adapter. tensor_types: Explicit set of :class:`TensorType` members to instantiate eagerly. Used only when ``eager`` is set; defaults to every applicable type. eager: If ``True``, create bindings up front and raise if none could be created. @@ -433,6 +455,13 @@ def attribute_names(self) -> list[str]: This is name *validity*, not availability for this view's prims -- a rigid-body view still lists ``"articulation_*"`` names. Use :attr:`available_attributes` for what is actually instantiated. + + .. note:: + A listed name is **not** a promise of correct dtype handling. The view is + float32-only (see the module docstring); every ``TensorType`` in the current wheel + is float32, but a future non-float binding would still be listed here yet not be + correctly readable until the wheel exposes dtype metadata. Filtering this to a + dtype-aware supported subset is deferred to that metadata (design doc §7 ask). """ return attribute_vocabulary() @@ -446,7 +475,8 @@ def has_attribute(self, name: str | Any) -> bool: This checks name *validity* for any view, not availability for these prims: it can return ``True`` for a name whose binding does not apply to this view's prims (in which - case :meth:`get_attribute` raises :class:`AttributeUnavailable`). + case :meth:`get_attribute` raises :class:`AttributeUnavailable`). It likewise does not + promise dtype support -- the view is float32-only (see :attr:`attribute_names`). """ try: self._resolve(name) diff --git a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py index 1b1fffe03acf..f5de647edc55 100644 --- a/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py +++ b/source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py @@ -8,7 +8,12 @@ These exercise the pure-Python name<->enum logic and the view's get/set/read-into dispatch (including the float32 reinterpret of structured buffers and the no-implicit-conversion device policy) against a fake ``PhysX`` + fake ``TensorBinding``. -Full read/write round-trips on a live sim are covered by the asset integration tests. + +Scope is intentionally the **API mechanics** against mock bindings. Live coverage against +real ovphysx bindings -- CPU-only properties on a GPU sim, read-only/write-only failures, +and structured ``read_into`` round-trips -- is provided by the asset-integration tests +(``test_articulation.py`` / ``test_rigid_object.py`` / ``test_rigid_object_collection.py``) +that adopt this view as ``root_view``; it is not (and is not meant to be) re-covered here. """ from __future__ import annotations From f4d7c63d8cb7eddf848e1c07b80bbd67db67488e Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Mon, 22 Jun 2026 17:21:10 +0200 Subject: [PATCH 11/12] refactor(ovphysx): route PVA sensor binding management through OvPhysxView Replace the three per-attribute create_tensor_binding calls and their manual float32 reinterpret views with a single OvPhysxView over the rigid-parent pattern. Bindings are obtained via binding_for and read via read_into, which fills the structured kernel buffers in place. The CPU-only RIGID_BODY_COM_POSE keeps its pinned-host staging on a GPU sim. No public API or numeric behavior changes. --- .../antoiner-feat-ovphysx_pva_view.skip | 0 .../isaaclab_ovphysx/sensors/pva/pva.py | 55 ++++++------------- 2 files changed, 18 insertions(+), 37 deletions(-) create mode 100644 source/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_pva_view.skip diff --git a/source/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_pva_view.skip b/source/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_pva_view.skip new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py index 7fb30e71e622..78d1765ead54 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py @@ -19,6 +19,7 @@ import isaaclab_ovphysx.tensor_types as TT from isaaclab_ovphysx.physics import OvPhysxManager as SimulationManager +from isaaclab_ovphysx.sim.views.ovphysx_view import OvPhysxView from .kernels import pva_reset_kernel, pva_update_kernel from .pva_data import PvaData @@ -67,7 +68,7 @@ def __init__(self, cfg: PvaCfg): self._rigid_parent_expr: str | None = None # Sentinel — set in :meth:`_initialize_impl`; ``None`` means the sensor has not been bound yet # (used by :meth:`_debug_vis_callback` to safely no-op before init). - self._pose_binding = None + self._root_view: OvPhysxView | None = None def __str__(self) -> str: """Returns: A string containing information about the instance.""" @@ -143,10 +144,8 @@ def _initialize_impl(self): # Translate the regex-style path expression to an ovphysx fnmatch glob. pattern = self._rigid_parent_expr.replace(".*", "*") - self._pose_binding = physx_instance.create_tensor_binding(pattern=pattern, tensor_type=TT.RIGID_BODY_POSE) - self._vel_binding = physx_instance.create_tensor_binding(pattern=pattern, tensor_type=TT.RIGID_BODY_VELOCITY) - self._com_binding = physx_instance.create_tensor_binding(pattern=pattern, tensor_type=TT.RIGID_BODY_COM_POSE) - self._num_bodies = self._pose_binding.count + self._root_view = OvPhysxView(physx_instance, pattern=pattern, device=self._device) + self._num_bodies = self._root_view.binding_for(TT.RIGID_BODY_POSE).count if self._num_bodies != self._num_envs: raise ValueError( @@ -182,15 +181,15 @@ def _update_buffers_impl(self, env_mask: wp.array | None = None): """Fills the buffers of the sensor data.""" env_mask = self._resolve_indices_and_mask(None, env_mask) - # ovphysx ``binding.read(dst)`` writes into the pre-allocated dst buffer; - # ``_*_view`` are float32 aliases of the structured-dtype buffers below. - self._pose_binding.read(self._transforms_view) - self._vel_binding.read(self._velocities_view) + # ``OvPhysxView.read_into`` fills the structured-dtype buffer in place via a + # cached float32 reinterpret; no manual float32 alias is needed. + self._root_view.read_into(TT.RIGID_BODY_POSE, self._transforms) + self._root_view.read_into(TT.RIGID_BODY_VELOCITY, self._velocities) # RIGID_BODY_COM_POSE is a CPU tensor type in the OVPhysX wheel. # For GPU simulations, stage on CPU then copy into the kernel buffer. - self._com_binding.read(self._coms_read_view) - if self._coms_read_view is not self._coms_gpu_view: - wp.copy(self._coms_gpu_view, self._coms_read_view) + self._root_view.read_into(TT.RIGID_BODY_COM_POSE, self._coms_read_view) + if self._coms_read_view is not self._coms_buffer: + wp.copy(self._coms_buffer, self._coms_read_view) wp.launch( pva_update_kernel, @@ -231,36 +230,18 @@ def _initialize_buffers_impl(self): self._offset_pos_b = wp.from_torch(offset_pos_torch.contiguous(), dtype=wp.vec3f) self._offset_quat_b = wp.from_torch(offset_quat_torch.contiguous(), dtype=wp.quatf) - # Structured-dtype buffers consumed by the kernel. + # Structured-dtype buffers filled in place by :meth:`OvPhysxView.read_into`. self._transforms = wp.zeros(self._num_bodies, dtype=wp.transformf, device=self._device) self._velocities = wp.zeros(self._num_bodies, dtype=wp.spatial_vectorf, device=self._device) self._coms_buffer = wp.zeros(self._num_bodies, dtype=wp.transformf, device=self._device) - self._transforms_view = wp.array( - ptr=self._transforms.ptr, - shape=self._pose_binding.shape, - dtype=wp.float32, - device=self._device, - copy=False, - ) - self._velocities_view = wp.array( - ptr=self._velocities.ptr, - shape=self._vel_binding.shape, - dtype=wp.float32, - device=self._device, - copy=False, - ) - self._coms_gpu_view = wp.array( - ptr=self._coms_buffer.ptr, - shape=self._com_binding.shape, - dtype=wp.float32, - device=self._device, - copy=False, - ) + # RIGID_BODY_COM_POSE is CPU-resident even on a GPU sim, so its binding requires a + # CPU destination. On a GPU sim, stage the read into a pinned CPU buffer and copy into + # the kernel buffer; on a CPU sim, read straight into the kernel buffer. if self._device == "cpu": - self._coms_read_view = self._coms_gpu_view + self._coms_read_view = self._coms_buffer else: - self._coms_read_view = wp.zeros(self._com_binding.shape, dtype=wp.float32, device="cpu", pinned=True) + self._coms_read_view = wp.zeros(self._num_bodies, dtype=wp.transformf, device="cpu", pinned=True) def _set_debug_vis_impl(self, debug_vis: bool): if debug_vis: @@ -273,7 +254,7 @@ def _set_debug_vis_impl(self, debug_vis: bool): def _debug_vis_callback(self, event): # safely return if the sensor has not been bound yet (matches the PhysX `_view is None` idiom) - if self._pose_binding is None: + if self._root_view is None: return # get marker location # -- base state (convert warp -> torch for visualization) From 7bd5f56287de02dc674de7e9ceec17c5dcbf65b7 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Thu, 25 Jun 2026 15:43:12 +0200 Subject: [PATCH 12/12] fix(ovphysx): drop view on PVA sensor invalidate Add _invalidate_initialize_callback to clear _root_view on simulation stop so a stale view (caching post-reset bindings) is not held across the reset; _initialize_impl rebuilds a fresh OvPhysxView on the next play. --- .../isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py b/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py index 78d1765ead54..569cd5f333db 100644 --- a/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py +++ b/source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/pva/pva.py @@ -177,6 +177,13 @@ def _initialize_impl(self): self._offset_pos_b = wp.from_torch(composed_p.contiguous(), dtype=wp.vec3f) self._offset_quat_b = wp.from_torch(composed_q.contiguous(), dtype=wp.quatf) + def _invalidate_initialize_callback(self, event) -> None: + """Drop the OVPhysX view when physics stops.""" + super()._invalidate_initialize_callback(event) + # Drop the view (and the bindings it caches) so a stale/destroyed handle is not held + # across the reset; ``_initialize_impl`` rebuilds a fresh view on the next play. + self._root_view = None + def _update_buffers_impl(self, env_mask: wp.array | None = None): """Fills the buffers of the sensor data.""" env_mask = self._resolve_indices_and_mask(None, env_mask)