Support Demo Scripts With Multiple Backends by YizeWang · Pull Request #5771 · isaac-sim/IsaacLab

YizeWang · 2026-05-25T16:53:51Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (existing functionality will not work without user modification)
Documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

I have read and understood the contribution guidelines
I have run the pre-commit checks with ./isaaclab.sh --format
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
I have updated the changelog and the corresponding version in the extension's config/extension.toml file
I have added my name to the CONTRIBUTORS.md or my name already exists there

Signed-off-by: Yize Wang <yizew@nvidia.com>

isaaclab-review-bot

Follow-up Review (a427283): New commits address several concerns and add new features.

✅ Issues Resolved

has_no_alive_visualizer_window infinite loop — FIXED 🎉
- Old: bool(sim.visualizers and not any(v.is_running() and not v.is_closed for v in sim.visualizers))
- New: visualizers = sim.visualizers or () + return not any(...)
- Empty/None visualizers now correctly return True ("no alive windows"), allowing the loop to exit. This was the most critical issue from the previous review.
PhysX + Newton visualizer combination — NOW SUPPORTED
- Previous TODO ("Not supported yet. Investigation needed.") has been implemented
- Comment clarifies: "PhysX still launches Isaac Sim Kit headless because it depends on Kit extensions"
- The if viz_type == "kit" or args.physics == "physx" condition correctly handles this case
Code simplification
- dt folded directly into SimulationCfg(dt=0.005, ...) — cleaner
- Articulation instantiation simplified: Articulation(CFG.replace(...)) instead of intermediate _cfg variables
- Loop condition cleaner: while not has_no_alive_visualizer_window(sim): instead of break-inside-while-True

🆕 New Additions

get_usage_examples() helper function — Extracts bash examples from docstrings for argparse epilog. Nice UX improvement for --help output.
Expanded docstring examples — More usage combinations documented (physx+kit, physx+newton, newton_mjwarp+kit, etc.)
Stricter visualizer validation — if not isinstance(args.visualizer, list) or len(args.visualizer) != 1: raise ValueError(...) — Explicit error instead of silent fallback

⚠️ Remaining Concerns (from original review)

Relative import from demo_helper import ... — Still assumes working directory is scripts/demos/. This may break if invoked from a different directory.
args.visualizer = [viz_type] mutation — Still present in resolve_backend_and_visualizer (L65). The helper mutates the args namespace before passing to AppLauncher.
conflict_handler="resolve" removal — Good: this was removed. But parser.set_defaults(visualizer=["kit"]) now sets a default that may conflict with AppLauncher's --visualizer handling. Worth verifying this doesn't cause issues.
No tests added — PR still lacks unit tests for the new helper functions.

📋 Summary

Previous Issues	Status
Infinite loop in `has_no_alive_visualizer_window`	✅ Fixed
PhysX + Newton visualizer unsupported	✅ Fixed
Relative import fragility	⏳ Not addressed
Args mutation side-effect	⏳ Not addressed
No tests	⏳ Not addressed

Verdict: The critical infinite loop bug is fixed 🎉. The remaining concerns are minor/stylistic. This looks much better than before.

_{🤖 Follow-up review by Isaac Lab Review Bot}

Update (b20cdab): New commits add two documentation comments to scripts/demos/quadrupeds.py:

Comment explaining why Kit visualizer is the default
Comment before main() call

These are purely cosmetic changes — no functional code modified. Previous concerns remain unchanged (see summary table above). No new issues.

Update (5e57982): Added physx_cfg parameter to resolve_backend_and_visualizer(), allowing callers to pass custom PhysX configuration instead of always using default PhysxCfg(). Clean enhancement with backward compatibility (parameter is optional). No new issues.

greptile-apps · 2026-05-25T16:58:25Z

Greptile Summary

This PR extracts a demo_helper.py module and updates quadrupeds.py to allow demo scripts to select a physics backend (physx or newton_mjwarp) and visualizer (kit or newton) independently via CLI flags, deferring Kit launch until the context manager decides it is required.

demo_helper.py (new): introduces resolve_backend_and_visualizer (a context manager that resolves physics/visualizer configs and conditionally launches Kit), has_no_alive_visualizer_window (loop-exit predicate), and get_usage_examples (argparse epilog helper).
quadrupeds.py: removes the hard-coded AppLauncher startup at module level and replaces the while simulation_app.is_running() loop with the backend-agnostic while not has_no_alive_visualizer_window(sim) condition, wrapping simulation setup inside the new context manager.

Confidence Score: 5/5

Safe to merge — the changes are additive, the loop-exit predicate is consistent with existing scripts, and the context manager correctly defers Kit launch without leaking resources.

The refactoring is narrowly scoped to the demo layer. is_closed is a @property on BaseVisualizer so the predicate in has_no_alive_visualizer_window is correct. The or () fallback for an empty visualizers list correctly lets the loop exit, matching the pattern in zero_agent.py and random_agent.py. AppLauncher is kept alive by the signal handlers it registers during construction, so the immediate discard of the instance after capturing .app.close does not cause premature cleanup. No wrong-data, stale-state, or resource-leak paths were found.

No files require special attention.

Important Files Changed

Filename	Overview
scripts/demos/demo_helper.py	New helper module: resolve_backend_and_visualizer context manager, has_no_alive_visualizer_window predicate (is_closed is correctly a @Property), and get_usage_examples parser helper. Logic is sound and consistent with existing codebase patterns.
scripts/demos/quadrupeds.py	Updated to use the new helper; Kit launch deferred into the context manager, loop-exit predicate replaced, imports of sim-extension types moved inside functions so they load only after Kit (or Newton) is ready.

_{Reviews (2): Last reviewed commit: "Recover 2 Comments" | Re-trigger Greptile}

greptile-apps · 2026-05-25T16:58:30Z

+    if needs_kit:
+        from isaaclab.app import AppLauncher
+
+        args.visualizer = [viz_type]
+        close_fn = AppLauncher(args).app.close


Mutable side-effect on caller's args namespace

args.visualizer is mutated in-place from a str (as parsed by the script's own --visualizer argument) to a list before AppLauncher is constructed. This leaks into the caller's namespace after the context manager exits and makes resolve_backend_and_visualizer non-reentrant. Using a shallow copy of the namespace would contain the mutation to the helper.

Suggested change

if needs_kit:

from isaaclab.app import AppLauncher

args.visualizer = [viz_type]

close_fn = AppLauncher(args).app.close

if needs_kit:

from isaaclab.app import AppLauncher

import copy

kit_args = copy.copy(args)

kit_args.visualizer = [viz_type]

close_fn = AppLauncher(kit_args).app.close

Signed-off-by: Yize Wang <yizew@nvidia.com>

hujc7

Maybe need to add some smoke test generally for demos?

hujc7 · 2026-05-27T04:01:40Z

    return scene_entities, origins


-def run_simulator(sim: sim_utils.SimulationContext, entities: dict[str, Articulation], origins: torch.Tensor):


Any reason to remove the type hint here?

Same reason as my last comment: https://github.com/isaac-sim/IsaacLab/pull/5771/changes#r3310474409. These types are not defined yet (these packages not imported). Let me know if my understanding in the previous comment is wrong.

I think we can use from __future__ import annotations and type checking for this. shouldn't be necessary to remove the type hints

Signed-off-by: Yize Wang <yizew@nvidia.com>

YizeWang · 2026-05-27T11:15:18Z

+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics newton_mjwarp --visualizer newton
+
+    # Usage with the PhysX backend without Kit (launches Newton visualizer).
+    # TODO(yizew@nvidia.com): Not supported yet. Investigation needed.


Is physx + newton expected to work by design? physx seems dependent on IsaacSim. I need to properly resolve this dependency if we want this pair to work.

Was it just the kit launch? or there are other issues?

nit: ideally this help message should also show up when run with -h/--help

Added help messages in the "Append Usage Examples to Help Menu" commit.

Signed-off-by: Yize Wang <yizew@nvidia.com>

hujc7 · 2026-05-27T17:52:47Z

+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics newton_mjwarp --visualizer newton
+
+    # Usage with the PhysX backend without Kit (launches Newton visualizer).
+    # TODO(yizew@nvidia.com): Not supported yet. Investigation needed.


Was it just the kit launch? or there are other issues?

Signed-off-by: Yize Wang <yizew@nvidia.com>

YizeWang · 2026-05-28T02:42:40Z

+
+
+@contextmanager
+def resolve_backend_and_visualizer(args, newton_cfg=None):


The option is limited inside ["kit", "newton"], to avoid dealing with rerun or vizer and to minimize the branches in demo_helper.py. Please let me know if these 2 visualizers are also supposed to be supported.

I think it'd be nice to also support rerun and viser so that we can be consistent across all the scripts

hujc7 · 2026-05-28T05:07:01Z

@greptileai review

kellyguo11 · 2026-05-28T04:37:50Z

+    # Resolve visualizer cfg
+    if not isinstance(args.visualizer, list) or len(args.visualizer) != 1:
+        raise ValueError("Demos support exactly one --visualizer value: kit or newton.")
+    viz_type = args.visualizer[0]


why do we only parse the first element? is there a limitation to supporting multiple visualizers like the training scripts?

kellyguo11 · 2026-05-28T04:38:27Z

+    elif args.physics == "newton_mjwarp":
+        from isaaclab_newton.physics import MJWarpSolverCfg, NewtonCfg
+
+        DEFAULT_NEWTON_CFG = NewtonCfg(


how generalizable is this cfg? do we expect this to work for most scripts? if not, it may be better to just keep this as a default MjWarpSolverCfg() object.

kellyguo11 · 2026-05-28T04:39:02Z

+
+
+@contextmanager
+def resolve_backend_and_visualizer(args, newton_cfg=None):


I think it'd be nice to also support rerun and viser so that we can be consistent across all the scripts

kellyguo11 · 2026-05-28T04:44:13Z

    # Simulate physics
-    while simulation_app.is_running():
+    # Exit when every visualizer window has been closed (works for kit and newton)
+    while not has_no_alive_visualizer_window(sim):


the double negative can be a bit confusing. would it make sense to make the utility has_alive_visualizer_window instead?

kellyguo11 · 2026-05-28T05:05:43Z

    return scene_entities, origins


-def run_simulator(sim: sim_utils.SimulationContext, entities: dict[str, Articulation], origins: torch.Tensor):


I think we can use from __future__ import annotations and type checking for this. shouldn't be necessary to remove the type hints

hujc7 · 2026-05-28T05:34:53Z

+    # Usage with the default PhysX backend (launches Isaac Sim Kit by default).
    ./isaaclab.sh -p scripts/demos/quadrupeds.py
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --visualizer kit
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics physx
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics physx --visualizer kit
+
+    # Usage with the kit-less Newton (MJWarp) backend (launches Isaac Sim Kit by default).
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics newton_mjwarp
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics newton_mjwarp --visualizer kit
+
+    # Usage with the Newton (MJWarp) backend without Kit (launches Newton visualizer).
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics newton_mjwarp --visualizer newton
+
+    # Usage with the PhysX backend and Newton visualizer.
+    # PhysX still launches Isaac Sim Kit headless because it depends on Kit extensions.
+    ./isaaclab.sh -p scripts/demos/quadrupeds.py --physics physx --visualizer newton


this could be simplified a bit to avoid duplicates

Signed-off-by: Yize Wang <yizew@nvidia.com>

Support Demo Scripts With Multiple Backends

05b1434

Signed-off-by: Yize Wang <yizew@nvidia.com>

YizeWang requested a review from ooctipus as a code owner May 25, 2026 16:53

github-actions Bot added the isaac-lab Related to Isaac Lab team label May 25, 2026

isaaclab-review-bot Bot reviewed May 25, 2026

View reviewed changes

greptile-apps Bot reviewed May 25, 2026

View reviewed changes

Minor Changes

c73177b

Signed-off-by: Yize Wang <yizew@nvidia.com>

hujc7 reviewed May 27, 2026

View reviewed changes

Yize Wang added 2 commits May 27, 2026 16:33

Resolve Comments

1beee90

Signed-off-by: Yize Wang <yizew@nvidia.com>

Fold dt Into Function Call

e85e64a

Signed-off-by: Yize Wang <yizew@nvidia.com>

YizeWang commented May 27, 2026

View reviewed changes

Comment thread scripts/demos/quadrupeds.py Outdated

YizeWang commented May 27, 2026

View reviewed changes

Comment thread scripts/demos/quadrupeds.py

Yize Wang added 3 commits May 27, 2026 19:20

Recover 3-Line Comments

35b051d

Signed-off-by: Yize Wang <yizew@nvidia.com>

Revert Articulation Import Location Change

34a07b1

Signed-off-by: Yize Wang <yizew@nvidia.com>

Add Instructions

018a1e6

Signed-off-by: Yize Wang <yizew@nvidia.com>

hujc7 reviewed May 27, 2026

View reviewed changes

Yize Wang added 3 commits May 28, 2026 10:07

Support Physx+NewtonViz

9181225

Signed-off-by: Yize Wang <yizew@nvidia.com>

Append Usage Examples to Help Menu

a427283

Signed-off-by: Yize Wang <yizew@nvidia.com>

Recover 2 Comments

b20cdab

Signed-off-by: Yize Wang <yizew@nvidia.com>

YizeWang commented May 28, 2026

View reviewed changes

kellyguo11 reviewed May 28, 2026

View reviewed changes

hujc7 mentioned this pull request May 28, 2026

[Test] Quadrupeds: relax import order (depends on #5826 + #5771) #5828

Draft

4 tasks

hujc7 reviewed May 28, 2026

View reviewed changes

Enable PhysxCfg Customization

5e57982

Signed-off-by: Yize Wang <yizew@nvidia.com>

		return scene_entities, origins


		def run_simulator(sim: sim_utils.SimulationContext, entities: dict[str, Articulation], origins: torch.Tensor):



		@contextmanager
		def resolve_backend_and_visualizer(args, newton_cfg=None):

Conversation

YizeWang commented May 25, 2026

Description

Type of change

Screenshots

Checklist

Uh oh!

isaaclab-review-bot Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

✅ Issues Resolved

🆕 New Additions

⚠️ Remaining Concerns (from original review)

📋 Summary

Uh oh!

greptile-apps Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

greptile-apps Bot May 25, 2026

Choose a reason for hiding this comment

Uh oh!

hujc7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YizeWang May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

YizeWang May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hujc7 May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hujc7 commented May 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

isaaclab-review-bot Bot left a comment •

edited

Loading

greptile-apps Bot commented May 25, 2026 •

edited

Loading

YizeWang May 27, 2026 •

edited

Loading

YizeWang May 27, 2026 •

edited

Loading

hujc7 May 27, 2026 •

edited

Loading