Re-generate lock, bump pins, fix platforms - #6768
Conversation
Greptile SummaryUpdates the supported-platform dependency resolution and regenerates the lockfile.
Confidence Score: 5/5The PR appears safe to merge with no concrete changed-code-triggered failure identified. The dependency metadata, lock resolution, platform restrictions, tests, and duplicated Newton override files remain internally aligned, and no affected transitive dependency was shown to be reachable through a vulnerable repository runtime path. Important Files Changed
Reviews (1): Last reviewed commit: "Re-generate lock, bumpy pins, fix platfo..." | Re-trigger Greptile |
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
Summary
This PR refreshes dependency metadata: it bumps the Isaac Sim pin to 6.0.1.0 and the Newton git commit (mirrored into both uv-overrides.txt copies), rewrites platform_machine in 'a b c' substring markers into explicit equality disjunctions, adds a [tool.uv] environments resolution universe, introduces a new aggregate ov extra with its isaacsim conflict entry, and regenerates uv.lock. The marker rewrites are the right fix (PEP 508 in against a space-joined string is substring matching, which silently matched unintended values) and the pin bumps are consistent across pyproject, the two override files, and the lock. Remaining comments are about drift guards for the newly duplicated OV/Newton pins, the untested new conflict entry, and the now-unreachable arm64 / Linux-AMD64 marker branches.
Architecture impact
No runtime or API surface changes. The notable structural additions are the [tool.uv] environments universe (which pins the lock to three supported platforms) and the new ov aggregate extra, both of which affect how contributors resolve and lock the environment.
Test coverage
The two pyproject-metadata unit tests were updated to track the new marker spelling and the ov extra, but coverage is shallow: the new ov pins are checked only with startswith, so they escape test_version_single_source_matches_literal_pins; the new isaacsim/ov conflict entry is not asserted anywhere; and the duplicated Newton pin in the two uv-overrides.txt files has no sync check. No new tests cover the environments block.
Implementation verdict
Minor fixes needed. Posted 6 actionable findings inline.
The PR exceeded the automated context budget, so some file content was truncated.
Automated comment-only review; human maintainers own approval decisions.
|
|
||
| isaacsim = ["isaacsim[all,extscache]==6.0.1.0"] | ||
|
|
||
| ov = ["ovphysx==0.5.9", "ovrtx>=0.4.0,<0.5.0", "ovstage>=0.1.0,<0.2.0"] |
There was a problem hiding this comment.
🟡 Warning — New ov extra duplicates OV pins outside the version drift guard
The ov extra hard-codes a third copy of ovphysx==0.5.9, ovrtx>=0.4.0,<0.5.0 and ovstage>=0.1.0,<0.2.0. test_version_single_source_matches_literal_pins only inspects optional["ovphysx"] and optional["ovrtx"], and the new assertions in test_uv_run_pyproject.py use startswith, so they pass for any version. The next OV bump can leave this copy stale with CI green, and uv sync --extra ov would then install a different stack than --extra ovphysx --extra ovrtx. Please extend test_version_single_source_matches_literal_pins with assert spec("ovphysx") in optional["ov"], assert spec("ovrtx") in optional["ov"] and assert spec("ovstage") in optional["ov"].
| [{ extra = "teleop" }, { extra = "mimic" }], | ||
| [{ extra = "teleop" }, { extra = "all" }], | ||
| [{ extra = "isaacsim" }, { extra = "teleop" }], | ||
| [{ extra = "isaacsim" }, { extra = "ov" }], |
There was a problem hiding this comment.
🟡 Warning — New isaacsim/ov conflict is not asserted by any test
test_uv_run_isaacsim_extra_is_conflict_forked iterates only over ("teleop", "ovphysx", "viser", "mimic", "all", "test"). If this newly added conflict pair is dropped later, no test fails even though uv sync --extra isaacsim would stop resolving — which is exactly why the pair was added. Add "ov" to that loop's tuple.
| # install CI matrix): Ubuntu 22.04 x86_64, Linux aarch64 (DGX Spark / Jetson), and | ||
| # Windows 11 x86_64. Restricting the resolution universe keeps macOS and other | ||
| # unsupported wheels out of uv.lock. | ||
| environments = [ |
There was a problem hiding this comment.
🔵 Suggestion — Resolution universe leaves arm64 / Linux-AMD64 marker branches unreachable
The new universe covers only linux/x86_64, linux/aarch64 and win32/AMD64, but the dependency markers still enumerate platform_machine == 'arm64' (usd-exchange, pytetwild[all], hf-xet, the pink IK stack) and [tool.uv.sources] still maps torch/torchvision/torchaudio for sys_platform == 'linux' and platform_machine == 'AMD64' / 'arm64'. Those branches can no longer be selected. If a supported host ever reports arm64 under Linux, uv sync now aborts with "no environment matches" instead of falling back to the cu130 index; if no supported host does, these are dead entries that mislead future readers. Please either add the matching environment or drop the unreachable branches in this PR.
| ] | ||
|
|
||
| assert usd_core_dependencies == ["usd-core>=25.11,<26.0 ; platform_machine in 'x86_64 AMD64'"] | ||
| assert usd_core_dependencies == [ |
There was a problem hiding this comment.
🔵 Suggestion — Marker fix is regression-tested for only two of the eight rewritten requirements
Eight requirement strings moved off the buggy platform_machine in '<space-separated list>' form, but only usd-core and usd-exchange are pinned by exact-string assertions. Reintroducing the substring form for pin, pin-pink, daqp, hf-xet, isaacteleop or dex-retargeting would not fail anything. A single scan over [project.dependencies] and [project.optional-dependencies] asserting no requirement contains platform_machine in ' would make the fix genuinely regression-proof instead of hard-coding two literals.
| mujoco~=3.10.0 | ||
| mujoco-warp~=3.10.0,>=3.10.0.2 | ||
| newton[sim,importers] @ git+https://github.com/newton-physics/newton.git@d7581b732d83843577411d36f6a58cccee909bce | ||
| newton[sim,importers] @ git+https://github.com/newton-physics/newton.git@92ae94541517104957e9b51b24d528b13dde223d |
There was a problem hiding this comment.
🔵 Suggestion — Missing changelog fragment for the touched isaaclab package
Repository guidelines require one fragment under source/<pkg>/changelog.d/ for every touched package. This PR modifies files under source/isaaclab/ (this override file plus the two CLI metadata tests) but adds no fragment. Since these edits are test/CI-only, add source/isaaclab/changelog.d/<slug>.skip so the nightly changelog job does not flag the package (use a .rst fragment instead if the Isaac Sim / Newton pin bump should be user-visible).
| mujoco~=3.10.0 | ||
| mujoco-warp~=3.10.0,>=3.10.0.2 | ||
| newton[sim,importers] @ git+https://github.com/newton-physics/newton.git@d7581b732d83843577411d36f6a58cccee909bce | ||
| newton[sim,importers] @ git+https://github.com/newton-physics/newton.git@92ae94541517104957e9b51b24d528b13dde223d |
There was a problem hiding this comment.
🔵 Suggestion — Newton pin now lives in three hand-maintained places with no sync check
The Newton commit is duplicated in [tool.uv].override-dependencies, this file, and source/isaaclab/test/install_ci/uv_pip/uv-overrides.txt. Only the pyproject copy is validated (any(dep.endswith(f"newton.git@{versions['newton']}") ...)). A future bump that misses one text file would silently build the wheel — or run install CI — against a different Newton revision. Consider adding a unit test under source/isaaclab/test/cli/ that asserts both override files match [tool.uv].override-dependencies line for line (keep it outside install_ci/ so that directory stays self-contained).
The bumped Newton commit imports static visual shapes and draws colliders only when USD says they are drawn. In the synthetic gaussian scene that made the ground plane's visual mesh cover the image center, which is what the shared PPISP assertions probe -- and that probe was reading the ground plane, not the gaussians, so it went to zero. Put a gaussian at the origin so the probe measures gaussian radiance regardless of the backdrop, and retune responsivity for that signal (the previous value saturates against a gaussian center). The same import change restores the missing table in the Newton Warp renderer, so drop the OMPE-103086 skips from the Franka soft and cloth rendering tests. Add the changelog fragment for the pin bump.
| Added | ||
| ^^^^^ | ||
|
|
||
| * Added an ``ov`` install extra that pulls both Omniverse backends (``ovphysx`` and ``ovrtx``); |
| # ``ov`` installs both Omniverse backends; ``ovphysx`` / ``ovrtx`` select one. | ||
| assert any(dep.startswith("skrl") for dep in optional_dependencies["skrl"]) | ||
| assert any(dep.startswith("ovphysx") for dep in optional_dependencies["ov"]) | ||
| assert any(dep.startswith("ovrtx") for dep in optional_dependencies["ov"]) |
There was a problem hiding this comment.
add test for ovstage here too
The bumped Newton pin imports static visual shapes and draws colliders only when USD says they are drawn, so the table now renders as its visual mesh instead of its collider and the ground plane arrives as a static visual. Both suites' newton_renderer goldens predate that and no longer match. Rebaked all 8 AOVs per suite. The legacy and ovstage stage variants agree on the new images, and the ovphysx combinations skip on these deformable presets, so they need no goldens.
| assert any(dep.startswith("ovphysx") for dep in optional_dependencies["ov"]) | ||
| assert any(dep.startswith("ovrtx") for dep in optional_dependencies["ov"]) | ||
| assert any(dep.startswith("ovphysx") for dep in optional_dependencies["ovphysx"]) | ||
| assert any(dep.startswith("ovrtx") for dep in optional_dependencies["ovrtx"]) |
|
PR title typo: “bumpy” → “bump”. :) |
StafaH
changed the title
The universe added in this branch covers Linux x86_64, Linux aarch64 and Windows AMD64, so Linux-AMD64 and arm64 branches can never be selected. Leaving them in reads as support that does not exist, and a host reporting arm64 under Linux would abort resolution rather than fall back to them. Drop those branches from the dependency markers and the torch index sources, and relock. Marker text only; no package or version changes.
3 participants
Description
Bump the newton pin. Regenerate the lock and fix previous issues and pins from the pyproject and lock.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there