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

[hujc7]

1. Summary

• Demonstrates that PR 5826's producer-side pxr deferral, applied on top of PR 5771's resolve_backend_and_visualizer helper, lets the demo script keep imports at module top — the canonical IsaacLab idiom
• The deferred-imports-inside-design_scene() workaround in PR 5771 is no longer needed
• 1 demo script touched (8 lines moved)

2. Branch composition

This branch chains three sets of changes for review-as-one. Diff order against develop:

1. PR [Fix] Defer pxr loading in SimulationContext and AssetBase #5826 commits (80b86a01070, 33d40434df5): producer-side pxr deferral in source/isaaclab/isaaclab/assets/asset_base.py and source/isaaclab/isaaclab/sim/simulation_context.py.
2. PR Support Demo Scripts With Multiple Backends #5771 commits (merged at 6c292c1802b): YizeWang's scripts/demos/demo_helper.py and scripts/demos/quadrupeds.py with resolve_backend_and_visualizer.
3. This branch's commit 690f2805f09: lift the deferred imports back to module top in scripts/demos/quadrupeds.py.

3. Mechanism

After (1), from isaaclab.assets import Articulation, from isaaclab.sim import SimulationContext, and import isaaclab.sim as sim_utils all keep sys.modules free of pxr/omni/carb/isaacsim until a runtime touch (e.g. sim_utils.SimulationContext.instance()).

(3) takes advantage of this: the 7 deferred imports inside design_scene() and 1 inside main() move back to module top, ordered after args_cli = parser.parse_args() to match the existing """Parse CLI first so we can decide whether to launch Isaac Sim Kit.""" block.

resolve_backend_and_visualizer from PR 5771 still gates the conditional AppLauncher launch inside the with block; the script's behavior under --physics newton_mjwarp --visualizer newton (no Kit) is unchanged.

4. Verification

Ran the script end-to-end (step-capped, headless) for all four backend combinations from the docstring:

Combo	Outcome
--physics newton_mjwarp --visualizer newton (kit-less)	[INFO]: Setup complete, sim stepped, rc=0
--physics newton_mjwarp --visualizer kit	rc=0
--physics physx --visualizer newton	rc=0
--physics physx --visualizer kit	rc=0

Plus an assert "pxr" not in sys.modules immediately after the module-top asset imports — passed in every case.

5. Out of scope

• This PR does not propose merging on its own. It chains 5826 and 5771 into a reviewable single state so reviewers can see the end shape if both land.
• If PR 5771 merges first, the relaxation here becomes a one-commit follow-up on top of 5826.
• If PR 5826 merges first, the relaxation here can be folded into PR 5771's branch directly.

6. Test plan

• ./isaaclab.sh -f clean
• End-to-end run of quadrupeds.py across all 4 backend combinations
• Existing demo / tutorial tests still green on PhysX backend
• Existing Newton kitless demo tests still green

[isaaclab-review-bot]

Code Review Summary

This PR demonstrates the combined effect of PRs #5826 and #5771, enabling module-top imports in demo scripts by deferring pxr loading until runtime. The approach is architecturally sound and maintains backward compatibility.

---

🔬 Architecture & Design

Design Assessment: The pxr deferral pattern is the correct approach for enabling kit-less workflows. Moving pxr imports to TYPE_CHECKING blocks and accessing them through sim_utils (which uses lazy module loading) cleanly separates import-time from runtime dependencies.

Cross-Module Impact: Well-contained. The changes to asset_base.py and simulation_context.py use the existing sim_utils namespace, so call sites remain unchanged. No API breaks.

File Placement: demo_helper.py in scripts/demos/ is appropriate for demo-specific utilities. The helper is not part of the core isaaclab package, which is correct—it's demo infrastructure.

---

🔍 Implementation Findings

🟡 Warning: scripts/demos/quadrupeds.py:193 — Timestep changed without documentation

The physics timestep changed from dt=0.01 to dt=0.005. This halves the timestep (doubling simulation accuracy but also computational cost). If intentional for Newton backend compatibility, please document in the PR description or add a comment explaining the choice.

# Current (PR):
sim_cfg = sim_utils.SimulationCfg(dt=0.005, device=args_cli.device, ...)

# Base (develop):
sim = sim_utils.SimulationContext(sim_utils.SimulationCfg(dt=0.01))

🔵 Improvement: scripts/demos/demo_helper.py:75 — Defensive check for visualizer attribute access

The has_no_alive_visualizer_window function accesses v.is_closed as an attribute, but if this were a method call that fails, it would raise an exception. Consider whether is_closed could ever be undefined on a visualizer object:

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

If is_closed is guaranteed by the BaseVisualizer interface, this is fine. Otherwise, consider getattr(v, 'is_closed', True) as a defensive fallback.

🔵 Improvement: scripts/demos/quadrupeds.py:139 — Type annotations loosened

The run_simulator signature changed from typed parameters to generic ones:

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

# After:
def run_simulator(sim, entities: dict, origins):

While acceptable for a demo script, keeping the type hints aids IDE support and documentation. Consider preserving them if the interface is stable.

---

🧪 Test Coverage

Coverage Assessment: This PR is a demonstration/refactoring change rather than a new feature. The core logic changes in asset_base.py and simulation_context.py are internal import restructuring that doesn't change runtime behavior.

Verification: The PR author's manual verification matrix (4 backend combinations + assert \"pxr\" not in sys.modules) is appropriate validation for import-order changes, which are difficult to unit test.

Recommendation: No additional tests required for this PR. The deferred import pattern is exercised by existing tests that use Articulation and SimulationContext.

---

✅ Summary

Aspect	Status
Architecture	✅ Sound pxr deferral approach
API Compatibility	✅ No breaking changes
Code Quality	✅ Clean with minor suggestions
Test Coverage	✅ Adequate for refactoring PR
Documentation	⚠️ dt change needs explanation

Verdict: Minor fixes needed (clarify dt change)

The pxr deferral mechanism is well-implemented and achieves the stated goal of allowing module-top imports without forcing Kit initialization. Once the timestep change is documented, this is ready to proceed.

---

Automated review by IsaacLab Review Bot 🤖