[pull] develop from isaac-sim:develop - #66
Merged
Merged
Conversation
## Description Keep 50 m of visible flat ground beyond each edge of the environment grid. Previously, large batches had only one environment spacing of margin, so locomotion robots could walk off the visible mesh while still standing on the infinite collision plane. | Environments | Spacing | Visual width before → after | Margin per side before → after | |---|---|---|---| | 1 | 1 m | 100 → 100 m | 50 → 50 m | | 4,096 | 4 m | 260 → 352 m | 4 → 50 m | | 16,384 | 2.5 m | 322.5 → 417.5 m | 2.5 → 50 m | The existing UV rescaling preserves **1 m checker cells / 2 m texture repeats** at every size. The infinite collision plane and explicit `import_ground_plane(size=...)` overrides are unchanged. **No rough-terrain, heightfield, solver, or task-parameter changes.** ## Validation - `uv run isaaclab -f`: passed. - Default-ground-plane asset/material tests: 3 passed. - CPU USD authoring checks at 1 / 4,096 / 16,384 origins verified the actual transformed mesh bounds and metric UV repeats. The margin assertion failed on the base and passed with this fix. - Headless GL before/after images at 20 m beyond a 16,384-environment grid corner: the old mesh ended behind the marker; the enlarged mesh covered it with unchanged checker size. This used a tiny static scene, not 16,384 simulated robots. - A 16-environment Go2-flat checkpoint replay passed 300 policy steps in the active training checkout with this same ground-size fix applied; robot and floor remained visible. - Fresh `develop` with Newton 1.6.0 produced a blank viewport with **both** the base and patched ground-size formulas (pixel-identical control frames). That playback was not counted as a successful visual check. - Extended the existing terrain-importer integration test across those batch sizes. The full Kit integration suite was not run locally. ## Type of change Bug fix (visualization only). ## Release backport - [ ] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] Formatting checks passed. - [x] Updated the API docstring and existing regression test. - [x] Added an `isaaclab` changelog fragment. Co-authored-by: ooctipus <ooctipus@users.noreply.github.com>
## Summary USD composition could fail for `scene.usda -> local robot.usda -> remote asset.usd`, or when a downloaded layer still referenced a remote URL. The common USD reference loader now prepares both local and remote files through one recursive dependency traversal. Changed layers are written as working copies, preserving authored files and raw downloads. Renderer-provided MDL identifiers such as `OmniPBR.mdl` and self-contained USDZ packages retain their resolution behavior. Completed managed local trees skip discovery while their file stamps match; incomplete downloads remain retryable, and remote URL requests retain freshness checks. Fixes #7999 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Validation - Full asset test suite: 66 passed. - Extended the existing composition test with renderer module identifiers and a remote USDZ case; all four composition cases fail on the previous PR revision and pass with this change. - Verified the common reference loader composes a real remote Cartpole through two local wrapper layers. - All 48 previously failing CI cases passed locally with Isaac Sim 6.1.0.0, OVRTX 0.5.0.377615, and OVPhysX 0.6.3. These include the existing Kit and Kitless image comparisons, visualizers, scene partitioning, USDZ demos, and later core/contributed-environment failures. Golden images and rendering thresholds were unchanged. - `uv run isaaclab -f`: passed, including changelog validation against upstream develop. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks with `uv run isaaclab -f` - [x] My changes generate no new warnings - [x] I have added a test that proves the fix - [x] I have added a changelog fragment for the touched package - [x] My name is already in `CONTRIBUTORS.md` --------- Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
# Package CI test jobs: 4:26 h → 2:09 h of runner time (−52%); longest job 29 → 13 min All package jobs pass. The comparison is a green `develop`-based PR run ([before](https://github.com/isaac-sim/IsaacLab/actions/runs/36102042654)) against this PR ([after](https://github.com/isaac-sim/IsaacLab/actions/runs/36126670863)): | Job | Before | After | |---|---:|---:| | `isaaclab_newton` | 26:35 | **7:45** (−71%) | | `isaaclab_ov` | 20:05 | **5:52** (−71%) | | `isaaclab_contrib` | 9:23 | **2:45** (−71%) | | `isaaclab (core)` 1/2/3 | 22:27 / 29:15 / 22:22 | **10:36 / 8:10 / 8:45** | | `isaaclab_tasks` 1/2/3 | 15:09 / 28:29 / 7:22 | **12:37 / 9:00 / 5:53** | | `isaaclab_rl` | 23:10 | **7:10** (−69%) | | `isaaclab_mimic` | 20:23 | **9:07** (−55%) | | `isaaclab_physx` | 14:49 | **8:46** (−41%) | | `isaaclab_visualizers` | 13:36 | 12:08 (−11%) | | `isaaclab_teleop` | 8:25 | 9:27 | | `isaaclab_assets` / `_experimental` | 3:04 / 1:44 | 5:15 / 5:45 | Assets and experimental lost to image-pull variance: their pulls took 1:25 and 3:58, against 0:02 in the baseline. Teleop and visualizers gain little because most of their files start a renderer, and renderer files run one at a time. ## Why xdist alone did not scale CI logs from a green run show most files spend more time starting than testing: - Kit files take a median 15.5 s before the first test, kitless files 3.2 s. - The first RTX renderer in a container waits 113–130 s. Splitting every file across xdist workers repeats that startup per worker, so the earlier renderer-file slowdowns were almost all startup: PhysX rigid-object rendering 113 s → 412 s, contrib visuotactile 119 s → 411 s. Only long files with cheap startup got faster, such as the leapp export. ## What changed - **`TEST_JOBS` / `test-jobs` input.** `tools/conftest.py` runs up to `TEST_JOBS` test files at once, each still in its own pytest process, so startup overlaps instead of repeating. Every package job sets `test-jobs: "4"`. Unset keeps today's serial, live-streamed behavior, and the multi-GPU lane is unchanged. - **Per-file settings** in `tools/test_settings.py`: - `PYTEST_WORKERS`: the few long-pole files that still split across xdist workers, holding one slot per worker and starting first. Today that is only `test_leapp_export_flow.py`. This replaces the job-wide `pytest-workers` input and `PYTEST_WORKER_LIMITS`. - `EXCLUSIVE_TESTS`: files that run alone. These are the two wall-clock performance tests; `test_robot_load_performance.py` failed with other files beside it. - **Renderers never overlap.** Files that start an RTX renderer (Kit cameras or OVRTX) never run beside each other; other files still run beside them. This avoids repeated cold shader compiles, which slowed startups several-fold, and garbled logs in the renderer log file they share. - **Output.** Output from concurrent files is printed whole per process attempt, so the job log does not interleave. - **Scheduling logic.** It lives in `tools/_file_scheduler.py`, with no pytest dependency. `tools/test_file_scheduler.py` covers it in the tools-tests job: slot limits, wide jobs not overtaken, renderer exclusivity, lazy claiming from the multi-GPU work queue, and error propagation. Each rule was checked by breaking it on purpose. - **Caches.** Every package job now restores the Warp kernel cache; nine did not. Jobs that render also restore the RTX shader cache. - **Pink IK race fix** (@ooctipus). Concurrent Pink IK controllers re-exported the same URDF and meshes into the shared temp dir, and the two GR1T2 mimic generation files read each other's half-written meshes. A file lock now covers export and load. This also affects users running several processes on one machine. ## Follow-ups - **RTX shader cache misses.** The restore finds no warmed entry for these runners' GPU and driver key (`sm89`, 595.58.03), so renderer startup still compiles shaders. `test_visualizer_golden_newton.py` still sometimes overruns the 120 s startup deadline and passes on retry. - **Non-atomic asset downloads.** `retrieve_file_path` downloads into the shared temp dir and reuses a file if it exists. Two processes fetching the same asset could read a partial file. This has not been observed. - **Slow asset-cache population.** `test_environments_isaacsim_physx.py` spends ~1,000 s populating the Git asset cache before its first test. ## Type of change - CI / test infrastructure (no user-facing change) ## 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` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] 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 ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` --------- Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
# Description `ActuatorNetLSTM.compute` used DC-motor clipping without updating its cached joint velocity, so the torque-speed limits were always evaluated at zero velocity. With the ANYmal actuator settings (saturation 120 N·m, effort limit 80 N·m, velocity limit 7.5 rad/s), it allowed 80 N·m of positive torque at 7.5 rad/s when the limit should be zero. Explicit PD and neural-network compute pass the current velocity directly into DC-motor clipping. This removes the cached `_joint_vel` tensor, its allocation and copies, and the redundant `DCMotor.compute` override. Clipping uses a local clamped value, preserving the measured velocity and the existing torque-speed equations. The base `_clip_effort(effort, *args, **kwargs)` only clamps effort and leaves model-specific inputs to subclasses. `DCMotor._clip_effort(effort, joint_vel)` owns velocity-dependent clipping; implicit actuators retain `_clip_effort(effort)`. Custom overrides used by explicit PD and neural-network actuators must accept the velocity argument, as documented in the changelog. ## Type of change - Bug fix and removal of redundant actuator state ## Tests Extended the existing DC-motor and ideal-PD tests; no new test file. - DC-motor, ideal-PD, and implicit-actuator suites: **41 passed**, including CPU and CUDA cases. Formatting and pre-commit checks passed. - The LSTM regression calls real `compute` with a constant 100 N·m network and checks expected efforts `80, 60, 0, 80` at velocities `0, 3.75, 7.5, -7.5`. Restoring the original production implementation fails numerically: two values differ, including 80 instead of zero at the velocity limit. - Existing four-quadrant DC-motor compute cases also verify that measured velocity stays unchanged. The LSTM regression guards against reintroducing cached `_joint_vel` state. - One assertion in the existing ideal-PD compute test verifies that base clipping needs only effort. It fails against the previous required-velocity signature. - A temporary two-step smoke check exercised DC-motor, LSTM, and MLP compute on CPU and CUDA with changing velocities beyond the curve's corner. Efforts matched the expected values and measured velocities stayed unchanged. ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the `pre-commit` checks with `./isaaclab.sh --format` - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/isaaclab/changelog.d/` - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Lynn <lynnhe02@gmail.com> Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
…7987) # Description `quat_apply` and `quat_apply_inverse` flattened quaternion and vector batches independently, losing broadcasting and causing `body_projected_gravity_b` to fail for multiple bodies. Both helpers now broadcast leading dimensions following NumPy rules and return the broadcast result shape. They use tensor views for broadcasting, without materializing repeated inputs. The observation only preserves an explicit body dimension, including integer body selection. The frame-transformer tutorial explicitly selects its single quaternion so its configured offset remains a flat 3-vector. Incompatible batch shapes now raise an error even when their element counts match; callers intentionally pairing flattened arrays must reshape them explicitly. The changelog also documents retained singleton batch dimensions and potentially noncontiguous outputs for transposed inputs; use `reshape` when flattening those outputs. Consolidated forward/inverse coverage in the existing math test and moved the gravity regression into `test_gravity_randomization.py`, using different gravity directions per environment. No new test file. Merged current `develop` and retained its multidimensional rotation coverage. ## Validation - Existing math, gravity, and reorientation checks passed. The caller audit additionally passed 66 tests: 36 CPU/CUDA quaternion cases, 21 operational-space controller cases, and 9 existing dexterous-task, FORGE, visualization, and sampler cases. - 42 CPU/CUDA comparisons of consumer tensor paths and gradients agreed with the old helpers. The tutorial offset failed backend-style stacking before its one-line correction and passed afterward with the analytic -90-degree yaw result. - Confirmed the broadcasting and gravity regressions fail with the old helpers and pass with the fix, including rejection of incompatible equal-element-count batches. - All pre-commit checks passed. - Single-thread CPU / RTX 5090 microbenchmarks: matching `(4096, 16)` GPU batches improved from about 27 to 25 microseconds; the gravity broadcast case improved from about 33 to 27 microseconds. Matching large CPU batches were about 17% slower (1.03 to 1.20 ms); matching 4096-element CPU batches were about unchanged (73 to 71 microseconds). ## Type of change - Bug fix, with an explicit compatibility change for callers relying on implicit flattened pairing. ## Release backport - [ ] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have run the `pre-commit` checks with `./isaaclab.sh --format` - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/isaaclab/changelog.d/` - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Lynn <lynnhe02@gmail.com> Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
# Newton test suite: 11:23 → 7:54 (−31%) The `isaaclab_newton` suite runs through `pytest tools` on the same machine with a warm Warp cache. All 38 files pass. | | `develop` | this PR | |---|---:|---:| | Suite wall time | 11:23 | **7:54** | | Test functions | 411 | 297 | | Test lines | 19,085 | 15,880 | ## What changed The whole test surface was pruned with the `isaaclab-auditing-tests` skill (#8007). **Method.** Every test was marked keep, fix, merge or delete. Whenever a deletion's keeper does not carry the assertions verbatim, a deliberate production mutation proves the keeper fails. An independent review then restored every gap it found. The only production change is `transform_to_vec_quat` raising its documented `ValueError`; it has a changelog entry. - **Merged:** tests that rebuilt the same scene now share one build: - frame transformer: 5 scenes → 1 - contact filter + stale-reset - IMU/PVA init + freefall - joint-wrench init/reset - limits: 14 builds → 4 - root/COM writes - dynamics accessors - rigid/collection write tests - **Deleted:** - manager class checks implied by the end-to-end solver test - `test_wrench_kernels.py` - private helper replays in `test_site_injection.py` - the joint-wrench oracle that repeated the production transform - the duplicate effort-limit test (the PhysX copy is kept) - **Fixed tests that could not fail:** - delayed/remotized PD equivalence, which passed with no delay authored - DC-motor clamp - IMU stale-data (#4970) - `set_coms_index` - joint targets, now checked by value instead of with a launch spy - **Restored:** four tests #8003 removed without a remaining proof (`TestDelayedPDAuthoring` and the material-property tests). - **Device axes:** device-independent bookkeeping runs on CUDA only. Every public writer family keeps a real-simulation CPU test. ## Product defects found (not fixed here) - **Newton `ArticulationView` misaddresses per-env shapes** for rigid-object collections. - **`RigidObject.body_link_vel_w` can read stale** after a COM velocity write. ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## 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] My changes generate no new warnings - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Octi Zhang <zhengyuz@nvidia.com> Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
# Description Sixth PR in a series that splits the core cleanup in #7949 into small, reviewable pieces. This one fixes two bugs in `isaaclab.benchmark`. Each fix has a regression test that fails against `develop` and passes here. ## Fixes - `benchmark.measurements.TestPhase.from_json`: phases written with `TestPhaseEncoder` store `"metadata"` as a list, but `from_json` passed `m["metadata"]` to `metadata_from_dict`, which indexes `["metadata"]` again. Phases with measurements raised `TypeError`, and phases without measurements silently lost their metadata because the assignment sat inside the measurement loop. It now passes the phase dict once, after the loop. This also fixes `TestPhase.aggregate_json_files`. - `benchmark.recorders.record_cpu_info.CPUInfoRecorder.get_data`: it read the mean/std/count from a dict that is only filled by `update()`, so calling it first raised `KeyError`. It now reads the running statistics directly, which start at zero. ## Tests - `test/benchmark/test_formatters.py`: `test_phase_json_round_trip_keeps_metadata` (with and without measurements). - `test/benchmark/test_recorders.py`: `TestCPUInfoRecorder.test_get_data_measurement_names` also calls `get_data()` before the first update. Validation: 25 formatter/recorder tests passed after merging `develop`. All three regression cases fail against the unfixed production code (metadata decoding, metadata without measurements, and CPU data before update). Formatting, lint, and changelog checks passed. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## 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` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] 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 ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )