Support Demo Scripts With Multiple Backends

[YizeWang]

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

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot — Multi-Perspective Analysis

PR #5771: Support Demo Scripts With Multiple Backends
Author: @YizeWang | Files Changed: 2 | SHA: 05b1434

---

🏗️ Isaac Lab Expert — Architecture & Design

Summary: This PR introduces a demo_helper.py utility (context manager + helper function) to allow demo scripts to run with either PhysX or Newton (MJWarp) physics backends, paired with Kit or Newton visualizers. The quadrupeds.py demo is refactored to use this new helper.

Positives:

• ✅ Clean separation of backend/visualizer resolution into a reusable helper
• ✅ Context manager pattern ensures Kit cleanup via finally block
• ✅ Dynamic instantiation via cfg.class_type(cfg) — idiomatic for Isaac Lab's config-driven design
• ✅ Moves imports inside functions for conditional loading (avoids importing PhysX when using Newton and vice versa)
• ✅ Good CLI ergonomics: --physics and --visualizer with clear choices

Concerns:

1. conflict_handler="resolve" on ArgumentParser (quadrupeds.py L19): This silently overwrites --visualizer added by AppLauncher.add_app_launcher_args(). This hides potential conflicts rather than surfacing them. If AppLauncher's --visualizer argument semantics change upstream, this will silently break.

2. args.visualizer = [viz_type] mutation (demo_helper.py L59): The helper mutates the parsed args namespace before passing to AppLauncher. This side-effect is non-obvious to callers and changes the type from str to list[str], which could confuse downstream code.

3. Relative import from demo_helper import ... (quadrupeds.py L40): This assumes the working directory is scripts/demos/. If the script is invoked from a different working directory, this import will fail. Consider using a relative path setup (e.g., sys.path.insert) or making demo_helper a proper module within the Isaac Lab package.

4. DEFAULT_NEWTON_CFG hardcoded in helper (demo_helper.py L24-33): Physics solver parameters (njmax=70, nconmax=70, ls_iterations=40, etc.) are scene-dependent. Hardcoding them in a general helper means other demos using this helper may need very different values. The newton_cfg override parameter helps, but the defaults could be misleading.

---

🔇 Silent Failure Hunter — Error Handling

1. has_no_alive_visualizer_window returns False when sim.visualizers is empty (demo_helper.py L72): If sim.visualizers is None or empty list, bool([]) is False, so the function returns False — meaning "there IS an alive window." This causes the simulation to run forever with no visualizer. The logic seems inverted for the empty-visualizers edge case.

   def has_no_alive_visualizer_window(sim) -> bool:
       return bool(sim.visualizers and not any(v.is_running() and not v.is_closed for v in sim.visualizers))

   When sim.visualizers is empty/None → bool(None and ...) → False → loop never breaks. This is a potential infinite loop.

2. No validation that AppLauncher import succeeds in non-Kit path: If viz_type != "kit", AppLauncher is never imported and close_fn stays None. But the script still calls AppLauncher.add_app_launcher_args(parser) at the top of quadrupeds.py (L17). If Isaac Sim Kit is not installed, this will crash before even reaching the helper.

3. Missing error handling for close_fn() (demo_helper.py L67): If Kit shutdown throws an exception during cleanup in the finally block, it could mask the original exception from the try block.

---

🧪 Test Coverage Analyzer

1. No tests added: The checklist item "I have added tests that prove my fix is effective or that my feature works" is unchecked. For multi-backend support, testing the following would be valuable:

   • resolve_backend_and_visualizer with each physics/visualizer combination
   • has_no_alive_visualizer_window edge cases (empty list, None, mixed states)
   • CLI argument parsing with the new --physics / --visualizer flags

2. No CI validation visible: Only the labeler check passed. No integration or unit test runs are visible for this PR.

---

📋 Summary

Category	Findings
Architecture	4 concerns
Silent Failures	3 issues (1 potential infinite loop)
Test Coverage	No tests added
Total	7 findings

Verdict: Needs attention — The potential infinite loop in has_no_alive_visualizer_window when no visualizers are present is the most critical issue. The relative import and args mutation are also worth addressing before merge.

---

_{🤖 Review by Isaac Lab Review Bot (multi-perspective ensemble analysis)}

---

Update (c73177b): Reviewed incremental changes. New commit contains:

• 📝 Improved docstrings and comments (helper description, has_no_alive_visualizer_window docstring, Kit cleanup comment)
• 🔧 Minor code simplification (removed intermediate needs_kit variable)
• 📋 Added TODO noting physx+newton visualizer combination is not yet supported

Previous concerns status: No changes to the flagged logic — the has_no_alive_visualizer_window infinite loop risk and args.visualizer mutation remain unaddressed.

New issues: None introduced in this update.

[greptile-apps]

Greptile Summary

This PR adds multi-backend support to the quadrupeds.py demo script by introducing a new demo_helper.py that provides a resolve_backend_and_visualizer context manager, allowing the demo to run with either PhysX or Newton (MJWarp) physics and either Kit or Newton visualizers.

• demo_helper.py introduces resolve_backend_and_visualizer, which lazily imports the correct physics/visualizer config objects and conditionally launches AppLauncher only when Kit is needed; the Kit app is closed via a finally block.
• quadrupeds.py defers heavy imports inside functions, replaces direct Articulation(cfg) construction with cfg.class_type(cfg), replaces the simulation_app.is_running() loop guard with has_no_alive_visualizer_window(sim), and drops the unconditional top-level AppLauncher instantiation.

Confidence Score: 3/5

The demo refactor is largely sound, but the new exit-condition helper has a logic inversion that will cause an unbreakable infinite loop if the simulation's visualizer list is ever empty.

The has_no_alive_visualizer_window function short-circuits to False when sim.visualizers is falsy, telling the loop to keep running forever instead of exiting. While current call sites always populate visualizer_cfgs, the function is intended as a reusable helper and the wrong default behavior makes it a silent footgun for any future demo that forgets to attach a visualizer. The args namespace mutation is a lesser concern but still reflects fragile coupling between the helper and callers.

scripts/demos/demo_helper.py — both the exit-condition helper and the args-mutation logic warrant a closer look.

Important Files Changed

Filename	Overview
scripts/demos/demo_helper.py	New helper introducing resolve_backend_and_visualizer context manager and has_no_alive_visualizer_window; has a logic inversion in the empty-visualizers case (infinite loop) and a side-effecting mutation of the caller's args namespace.
scripts/demos/quadrupeds.py	Refactored to support multiple physics and visualizer backends; defers imports and AppLauncher creation; uses cfg.class_type pattern for robot construction; broadly clean but relies on demo_helper for exit logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[quadrupeds.py main] --> B[resolve_backend_and_visualizer context manager]
    B --> C{args.physics}
    C -->|physx| D[PhysxCfg]
    C -->|newton_mjwarp| E[NewtonCfg / MJWarpSolverCfg]
    B --> F{args.visualizer}
    F -->|kit| G[KitVisualizerCfg]
    F -->|newton| H[NewtonVisualizerCfg]
    B --> I{needs_kit?}
    I -->|yes| J[AppLauncher created, close_fn stored]
    I -->|no| K[close_fn = None]
    B --> L[yield physics_cfg, visualizer_cfg]
    L --> M[SimulationContext created with physics + visualizer cfgs]
    M --> N[design_scene]
    N --> O[run_simulator loop]
    O --> P{has_no_alive_visualizer_window?}
    P -->|yes| Q[break]
    P -->|no| O
    Q --> R[finally: close_fn if Kit]

Loading

_{Reviews (1): Last reviewed commit: "Support Demo Scripts With Multiple Backe..." | Re-trigger Greptile}

[greptile-apps]

Infinite loop when sim.visualizers is empty

bool(sim.visualizers and ...) short-circuits to False when sim.visualizers is falsy (empty list, None, etc.), meaning the function returns False — "there ARE alive visualizer windows" — even when none were ever registered. Any caller whose SimulationContext ends up with an empty visualizers list will spin in the while True loop indefinitely with no exit path. The guard should return True (exit the loop) when no visualizers exist, not False.

[greptile-apps]

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