Migrate OVPhysX RigidObjectCollection onto OvPhysxView (view series, part 4)

[AntoineRichard]

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

This PR depends on #6224 and is branched off it (a sibling of the Articulation #6225
and RigidObject #6226 migrations, not stacked on them). It targets develop, but because
#6224 has not merged yet, the diff below includes the Part 1 OvPhysxView commits as well
as the Part 4 migration. Please review #6224 first. Once #6224 merges this branch will
be rebased onto develop and the diff will shrink to the collection migration only.

Description

Part 4 of the OVPhysX view-migration series. It migrates the OVPhysX RigidObjectCollection
onto OvPhysxView (introduced in #6224), following Parts 2 (#6225) and 3 (#6226).

This is the part that exercises the view's fused multi-prim path — prim_paths=[...] plus
key_aliases — which the earlier parts did not touch, so it dogfoods that slice of the API.

What changed

• RigidObjectCollection._initialize_impl builds a single
  OvPhysxView(prim_paths=[g0, …], key_aliases={LINK_POSE: RIGID_BODY_POSE, …}). The view
  creates each fused binding from its underlying RIGID_BODY_* type but stores it under the
  collection's LINK_*/BODY_* data-class key via key_aliases (every alias stays within its
  CPU/GPU residency class, which the view requires). Eager creation is fail-loud.
• RigidObjectCollectionData is built from the view; both _get_binding helpers delegate to
  OvPhysxView.try_binding_for, and the dead _binding_getter callback is removed.
• Reads route through root_view.read_into(...) on the native fused path (cached
  reinterpret); the articulation-mode mock read path (iface tests) keeps its raw binding.read
  because its buffer device intentionally bypasses the view's strict device check.
• Writes route through root_view.set_attribute(...) — the wrench plus the body-major
  reshape write helper, including CPU-only mass/COM/inertia (pre-staged to pinned CPU).
• RigidObjectCollection.root_view now returns the OvPhysxView instead of a raw
  dict[TensorType, TensorBinding] (breaking).

Breaking change / migration

RigidObjectCollection.root_view returns an OvPhysxView, not a dict:

• root_view[tensor_type] → root_view.try_binding_for(tensor_type)
• full-tensor read → root_view.get_attribute(tensor_type)

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Bug fix (non-breaking change which fixes an issue)
• This change requires a documentation update

Checklist

• I have run the pre-commit checks with ./isaaclab.sh --format
• I have added tests that prove my fix is effective or that my feature works (collection tests exercise the migrated read/write paths)
• I have updated the changelog (source/isaaclab_ovphysx/changelog.d/)
• I have updated the documentation accordingly
• I have added my changes to the right changelog.d fragment(s)

Testing

Run kitless; cpu and cuda must be separate invocations (process-global device lock):

72 passed, 4 xfailed   # test_rigid_object_collection.py -k cpu
72 passed, 4 xfailed   # test_rigid_object_collection.py -k cuda

[greptile-apps]

Greptile Summary

Part 4 of the OVPhysX view-migration series: migrates RigidObjectCollection and its data container from a raw dict[TensorType, TensorBinding] to an OvPhysxView, following the same pattern established in Parts 2-3 for articulations and rigid objects.

• _initialize_impl now constructs a single OvPhysxView(prim_paths=[...], key_aliases={LINK_*: RIGID_BODY_*}) instead of populating a plain bindings dict; all six fused bindings are eagerly created and the view's device/dtype/read-only guards apply automatically.
• All reads route through root_view.read_into(...) and all writes through root_view.set_attribute(...); the mock/native dispatch in _read_binding_into_instance_major is preserved.
• RigidObjectCollection.root_view now returns OvPhysxView instead of the dict (breaking change, documented in changelog).

Confidence Score: 4/5

The migration is structurally sound and consistent with Parts 2-3; all read/write dispatch paths are plausible given the 72-pass test suite.

All changed lines route correctly through the view's device/dtype guards, the mock-vs-native dispatch is explicitly preserved, and the tests cover CPU and CUDA paths for all major operations. The three findings are about pre-initialization robustness and type annotation completeness, not incorrect runtime behavior.

rigid_object_collection.py deserves a second look for the root_view return annotation and the broad except Exception in the binding-creation loop.

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/rigid_object_collection/rigid_object_collection.py	Core migration: replaces the raw dict with an OvPhysxView; minor issues with root_view return annotation omitting None and _get_binding crashing before initialization.
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/rigid_object_collection/rigid_object_collection_data.py	Data container migrated cleanly: _bindings dict and _binding_getter callback replaced by a single _view (OvPhysxView); mock/native dispatch preserved.
source/isaaclab_ovphysx/test/assets/test_rigid_object_collection.py	Minimal correct update: _MATERIAL_GAP_REASON string updated to reflect root_view is now an OvPhysxView rather than a dict.
source/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_rigidobjectcollection_view.major.rst	Changelog accurately documents the breaking root_view return-type change and migration path.

Comments Outside Diff (1)

1. source/isaaclab_ovphysx/isaaclab_ovphysx/assets/rigid_object_collection/rigid_object_collection.py, line 1210-1224 (link)

   _get_binding crashes before initialization instead of returning None

   Before this PR, _get_binding used self._bindings.get(tensor_type) which safely returned None when _bindings was still an empty dict (before _initialize_impl ran). The new implementation calls self._root_view.try_binding_for(tensor_type), but _root_view is None until _initialize_impl completes, so any early call will raise AttributeError: 'NoneType' object has no attribute 'try_binding_for' rather than returning None silently. A simple guard if self._root_view is None: return None would restore the prior safety contract.

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

[greptile-apps]

root_view return-type annotation omits None

_root_view is initialized to None in __init__ and is only assigned a real OvPhysxView during _initialize_impl. The property's return annotation is OvPhysxView (not OvPhysxView | None), so any caller who accesses root_view before sim.reset() will get None while the type system promises a live view. This becomes especially visible because the PR description explicitly calls out root_view as part of the public breaking-change migration guide.

[greptile-apps]

Binding creation loop uses overly broad except Exception

The loop calls self._root_view.binding_for(store_key) and wraps it in except Exception. This swallows unexpected errors from the wheel's create_tensor_binding and re-raises them as a generic RuntimeError, discarding the original exception type. Catching OvPhysxView.AttributeUnavailable specifically (or at least OvPhysxView.OvPhysxViewError) would preserve unexpected failures while still providing the helpful user-facing message for the expected "no matching prim" case.