Skip to content

Enable physics reset for ALL background - #1177

Merged
xyao-nv merged 2 commits into
mainfrom
xyao/fix/background_reset_table
Sep 3, 2026
Merged

Enable physics reset for ALL background#1177
xyao-nv merged 2 commits into
mainfrom
xyao/fix/background_reset_table

Conversation

@xyao-nv

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

Copy link
Copy Markdown
Collaborator

Summary

Enable physics reset for ALL backrgound

Detailed description

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

@xyao-nv
xyao-nv marked this pull request as ready for review September 3, 2026 00:05
@xyao-nv xyao-nv changed the title Enable physics reset for ALL backrgound Enable physics reset for ALL background Sep 3, 2026
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR enables nested-physics pose caching and reset by default for every library background while retaining an explicit opt-out.

  • Changes LibraryBackground.reset_nested_physics from false to true and removes redundant kitchen-specific overrides.
  • Updates terminology and documentation to describe reset-enabled backgrounds and the opt-out behavior.
  • Adds an integration test proving that a displaced Maple table has its pose and velocity restored on reset.

Confidence Score: 5/5

The PR appears safe to merge because no concrete blocking or non-blocking defect remains in the changed behavior.

The default propagates correctly through library background construction, explicit false values still opt out, and the reset implementation scopes restored state to the requested environments.

Important Files Changed

Filename Overview
isaaclab_arena/assets/background_library.py Makes nested-physics reset the library-wide default while preserving explicit constructor opt-out semantics.
isaaclab_arena/tests/test_background_physics_reset.py Adds an end-to-end Maple table test covering pose restoration and velocity clearing.
isaaclab_arena/scene/scene.py Updates accessor documentation from opt-in terminology to reset-enabled terminology without behavioral changes.
isaaclab_arena/terms/events.py Updates an assertion message to match the new default-enabled terminology.
docs/pages/concepts/scene/concept_assets_design.rst Documents default nested-physics resets and the explicit false opt-out.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Library background constructed] --> B{reset_nested_physics}
  B -->|true by default| C[Discover nested rigid and articulation roots]
  B -->|explicit false| D[Skip nested-physics reset registration]
  C --> E[Register reset views]
  E --> F[Cache initial environment-local state]
  F --> G[Episode reset]
  G --> H[Restore pose, velocity, and articulation state]
Loading

Reviews (1): Last reviewed commit: "enable background reset for all" | Re-trigger Greptile

Comment thread isaaclab_arena/assets/background_library.py Outdated
Comment thread docs/pages/concepts/scene/concept_assets_design.rst
@arena-review-bot

Copy link
Copy Markdown
Contributor

🤖 Isaac Lab-Arena Review Bot

Summary

Flips reset_nested_physics from opt-in to on-by-default for library backgrounds, so nested rigid bodies and articulations in backgrounds like the Maple table are pose-cached and restored on reset. The direction looks right — the previous opt-in list was easy to forget, which is exactly the bug that was caught. Two things to sort out: the default only moved on LibraryBackground, and the opt-out the docs now advertise is not actually callable on most library backgrounds.

Design, Boundaries & Scope

This turns on real per-run work for nine backgrounds that previously did none: kitchen, kitchen_with_open_drawer, packing_table, galileo, galileo_locomanip, table, office_table_background, maple_table_robolab, table_oak_robolab. For each of those, _get_spawn_cfg now swaps in _spawn_from_usd_with_resettable_nested_physics, which calls SetInstanceable(False) on every instanceable subtree containing physics — and then the whole thing gets cloned per environment.

Has this been checked at higher --num_envs on the large rooms (galileo, kitchen)? De-instancing trades USD instancing for per-env copies, so if those backgrounds carry instanced physics props the memory and load cost scales with env count in a way it did not before. If it turns out expensive on a particular background, opting that one back out is the escape hatch — which is the second finding below.

Also worth a sanity check: _validate_runtime_composition hard-asserts that source-USD discovery exactly matches the live composed stage. That assert now runs for all nine of the newly-enabled backgrounds instead of just the kitchens, so any discovery gap becomes a crash at first reset rather than a silently skipped background.

Findings

🟡 Warning: isaaclab_arena/assets/background_library.py:33 — the default moved to LibraryBackground, but Background.__init__ still defaults to False, so the docs' "by default" only holds for library backgrounds. Moving it to Background would also let the class attribute and the None-sentinel plumbing in LibraryBackground.__init__ go away, since nothing overrides it any more.

🟡 Warning: docs/pages/concepts/scene/concept_assets_design.rst:62 — eight of the library backgrounds declare def __init__(self) with no arguments, so get_asset_by_name("kitchen")(reset_nested_physics=False) is a TypeError. The documented opt-out currently requires subclassing.

Test Coverage

test_maple_table_pose_restored_on_reset is a proper regression test for the reported bug — it fails before the flip (the table would not be registered) and passes after — and it follows the inner/outer pattern with deferred imports, landing correctly in Phase 1. Coverage for the other newly-enabled backgrounds comes indirectly from the existing kitchen and packing_table tests, so the flip does get exercised beyond the one asset. Worth waiting on the full test run, which is still pending — that is where a _validate_runtime_composition mismatch on one of the newly-enabled backgrounds would show up.

Verdict

Minor fixes needed

Signed-off-by: Xinjie Yao <xyao@nvidia.com>

@qianl-nv qianl-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@xyao-nv
xyao-nv merged commit 1128ec4 into main Sep 3, 2026
10 checks passed
xyao-nv added a commit that referenced this pull request Sep 3, 2026
## Summary
Enable physics reset for ALL background
#1177

## 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

Signed-off-by: Xinjie Yao <xyao@nvidia.com>
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