[Fix] Pin Newton to the commit carrying newton#4017 - #7445
Draft
hujc7 wants to merge 28 commits into
Draft
Conversation
Adds a composable teleop-no-isaacsim extra so the lock path selects the teleop requirements without pulling the isaacsim wheel into an image that already ships Kit. Derives the submodule extras from OPTIONAL_SUBMODULE_ROOT_EXTRAS instead of a second hardcoded table, and falls back to pip rather than installing less than was asked for. Pins the uv pip calls to the venv: UV_PYTHON selects the target environment, so they were writing into Isaac Sim's site-packages.
Reverts the install CLI lock path: it also caught Conda environments, where uv sync targets the project environment rather than the active one, breaking the install_ci Conda tests. The images control VIRTUAL_ENV and UV_PROJECT_ENVIRONMENT themselves, so Dockerfile.base syncs the lock directly through a named kit-image extra. imageio-ffmpeg is excluded during the sync instead of uninstalled after.
The extras set gained teleop-no-isaacsim and kit-image. teleop now composes from the former, so asserting on its literal requirement list no longer sees isaaclab-mimic; flatten the isaaclab-dev[...] self-reference first so the contract stays on what the extra resolves to.
run_tests.sh launches the test container with --user ${host_uid}:${host_gid}, so a
venv owned by the image's own user is read-only there and the test-time extras
install (pytetwild for tetrahedralization) fails with EACCES. Widen the mode
instead of chowning to a uid the image cannot know.
The teleop extra now composes through an isaaclab-dev[teleop-no-isaacsim] self-reference. _root_extra_dependencies strips every isaaclab* requirement as a workspace self-reference and isaacsim separately, so the list came back empty and ./isaaclab.sh -i teleop installed none of the teleop stack. Expand root self-references before the workspace filter runs, guarding against a cycle with a seen set.
The test extra pulls the whole Sphinx docs stack, so the images installed a bare pytest and CI layered pytest-mock, junitparser, flaky and coverage on top in a separate post-build docker build. Split those runners into a test-runtime extra that test composes from, and pull it into kit-image so both images carry them from the lock. In-image pytest goes from 238 passed / 23 failed to 257 passed / 4 failed; the 4 remaining fail identically without this change.
Restore the uv cache mount and drop UV_NO_CACHE=1, which disabled the only working build cache. Resolve the third-party packages from workspace manifests above the source copy so a source edit re-runs only the workspace install (240 packages in 6.0s -> 14 in 4ms). Set ownership in the layer that creates the venv instead of a recursive chown afterwards: on overlayfs a chown copies up every file it touches, so the base image stored the 11.5GB venv twice. That layer drops from 1003.9s to 36.1s and the image from 44.7GB to 33.2GB, with 0 of 98562 common files differing in owner or mode. Kit-less sets the modes in its builder stage so the stage copy carries them rather than adding an equivalent layer. Also hoist the layers that depend on no repo content above the copies, give isaaclab.sh an explicit 755 (sed -i rewrote it under the build umask, leaving it unrunnable by other uids), and keep local ruff/pytest caches out of the context.
test_dockerfile_nonroot.py only globs Dockerfile.* and parses their text, but build.yaml ran it inside the image via /isaac-sim/python.sh in both the isaaclab_ov and curobo jobs. Kit's interpreter has no pytest once the install moves to the venv, so those steps failed, and a text lint held a self-hosted GPU runner in two places. Move it to a ubuntu-latest job using the invocation kitless-docker.yml already uses. The steps that verify the running container's uid stay where they are.
Two assumptions broke when the install moved into a venv. The uv path was derived from site.getuserbase() and bootstrapped with pip install uv, which only landed there because Isaac Sim's site-packages was read-only; pip installs into the venv's own bin instead, so $HOME/.local/bin/uv never appeared and record-video died with No such file or directory. Prefer uv from PATH, which the image ships, and keep the user-base bootstrap for images that do not. The [INFO] banner isaaclab.sh writes to stdout now follows the command output rather than preceding it, so tail -n 1 captured the banner for the interpreter, user-site and uv paths. Filter the banner instead of skipping by position.
Isaac Sim ships prebundled copies under pip_prebundle that setup_python_env.sh puts on PYTHONPATH. isaaclab.sh deprioritizes them, but launch paths that never import isaaclab (runheadless.sh, isaac-sim.streaming.sh) do not, and the prebundled torch loads a libtorch_cuda.so needing an NCCL symbol the bundled NCCL does not export (nvbugs 6343978). The pip install path repointed them itself; installing into a venv leaves them untouched, so test_no_shadowing_prebundled_torch_in_isaac_sim failed. Reuse the same repoint after the workspace sync, while the build is still root: 17 packages become symlinks into the venv and the shadowing directory count drops to 0.
The MuJoCo tendon adapter reads mujoco.actuator_trntype through an ArticulationView, which every Newton 1.5.x release rejects with "has custom frequency 'mujoco:actuator' which is not supported by ArticulationView". newton#4017 makes ArticulationView generic over custom frequencies. The fix landed on main and is in no 1.5.x release, so the pin moves from the PyPI form to the merge commit and Newton reports 1.6.0.dev0. Regenerating uv.lock is what carries the pin into the Docker images.
github-actions
Bot
added
documentation
hujc7
added
ci:run-docker
8 tasks
The install CI hard-coded ``m.version('newton') == '1.5.1'``, which the pin bump
breaks. Read the requirement from [tool.uv].override-dependencies and compare the
commit recorded in the distribution's direct_url.json, so the assertion follows a
bump rather than going stale on every one.
A version string cannot identify the pin: every commit on Newton's main branch
reports the same 1.6.0.dev0.
hujc7
added
ci:run-docker
This reverts commit 3037385. The premise was wrong. That test installs with ``uv pip install <wheel>[all]`` and no overrides file, so Newton resolves from PyPI against the wheel's loose ``newton[sim]>=1.2.0`` bound -- 1.5.1, recorded without a direct_url.json. The git pin only reaches ``uv sync`` paths, so it does not change what this test sees, and asserting the pinned commit here made the assertion unsatisfiable.
The images excluded imageio-ffmpeg, so moviepy.editor - which isaaclab/envs/utils/video_recorder.py imports - fell through to imageio's TIFF writer and raised TypeError on write. Video recording only worked because CI installs moviepy into the container at test time. Pull the existing video extra into kit-image and drop the exclusion. Costs 31.6 MB (moviepy, imageio-ffmpeg, proglog) on a 33 GB image; verified in-image that ImageSequenceClip.write_videofile produces a real mp4.
hujc7
added
ci:run-docker
This reverts commit 2f057e7.
hujc7
added
ci:run-docker
The pinned Newton rejects a non-"fixed" penalty_update_method unless sparse_dynamics is set, and sparse dynamics in turn require a sparse Jacobian (solver_kamino.py:397 and :439). The round-trip test built the default combination, so to_solver_config() raised before any field could be checked. Set both sparse flags in the test. They are valid for every parametrized field, and the test asserts field forwarding rather than solver-combination validity.
Isaac Sim ships some prebundled packages twice: a flat directory and an extras-qualified mirror, <pkg>[extras]/<pkg>-<version>-*/<pkg>, whose contents are per-file symlinks into the flat copy. Only the flat directory was repointed, so the mirror kept pointing at the shipped version file list. MEASURED on the Isaac Sim image: newton[sim]/newton-1.5.0-py3-none-any/newton holds 842 symlinks, and 330 of their targets do not exist in the newton the pin installs, 232 of them under _src/solvers/kamino. The install then aborts on its own dangling-symlink guard, which is correct: extensions resolving through the mirror would hit missing modules at runtime. Collapse each mirror to the same symlink the flat directory already gets, so one directory link replaces a per-file tree that goes stale on every bump. Mirrors are matched by name rather than glob, since newton[sim] is a valid character class that would otherwise match newtons.
… package" This reverts commit 7e1fd35.
hujc7
added
ci:run-docker
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maincommit merging newton#4017, unblocking the MuJoCo tendon adapter that PR [Task Clean-up][Assets] Serve the Shadow Hand from one asset with native PhysX tendons #7161 needs.95b91be) is this PR. Review range:59c5078...95b91be.uv.lockis the whole bump lever on top of [Docker] Install the Docker images from uv.lock #7405: 9 lines of lock diff.Description
resolve_fixed_tendon_actuator_columnsreadsmujoco.actuator_trntypethrough anArticulationView. Every Newton 1.5.x release rejects that with "has custom frequency'mujoco:actuator' which is not supported by ArticulationView"; newton#4017 makes
ArticulationViewgeneric over custom frequencies.The fix is on
mainand in no 1.5.x release — verified againstrelease-1.5, thev1.5.1tag, and
v1.5.0, all of which still carry the pre-fix rejection string, and PyPI, whoselatest
newtonis 1.5.1. The pin therefore moves from the PyPI release form back to a gitcommit, and Newton reports
1.6.0.dev0.Three pin sites mirror each other and are updated together:
[tool.uv].override-dependenciesin the rootpyproject.tomltools/wheel_builder/uv-overrides.txtsource/isaaclab/test/install_ci/uv_pip/uv-overrides.txtisaaclab.sh --installneeds no change: #7405's_ensure_newtonalready parses both the@<sha>and==<version>pin forms.Type of change
Release backport
developChecklist
pre-commitchecks with./isaaclab.sh --format