Pulling v0.3.0 up to Isaac Sim 6.1 - #1194
Conversation
Greptile SummaryThis PR updates Arena to the Isaac Sim 6.1 stack, advances the Isaac Lab submodule, consolidates Isaac Lab dependency installation, enables GPU+Fabric stage-rebuild coverage, and pins OSMO experiments to the v0.3.0 Sim 6.1 image.
Confidence Score: 2/5The PR is not safe to merge until the stale workflow test, inaccessible default build image, and retained production CPU/Fabric workaround are addressed. The image-default change deterministically contradicts an existing test, the Docker quickstart now defaults to an acknowledged internal image without an authentication path, and multi-build production experiments still get forced onto CPU despite the PR enabling the fixed GPU+Fabric path. Files Needing Attention: docker/Dockerfile.isaaclab_arena, osmo/tasks/experiment_runner_task.py, isaaclab_arena/tests/test_render_after_stage_rebuild.py, isaaclab_arena/evaluation/run_execution.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Base[Internal Isaac Sim 6.1 image] --> Docker[Arena container build]
Lab[Isaac Lab 8911377f] --> Docker
Docker --> Tag[v0.3.0_isaac_sim_6.1]
Tag --> OSMO[OSMO experiment task]
OSMO --> Runner[Experiment runner]
Runner --> Workaround{More than one build?}
Workaround -->|Yes| CPU[Force CPU and disable Fabric]
Workaround -->|No| Configured[Use configured device and Fabric]
Tests[Stage-rebuild regression] --> Fabric[GPU plus Fabric path]
Reviews (1): Last reviewed commit: "Fix rendering issues by pulling up to Is..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| EXPERIMENT_RUNNER_SCRIPT = "isaaclab_arena/evaluation/experiment_runner.py" | ||
| # Default container image containing Arena and its runtime dependencies. | ||
| DEFAULT_EXPERIMENT_RUNNER_IMAGE = "nvcr.io/nvstaging/isaac-amr/isaaclab_arena:latest" | ||
| DEFAULT_EXPERIMENT_RUNNER_IMAGE = "nvcr.io/nvstaging/isaac-amr/isaaclab_arena:v0.3.0_isaac_sim_6.1" |
There was a problem hiding this comment.
The new default image uses :v0.3.0_isaac_sim_6.1, but test_explicit_experiment_composes_typed_defaults still requires :latest. Constructing ExperimentRunnerTaskCfg() now contradicts that hard-coded assertion, so the workflow test suite fails.
| # TODO (peterd): Change this to the public Sim 6.1 image once it's released | ||
| ARG BASE_IMAGE=nvcr.io/0947644777160149/internal/isaac-sim:latest-release-6-1 |
There was a problem hiding this comment.
The Dockerfile now defaults to an explicitly internal Isaac Sim image. The documented developer build runs docker build --pull without authenticating to NGC or explaining how to obtain access, so developers without credentials for this namespace cannot build the repository's default container.
| @pytest.mark.with_cameras | ||
| def test_render_after_stage_rebuild_with_fabric(): |
There was a problem hiding this comment.
Unskipping the GPU+Fabric rebuild test indicates that the Sim 6.1 update resolves the rendering defect, but production still calls disable_fabric_for_runs. Any experiment with more than one build—including the shipped five-rebuild DROID experiment—is therefore still forced onto CPU with Fabric disabled, retaining the performance penalty this update is intended to remove.
Knowledge Base Used: Force CPU rendering after stage rebuilds
| MIN_IMAGE_STD = 1.0 | ||
| # Set True to dump the compared renders as PNGs into IMAGE_OUTPUT_DIR, which is created on demand. | ||
| SAVE_IMAGES = False | ||
| SAVE_IMAGES = True |
There was a problem hiding this comment.
SAVE_IMAGES is documented as an opt-in debugging switch, but enabling it globally makes routine executions of both camera tests encode and write before, after, and difference PNGs for every camera. This adds unnecessary disk I/O and leaves persistent debug artifacts during normal local and CI runs.
| SAVE_IMAGES = True | |
| SAVE_IMAGES = False |
Knowledge Base Used: Force CPU rendering after stage rebuilds
![arena-review-bot[bot]](https://avatars.githubusercontent.com/u/195794629?s=60&v=4){kind=small}
| @@ -1 +1 @@ | |||
| Subproject commit af1bab4dc173ba69b08fab779c14ead61d13fd33 | |||
| Subproject commit 8911377fb8d63fc185a4a11bd53749739d587041 | |||
There was a problem hiding this comment.
🔴 Submodule points at a commit that lives on no branch
8911377 ("Update Kellys branch to get Arena tests passing") isn't the head of any branch in isaac-sim/IsaacLab, and it isn't on IsaacLab#7546's branch either — that branch is now 16 commits further along. The old pin af1bab4 was a plain ancestor of release/3.0.0-beta2.
An unreferenced commit is GC-eligible and won't survive #7546 being squash-merged, at which point git submodule update --init breaks for every clone and every image build. Could we wait for #7546 to land on release/3.0.0-beta2 and pin to a commit on that branch instead?
| @@ -1,4 +1,5 @@ | |||
| ARG BASE_IMAGE=nvcr.io/nvidia/isaac-sim:6.0.1 | |||
| # TODO (peterd): Change this to the public Sim 6.1 image once it's released | |||
| ARG BASE_IMAGE=nvcr.io/0947644777160149/internal/isaac-sim:latest-release-6-1 | |||
There was a problem hiding this comment.
🟡 Base image isn't reachable or reproducible
Two things beyond the TODO: nvcr.io/0947644777160149/internal/... is an internal org, so anyone outside NVIDIA can no longer build this container at all; and latest-release-6-1 is a moving tag, so two builds weeks apart can silently pick up different Sim versions.
Is the plan to land this before the public 6.1 image exists, or wait for it? Either way, could we pin a fixed 6.1.x tag or a digest rather than a floating one?
| MIN_IMAGE_STD = 1.0 | ||
| # Set True to dump the compared renders as PNGs into IMAGE_OUTPUT_DIR, which is created on demand. | ||
| SAVE_IMAGES = False | ||
| SAVE_IMAGES = True |
There was a problem hiding this comment.
🟡 Debug flag left switched on
This looks like it was flipped while chasing the bug — it makes every with_cameras run pull in PIL and write three PNGs per camera. Back to False?
| SAVE_IMAGES = True | |
| SAVE_IMAGES = False |
| @pytest.mark.skip(reason="[lab-render-after-rebuild-bug] Rebuilds render incorrectly under GPU+Fabric.") | ||
| @pytest.mark.with_cameras | ||
| def test_render_after_stage_rebuild_with_fabric(): | ||
| """Rebuilds should also render correctly with Fabric on, which is the default outside this bug.""" |
There was a problem hiding this comment.
🔵 Comments still describe the bug as live
"which is the default outside this bug" here, and "Opt out of the suite-wide override" on line 170, are both stale now — force_disable_fabric defaults to False, so there's no suite-wide override left to opt out of. Worth rewording both, or dropping them along with the workaround itself.
| headless: bool = True, | ||
| enable_cameras: bool = False, | ||
| force_disable_fabric: bool = True, | ||
| force_disable_fabric: bool = False, |
There was a problem hiding this comment.
🟡 Can the whole Fabric workaround go now?
The TODO on _fabric_disabled_for_env_builds says to remove it once the render-after-rebuild bug is fixed in Lab, and this PR is that fix. The only caller still passing True is test_render_after_stage_rebuild_without_fabric, which already gets Fabric off through --disable_fabric in its own CLI args — so the monkeypatch looks redundant there too.
Could the context manager and the force_disable_fabric parameter both be deleted, leaving just the per-test --disable_fabric flag?
| EXPERIMENT_RUNNER_SCRIPT = "isaaclab_arena/evaluation/experiment_runner.py" | ||
| # Default container image containing Arena and its runtime dependencies. | ||
| DEFAULT_EXPERIMENT_RUNNER_IMAGE = "nvcr.io/nvstaging/isaac-amr/isaaclab_arena:latest" | ||
| DEFAULT_EXPERIMENT_RUNNER_IMAGE = "nvcr.io/nvstaging/isaac-amr/isaaclab_arena:v0.3.0_isaac_sim_6.1" |
There was a problem hiding this comment.
🟡 Default OSMO image is now frozen
This swaps a tracking :latest for a fixed v0.3.0_isaac_sim_6.1 tag, so OSMO runs off main stop picking up newly built containers. Is that meant to stick, or is it a transition pin to revert once :latest is rebuilt on 6.1? A short TODO next to it would make the intent clear either way.
🤖 Isaac Lab-Arena Review BotSummaryBumps Arena to Isaac Sim 6.1, bumps the Isaac Lab submodule, and un-skips the Fabric variant of the render-after-rebuild test. The Dockerfile cleanup is the strongest part of the change — I checked the new submodule pin and Findings🟡 Warning: 🔵 Improvement: The remaining findings are inline: the submodule pin (🔴), the base image (🟡), Test CoverageUn-skipping VerdictMinor fixes needed — the submodule pin should be re-pointed at a commit on an upstream branch before this merges, and it is worth settling whether |
- Point the Docker base image at the released nvcr.io/nvidia/isaac-sim:6.1.0 instead of the internal 6.1 release image. - Pin isaacsim[all,extscache]==6.1.0.0 in the isaaclab-from-source group rather than taking Isaac Lab's isaacsim extra, which still pins 6.0.1.0 on the 6.1 branch. Scoping the pin to that group leaves the wheel flavor on 6.0. - Relax the mujoco-usd-converter constraint to a floor: isaacsim-core pins it exactly, and the two install flavors need different versions (0.2.0 / 0.5.0). - Restore SAVE_IMAGES=False in test_render_after_stage_rebuild.py. Signed-off-by: alex <amillane@nvidia.com>
8c7a297 to
b07e859
Compare
- test_explicit_experiment_composes_typed_defaults still expected isaaclab_arena:latest, while this branch's DEFAULT_EXPERIMENT_RUNNER_IMAGE now points at v0.3.0_isaac_sim_6.1. - Refresh the websockets override comment: isaacsim-kernel 6.1 declares >=12.0,<15, not ==12.0. Signed-off-by: alex <amillane@nvidia.com>
- Newton dropped its usd-core dependency in 1.5.0, so the 6.1 resolution left the uv install with only the empty pxr namespace directory shipped by the schema packages, breaking Kit-less `from pxr import Gf` in subprocess tests. - Pins usd-core>=25.11,<26.0 in the isaaclab-from-source group, matching the USD version Isaac Sim 6.1 builds against. Docker is unaffected: it resolves pxr from the Isaac Sim prebundle. Signed-off-by: alex <amillane@nvidia.com>
- Moves the gitlink from 8911377fb8 to 206e452cc1, the current tip of kellyguo11/IsaacLab-public codex/beta2-isaacsim61-compat. - Drops the local "Update Kellys branch to get Arena tests passing" commit: every hunk is superseded upstream. ModelFlags now resolves through the isaaclab_newton._newton_compat shim, the joint target attributes have hasattr fallbacks, and the exact newton/warp/aiohttp pins became ranges with newton[importers] supplying newton-usd-schemas. Signed-off-by: alex <amillane@nvidia.com>
- The submodule bump changed Isaac Lab's declared dependencies (newton gained the importers extra, warp-lang and aiohttp became ranges), so `uv lock --check` failed in CI and skipped every step after it. - Re-locks against the new tip: the source flavor keeps Isaac Sim 6.1.0.0 with newton 1.5.0 and warp-lang 1.16.0, and picks up the newton importers packages (open3d, meshio, alphashape, ...). Signed-off-by: alex <amillane@nvidia.com>
- Points the submodule at 771ef0da80, which declares newton[sim]==1.5.0 to match the copy Isaac Sim prebundles instead of newton[importers,sim]. The extra made pip reinstall Newton over the prebundle and strand the symlinks other Isaac Sim extensions share into it, so `isaaclab.sh -i` aborted and the Arena image could not be built on Isaac Sim 6.1. - Re-locks: the packages the importers extra supplied now come from isaaclab itself, so the resolution keeps open3d, meshio, alphashape and friends with newton 1.5.0 and warp-lang 1.16.0 unchanged. Signed-off-by: alex <amillane@nvidia.com>
- Isaac Sim 6.1 renders correctly after a stage rebuild, so experiments no longer need disable_fabric_for_runs, which forced Fabric off and the CPU device on every run whenever a process would build the environment more than once. Multi-run experiments run on GPU with Fabric again. - Drops the matching note in the test helper. Its force_disable_fabric switch stays: test_render_after_stage_rebuild uses it to cover the with- and without-Fabric paths deliberately. Signed-off-by: alex <amillane@nvidia.com>
1 participant
Summary
Pull Arena
v0.3.0up to Isaac Sim 6.1Detailed description
nvcr.io/nvidia/isaac-sim:6.1.0base image and the publicisaacsim==6.1.0.0wheel, so both the Docker and native uv installs run the same Isaac Sim.newton[sim]==1.5.0to match the copy Isaac Sim prebundles; asking for theimportersextra made pip replace it and strand the symlinks other extensions share, which broke the image build.usd-corefor the native uv install, which lost its only OpenUSD provider once Newton 1.5 stopped depending on it.Video
After the upgrade 6.1 we can run a multi-environment experiment without parts of the robot disappearing.
robolab_10_tasks_pi0_3cam_5x.mp4