Skip to content

Cherrypick: Enable physics reset for ALL background (#1177) - #1180

Merged
xyao-nv merged 1 commit into
release/0.3.0from
xyao/fix/backport-background-reset
Sep 3, 2026
Merged

Cherrypick: Enable physics reset for ALL background (#1177)#1180
xyao-nv merged 1 commit into
release/0.3.0from
xyao/fix/backport-background-reset

Conversation

@xyao-nv

@xyao-nv xyao-nv commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Enable physics reset for ALL background #1177

Detailed description

  • Caught bug here
  • Enabled nested physics pose caching and reset by default for all library backgrounds, instead of kitchen only

## Summary
Enable physics reset for ALL backrgound

## Detailed description
- Caught bug
[here](https://nvidia.slack.com/archives/C0BR78Z3KAR/p1788389583037159)
- Enabled nested physics pose caching and reset by default for all
library backgrounds, instead of kitchen only



https://github.com/user-attachments/assets/dba87d32-01b2-4dc1-a6f7-50a9e3e1d975

---------

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
@xyao-nv
xyao-nv marked this pull request as ready for review September 3, 2026 19:34
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR enables nested physics-state reset by default for all backgrounds while retaining an explicit opt-out.

  • Changes the Background default and simplifies library background constructors around that default.
  • Updates reset-related terminology and documentation.
  • Adds an integration test verifying that a displaced Maple table and its velocity are restored on reset.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified.

The default-on reset path remains explicitly configurable, current library backgrounds use compatible USD spawners, and the added integration coverage verifies restoration of nested rigid-body state.

Important Files Changed

Filename Overview
isaaclab_arena/assets/background.py Changes nested physics reset from opt-in to default-on while preserving the constructor-level opt-out.
isaaclab_arena/assets/background_library.py Removes obsolete per-library reset defaults and forwards constructor overrides through the common background implementation.
isaaclab_arena/scene/scene.py Updates accessor documentation to describe reset-enabled rather than opted-in backgrounds.
isaaclab_arena/terms/events.py Updates a reset validation assertion message without changing behavior.
isaaclab_arena/tests/test_background_physics_reset.py Verifies the new default, explicit opt-out, and restoration of a moved Maple table's pose and velocity.
docs/pages/concepts/scene/concept_assets_design.rst Documents default nested physics resetting and the explicit opt-out.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Construct Background] --> B{reset_nested_physics}
  B -->|false| C[Use standard spawn path]
  B -->|true by default| D[Discover and de-instance nested physics roots]
  D --> E[Register reset-enabled paths in Scene]
  E --> F[Cache initial rigid and articulation state]
  F --> G[Environment reset]
  G --> H[Restore cached poses, velocities, and joint state]
Loading

Reviews (1): Last reviewed commit: "Enable physics reset for ALL background ..." | Re-trigger Greptile

Comment thread isaaclab_arena/assets/background.py
Comment thread isaaclab_arena/assets/background_library.py
@arena-review-bot

Copy link
Copy Markdown
Contributor

🤖 Isaac Lab-Arena Review Bot

Summary

Cherrypick of #1177 onto release/0.3.0: reset_nested_physics flips from opt-in to on-by-default, the per-class reset_nested_physics plumbing in LibraryBackground is dropped along with seven no-op __init__ overrides, and a regression test resets a nested body in maple_table_robolab. I diffed this branch against #1177 — it is byte-identical, so the cherrypick itself is clean and the question is whether the default flip is safe to land on a release branch.

Design, Boundaries & Scope

The flip is a real behavior change for everyone, not just the kitchen backgrounds that were opted in. Every background now goes through the de-instancing spawner, downloads and traverses its source USD at config-composition time, gets private PhysX views per nested rigid/articulation root, and runs ResetBackgroundPhysics._validate_runtime_composition — which asserts that source-USD discovery exactly matches the live composed stage. Where those diverge, a background that previously worked (with no reset) now hard-fails on the first reset. Details inline on isaaclab_arena/assets/background.py:36.

Resetting background physics is arguably the right default and the old one was the bug, so this is about validation coverage rather than the direction. The new test exercises exactly one library background; the ones with the most nested physics (galileo, packing_table, office_table, the lightwheel and replicator kitchens) are the ones most likely to trip the validation assert.

Findings

🟡 Warning: isaaclab_arena/assets/background.py:36 — default flip enables a hard-asserting reset path for every background; only maple_table_robolab is covered by a test. (inline)

🔵 Improvement: isaaclab_arena/assets/background_library.py:144OfficeTableBackground.__init__ is the last remaining override and only forwards a class attribute; it also raises TypeError if a caller now passes scale=. (inline)

🔵 Improvement: isaaclab_arena/scene/scene.py:92-98get_scene_cfg records an entry for a reset-enabled background even when discovery returns no physics roots, so _background_physics_paths becomes e.g. {"table": {}}. arena_env_builder only checks the outer dict for truthiness, so a physics-free background (table, table_oak_robolab) now gets a ResetBackgroundPhysics term that resets nothing but still runs the runtime-composition traversal on first reset. Skipping backgrounds whose discovery is empty would keep this off for them. (Not on a changed line, but the default flip is what makes it reachable.)

Test Coverage

The new test_maple_table_pose_restored_on_reset follows the file's existing inner/outer pattern via run_function_with_persistent_simulation_app, lands in Phase 1 like its siblings, and asserts real state (pose restored to the cached value, velocity zeroed) rather than just "no exception" — that is a genuine regression test for the reported bug, and it also covers the reset_nested_physics=False opt-out path.

The gap is breadth: it is the only library background verified against the newly-default-on path. Run tests, Run tests with subprocess, and the GR00T E2E job were still pending at review time; those results are the main signal for whether the other backgrounds survive _validate_runtime_composition.

Verdict

Minor fixes needed — the cherrypick is faithful and the test is solid; please confirm the heavier library backgrounds were exercised before this lands on release/0.3.0.

@xyao-nv
xyao-nv enabled auto-merge (squash) September 3, 2026 20:44
@xyao-nv
xyao-nv disabled auto-merge September 3, 2026 20:54
@xyao-nv
xyao-nv merged commit ea55c38 into release/0.3.0 Sep 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants