[ooctipus]

Description

Fixes the Kit startup crash introduced by #8125 when MoviePy is installed. Importing the RL entrypoints pulled in the environment runtime and moviepy.editor, including unrelated editing dependencies, before Kit launched.

• Import the MARL wrapper only when constructing the wrapped environment, after simulation launch.
• Import ImageSequenceClip directly in the recorder instead of loading moviepy.editor. This preserves recording without importing SciPy, OpenCV, or matplotlib.
• Add one case to the existing subprocess import test. It catches eager environment imports even without MoviePy installed.

There are no launcher or distributed-device changes. Torch remains available before Kit; moving one import did not change that boundary.

Validation

• 121 tests passed across the entrypoint, distributed-device, and video-recorder suites, with MoviePy installed.
• The added regression failed when the original eager MARL import was restored and passed with the fix.
• The recorder wrote and decoded a three-frame MP4 using the direct import.
• GPU 0: Isaac-Cartpole-Direct, Isaac PhysX, 16 environments, one RSL-RL training iteration completed. A second run with the environment/video recorder deliberately imported before Kit also completed.
• Formatting, lint, and changelog checks passed.

Release backport

• Backport this pull request to the active release branch after it merges into develop

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks
• My changes generate no new warnings
• I have added tests that prove my fix is effective
• I have added changelog fragments for every touched package

[ooctipus]

run-ci

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Defers torch and environment imports to avoid startup crashes.

The PR should not merge until distributed Kit launches avoid the remaining pre-startup torch import.

Findings

1.  Distributed launches import torch early ▶
2.  Import order lacks regression coverage ▶

Summary

The PR defers the MARL converter import until environment creation and moves torch out of sim_launcher’s module-level imports to reduce pre-Kit loading.

• The converter import is deferred until after launch in the checked RL entry points.
• Distributed device resolution still imports torch before Kit starts, and the import-order fix lacks a targeted regression test.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[launch_simulation] --> B{distributed?}
  B -- yes --> C[Import torch to resolve device]
  B -- no --> D[Skip torch import]
  C --> E[Start Kit]
  D --> E
  E --> F[Create RL environment]
  F --> G[Import MARL converter if needed]

Loading

_{Reviews (1) · Last reviewed commit: "Keep the environment runtime out of the ..."}

[greptile-apps]

Distributed launches import torch early
With --distributed and an Isaac Sim/Kit configuration, launch_simulation calls _resolve_distributed_device before it starts Kit. This line therefore still imports torch before Kit starts, leaving distributed runs exposed to the NumPy/OpenBLAS startup-fork crash this PR aims to prevent.

[greptile-apps]

Import order lacks regression coverage
This fix relies on importing common without loading the environment runtime, but the existing import-isolation tests do not import common or check for moviepy and SciPy. A subprocess test for those imports would catch an eager import being restored; without it, the startup crash could return unnoticed.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[isaaclab-review-bot]

Isaac Lab Review Bot

The import deferrals preserve the existing execution paths while avoiding heavyweight dependencies before Kit startup. One style cleanup remains: remove the unnecessary # noqa: PLC0415 suppressions, which are inconsistent with equivalent deferred imports already present in the file.

• Design and architecture: Deferring multi_agent_to_single_agent to the MARL conversion branch and torch to distributed-device resolution follows the repository pattern for runtime-dependent imports. No new abstraction or ownership boundary is introduced.
• API: The signatures, defaults, return behavior, and owning-module exports remain unchanged. The conversion helper is still imported from isaaclab.envs at its point of use.
• Compatibility and deprecation: Breaking changes: none identified. The changed imports affect initialization timing rather than the documented entry-point contracts, and the symbols remain available from their owning modules.
• Implementation: The deferred imports precede their uses on the relevant branches: torch is loaded only after distributed mode is selected, and multi_agent_to_single_agent is loaded only when the created environment requires MARL conversion. The control flow remains otherwise unchanged.
• Style consistency: The rationale comments document the non-obvious startup constraint. However, the new # noqa: PLC0415 markers are inconsistent with sibling deferred imports such as WarpFrontend and the pretrained-checkpoint utilities, which need no suppression. Remove both new markers while retaining the rationale comments.
• Test quality: No test files changed, so there were no added cases to audit for necessity, ownership, or duplication. The PR reports validation through the existing entry-point, common-entry-point, and distributed-device-resolution suites.

Minor fixes needed. Posted 1 actionable finding inline.

Automated review; human maintainers own approval decisions.

[isaaclab-review-bot]

🔵 Suggestion · Style Consistency — noqa marker inconsistent with sibling deferred imports

The two other deferred imports in this same file (WarpFrontend at line 500 and pretrained_checkpoint at line 542) carry only a rationale comment and no # noqa: PLC0415, so the rule is evidently not enforced here and the new suppression is inert. Drop the # noqa: PLC0415 here and on the matching import torch in sim_launcher.py, keeping the explanatory comments.

[ooctipus]

run-ci