Skip to content

Fix latest OV stack integration - #7626

Draft
kellyguo11 wants to merge 2 commits into
isaac-sim:developfrom
kellyguo11:fix/ovstack-latest-integration
Draft

Fix latest OV stack integration#7626
kellyguo11 wants to merge 2 commits into
isaac-sim:developfrom
kellyguo11:fix/ovstack-latest-integration

Conversation

@kellyguo11

@kellyguo11 kellyguo11 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Description

Fix the Isaac Lab integration boundaries exposed by testing the latest internal OV stack:

  • Resolve the exact omniverseclient release embedded in the installed OvPhysX wheel and install it before importing OvPhysX in wheelhouse CI jobs.
  • Reject manually assembled OvPhysX environments early with an actionable version-mismatch error instead of allowing native plugin initialization to fail later.
  • Source OVRTX OVStage rigid transforms from the active SceneDataBackend, enabling OvPhysX as well as Newton while preserving authored scale and world-pose semantics.
  • Register the OvPhysX codeless schema root with OVStage's separate population registry, preserving PhysX contact-report and articulation-tendon schemas when OvPhysX 0.6.1+ populates a stage.

The original internal run used omniverseclient==2.72.3 with an OvPhysX wheel whose ovstage-omniclient.version requires 2.74.0-release.7316+gl.ec99a64b. That mismatch caused 544 of 569 isaaclab_ov failures to share the same ovphysx_create_instance() startup error. Installing the marker-selected release removes that cascade.

The subsequent rendering run exposed 22 additional physics failures. The USD stage contained the required PhysX schemas, but OVStage population dropped them because its schema registry is independent of the host USD Plug.Registry. Registering ovphysx.codeless_schema_root() with ovstage.population.register_usd_schemas() fixes all 11 contact-reporter and all 11 fixed-tendon rendering cases.

Original run: https://github.com/NVIDIA-Omniverse/IsaacLab-Internal/actions/runs/33908292244

Baseline run: https://github.com/NVIDIA-Omniverse/IsaacLab-Internal/actions/runs/33908863254

The current public package pins remain unchanged; the dynamic OmniClient resolution is limited to internal wheelhouse overlays. Schema registration is feature-gated for compatibility with the currently pinned public OvPhysX/OVStage stack.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Validation

Tested with:

  • OVRTX 0.5.0.377615
  • OvPhysX 0.6.2
  • OVStage 0.2.0.377349
  • OmniClient 2.74.0
  • Newton 1.6.0rc1
  • NVIDIA RTX PRO 6000 Blackwell, driver 595.58.03

Passing focused checks:

  • 19 passed: OvPhysX manager lifecycle and schema-registration tests.
  • Contact module, split by device: CUDA 14 passed, 4 skipped; CPU 15 passed, 4 skipped.
  • GPU tendon-focused articulation tests: 3 passed, 2 skipped.
  • Homogeneous-Kuka contact filtering and fixed-tendon target-write regressions: passed.
  • 60 passed: runtime-import, OVRTX renderer-contract, and deformable-binding focused tests from the first integration pass.
  • uv run isaaclab -f.

Full rendering correctness after the schema fix:

Path Before After Physics failures after
OVRTX 0.5 legacy 15 failed, 39 passed, 36 skipped 4 failed, 50 passed, 36 skipped 0
OVRTX 0.5 + OVStage 0.2 41 failed, 3 passed, 32 skipped 39 failed, 5 passed, 32 skipped 0

All 22 physics-caused rendering failures are fixed. The remaining failures are image comparisons: four on the legacy path and 39 on the OVStage path. Nine image mismatches were previously masked by the physics initialization failures.

A separate CPU-only Shadow Hand tendon run now reaches an OvPhysX 0.6.2 native segmentation fault in PhysX.step_sync. The GPU path used by rendering passes; this appears to be an upstream OvPhysX CPU issue and is not hidden by this change.

Screenshots

The investigation generated full golden/actual/diff galleries and JUnit XML as a local test artifact. They are intentionally not committed because they are diagnostic output rather than source assets.

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 or that my feature works
  • I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@kellyguo11
kellyguo11 requested a review from a team September 7, 2026 21:49
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team infrastructure labels Sep 7, 2026
@kellyguo11

Copy link
Copy Markdown
Contributor Author

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 7, 2026
@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR repairs integration with newer OvPhysX/OVStage/OVRTX packages by deriving the required OmniClient release from installed OvPhysX metadata and publishing rigid transforms through the active scene-data backend.

  • Resolves and installs the OvPhysX-compatible OmniClient release in wheelhouse CI.
  • Adds an actionable runtime error for incompatible manually assembled environments.
  • Generalizes OVStage rigid-transform synchronization from Newton-specific state to the active physics backend.
  • Preserves authored scale while publishing backend-provided world poses.
  • Adds focused runtime-import and renderer-contract coverage.

Confidence Score: 5/5

The PR appears safe to merge, with the changed dependency-resolution and transform-publication paths matching the current runtime and backend contracts.

No concrete behavioral, security, or repository-rule violation remains after checking the runtime initialization order, CI execution context, compatibility-marker behavior, and current Newton and OvPhysX scene-data implementations.

Important Files Changed

Filename Overview
.github/actions/run-tests/run_tests.sh Resolves and force-reinstalls the OmniClient release required by the wheelhouse-provided OvPhysX package before runtime validation.
tools/resolve_ovphysx_omniclient.py Adds a metadata-only utility that emits the exact pip requirement derived from the installed OvPhysX compatibility marker.
source/isaaclab_ov/isaaclab_ov/_runtime.py Parses OvPhysX compatibility metadata and rejects mismatched OmniClient installations before the demonstrated native bootstrap path.
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py Sources OVStage rigid paths and world transforms from the active scene-data backend instead of directly depending on Newton state.
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer_kernels.py Renames and generalizes the rigid-transform composition kernel for backend-independent physics transforms.
source/isaaclab_ov/test/test_ovrtx_renderer_contract.py Verifies active-backend transform selection, object filtering, authored-scale preservation, world matrices, and transform-stack reset behavior.
source/isaaclab_ov/test/test_runtime_imports.py Covers compatibility-marker parsing, malformed metadata, and actionable mismatch reporting.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  W[Installed OvPhysX wheel] --> M[Read OVStage OmniClient marker]
  M --> V[Resolve omniverseclient version]
  V --> C[Install matched dependency in wheelhouse CI]
  V --> R[Validate runtime environment]
  R --> P[Bootstrap OvPhysX]

  S[SimulationContext] --> D[Active SceneDataBackend]
  D --> T[World rigid transforms and paths]
  A[Authored USD scales] --> K[Transform composition kernel]
  T --> K
  K --> O[OVStage omni:xform updates]
Loading

Reviews (1): Last reviewed commit: "Fix latest OV stack integration" | Re-trigger Greptile

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isaac Lab Review Bot

The OvPhysX/OmniClient compatibility guard and backend-agnostic OVStage transform sourcing are internally consistent, but the changed exception contract of import_ovphysx needs to be documented.

  • Design and architecture: Using the active SceneDataBackend for OVStage rigid transforms cleanly generalizes the path beyond Newton while preserving scale and world-pose handling. The stricter missing-marker behavior in the CI resolver is appropriate for wheelhouse jobs that must resolve an exact matched dependency and need not mirror the runtime fallback.
  • API: import_ovphysx now raises RuntimeError for malformed compatibility metadata or an OmniClient mismatch, but its Raises section still documents only ModuleNotFoundError. The public helper’s documented exception contract should include this new failure mode.
  • Implementation: The marker parsing, installed-distribution lookup, CI installation path, backend transform indexing, Warp kernel inputs, zero-copy write ordering, and teardown state were traced and are internally consistent. The private kernel and state renames are updated across their consumers and tests.

Minor fixes needed. Posted 1 actionable finding inline.

Automated review; human maintainers own approval decisions.

if exc.name != "ovphysx":
raise
raise ModuleNotFoundError(_OVPHYSX_INSTALL_MESSAGE, name="ovphysx") from exc
_validate_ovphysx_omniverseclient()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Suggestion · Api — New RuntimeError not in documented Raises

_validate_ovphysx_omniverseclient() can raise RuntimeError for a mismatched or malformed marker, but the import_ovphysx docstring still declares only ModuleNotFoundError. Callers that guard optional-backend selection with except ModuleNotFoundError will now see an undocumented error escape. Add the RuntimeError case to the Raises: section so the stated contract matches the implementation.

@kellyguo11
kellyguo11 marked this pull request as draft September 7, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant