[hujc7]

Review Map

• Exact changes: a stacked PR's page shows the cumulative diff of its dependency chain; the link pins the commit range that is the PR's own contribution.
• Links pin specific SHAs and can go stale after a branch update — the table on [DO-NOT-MERGE][Task Clean-up] Dexterous: lumped validation branch (series reference) #6324 is refreshed first.

PR	Status	Depends on	Exact changes
#6410 [Docs] Environment overview regen		—	—
#6411 Part 1/8: Newton cloner/cubric/visualizer fixes		—	—
📌 #6412 Part 2/8: OVPhysX articulation + manager runtime (this PR)		—	—
#6413 Part 3/8: Reorient Direct, torch		—	—
#6414 Part 4/8: Handover + camera Direct, torch		#6413	changes
#6418 Part 5/8: Reorient manager counterparts		#6413	changes
#6421 Part 6/8: Handover + camera manager counterparts		#6413, #6414, #6418	changes
#6415 Part 7/8: Benchmark success-rate utilities + docs		#6413, #6414, #6418, #6421	changes
#6582 Part 8/8: Warp variants → experimental (draft; merges last)		#6413	changes
#6324 [DO-NOT-MERGE] Lumped validation reference		ALL	—

Summary

• Fixes OVPhysX articulation handling and manager runtime behavior for the dexterous task family.

Stacking

• Independent; based on develop.

Review history

• Approved; unchanged since approval apart from develop syncs.

[greptile-apps]

Greptile Summary

This PR fixes three OVPhysX correctness issues: actuator joint indices now follow the common isaaclab contract (slice(None) for full-coverage, torch.int32 tensor for partial), Kit's pre-registered physxSchema provider is respected to avoid duplicate-type errors, and PhysX initialization/stepping/reset are refactored into helpers that dispatch across both the declared public and the current runtime API.

• Articulation (articulation.py): _process_actuators_cfg computes actuator_joint_ids as slice(None) or torch.int32 tensor before constructing each actuator; _apply_actuator_model switches from act.joint_indices to the always-list _joint_ids_per_actuator dict.
• Manager (ovphysx_manager.py): Extracts _create_physx_instance, _step_physx, and _reset_physx_stage as @staticmethod helpers; _ensure_physx_schemas_registered now short-circuits if Kit already owns physxSchema.
• Tests: New assertions confirm joint_indices dtype and device for partial actuators; new backend tests mock both runtime variants end-to-end.

Confidence Score: 3/5

The current-API path used in training is well-exercised; the declared-legacy path has a real gap where four carbonite overrides that suppress USD write-back are silently skipped when set_setting is absent.

The declared-legacy path in _create_physx_instance calls only set_config_int32(NUM_THREADS, 8) when set_setting is unavailable, silently omitting physxDispatcher, updateToUsd, updateVelocitiesToUsd, and updateParticlesToUsd. If those settings default to enabled in that API variant, physics data would be written to USD every step. The inspect.signature call on line 743 also has no guard against C extension types that do not expose a Python signature, which would crash initialization entirely.

ovphysx_manager.py lines 742-763 (_create_physx_instance legacy branch) and articulation.py line 3930 (torch.int32 index tensor).

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py	Adds dual-API support for OVPhysX initialization, stepping, and stage reset. The legacy path that uses set_config_int32 silently drops four critical carbonite overrides, and inspect.signature is called without a guard against C extension failures.
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/articulation/articulation.py	Fixes actuator joint indexing contract with slice(None) or torch.int32 tensor; decouples _apply_actuator_model from act.joint_indices. The int32 dtype may cause PyTorch advanced-indexing errors on some bundled versions.
source/isaaclab_ovphysx/test/physics/test_ovphysx_scene_data_backend.py	Adds three new unit tests covering declared legacy runtime, current runtime, and Kit physxSchema provider guard. Does not assert physxDispatcher/updateToUsd overrides in the legacy set_config_int32 path.
source/isaaclab_ovphysx/test/assets/test_articulation.py	Adds assertions for the new joint_indices contract: slice(None) for full-coverage and a device-matching torch.int32 tensor for partial-coverage actuators.
source/isaaclab_ovphysx/changelog.d/task-cleanup-dex-part02.rst	New changelog entry summarising the three fixes included in this PR.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_create_physx_instance] --> B{hasattr PhysX set_cpu_mode?}
    B -- Yes/current --> C[set_cpu_mode + PhysXConfig full overrides]
    B -- No/legacy --> D[inspect.signature PhysX.parameters]
    D --> E{active_cuda_gpus in params AND gpu?}
    E -- Yes --> F[PhysXConfig suppressReadback only]
    E -- No --> G{gpu_index in params?}
    G -- Yes --> H[add gpu_index to kwargs]
    G -- No --> I[device only]
    F --> J[physx = PhysX kwargs]
    H --> J
    I --> J
    J --> K{hasattr physx set_setting?}
    K -- Yes --> L[set_setting all overrides]
    K -- No --> M[set_config_int32 NUM_THREADS only - missing physxDispatcher updateToUsd etc]

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"}}}%%
flowchart TD
    A[_create_physx_instance] --> B{hasattr PhysX set_cpu_mode?}
    B -- Yes/current --> C[set_cpu_mode + PhysXConfig full overrides]
    B -- No/legacy --> D[inspect.signature PhysX.parameters]
    D --> E{active_cuda_gpus in params AND gpu?}
    E -- Yes --> F[PhysXConfig suppressReadback only]
    E -- No --> G{gpu_index in params?}
    G -- Yes --> H[add gpu_index to kwargs]
    G -- No --> I[device only]
    F --> J[physx = PhysX kwargs]
    H --> J
    I --> J
    J --> K{hasattr physx set_setting?}
    K -- Yes --> L[set_setting all overrides]
    K -- No --> M[set_config_int32 NUM_THREADS only - missing physxDispatcher updateToUsd etc]

Loading

_{Reviews (1): Last reviewed commit: "Fix OVPhysX articulation indexing and ma..." | Re-trigger Greptile}

[greptile-apps]

Missing carbonite overrides in legacy path when set_setting is unavailable

The else branch only applies NUM_THREADS via set_config_int32, silently skipping physxDispatcher, updateToUsd, updateVelocitiesToUsd, and updateParticlesToUsd. The PhysXConfig passed to the constructor (lines 746–751) only contains suppressReadback and suppressFabricUpdate, so these four settings are never applied.

If the declared legacy wheel defaults updateToUsd to True, physics state would be written back to the USD stage on every step — causing severe performance degradation and potentially inconsistent state. The original code (before this PR) applied all four overrides unconditionally via carbonite_overrides. The new test for the legacy API (test_manager_supports_declared_legacy_runtime_api) only asserts that set_config_int32(NUM_THREADS, 8) is called, leaving this gap unverified.

[greptile-apps]

inspect.signature may fail on C extension types without recovery

inspect.signature(ovphysx.PhysX) is called without a try/except guard. For C extension types (e.g., pybind11 or Cython bindings) that do not expose a Python-visible __init__ signature via __text_signature__, this raises ValueError: callable ... is not supported by signature. The caller (_warmup_and_load) does not catch this, so PhysX initialization would abort with an unhandled exception. A try/except (ValueError, TypeError) fallback that treats missing parameters as empty (and proceeds to the set_setting/set_config_int32 post-construction path) would make this branch safe.

[greptile-apps]

torch.int32 index tensor may not be accepted by all PyTorch advanced-indexing paths

actuator_joint_ids is created with dtype=torch.int32 and is immediately used to index several .torch tensors at lines 3943–3950. Standard PyTorch advanced indexing expects int64 (LongTensor); older bundled PyTorch versions raise RuntimeError: expected scalar type Long but found Int for int32 index tensors. Using dtype=torch.int64 (or inserting .long() casts at the indexing sites) would eliminate this version dependency.

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!

[AntoineRichard]

AI-generated review comment: source/isaaclab_ovphysx/test/physics/test_ovphysx_scene_data_backend.py:6 still describes this module as containing only OvPhysxSceneDataBackend unit tests, but this PR adds several OvPhysxManager tests to the same file. Please broaden the module docstring so it accurately describes both the scene-data backend and manager coverage.

[AntoineRichard]

Looks Ok to me.