Migrate OVPhysX joint-wrench sensor onto OvPhysxView (view series, part 5)

[AntoineRichard]

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

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

Description

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

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

Type of change

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

Checklist

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

Testing

test_joint_wrench_sensor.py — cpu: 11 passed, cuda: 11 passed.

[greptile-apps]

Greptile Summary

Routes the joint-wrench sensor's tensor-binding lifecycle through the new OvPhysxView class, replacing direct create_tensor_binding calls and a manually-managed float32 alias view with a string-keyed binding manager that caches structured-dtype reinterpret views internally. The public sensor API is unchanged.

• ovphysx_view.py — new 705-line OvPhysxView class with lazy/eager binding creation, device-residency enforcement (no implicit CPU↔GPU staging), dtype reinterpret guards, and a _read_views cache that keeps the OVPhysX wheel's object-identity read-cache warm across steps.
• joint_wrench_sensor.py — _initialize_impl now creates one OvPhysxView and retrieves the binding from it; _update_buffers_impl calls read_into directly with the spatial_vectorf buffer; _invalidate_initialize_callback correctly nulls the view before the buffer to prevent Warp's deallocator from aborting on a freed-but-still-referenced allocation.
• test_ovphysx_view.py — 587-line unit tests covering name resolution, structured-dtype reinterpret, device policy, key aliases, and the read-cache non-growth assertion for no-out get_attribute calls.

Confidence Score: 4/5

Safe to merge — this is a pure internal refactor with no public API changes, the sensor's numeric behavior is unchanged, and the teardown ordering is correct.

The _read_views cache in OvPhysxView accumulates entries without eviction. For the sensor's single reused buffer this is harmless, but as a general-purpose class it could quietly retain dead reinterpret-view objects for callers who cycle through many destination buffers. Everything else — device-residency enforcement, structured-dtype reinterpret, binding caching, invalidation ordering, and test coverage — looks solid.

The _read_view method in ovphysx_view.py (lines 802–818) warrants a second look on the cache eviction question before this class is used more broadly.

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py	New 705-line OvPhysxView class — string-keyed binding manager over OVPhysX TensorBinding. Well-structured with typed errors, device-residency policy, and a read-cache for structured-dtype reinterprets. The _read_views cache grows without eviction, which is benign for single-buffer callers but a latent memory concern for views used with many distinct destination buffers.
source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/joint_wrench/joint_wrench_sensor.py	Initialization now builds one OvPhysxView and retrieves the LINK_INCOMING_JOINT_FORCE binding from it; per-step reads route through read_into instead of a manual float32 alias view. Ordering in _invalidate_initialize_callback (view before buffer) is correct. _wrench_binding is still maintained as an instance attribute but no longer participates in the guard check.
source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py	587-line unit test file covering pure helpers, construction (eager/lazy), get_attribute, read_into, set_attribute, device policy, dtype safety, key aliases, and adversarial cases via a fake PhysX+TensorBinding. Includes a dedicated test asserting that the read-cache is not populated on no-out get_attribute calls.
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/init.pyi	Adds OvPhysxView to all and the re-export list in the stub file. Clean, no issues.
docs/source/api/lab_ovphysx/isaaclab_ovphysx.sim.views.rst	New RST page for isaaclab_ovphysx.sim.views using automodule. Correctly indexed in docs/source/api/index.rst.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant S as JointWrenchSensor
    participant V as OvPhysxView
    participant PX as OVPhysX (PhysX)
    participant B as TensorBinding

    Note over S,B: _initialize_impl
    S->>V: OvPhysxView(physx, pattern, device)
    S->>V: binding_for(LINK_INCOMING_JOINT_FORCE)
    V->>PX: create_tensor_binding(tensor_type, pattern)
    PX-->>V: TensorBinding (cached in _bindings)
    V-->>S: _wrench_binding (raw handle)
    S->>S: validate body_count / count
    S->>S: allocate _wrench_buf (spatial_vectorf)

    Note over S,B: _update_buffers_impl (each step)
    S->>V: read_into(LINK_INCOMING_JOINT_FORCE, _wrench_buf)
    V->>V: "_resolve -> TensorType"
    V->>V: "_binding(tt) -> cached TensorBinding"
    V->>V: _check_device(_wrench_buf, device)
    V->>V: _read_view(_wrench_buf, binding)
    Note right of V: First call: build float32 reinterpret,<br/>cache in _read_views[id(buf)].<br/>Subsequent calls: reuse cached view.
    V->>B: binding.read(float32_reinterpret_view)
    B-->>V: fills buffer in-place

    Note over S,B: _invalidate_initialize_callback
    S->>S: "_root_view = None (releases _read_views)"
    S->>S: "_wrench_binding = None"
    S->>S: "_wrench_buf = None (safe: reinterpret already released)"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant S as JointWrenchSensor
    participant V as OvPhysxView
    participant PX as OVPhysX (PhysX)
    participant B as TensorBinding

    Note over S,B: _initialize_impl
    S->>V: OvPhysxView(physx, pattern, device)
    S->>V: binding_for(LINK_INCOMING_JOINT_FORCE)
    V->>PX: create_tensor_binding(tensor_type, pattern)
    PX-->>V: TensorBinding (cached in _bindings)
    V-->>S: _wrench_binding (raw handle)
    S->>S: validate body_count / count
    S->>S: allocate _wrench_buf (spatial_vectorf)

    Note over S,B: _update_buffers_impl (each step)
    S->>V: read_into(LINK_INCOMING_JOINT_FORCE, _wrench_buf)
    V->>V: "_resolve -> TensorType"
    V->>V: "_binding(tt) -> cached TensorBinding"
    V->>V: _check_device(_wrench_buf, device)
    V->>V: _read_view(_wrench_buf, binding)
    Note right of V: First call: build float32 reinterpret,<br/>cache in _read_views[id(buf)].<br/>Subsequent calls: reuse cached view.
    V->>B: binding.read(float32_reinterpret_view)
    B-->>V: fills buffer in-place

    Note over S,B: _invalidate_initialize_callback
    S->>S: "_root_view = None (releases _read_views)"
    S->>S: "_wrench_binding = None"
    S->>S: "_wrench_buf = None (safe: reinterpret already released)"

Loading

Comments Outside Diff (2)

1. source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py, line 802-818 (link)

   _read_views cache grows without eviction

   The cache maps id(dst) → reinterpret view and entries are never removed. For the current sensor usage (one reused _wrench_buf), this is a non-issue — the cache stays at size 1 for the view's lifetime. But OvPhysxView is a general-purpose class; a caller who passes a new structured dst on every call to read_into or get_attribute(out=...) will accumulate one dead entry per unique buffer object, keeping each float32 reinterpret (and by extension a raw pointer into the already-freed backing memory) alive in the dict indefinitely.

   A weakref-keyed eviction is the standard fix (weakref.WeakKeyDictionary doesn't work here because wp.array is likely not weak-referenceable, but a __del__-based sentinel or a size-bounded LRU would be sufficient). At minimum, a docstring note warning callers to reuse dst would make the expected usage pattern explicit.

2. source/isaaclab_ovphysx/isaaclab_ovphysx/sensors/joint_wrench/joint_wrench_sensor.py, line 63-65 (link)

   _wrench_binding is now only used inside _initialize_impl

   After this refactor, _wrench_binding is assigned from _root_view.binding_for(...), used only for the body_count/count guard, and then stored as an instance attribute that is cleared in _invalidate_initialize_callback — it no longer participates in the _update_buffers_impl guard (_root_view is None or _wrench_buf is None). Making it a local variable in _initialize_impl would remove the ambiguity about whether it signals initialization state. If it needs to stay on the instance for future callers or introspection, a comment explaining that intent would help.

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

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