Update OvPhysX to 0.5.10 - #7140
Conversation
OvPhysX 0.5.10 requires ovstage 0.1.1.355824, so the two move together: bumping ovphysx alone leaves the environment unresolvable. The code change this depends on is already on develop. OvPhysX reads sealed data only, so attaching at an unsealed ordinal silently yields an empty scene, which surfaced as a null joint-friction buffer during articulation setup. `advance_write_floor` in the OvPhysX manager commits the ordinal first. Verified locally on a clean environment: Isaac-Velocity-Flat-AnymalD with physics=ovphysx trains, which is the articulation path that failed before the fix. That is a smoke test rather than a soak.
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
There was a problem hiding this comment.
Isaac Lab Review Bot
The OvPhysX and canonical ovstage versions were updated, but the root OV extras still pin the previous ovstage build, leaving dependency metadata inconsistent and the environment unresolvable.
- Design and architecture: The single-source version table is not mirrored consistently:
[tool.isaaclab.versions].ovstageandisaaclab_ovrequire 0.1.1.355824, while theov,ovphysx, andovrtxextras retain 0.1.0.346039. All mirrored pins must be updated together. - API: The changed dependency surface exposes conflicting exact ovstage constraints when the OV extras are installed with
isaaclab_ov; the extras should all reference ovstage 0.1.1.355824. - Implementation:
test_version_single_source_matches_literal_pinsrequires the canonical ovstage specification in both theovphysxandovrtxextras, so the current metadata will fail that test. Update all three root OV extras to the new ovstage build.
Significant concerns. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| ov = ["ovphysx==0.5.9", "ovrtx==0.4.1.364340", "ovstage==0.1.0.346039"] | ||
| ovphysx = ["ovphysx==0.5.9", "ovstage==0.1.0.346039"] | ||
| ov = ["ovphysx==0.5.10", "ovrtx==0.4.1.364340", "ovstage==0.1.0.346039"] | ||
| ovphysx = ["ovphysx==0.5.10", "ovstage==0.1.0.346039"] |
There was a problem hiding this comment.
🔴 Critical · Design Architecture — ovstage pin drifts from versions table
[tool.isaaclab.versions].ovstage now reads 0.1.1.355824 and source/isaaclab_ov/pyproject.toml requires that exact build, but the ov, ovphysx, and ovrtx extras still pin ovstage==0.1.0.346039. test_version_single_source_matches_literal_pins asserts spec("ovstage") appears in optional["ovphysx"] and optional["ovrtx"], so it fails, and installing any OV extra alongside isaaclab_ov yields conflicting ovstage constraints. Mirror 0.1.1.355824 in all three extras.
Greptile SummaryThis PR updates the OvPhysX and matching OvStage dependency versions and regenerates the lockfile, but leaves the root OV extras pinned to the previous OvStage build.
Confidence Score: 4/5The PR should not merge until the root OV extras use the matching OvStage build so fresh resolution and lock validation succeed. The workspace and lockfile require OvStage 0.1.1.355824, while the changed root extras combine OvPhysX 0.5.10 with an incompatible exact pin to OvStage 0.1.0.346039. Files Needing Attention: pyproject.toml Important Files Changed
Reviews (1): Last reviewed commit: "Merge branch 'develop' into antoiner/ovp..." | Re-trigger Greptile |
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
| ov = ["ovphysx==0.5.10", "ovrtx==0.4.1.364340", "ovstage==0.1.0.346039"] | ||
| ovphysx = ["ovphysx==0.5.10", "ovstage==0.1.0.346039"] |
There was a problem hiding this comment.
Stale OvStage pins break resolution
When a user resolves the ov, ovphysx, or transitive all extra, these declarations require ovstage==0.1.0.346039 while isaaclab-ov and the lockfile require ovstage==0.1.1.355824, causing fresh dependency resolution, uv lock --check, and the literal-pin synchronization test to fail.
Signed-off-by: Kelly Guo <kellyg@nvidia.com>
| ov = ["ovphysx==0.5.9", "ovrtx==0.4.1.364340", "ovstage==0.1.0.346039"] | ||
| ovphysx = ["ovphysx==0.5.9", "ovstage==0.1.0.346039"] | ||
| ovrtx = ["ovrtx==0.4.1.364340", "ovstage==0.1.0.346039"] | ||
| ov = ["ovphysx==0.5.10", "ovrtx==0.4.1.364340", "ovstage==0.1.0.346039"] |
There was a problem hiding this comment.
should we update the "ovstage==0.1.0.346039" version here
|
closing in favor of #7150 |
## Summary - Updates OvPhysX to `0.5.10` and OVStage to `0.1.1.355824` while retaining OVRTX `0.4.1.364340`. - Adds the OVStage proprietary-license alias and regenerates `uv.lock`. - Supersedes #7140; thanks to @AntoineRichard for the original dependency work. #7143 remains validation-only evidence and will not be merged. ## Validation - Resolved and imported the exact OvPhysX, OVStage, and OVRTX versions. - Focused dependency, renderer, runtime, and OvPhysX environment tests: 105 passed. - Six-file legacy rendering sweep: 36 passed, 36 intentional unsupported-preset skips, no retries, and no golden changes. - Focused OVStage rendering sweep: 36 passed, 36 intentional skips, no retries, and no golden changes. - Lockfile, changelog, pre-commit, and diff checks passed. ## Type of change - Dependency update ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there
Summary
ovphysxto 0.5.10 andovstageto the matching 0.1.1.355824 build (OvPhysX 0.5.10 requires this exact ovstage build; bumping ovphysx alone leaves the environment unresolvable).ovrtxpin to>=0.4.0,<0.5.0.Test plan
uv run isaaclab -fuv lock --check