Support OVPhysX 0.5.11 and 0.6 lifecycle APIs

[kellyguo11]

Description

Support both the released OVPhysX 0.5.11 lifecycle API and the OVPhysX 0.6 lifecycle API without changing Isaac Lab's dependency pins.

This follows the existing OVRTX 0.4.1/0.5 compatibility pattern:

• Detect the installed ovphysx distribution version once at import.
• Resolve an immutable lifecycle mapping to warmup_gpu() / release() for OVPhysX 0.5.11 and to warmup() / destroy() for the OVPhysX 0.6 release line and later, including prerelease and development builds.
• Default missing or unparsable metadata to the supported OVPhysX 0.5.11 path.
• Keep the public ov and ovphysx extras pinned to ovphysx==0.5.11; OVPhysX 0.6 remains a manual internal-testing overlay.

The lifecycle changes preserve existing 0.5.11 cleanup semantics. For OVPhysX 0.6 only, they also retain OVPhysX and OVStage ownership when destroy() fails before native teardown so cleanup can be retried; a terminal destroy error releases dead owners.

The scope is limited to OVPhysX lifecycle compatibility code, tests, and the isaaclab_ov changelog fragment. No actuator changes or dependency updates are included.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Validation

• uv run --frozen --extra test --extra ovphysx python -m pytest source/isaaclab_ov/test/physics/test_ovphysx_compat.py source/isaaclab_ov/test/physics/test_ovphysx_scene_data_backend.py -q — 50 passed against the pinned OVPhysX 0.5.11 environment.
• The same 50-test suite passed with installed OVPhysX metadata simulated as 0.6.0, including the version-independent OVStage ownership test and version-selected 0.6 lifecycle doubles.
• The 0.6 development-build regression fails on the prior PR head and passes with the release-line classifier.
• uv run --extra test --extra ovphysx python -m pytest source/isaaclab_ov/test/assets/test_rigid_object.py::test_warmup_attach_stage_not_called_for_cpu -q — 1 passed against the real OVPhysX 0.5.11 CPU runtime.
• uv run --frozen --extra test --extra ov python -m pytest source/isaaclab_ov/test/test_ovrtx_clone_plan.py -q — 17 passed after merging upstream fix Fix OVRTX clone-plan test import #7582.
• Changelog fragment validation against current develop — passed.
• uv run isaaclab -f — passed.

Screenshots

Not applicable.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with uv run isaaclab -f
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective and that the feature works
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package
• The contributors already exist in CONTRIBUTORS.md

[greptile-apps]

Greptile Summary

The PR adds version-selected OVPhysX 0.5.11/0.6 lifecycle dispatch and preserves runtime ownership when 0.6 destruction fails before teardown. It also initializes actuator-resolved soft velocity limits during collection construction and updates the associated tests, documentation, and changelogs.

• Resolves OVPhysX warmup and destruction entry points once from installed distribution metadata.
• Retains OVPhysX and OVStage owners when destruction remains retryable.
• Initializes soft joint velocity limits before the first actuator update, including unbounded Remotized actuator semantics.
• Adds lifecycle, retry, actuator-limit, and CPU warmup coverage.

Confidence Score: 5/5

The PR appears safe to merge; no concrete blocking or independently actionable non-blocking defects were identified.

The lifecycle routing, destruction ownership handling, and construction-time actuator-limit initialization are internally consistent with their covered runtime paths, and the investigated edge cases did not establish a reachable failure.

Important Files Changed

Filename	Overview
source/isaaclab_ov/isaaclab_ov/physics/ovphysx_compat.py	Adds immutable, metadata-driven lifecycle entry-point selection for legacy and current OVPhysX versions.
source/isaaclab_ov/isaaclab_ov/physics/ovphysx_manager.py	Routes warmup and teardown through compatibility entry points and retains owners after retryable destruction failures.
source/isaaclab/isaaclab/actuators/actuator_collection.py	Seeds soft joint velocity limits from resolved actuator values during collection construction.
source/isaaclab/test/actuators/test_actuator_collection.py	Covers construction-time soft-limit initialization for Lab, native-test-double, and Remotized configurations.
source/isaaclab_ov/test/physics/test_ovphysx_compat.py	Verifies version detection, lifecycle classification, and immutable mapping construction.
source/isaaclab_ov/test/physics/test_ovphysx_scene_data_backend.py	Extends lifecycle dispatch and destruction ownership/retry coverage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Import compatibility layer] --> B{Installed OVPhysX version >= 0.6?}
    B -- Yes --> C[warmup / destroy]
    B -- No or unknown --> D[warmup_gpu / release]
    C --> E[Initialize and run simulation]
    D --> E
    E --> F[Close manager]
    F --> G{Destroy completed or handle is terminal?}
    G -- Yes --> H[Release PhysX and OVStage owners]
    G -- No --> I[Preserve owners for retry]

Loading

_{Reviews (1): Last reviewed commit: "Select OVPhysX lifecycle APIs by install..." | Re-trigger Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

The version-selected OVPhysX lifecycle mapping and actuator soft-limit initialization are coherent, but the new retry-oriented teardown behavior unintentionally changes failure handling for the pinned OVPhysX 0.5.11 release path.

• Design and architecture: Resolving immutable lifecycle entry points once at import cleanly isolates OVPhysX 0.5.11/0.6 compatibility. However, _release_physx applies the 0.6-specific owner-retention policy to both lifecycle generations, changing established 0.5.11 cleanup semantics when release() raises. Retention should be gated to the 0.6 destroy() path.
• API: The compatibility helpers are documented and internally consistent, dependency pins remain unchanged, and the actuator velocity-limit documentation matches the implemented resolution behavior. No public API compatibility issue was identified.
• Implementation: Per-group soft joint velocity limits are initialized for Lab and Newton-managed actuator paths, including unbounded remotized actuators. Teardown still attempts the selected lifecycle method, but a 0.5.11 release() failure can now leave _physx and _ovstage retained while close() resets the rest of the manager state; preserve the prior legacy cleanup behavior and restrict retry retention to the 0.6 lifecycle.

Minor fixes needed. Posted 1 actionable finding inline.

Automated review; human maintainers own approval decisions.

[isaaclab-review-bot]

🟡 Warning · Design Architecture — Destroy-failure retention not gated on 0.6 lifecycle

This retain-on-failure policy also governs the pinned 0.5.11 release() path. Previously cls._physx was cleared and _destroy_ovstage() ran unconditionally. Now, if release() raises and the handle probe does not raise RuntimeError (e.g. AttributeError on a 0.5.11 object), destroyed stays false: the OVStage leaks and close() returns with cls._physx still set while all other manager state is reset. Gate retention on the 0.6 lifecycle selection.

[kellyguo11]

Addressed in 5cd8349. The legacy release() path now preserves its prior unconditional owner cleanup, while handle probing and retry retention are gated to the selected OVPhysX 0.6 destroy() path. I added a regression test that fails on the reviewed commit and passes with this fix.

[kellyguo11]

run-ci

[AntoineRichard]

🤖 AI-assisted suggestion (non-blocking): Could we consolidate this classifier table and the mapping tests below into one parameterized test of the final lifecycle mapping?

uses_current_lifecycle_api() is exercised again through build_lifecycle_entry_points(), so separately asserting both layers duplicates the same < 0.6 decision. One table covering None, 0.5.11, 0.6, a local 0.6 build, and 1.0 would preserve fallback, boundary, local-version, and forward-compatibility coverage.

I would keep the three metadata-detection tests and the read-only mapping test, but remove test_installed_entry_points_match_the_installed_version(), since it largely restates the module initializer. This would reduce this file from 14 collected cases to about 9 while still testing the observable compatibility contract.

[kellyguo11]

Addressed in 2200d71. The classifier and mapping coverage is now one parameterized test of the final lifecycle mapping, and the initializer-restatement test was removed. The file now collects 9 cases while retaining metadata, fallback, boundary, prerelease/local-build, forward-compatibility, and immutability coverage.

[AntoineRichard]

🤖 AI-assisted suggestion (non-blocking): These six tests cover useful behavior, but most of their setup is identical. Could we parameterize by lifecycle generation and exercise both selected methods in each positive case?

One legacy row could assert warmup_gpu() followed by release(), and one current row could assert warmup() followed by destroy(). The missing-entry-point checks could become a second parameterized test over warmup and destroy.

That would preserve selection and diagnostic coverage while reducing the four positive tests to two collected cases and removing most of the repeated SimpleNamespace, mapping, and call-recording setup. I would keep the three ownership-state tests below separate because legacy failure, retryable current failure, and terminal current failure assert genuinely distinct postconditions.

[kellyguo11]

Addressed in 2200d71. The positive dispatch coverage now has one legacy row and one current row that each exercise both lifecycle calls, and the missing-entry-point checks are parameterized by operation. The three ownership-state tests remain separate.

[AntoineRichard]

🤖 AI-assisted review — test maintainability

The lifecycle coverage is valuable, particularly the three teardown tests for legacy cleanup, retryable OVPhysX 0.6 destruction, and terminal OVPhysX 0.6 destruction. Those tests protect distinct ownership invariants and should remain separate.

The reported 57 passing tests include 34 pre-existing cases. This PR adds 23 collected cases: 14 compatibility cases and 9 manager cases; the CPU runtime test is modified rather than added.

I left two non-blocking inline suggestions to consolidate overlapping classification/mapping coverage and parameterize the lifecycle dispatch wrappers. Together, those changes could reduce the additions from 23 to roughly 16 collected cases, about 30%, without removing meaningful regression coverage.

No blocking change is requested from this test-volume review.

[marcodiiga]

Non-blocking, but worth fixing while you're here: the pre-existing test_manager_attaches_and_releases_owned_ovstage (L464) also drives _release_physx() with a fake that only implements release(), and unlike this test it does not pin OVPHYSX_LIFECYCLE_ENTRY_POINTS. With the installed ovphysx metadata reporting 0.6.x it fails with

AttributeError: OVPhysX does not expose the selected destroy() lifecycle entry point

(reproduced by patching importlib.metadata.version to return 0.6.0; the other 56 tests in these two files pass under both versions). Either pin the legacy mapping there as done here, or add destroy = release to that fake so the suite stays green on a 0.6 install.

[kellyguo11]

Addressed in 2200d71. The existing OVStage ownership test now explicitly pins the legacy mapping, and the full compatibility suite passes with installed metadata simulated as 0.6.0. I also handled the review note about prereleases by classifying on version.release[:2], with a 0.6 development/local-build case.

[marcodiiga]

LGTM. Checked the mapping against both the 0.5.11 wheel and the 0.6 API: warmup_gpu/release -> warmup/destroy is correct, and the owner-retention logic matches the 0.6 destroy() contract (a native or process-shutdown failure marks the instance destroyed before raising, so probing handle for RuntimeError is the right terminal check; only a pre-teardown failure keeps it valid for a retry). Skipping the explicit CPU warmup on 0.6 is fine since warmup() runs lazily on the first tensor read.

Two non-blocking notes:

• One pre-existing test is not version-agnostic (inline comment).
• uses_current_lifecycle_api compares against Version("0.6"). A 0.6 pre-release tag (e.g. 0.6.0rc1 or 0.6.0.dev1) sorts below that under PEP 440 and would select the legacy names; comparing version.release[:2] >= (0, 6) avoids it if such builds are ever used for testing.

The failing multi-GPU smoke job is a runner setup issue (Isaac Sim package vs. virtualenv), unrelated to this change.

[hujc7]

run-ci

[kellyguo11]

run-ci

[kellyguo11]

run-ci