Updates usd-core to 25.11.0#5495
Conversation
![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 bumps usd-core from 25.8.0 to 25.11.0 to align with Kit 110.1.1, and adds comprehensive CI tests for installation workflows covering both uv and conda environments across kitless Newton and Isaac Sim/PhysX backends. The implementation is well-structured with proper test isolation and environment cleanup patterns.
Architecture Impact
- install.py: The
_install_isaacsim()function now supports environment variable overrides for internal NVIDIA registries (ISAACSIM_EXTRA_INDEX_URLS,ISAACSIM_VERSION_SPEC,ISAACSIM_USE_PRE,ISAACSIM_EXTRAS). This is a non-breaking extension that enables CI to test against pre-release builds. - CI Workflow: The single
install-testsjob is now split intoinstall-tests(uv) andinstall-tests-conda(conda), running in parallel. The path-based gating logic is removed — tests now always run. - Test Infrastructure: New mixins (
Conda_Mixin,IsaacSimSource_Mixin,UV_Mixin) provide reusable environment management for installation tests.
Implementation Verdict
Minor fixes needed — The core functionality is sound but there are a few issues that should be addressed before merge.
Test Coverage
The test coverage is excellent. The new test_install_workflow_training.py covers:
- 7 distinct installation scenarios matching the documentation matrix
- Both uv and conda environment managers
- Kitless Newton, Isaac Sim pip, and Isaac Sim source workflows
- Proper teardown in
finallyblocks ensuring environment cleanup
Each test runs a real training loop (3 iterations of Cartpole) which validates end-to-end functionality. The marker system (@pytest.mark.uv, @pytest.mark.conda, @pytest.mark.isaacsim_source) enables selective execution.
CI Status
No CI checks available yet — manual verification recommended.
Findings
🟡 Warning: source/isaaclab/test/install_ci/test_install_workflow_training.py:243 — Test_UV_IsaacSim_Source_Training lacks teardown
Unlike all other test classes that use try/finally to ensure environment cleanup, this class runs commands directly without any teardown mechanism. While it doesn't create a venv (by design), if the install step partially succeeds and modifies the repo state (e.g., editable installs in _isaac_sim's site-packages), subsequent tests could be affected.
def test_install_and_train_physx(self, isaaclab_root):
"""``isaaclab.sh -i physx,…`` (no isaacsim token) then train with PhysX."""
cli = str(isaaclab_root / "isaaclab.sh")
# No try/finally cleanup — any installed packages persist
result = self.run_without_venv([cli, "-i", _ISAACSIM_SOURCE_INSTALL], cwd=isaaclab_root)Consider whether test isolation is needed here or document why it's intentionally omitted.
🟡 Warning: source/isaaclab/test/install_ci/utils.py:219-222 — Parenthesized assertion may confuse readers
assert result.returncode == 0, (
f"conda env creation via isaaclab.sh -c failed:\n{result.stdout}\n{result.stderr}"
)The parentheses here create a tuple with the assertion message, which works but is an unusual pattern. While functionally correct, it could be misread as an accidental tuple. Standard practice is to use backslash continuation or dedent the string.
🟡 Warning: docker/Dockerfile.installci-conda:56-57 — x86_64-only Miniconda installer
RUN wget -q "https://repo.anaconda.com/miniconda/Miniconda3-${MINICONDA_VERSION}-Linux-x86_64.sh" \The Dockerfile hardcodes x86_64 in the Miniconda URL. If this image is ever built on ARM (e.g., DGX Spark), it will silently download the wrong installer. Consider using $(uname -m) or a build-arg for architecture selection, consistent with the ARM awareness shown in install.py.
🔵 Improvement: source/isaaclab/isaaclab/cli/commands/install.py:328-331 — Redundant URL deduplication logic
for url in _INTERNAL_ISAACSIM_INDEX_URLS:
if url not in extra_index_urls:
extra_index_urls.insert(0, url)The if url not in check is always true here since extra_index_urls starts as [NVIDIA_INDEX_URL] and _INTERNAL_ISAACSIM_INDEX_URLS contains different URLs. The check is defensive but adds cognitive overhead. Either document why it's needed (future-proofing) or simplify.
🔵 Improvement: source/isaaclab/test/install_ci/test_install_workflow_training.py:93-94 — Training command has positional argument without --
_KITLESS_TRAIN_CMD = [
...
"presets=newton", # This looks like a Hydra override, not a flag
"--visualizer",The presets=newton argument appears to be a Hydra-style override mixed with argparse flags. This works if the training script uses Hydra, but the inconsistent style (no --) could confuse readers. Consider adding a comment clarifying this is intentional Hydra syntax.
🔵 Improvement: .github/workflows/install-ci.yml:97-98 — changes job output is now unused
outputs:
run_install_tests: ${{ steps.detect.outputs.run_install_tests }}The workflow still computes and outputs run_install_tests, but the downstream jobs no longer have if: needs.changes.outputs.run_install_tests == 'true'. The output and the any_match function are now dead code — they only feed the summary table. Consider removing the unused logic or adding a comment explaining it's kept for summary generation only.
🔵 Improvement: source/isaaclab/test/install_ci/utils.py:250-252 — find_isaac_sim_symlink is defined but never called
@staticmethod
def find_isaac_sim_symlink(isaaclab_root: Path) -> Path | None:
"""Return the ``_isaac_sim`` path if it exists, else ``None``."""This method exists in IsaacSimSource_Mixin but is never used — tests call find_isaaclab_root() directly and check _isaac_sim existence inline in _isaacsim_source_available(). Consider either using this method for consistency or removing it.
Greptile SummaryThis PR bumps the kitless OpenUSD dependency (
Confidence Score: 4/5Safe to merge with the understanding that one test class will never execute in automated CI.
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[GitHub Actions trigger] --> B[changes job\nPR summary only]
B --> C[install-tests uv\n-m uv]
B --> D[install-tests-conda\n-m conda]
C --> E[run_install_ci.py docker --gpu]
D --> F[run_install_ci.py docker --conda --gpu]
E --> G[_build Dockerfile.installci uv image]
F --> G
G --> H{--conda?}
H -- No --> I[docker run uv image -m uv tests]
H -- Yes --> J[_build Dockerfile.installci-conda]
J --> K[docker run conda image -m conda tests]
I --> L[Test_UV_Kitless_Training xfail strict]
I --> M[Test_UV_IsaacSim_Pip_Training]
I --> N[Test_UV_IsaacSim_Source_With_Env_Training]
I --> O[Test_UV_IsaacSim_Source_Training - never selected]
K --> P[Test_Conda_Kitless_Training xfail strict]
K --> Q[Test_Conda_IsaacSim_Pip_Training]
K --> R[Test_Conda_IsaacSim_Source_Training]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if extra_urls_env: | ||
| # Prepend the two internal registries so they are tried before pypi.org. | ||
| for url in _INTERNAL_ISAACSIM_INDEX_URLS: | ||
| if url not in extra_index_urls: | ||
| extra_index_urls.insert(0, url) | ||
| for url in extra_urls_env: | ||
| if url not in extra_index_urls: |
There was a problem hiding this comment.
insert(0, url) inside a forward loop reverses the list defined in _INTERNAL_ISAACSIM_INDEX_URLS. After the loop, the order is [ct-omniverse-pypi, sw-isaacsim-pypi, pypi.nvidia.com] — opposite to the declared order [sw-isaacsim-pypi, ct-omniverse-pypi]. This affects which internal registry gets priority when pip/uv resolves version conflicts across the two registries. Consider replacing the loop with a slice-insert or inserting the pre-built prefix slice at once to preserve list ordering.
| if args.conda: | ||
| dockerfile = repo_root / "docker" / "Dockerfile.installci-conda" | ||
| image_tag = f"isaaclab-installci-conda:{args.base_image.replace(':', '-').replace('/', '-')}" | ||
| # Default: run only conda-marked tests | ||
| default_pytest_args = ["-v", "--tb=short", "-m", "conda"] | ||
| timeout_seconds = 7200 # conda installs are heavier; allow 2 h |
There was a problem hiding this comment.
The _cmd_docker function mounts pip and uv caches as named Docker volumes for both the uv and conda images, but the conda package cache (/root/.conda/pkgs and /opt/miniconda3/pkgs) has no corresponding volume mount. Each conda CI run will re-download all packages from scratch, making conda jobs significantly slower than they could be and increasing network load on the registries.
| render_table() { | ||
| local files="$1" entry regex desc count sample |
There was a problem hiding this comment.
run_install_tests output is set but never consumed
The changes job still writes run_install_tests=true to $GITHUB_OUTPUT, but neither install-tests nor install-tests-conda reference needs.changes.outputs.run_install_tests in an if: condition anymore (both jobs now run unconditionally). The output can be removed to avoid misleading future readers into thinking it still gates anything.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The new commit addresses the ARM architecture concern I raised previously:
Previous concern addressed: The Dockerfile.installci-conda now uses $(uname -m) to dynamically select the correct Miniconda installer for the build platform (line 60-63), fixing the x86_64 hardcoding issue.
The remaining previous findings (unused find_isaac_sim_symlink method, unused changes job output, parenthesized assertion style, Test_UV_IsaacSim_Source_Training lacking teardown, redundant URL deduplication, Hydra override comment) are still present but are minor improvements/warnings that don't block merge.
No new issues introduced by the latest commit.
Implementation Verdict
Ship it — The ARM fix was the most actionable concern, and it's now resolved. The remaining items are code quality suggestions that can be addressed in follow-up PRs if desired.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
No new commits since the previous review — the HEAD is still 7970c012. All previous findings remain as noted:
- ✅ ARM architecture concern was already addressed in the previous commit
- Remaining minor items (unused
find_isaac_sim_symlink, unusedchangesjob output, parenthesized assertion, missing teardown inTest_UV_IsaacSim_Source_Training, redundant URL dedup, Hydra override comment) are unchanged
CI Status
The conda CI job is failing (❌ Installation Tests (conda): failure). This should be investigated before merge — check whether it's a flaky infrastructure issue or a real test failure introduced by this PR.
Implementation Verdict
Minor fixes needed — Investigate the conda CI failure. If it's transient/infrastructure-related, re-run; if it's a real failure, fix before merge.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The new commit b11a1174 addresses the CI failure by removing the conditional if: needs.changes.outputs.run_install_tests == 'true' from both installation test jobs — they now always run. The changes job output is still computed but only used for the PR summary, which is acceptable.
Previous minor findings (unused find_isaac_sim_symlink, parenthesized assertion, missing teardown in Test_UV_IsaacSim_Source_Training, redundant URL dedup, Hydra override comment) remain unchanged but are all 🔵 improvements, not blockers.
CI Status
✅ Installation Tests (conda): success — the conda CI is now passing. The uv job is still pending but pre-commit and other checks pass.
Implementation Verdict
Ship it — The core changes are correct. The remaining minor items are cosmetic and can be addressed in a follow-up if desired.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Follow-up Review
The latest commit 7049ea2f appears identical to the previous reviewed state — no new changes were introduced. All previous findings remain as noted:
- ✅ CI conditional removal was correctly implemented (installation tests now always run)
- 🔵 Minor improvements (unused
find_isaac_sim_symlink, parenthesized assertion style, missing teardown in source-only test) remain cosmetic and non-blocking
CI Status
Most jobs are still pending, but Installation Tests (conda): success and core infrastructure checks (pre-commit, Build Wheel, Build Base Docker Image) are passing.
Implementation Verdict
Ship it — No new issues. The PR is ready to merge once the remaining CI jobs complete.
kellyguo11
requested review from
Mayankm96,
jtigue-bdai,
matthewtrepte and
ossamaAhmed
as code owners
github-actions
Bot
added
the
documentation
#5538) ## Summary Two unrelated CI breakages on develop, bundled here so develop turns green in one PR. ### 1. Skip the failing viewergl test `test_cartpole_newton_visualizer_viewergl_rgb_motion[physx,newton]` started returning all-black frames on develop after `nvcr.io/nvidian/isaac-sim:latest-develop` flipped to a Kit 110.1.1 + USD 25.11 base. The failure has been deterministic across multiple PRs (#5523, #5495, #5408, …). Investigation so far has ruled out: - PR #5521 (revert in #5539 still failed) - Newton 1.0 → 1.2.0rc2 viewer code regression (only 7-line addition; ViewerGL alone yields 1.08M nonzero pixels) - warp 1.12 → 1.13 RegisteredGLBuffer ABI (byte-identical) - Module-load side effects of `isaaclab_physx.renderers` - CUDA-GL interop (PR #5540 diagnostic confirms direct CPU FBO readback also returns zeros, with `GL_NO_ERROR`) - GL context-currency (PR #5541 H6 attempt: still fails) - GL/CUDA sync (PR #5542 H4 attempt: still fails) Diagnostic output (PR #5540 v2): ``` [VIZDIAG] fbo=c_uint(8) pbo=None size=600x600 [VIZDIAG] glGetError before: GL_NO_ERROR [VIZDIAG] CPU-readback: nonzero=0/1080000 max=0 err=GL_NO_ERROR [VIZDIAG] PBO-result: nonzero=0/1080000 max=0 ``` The FBO itself is empty — Newton's pyglet/EGL renderer is not depositing pixels under Kit 110.1.1, even though `tiled_camera_rgb_non_black` (Kit RTX path) on the same env passes. Underlying root cause still being chased; this PR ships the skip to unblock develop. ### 2. Fix warp intersphinx 404 in docs build `https://nvidia.github.io/warp/objects.inv` started returning 404 — Warp's `objects.inv` only lives at `/stable/` and `/latest/` now. With Sphinx's `warnings_treated_as_errors`, the broken intersphinx fetch fails the docs build on every PR. Pinning to `/stable/` (matches the existing PyTorch `/docs/2.11/` workaround pattern in the same file). Verified `https://nvidia.github.io/warp/stable/objects.inv` returns 200. ## Test plan - [x] CI `isaaclab_visualizers` on this branch — was passing earlier with the skip; will re-verify with the bundled docs fix - [ ] CI `Build Latest Docs` on this branch — must turn green (was failing on every recent PR before this fix) ## Re-enable plan Once the underlying viewergl bug is identified and fixed, drop the `@pytest.mark.skip` decorator and remove the `jichuanh-disable-viewergl-flaky.skip` fragment.
Align the kitless OpenUSD dependency with Kit 110.1.1 so installs resolve the matching USD runtime. Co-authored-by: Cursor <cursoragent@cursor.com>
kellyguo11
force-pushed
the
kellyg/usd-core-25-11
branch
from
55250ab to
c1d8ae5
Compare
kellyguo11
changed the title
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
Clean dependency version bump from usd-core==25.8.0 to usd-core==25.11.0 to align the kitless OpenUSD dependency with Kit 110.1.1. The change is minimal (1 line in setup.py) with a properly formatted changelog fragment.
Design Assessment
Design is sound. The exact pin (==) is appropriate here — this intentionally locks to a specific USD version matching the Kit SDK, preventing resolver conflicts between Isaac Sim's bundled USD runtime and the pip-only dependency. The existing architecture markers (SUPPORTED_ARCHS) are correctly preserved, ensuring usd-core only installs on x86_64/AMD64 where wheels are available.
Findings
No critical or warning-level issues found. Specific validations:
- ✅
usd-exchange>=2.2does not pinusd-corein its requirements — no dependency conflict - ✅
usd-core25.11 supports Python>=3.8,<3.14, compatible with isaaclab's>=3.12requirement - ✅ PEP 440 version normalization handles
25.11.0correctly (maps to PyPI's25.11release) - ✅ No other files in the repo reference the old
25.8.0version - ✅ Changelog fragment follows project conventions (
Changedcategory, proper RST formatting) - ✅ Existing code uses direct
from pxr importpatterns — any incompatibility would fail loudly at import time, not silently
Test Coverage
- New tests needed: No — this is a configuration change, not new functionality
- CI coverage: The Installation Tests workflow is specifically designed to catch dependency resolution failures and import errors. It triggers on
source/path changes and tests in a fresh Docker environment. - The author correctly left the "added tests" checklist item unchecked
CI Status
- Build Wheel: ✅ pass
- Check changelog fragments: ✅ pass
- pre-commit: ✅ pass
- Check for Broken Links: ❌ fail (unrelated to this PR — pre-existing docs link issue)
- Installation Tests: ⏳ pending (this is the key check — recommend waiting for it to pass before merge)
Verdict
Ship it ✅
Straightforward, well-executed dependency alignment. No compatibility concerns detected. Recommend merging once Installation Tests pass.
1 participant
Description
Align the kitless OpenUSD dependency with Kit 110.1.1 so installs resolve the matching USD runtime.
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there