Repository navigation
[Backport release/3.0.0] Backport six non-breaking bug fixes - #8249
Merged
kellyguo11 merged 6 commits intoOct 2, 2026
Merged
kellyguo11 merged 6 commits into
kellyguo11 merged 6 commits into
Conversation
Backport isaac-sim#7131 to release/3.0.0. (cherry picked from commit 7c05e96)
StafaH
requested review from
AntoineRichard,
fatimaanes,
kellyguo11,
ooctipus and
pbarejko
as code owners
October 1, 2026 20:33
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Isaac Lab Review Bot
The backport adds an initialization guard to CircularBuffer.buffer, replacing an implementation-level PyTorch failure before the first append with a documented RuntimeError. The focused implementation, changelog entry, and regression coverage are consistent with the existing buffer lifecycle.
- Design and architecture: The validation remains at the property that owns access to
_buffer, matching the lifecycle checks already used by__getitem__andstacked. No new abstraction, state duplication, or cross-layer responsibility is introduced. - API: The reviewed public surface is
CircularBuffer.buffer. Its post-append shape, oldest-to-newest ordering, device, dtype, and legacy versusstack_dimbehavior remain unchanged. The docstring now explicitly documents the pre-appendRuntimeError. - Compatibility and deprecation: Breaking changes: none identified. Pre-append access previously leaked an incidental PyTorch
TypeErrorfrom operating onNone; the patch intentionally replaces that failure with a clear, documentedRuntimeError. No evidence establishes the former implementation error as a supported contract, while all initialized access paths remain unchanged. - Implementation: The new
_buffer is Noneguard runs before both existing layout branches. Once initialized, the property still delegates directly totorch.transposein legacy mode ortorch.movedimin stacked-output mode, preserving the existing data path without added allocation or synchronization. The package changelog records the corrected failure behavior. - Style consistency: The added
Raises:section follows the neighboring Google-style property documentation, and the concise error message gives the caller a concrete remedy. The implementation adds no redundant comments or structural churn, and the changelog wording is specific to the corrected behavior. - Test quality: The changed tests were audited for ownership and duplication. The new empty-property assertion directly covers the regression at the
CircularBuffer.bufferboundary and checks both exception type and actionable wording. The first-append assertion independently preserves the default-mode warmup-fill contract before the existing overflow checks; it does not duplicate the separate stacked-output-mode case.
No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.
Automated review; human maintainers own approval decisions.
Contributor
 |
Collaborator
Author
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
StafaH
changed the title
# Description
Fixes Blender subprocess invocation in
`scripts/tools/process_meshes_to_obj.py`.
The helper previously constructed the command as one formatted string
and then called `.split(" ")`. Any valid Blender, input, output, or
script path containing whitespace was therefore split into multiple
command-line arguments.
The subprocess was also launched without `check=True`, so a failed
Blender conversion could be silently treated as success by the wrapper.
The helper now builds the subprocess argument list directly, preserves
each path as one argument, requests `check=True`, and raises a clear
error when Blender cannot be found on `PATH`.
## Type of change
- Bug fix
## Validation
- Added a unit regression test with Blender, input, and output paths
containing spaces.
- The test verifies each path remains one subprocess argument and
`check=True` is used.
- Added coverage for the missing-Blender error path.
- No Blender installation is required by the tests.
- No package changelog fragment is required because this changes a
repository script, not a `source/<pkg>/` package.
## Checklist
- [x] I have read and understood the contribution guidelines
- [x] I have added regression tests
- [x] No new dependencies
---------
Co-authored-by: Kelly Guo <kellyg@nvidia.com>
(cherry picked from commit b9074a3)
# Description
Fixes two path-handling bugs in `scripts/tools/blender_obj.py`.
When the output is a basename such as `robot.obj`, `os.path.dirname()`
returns an empty string. The converter previously tried to create that
empty path with `os.makedirs("")`, so writing directly to the current
working directory failed.
The optional USD export also used `out_file.replace("obj", "usd")`,
which replaces every matching substring in the full path. A valid path
such as `objects/robot.obj` could therefore be changed to
`usdects/robot.usd`.
The converter now creates a parent directory only when one is present
and derives the USD path by replacing only the file extension.
## Type of change
- Bug fix
## Validation
- Added a unit regression test for an OBJ basename in the current
working directory.
- Added a regression test verifying `objects/robot.obj` maps to
`objects/robot.usd` without changing directory names.
- Blender is mocked, so the tests do not require a Blender installation.
- Added the required `isaaclab` `.skip` changelog marker because managed
package tests are touched.
## Checklist
- [x] I have read and understood the contribution guidelines
- [x] I have added regression tests
- [x] No new dependencies
---------
Co-authored-by: Kelly Guo <kellyg@nvidia.com>
(cherry picked from commit b18ab03)
# Description Fixes `dump_yaml()` changing valid `.yml` filenames. The helper currently checks only whether the filename ends with the literal string `yaml`. Passing a standard YAML filename such as `config.yml` therefore writes `config.yml.yaml` instead of the requested path. The extension check now accepts both `.yaml` and `.yml` case-insensitively. Filenames without a YAML extension continue to receive the default `.yaml` suffix. ## Type of change - Bug fix ## Validation - Extended the existing YAML I/O unit tests. - Added a regression test verifying `config.yml` is written unchanged and `config.yml.yaml` is not created. - Existing coverage still verifies extensionless filenames receive `.yaml`. - Added the required `isaaclab` changelog fragment. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have added a regression test - [x] I have added the required changelog fragment - [x] No new dependencies (cherry picked from commit 7c69c9b)
# Description Removes stray debug output from `ActionManager.get_IO_descriptors`. Successful descriptor export currently executes `print(item["export"])` for every action term, so normal export writes unrelated `True` / `False` lines to stdout. The export flag is already consumed immediately afterwards and does not need to be printed. This removes only that debug print. The existing error message for descriptors that actually fail to build is unchanged. ## Type of change - Bug fix ## Validation - Added a `pytest.mark.unit` regression test using a valid action descriptor. - The test captures stdout and verifies successful descriptor export is silent. - The exported descriptor content is still checked. - Added the required `isaaclab` changelog fragment. ## Checklist - [x] I have read and understood the contribution guidelines - [x] I have added a regression test - [x] I have added the required changelog fragment - [x] No new dependencies --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com> (cherry picked from commit 07d2c31)
# Description Fixes Newton-native actuators driving only the first asset of a multi-asset scene. **Symptom.** In `examples/visual_color_randomization.py` with `physics=newton_mjwarp`, only the stock ANYmal (the first asset) holds its default pose. The other four styles have no joint control and collapse to the floor within about 50 steps. **Root cause.** `define_actuator_properties` authored `NewtonActuator` USD prims on only the *first* prim matching the articulation path (`find_first_matching_prim`). `MultiAssetSpawner` spawns a distinct articulation per asset at `env_<i>/Robot` before cloning, so the other assets had no actuator prims. Newton's `ModelBuilder` only sees actuators authored on the prim it imports, so those assets imported with no actuators and their joints received no torque. With a single asset, only `env_0` exists at spawn time, so "first match" happened to be enough. **Fix.** Author the prims on every matching articulation, and resolve the matches on the stage that was passed in rather than the current stage. For a single asset there is one match, so behavior is unchanged. Visual materials are not involved. I isolated this on pristine `develop` with five *identical* plain assets through the multi-asset spawner, which reproduces the collapse exactly, while a single asset controls all envs. Fixes # (issue) ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Verification New test `test_define_actuator_properties_authors_every_spawned_articulation`: fails without the change, passes with it. `source/isaaclab/test/actuators/test_newton_adapter.py`: 12 passed. ruff 0.14.10 check and format clean. Real demo, `examples/visual_color_randomization.py`, 10 envs, `newton_mjwarp`, no visualizer, before and after on `develop`: | | before | after | |---|---|---| | `NewtonActuator` prims on stage | `env_0` only | one per asset source (`env_0, 2, 4, 6, 8`) | | native actuator indices in the Newton model | 24 (worlds 0 and 1) | 120 (all 10 worlds) | | envs with nonzero applied torque | 2 of 10 | 10 of 10 | | root height at step 100 | 0.43 (2 envs), 0.07 (8 envs) | 0.43 (all 10) | The number of controlled envs equaled the first asset's block size in every case I tried (2 of 10, 3 of 15, 1 of 7). ## Not covered I checked the PhysX and OVPhysX tests that mention these helpers; they patch their own backend's `find_first_matching_prim` and do not depend on this function. I did not run those GPU/runtime suites. ## Checklist - [ ] I have read and understood the contribution guidelines - [ ] I have run the `pre-commit` checks with `uv run isaaclab --format` (I ran ruff 0.14.10 check and format, the same hook, on the changed files only) - [x] I have made corresponding changes to the documentation (docstring updated; no user-facing docs affected) - [ ] My changes generate no new warnings - [x] I have validated the changed behavior with focused coverage - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [ ] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit c1242ed)
StafaH
changed the title
Collaborator
Author
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
StafaH
force-pushed
the
backport/release-3.0.0-circular-buffer
branch
from
October 2, 2026 00:22
5db9d43 to
43df6ad
Compare
StafaH
changed the title
Collaborator
Author
|
run-ci |
isaaclab-bot
Bot
added
ci:run-docker
kellyguo11
approved these changes
Oct 2, 2026
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
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.
Description
Backport six non-breaking bug fixes to
release/3.0.0:.ymlfilenames.All six fixes are exact cherry-picks. Original tests and changelog fragments are included; no new dependencies.
Validation
git diff --checkpassed.Type of change