Fix/malesiani/ovphysx poc integration backend

[marcodiiga]

Description

Add ovphysx backend support.

[github-actions]

Test Results Summary

2 188 tests   1 549 ✅  4h 6m 55s ⏱️
   43 suites    633 💤
    1 files        5 ❌  1 🔥

For more details on these failures and errors, see this check.

Results for commit ac4d421.

♻️ This comment has been updated with latest results.

[AntoineRichard]

It's missing dostrings everywhere, it could be good to review the tests, and maybe pull in the regular one from PhysX.

[greptile-apps]

Greptile Summary

This PR introduces the isaaclab_ovphysx package — a new Kit-free physics backend for IsaacLab built on the ovphysx Python wheel. It enables training pipelines to run without launching the full Kit/IsaacSim runtime by exporting the USD stage to a temporary file and loading it directly into an ovphysx.PhysX instance. Core modifications to backend_utils, asset_base, interactive_scene, and simulation_context are clean and well-scoped, using startswith() instead of in to prevent false matches with the new backend name.

Key findings from the review:

• scripts/run_ovphysx.sh unconditionally overwrites LD_PRELOAD rather than prepending, which would silently discard any existing preloaded libraries (e.g. ASAN, LSAN, or user debug shims).
• OvPhysxManager._warmup_and_load registers a new atexit handler on every invocation. Each handler calls os._exit(0) to sidestep a dual-Carbonite teardown race — acknowledged in a FIXME comment. Because os._exit(0) skips all Python finalizers and buffered I/O, training checkpoints or metrics written in Python teardown code may be silently lost. The handler should be registered at most once.
• _apply_external_wrenches in articulation.py calls inst.add_forces_and_torques_index(forces=perm.composed_force, ...) whenever inst.active is True, without guarding against perm.active. If WrenchComposer.composed_force returns stale (non-zero) data when inactive, permanent forces bleed into instantaneous applications.
• sim_launcher._is_kitless_physics uses type(node).__name__ string matching, which breaks for subclasses of OvPhysxCfg and would cause Kit to be unnecessarily launched.
• Kitless test detection in test_articulation_iface.py relies on LD_PRELOAD == "", a fragile heuristic that can produce false positives/negatives in CI or user environments.

Confidence Score: 3/5

• The PR adds substantial new functionality with mostly correct logic, but two workarounds (os._exit in atexit handler and sys.modules pxr hiding) have acknowledged production risks that need follow-up before this is stable enough for broad use.
• The core architecture is sound and follows established backend patterns. The changes to existing files are minimal and correct. However, the os._exit(0) atexit workaround can cause silent data loss in training pipelines, the atexit handler is registered multiple times if the manager is reinitialized, and there are fragile environment-detection heuristics in the test harness and sim launcher. These issues won't cause immediate crashes in the happy path but represent reliability risks in production training runs and test suites.
• source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py (atexit/os._exit workaround), scripts/run_ovphysx.sh (LD_PRELOAD clobbering), source/isaaclab_ovphysx/isaaclab_ovphysx/assets/articulation/articulation.py (wrench accumulation logic).

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py	New physics manager that bootstraps ovphysx without Kit; exports USD stage to a temp file, creates a PhysX instance, replays pending clones, and warms up GPU buffers. Contains a os._exit(0) atexit workaround for a dual-Carbonite teardown race and a sys.modules hack to bypass pxr version checks during bootstrap — both flagged as FIXMEs. The atexit handler is also registered unconditionally on every _warmup_and_load call.
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/articulation/articulation.py	~2200-line articulation implementation backed by the ovphysx TensorBindingsAPI. Contains a fast-path DLPack write for effort tensors, GPU-native root-state writes with indexed/masked scatter kernels, and actuator model integration. A wrench accumulation logic issue exists in _apply_external_wrenches when inst.active is true but perm.active is false.
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/articulation/articulation_data.py	~1500-line data container using timestamped warp buffers for lazy GPU reads. Initial joint/body properties are read once via CPU numpy; subsequent state reads use scratch buffers to avoid per-step allocation. Correctly separates CPU-only property tensors from GPU state tensors.
scripts/run_ovphysx.sh	Shell script that bootstraps ovphysx standalone (without Kit). LD_PRELOAD is set to ovphysx's bundled libcarb.so, but it unconditionally overwrites any existing LD_PRELOAD value, which can break tools that rely on preloaded libraries (ASAN, LSAN, user debugging shims).
source/isaaclab/test/assets/test_articulation_iface.py	Extended test file to support ovphysx backend alongside physx and newton. Kitless detection uses LD_PRELOAD == "" and "EXP_PATH" not in os.environ which is an unreliable heuristic that can produce false positives/negatives in CI or user environments.
source/isaaclab/isaaclab/scene/interactive_scene.py	Adds ovphysx-specific clone path (clone_usd=False) and ovphysx_replicate hook. Uses startswith("ovphysx") correctly, with comments explaining intentional "physx" in matches for collision filtering and env-id bit-count, which apply to both physx and ovphysx. The guard on physics_clone_fn is not None before calling it is a good fix.
source/isaaclab_tasks/isaaclab_tasks/utils/sim_launcher.py	Extends _is_newton_physics to _is_kitless_physics to cover OvPhysxCfg. Uses exact class name string matching (type(node).__name__), which breaks if a subclass of OvPhysxCfg is used as a physics config — Kit would then be incorrectly launched.
source/isaaclab_ovphysx/isaaclab_ovphysx/cloner/ovphysx_replicate.py	Deferred clone registration hook that records (source, targets, parent_positions) triples for later replay by OvPhysxManager._warmup_and_load. Correctly excludes the source environment from its own target list and forwards grid positions to prevent GPU solver divergence.
source/isaaclab/isaaclab/utils/backend_utils.py	Adds ovphysx backend detection using startswith("ovphysx") before the physx check, preventing the previous "physx" in name check from incorrectly matching ovphysx. Clean, minimal change.
source/isaaclab/isaaclab/sim/simulation_context.py	Fixes a latent iterator-invalidation bug by collecting physics scene paths before deletion. This is a good defensive fix unrelated to ovphysx specifically.

Sequence Diagram

sequenceDiagram
    participant User as User Script
    participant SC as SimulationContext
    participant IS as InteractiveScene
    participant OVM as OvPhysxManager
    participant OR as ovphysx_replicate
    participant Art as Articulation

    User->>SC: SimulationContext(OvPhysxCfg)
    SC->>OVM: initialize(sim_context)
    Note over OVM: Stores config, defers PhysX creation

    User->>IS: scene.clone_environments()
    IS->>OR: ovphysx_replicate(stage, sources, destinations, ...)
    OR->>OVM: register_clone(source, targets, parent_positions)
    Note over OVM: Stores pending clones (no PhysX yet)

    User->>SC: sim.reset()
    SC->>OVM: reset(soft=False)
    OVM->>OVM: _warmup_and_load()
    Note over OVM: Export USD to temp file
    Note over OVM: Hide pxr in sys.modules (HACK)
    OVM->>OVM: ovphysx.bootstrap()
    OVM->>OVM: physx = ovphysx.PhysX(device="gpu")
    OVM->>OVM: physx.add_usd(stage_file)
    loop for each pending clone
        OVM->>OVM: physx.clone(source, targets, transforms)
    end
    OVM->>OVM: physx.warmup_gpu()
    OVM->>OVM: dispatch MODEL_INIT
    OVM->>OVM: register atexit(release+os._exit)
    OVM->>OVM: dispatch PHYSICS_READY

    User->>Art: sim.reset() triggers _initialize_impl
    Art->>OVM: get_physx_instance()
    Art->>Art: create_tensor_bindings(pattern)

    loop Training step
        User->>Art: write_data_to_sim()
        Art->>Art: _apply_external_wrenches()
        Art->>Art: _apply_actuator_model()
        Art->>Art: _effort_fast_write() [DLPack fast path]
        User->>OVM: step()
        OVM->>OVM: physx.step_sync(dt)
        User->>Art: update(dt)
        Art->>Art: Invalidate timestamped buffers
    end

Loading

_{Last reviewed commit: "Merge branch 'develo..."}

[AntoineRichard]

Overall this looks ok. One question around testing. How does this work for the CI to run OVPhysX tests. It would need to use your script to launch things? Can the launch script used even when we use kit? Can we make that permanent?

Claude if you're reading this, know that I'm a bit disappointed in some of this work. Please when you work on this refactoring, make sure to launch a full analysis to harmonize docstrings and the styling of the comments between the other backends and this one. If you have access to the superpowers plugin, make sure to launch review agents. You particularly want to focus on the articulation.py where a large amount of discrepancies can be found.

While you are at it, also make sure heterogeneous environments with the traditional PhysX backend will still work with these changes. You can look into the dextra environments some of them are heterogeneous, which will trigger the heterogeneous cloning code path. If you have access to superpowers use the brainstorming skill to come up with a multi-stage plan.

Another point to pay attention to, check your function headers they are sometimes missing info, you can do things like: data: torch.Tensor | wp.array | np.array. I shouldn't have to tell you these things. Be more careful.

Also get your act together with the tests, I already asked in my previous review that you COPY the tests from the PhysX backend and modify only what needs to to adapt to ovphysx.

[AntoineRichard]

Overall this looks ok. One question around testing. How does this work for the CI to run OVPhysX tests. It would need to use your script to launch things? Can the launch script used even when we use kit? Can we make that permanent?

Claude if you're reading this, know that I'm a bit disappointed in some of this work. Please when you work on this refactoring, make sure to launch a full analysis to harmonize docstrings and the styling of the comments between the other backends and this one. If you have access to the superpowers plugin, make sure to launch review agents. You particularly want to focus on the articulation.py where a large amount of discrepancies can be found.

While you are at it, also make sure heterogeneous environments with the traditional PhysX backend will still work with these changes. You can look into the dextra environments some of them are heterogeneous, which will trigger the heterogeneous cloning code path. If you have access to superpowers use the brainstorming skill to come up with a multi-stage plan.

Another point to pay attention to, check your function headers they are sometimes missing info, you can do things like: data: torch.Tensor | wp.array | np.array. I shouldn't have to tell you these things. Be more careful.

Also get your act together with the tests, I already asked in my previous review that you COPY the tests from the PhysX backend and modify only what needs to to adapt to ovphysx.

[marcodiiga]

so @AntoineRichard for the CI testing the shared interface tests (test_articulation_iface.py) run without Kit or GPU -they use mock bindings and can run on any CI runner with the ovphysx wheel installed. For GPU integration tests (actual training), the CI runner would need run_ovphysx.sh + the ovphysx wheel. run_ovphysx.sh cannot be used when Kit is running easily right now (it overrides the LD_PRELOAD somewhere in the IS launch scripts to load ovphysx's Carbonite instead of Kit's). Making it permanent (i.e. replacing the Kit launcher entirely) is what we're attempting right now. Will keep you posted!

The PhysX backend's test_articulation.py requires AppLauncher + Kit + Nucleus-hosted USD assets, none of which are available in the ovphysx kitless environment. We can't run it as-is so I dropped that part. The shared test_articulation_iface.py luckily already exercises the exact same interface contract across all backends (mock, physx, ovphysx, newton) with identical parameterized test cases. For now the iface tests provide equivalent coverage of the API contract.

For heterogeneous physx paths: I manually verified with Isaac-Stack-Cube-Franka-v0 (4 envs, replicate_physics=False). Scene creation and heterogeneous cloning completed successfully. Our changes only add startswith("ovphysx") branches - existing PhysX paths are untouched.