From 113fbd477411f1205a3a2ad8c4c4b6a4d8672fe0 Mon Sep 17 00:00:00 2001 From: Octi Zhang Date: Mon, 28 Sep 2026 08:28:40 -0700 Subject: [PATCH 1/3] Inline launch-stack helpers that only restate or forward launch_simulation and scan turn launcher args into a dict once, so the Namespace/dict accessors and the separate logging-level resolver go away. The livestream mode and Kit-visualizer intent are resolved once, one-line predicates are inlined, and SettingsManager keeps one accessor and one setter. fuse_kit_args moves to a lightweight isaaclab.app.argv module, so dispatchers import it directly. KitLauncher inlines single-use helpers, merges the toolbar button helpers, and names its argument hook add_launcher_args. --- docs/source/api/lab/isaaclab.app.rst | 4 +- .../launch-helper-cleanup.major.rst | 16 ++ source/isaaclab/isaaclab/app/__init__.pyi | 4 +- source/isaaclab/isaaclab/app/argv.py | 39 +++ source/isaaclab/isaaclab/app/logging_utils.py | 36 +-- .../isaaclab/isaaclab/app/settings_manager.py | 51 ---- source/isaaclab/isaaclab/app/sim_launcher.py | 233 +++++++----------- .../isaaclab/isaaclab/benchmark/dispatch.py | 17 +- .../isaaclab/isaaclab/cli/commands/deploy.py | 2 +- .../isaaclab/devices/gamepad/se2_gamepad.py | 2 +- .../isaaclab/devices/gamepad/se3_gamepad.py | 2 +- .../isaaclab/sensors/camera/camera.py | 10 +- .../isaaclab/sim/simulation_context.py | 16 +- .../test_train_multigpu_command_building.py | 2 +- .../envs/test_action_state_recorder_term.py | 2 +- ...on_state_recorder_term_task_integration.py | 2 +- .../test/sensors/test_outdated_sensor.py | 2 +- .../test_simulation_context_visualizers.py | 4 +- .../changelog.d/launch-helper-cleanup.skip | 1 + .../test_ovphysx_scene_data_backend.py | 4 +- .../changelog.d/launch-helper-cleanup.rst | 6 + .../isaaclab_physx/app/kit_launcher.py | 135 +++++----- .../renderers/isaac_rtx_renderer.py | 2 +- .../sensors/contact_sensor/contact_sensor.py | 2 +- .../sim/views/fabric_frame_view.py | 4 +- .../test/app/test_kit_launcher.py | 40 +-- .../test/envs/test_manager_based_rl_env_ui.py | 4 +- .../test_isaac_rtx_renderer_contract.py | 8 +- .../test/sensors/test_contact_sensor.py | 10 +- .../changelog.d/launch-helper-cleanup.skip | 1 + .../isaaclab_rl/entrypoints/common.py | 2 +- .../isaaclab_rl/entrypoints/dispatch.py | 5 +- .../export/leapp_initialized_checkpoints.py | 4 +- .../changelog.d/launch-helper-cleanup.skip | 1 + .../test_distributed_device_resolution.py | 31 +-- .../test/core/test_environment_determinism.py | 2 +- .../test/core/test_rl_device_separation.py | 2 +- .../test/core/test_runtime_compatibility.py | 5 +- .../test_sim_launcher_visualizer_intent.py | 24 +- source/isaaclab_tasks/test/env_test_utils.py | 4 +- .../changelog.d/launch-helper-cleanup.skip | 1 + .../kit/kit_visualizer.py | 8 +- 42 files changed, 301 insertions(+), 449 deletions(-) create mode 100644 source/isaaclab/changelog.d/launch-helper-cleanup.major.rst create mode 100644 source/isaaclab/isaaclab/app/argv.py create mode 100644 source/isaaclab_ov/changelog.d/launch-helper-cleanup.skip create mode 100644 source/isaaclab_physx/changelog.d/launch-helper-cleanup.rst create mode 100644 source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip create mode 100644 source/isaaclab_tasks/changelog.d/launch-helper-cleanup.skip create mode 100644 source/isaaclab_visualizers/changelog.d/launch-helper-cleanup.skip diff --git a/docs/source/api/lab/isaaclab.app.rst b/docs/source/api/lab/isaaclab.app.rst index b5d5a4283861..46e34a320dff 100644 --- a/docs/source/api/lab/isaaclab.app.rst +++ b/docs/source/api/lab/isaaclab.app.rst @@ -17,8 +17,8 @@ .. autosummary:: add_launcher_args + fuse_kit_args launch_simulation - make_physics_cfg report_activity scan @@ -36,7 +36,7 @@ Simulation Launcher .. autoclass:: AppLauncher :members: -.. autofunction:: make_physics_cfg +.. autofunction:: fuse_kit_args .. autofunction:: scan diff --git a/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst b/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst new file mode 100644 index 000000000000..2a5390ed44f5 --- /dev/null +++ b/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst @@ -0,0 +1,16 @@ +Added +^^^^^ + +* Added :func:`~isaaclab.app.fuse_kit_args` to the :mod:`isaaclab.app` exports. It moved to the lightweight + ``isaaclab.app.argv`` module; import it from :mod:`isaaclab.app` instead of ``isaaclab.app.sim_launcher``. + +Removed +^^^^^^^ + +* **Breaking:** Removed ``SettingsManager.instance()``. Use :func:`~isaaclab.app.get_settings_manager`. +* **Breaking:** Removed ``SettingsManager.set_bool``, ``set_int``, ``set_float``, and ``set_string``. Use + :meth:`~isaaclab.app.SettingsManager.set`, which dispatches on the value type. +* **Breaking:** Removed ``isaaclab.app.make_physics_cfg``. Select the backend with the ``physics`` launcher + argument of :func:`~isaaclab.app.launch_simulation`, or construct the physics config directly. +* **Breaking:** Removed ``isaaclab.app.logging_utils.resolve_python_logging_level``. + :func:`~isaaclab.app.launch_simulation` applies the ``--verbose`` / ``--info`` level itself. diff --git a/source/isaaclab/isaaclab/app/__init__.pyi b/source/isaaclab/isaaclab/app/__init__.pyi index 5cdf56e10f14..ab9d8cc30013 100644 --- a/source/isaaclab/isaaclab/app/__init__.pyi +++ b/source/isaaclab/isaaclab/app/__init__.pyi @@ -9,8 +9,8 @@ __all__ = [ "SettingsManager", "get_settings_manager", "add_launcher_args", + "fuse_kit_args", "launch_simulation", - "make_physics_cfg", "scan", "Scan", "LoadingScreen", @@ -18,6 +18,7 @@ __all__ = [ ] from .app_launcher import AppLauncher +from .argv import fuse_kit_args from .loading_screen import LoadingScreen, report_activity from .settings_manager import SettingsManager, get_settings_manager from .sim_launcher import ( @@ -25,6 +26,5 @@ from .sim_launcher import ( SimulationLauncher, add_launcher_args, launch_simulation, - make_physics_cfg, scan, ) diff --git a/source/isaaclab/isaaclab/app/argv.py b/source/isaaclab/isaaclab/app/argv.py new file mode 100644 index 000000000000..efbc422d0e86 --- /dev/null +++ b/source/isaaclab/isaaclab/app/argv.py @@ -0,0 +1,39 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +"""Command-line token helpers for the launcher arguments. + +This module imports nothing heavy, so CLI dispatchers can use it before choosing a backend. +""" + +from __future__ import annotations + + +def fuse_kit_args(argv: list[str]) -> list[str]: + """Fuse ``["--kit_args", ""]`` pairs into single ``--kit_args=`` tokens. + + Argparse rejects a value token that itself looks like an option (starts with ``-`` and contains + no space) with "expected one argument", and Kit arguments always start with ``--``. Fusing the + pair into the ``=``-attached form before parsing makes the documented space-separated form work + for a single Kit argument. All other forms pass through unchanged. + + Args: + argv: Command-line tokens, excluding the program name. + + Returns: + Tokens with any affected pair replaced by one fused token. + """ + fused: list[str] = [] + index = 0 + while index < len(argv): + token = argv[index] + next_token = argv[index + 1] if index + 1 < len(argv) else None + if token == "--kit_args" and next_token is not None and next_token.startswith("-") and " " not in next_token: + fused.append(f"--kit_args={next_token}") + index += 2 + else: + fused.append(token) + index += 1 + return fused diff --git a/source/isaaclab/isaaclab/app/logging_utils.py b/source/isaaclab/isaaclab/app/logging_utils.py index f3b6ae9a69b4..0bf10e23b97f 100644 --- a/source/isaaclab/isaaclab/app/logging_utils.py +++ b/source/isaaclab/isaaclab/app/logging_utils.py @@ -3,52 +3,20 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Backend-agnostic resolution and application of the Python logging level. +"""Backend-agnostic application of the Python logging level. The logging intent expressed by the ``--verbose`` / ``--info`` CLI arguments must be -honored by every simulation backend, not just the Kit-based one. Keeping the resolution +honored by every simulation backend, not just the Kit-based one. Keeping the application here (rather than inside :class:`~isaaclab_physx.app.KitLauncher`) lets the kitless launch path apply the same level without constructing Kit. """ from __future__ import annotations -import argparse import logging -import sys from contextlib import contextmanager -def resolve_python_logging_level(launcher_args: argparse.Namespace | dict | None = None) -> int: - """Resolve the intended Python logging level from launcher args and ``sys.argv``. - - ``--verbose`` maps to :data:`logging.DEBUG` and ``--info`` to :data:`logging.INFO`. - When neither is requested, the current effective level of the root logger is kept - (falling back to :data:`logging.WARNING` when unset). - - Args: - launcher_args: Parsed launcher arguments as a namespace or dict. Defaults to None, - which is treated as an empty set of arguments. - - Returns: - The resolved logging level. - """ - if launcher_args is None: - args = {} - elif isinstance(launcher_args, argparse.Namespace): - args = launcher_args.__dict__ - else: - args = launcher_args - - if args.get("verbose", False) or "--verbose" in sys.argv: - return logging.DEBUG - if args.get("info", False) or "--info" in sys.argv: - return logging.INFO - - level = logging.getLogger().getEffectiveLevel() - return logging.WARNING if level == logging.NOTSET else level - - @contextmanager def force_log_level(level: int): """Context manager that temporarily lowers the root logger and its handlers to *level*, then restores them. diff --git a/source/isaaclab/isaaclab/app/settings_manager.py b/source/isaaclab/isaaclab/app/settings_manager.py index c1b13c964d74..e805d5460615 100644 --- a/source/isaaclab/isaaclab/app/settings_manager.py +++ b/source/isaaclab/isaaclab/app/settings_manager.py @@ -58,21 +58,6 @@ def __init__(self): self._backend = None self._needs_init = False - @classmethod - def instance(cls) -> "SettingsManager": - """Get the singleton instance of the settings manager. - - Returns: - The singleton SettingsManager instance - """ - # Get instance from sys.modules (survives module reloads) - instance = sys.modules.get(_SINGLETON_KEY) - - if instance is None: - instance = cls() - - return instance - def set_backend(self, backend: Any) -> None: """Delegate all settings to *backend* from now on. @@ -124,42 +109,6 @@ def get(self, path: str, default: Any = None) -> Any: # Standalone mode - use dictionary return self._standalone_settings.get(path, default) - def set_bool(self, path: str, value: bool) -> None: - """Set a boolean setting value. - - Args: - path: The settings path - value: The boolean value to set - """ - self.set(path, value) - - def set_int(self, path: str, value: int) -> None: - """Set an integer setting value. - - Args: - path: The settings path - value: The integer value to set - """ - self.set(path, value) - - def set_float(self, path: str, value: float) -> None: - """Set a float setting value. - - Args: - path: The settings path - value: The float value to set - """ - self.set(path, value) - - def set_string(self, path: str, value: str) -> None: - """Set a string setting value. - - Args: - path: The settings path - value: The string value to set - """ - self.set(path, value) - def get_settings_manager() -> SettingsManager: """Get the global settings manager instance. diff --git a/source/isaaclab/isaaclab/app/sim_launcher.py b/source/isaaclab/isaaclab/app/sim_launcher.py index 7739cb1f8084..2ee9c3e5d9d6 100644 --- a/source/isaaclab/isaaclab/app/sim_launcher.py +++ b/source/isaaclab/isaaclab/app/sim_launcher.py @@ -15,6 +15,7 @@ import argparse import logging import os +import sys import warnings from collections.abc import Generator from contextlib import contextmanager @@ -32,7 +33,7 @@ from ..sensors.camera.camera_cfg import CameraCfg from ..utils.device import set_cuda_device from ..utils.string import string_to_callable -from .logging_utils import apply_python_logging_level, resolve_python_logging_level +from .logging_utils import apply_python_logging_level from .settings_manager import get_settings_manager logger = logging.getLogger(__name__) @@ -76,38 +77,10 @@ def add_launcher_args(parser: argparse.ArgumentParser) -> None: The arguments are defined by the Kit launcher, which consumes most of them. """ - string_to_callable(_KIT_LAUNCHER).add_app_launcher_args(parser) + string_to_callable(_KIT_LAUNCHER).add_launcher_args(parser) -def fuse_kit_args(argv: list[str]) -> list[str]: - """Fuse ``["--kit_args", ""]`` pairs into single ``--kit_args=`` tokens. - - Argparse rejects a value token that itself looks like an option (starts with ``-`` and contains - no space) with "expected one argument", and Kit arguments always start with ``--``. Fusing the - pair into the ``=``-attached form before parsing makes the documented space-separated form work - for a single Kit argument. All other forms pass through unchanged. - - Args: - argv: Command-line tokens, excluding the program name. - - Returns: - Tokens with any affected pair replaced by one fused token. - """ - fused: list[str] = [] - index = 0 - while index < len(argv): - token = argv[index] - next_token = argv[index + 1] if index + 1 < len(argv) else None - if token == "--kit_args" and next_token is not None and next_token.startswith("-") and " " not in next_token: - fused.append(f"--kit_args={next_token}") - index += 2 - else: - fused.append(token) - index += 1 - return fused - - -def make_physics_cfg(physics_cfg_str: str) -> PhysicsCfg: +def _make_physics_cfg(physics_cfg_str: str) -> PhysicsCfg: """Build a physics config for the requested backend. Args: @@ -143,21 +116,6 @@ def make_physics_cfg(physics_cfg_str: str) -> PhysicsCfg: """ -def _is_ovrtx_renderer(node) -> bool: - """True when the node is an OVRTX renderer config.""" - return isinstance(node, RendererCfg) and getattr(node, "renderer_type", None) == "ovrtx" - - -def _is_auto_rtx_renderer(node) -> bool: - """True when the node is an automatic RTX renderer placeholder.""" - return getattr(node, "renderer_type", None) == "auto_rtx" - - -def _is_auto_physx_physics(node) -> bool: - """True when the node is an automatic PhysX placeholder.""" - return isinstance(node, PhysxAutoCfg) - - def _is_kit_camera(node) -> bool: """True for a CameraCfg whose renderer requires Kit (not Newton).""" if not isinstance(node, CameraCfg): @@ -165,7 +123,7 @@ def _is_kit_camera(node) -> bool: renderer_cfg = getattr(node, "renderer_cfg", None) if renderer_cfg is None: return True - if _is_auto_rtx_renderer(renderer_cfg): + if getattr(renderer_cfg, "renderer_type", None) == "auto_rtx": # ``auto_rtx`` is resolved after the initial scan once physics and # visualizer intent are known; ie. it may become OVRTX for a kitless run. return False @@ -181,21 +139,11 @@ def _is_kit_camera(node) -> bool: """ -def _get_arg(launcher_args: argparse.Namespace | dict | None, key: str, default: Any = None) -> Any: - """Read *key* from launcher args, whether a namespace, dict, or ``None``.""" - if isinstance(launcher_args, argparse.Namespace): - return getattr(launcher_args, key, default) - if isinstance(launcher_args, dict): - return launcher_args.get(key, default) - return default - - -def _set_arg(launcher_args: argparse.Namespace | dict | None, key: str, value: Any) -> None: - """Write *key* on launcher args when it is a namespace or dict.""" +def _as_dict(launcher_args: argparse.Namespace | dict | None) -> dict: + """Return launcher args as a dict; writes to it reach the caller's namespace or dict.""" if isinstance(launcher_args, argparse.Namespace): - setattr(launcher_args, key, value) - elif isinstance(launcher_args, dict): - launcher_args[key] = value + return vars(launcher_args) + return {} if launcher_args is None else launcher_args def _parse_visualizer_csv(value: str) -> list[str] | None: @@ -239,29 +187,22 @@ def _parse_visualizer_csv(value: str) -> list[str] | None: return list(dict.fromkeys(names)) -def _get_livestream_mode(launcher_args: argparse.Namespace | dict | None) -> int: - """Return the livestream mode: ``--livestream`` when set (>= 0), else the ``LIVESTREAM`` environment variable.""" - livestream = _get_arg(launcher_args, "livestream", -1) +def _normalize_launcher_args(args: dict) -> None: + """Resolve the livestream mode and the visualizer selection in place, once for every consumer. + + Writes ``livestream`` (the effective mode: ``--livestream`` when set (>= 0), else the ``LIVESTREAM`` + environment variable), ``visualizer`` (canonical names, or None when all are disabled), + ``visualizer_explicit`` and ``visualizer_disable_all``. Livestreaming adds the Kit visualizer, whose + viewport produces the stream. Normalizing twice gives the same result. + """ + livestream = args.get("livestream", -1) if livestream is None or int(livestream) < 0: livestream = os.environ.get("LIVESTREAM", 0) livestream = int(livestream) if livestream not in (0, 1, 2): raise ValueError(f"Invalid livestream mode: {livestream}. Expected 0 (disabled), 1, or 2.") - return livestream - - -def _normalize_launcher_args(launcher_args: argparse.Namespace | dict | None) -> None: - """Resolve the livestream mode and the visualizer selection in place, once for every consumer. - - Writes ``livestream`` (the effective mode), ``visualizer`` (canonical names, or None when all are - disabled), ``visualizer_explicit`` and ``visualizer_disable_all``. Livestreaming adds the Kit - visualizer, whose viewport produces the stream. Normalizing twice gives the same result. - """ - if launcher_args is None: - return - livestream = _get_livestream_mode(launcher_args) - visualizers = _get_arg(launcher_args, "visualizer") - explicit = bool(_get_arg(launcher_args, "visualizer_explicit", False)) or visualizers is not None + visualizers = args.get("visualizer") + explicit = bool(args.get("visualizer_explicit", False)) or visualizers is not None if visualizers and not isinstance(visualizers, str): visualizers = ",".join(str(visualizer).strip() for visualizer in visualizers) if visualizers: @@ -276,29 +217,40 @@ def _normalize_launcher_args(launcher_args: argparse.Namespace | dict | None) -> if "kit" not in (visualizers or []): visualizers = [*(visualizers or []), "kit"] explicit = True - _set_arg(launcher_args, "livestream", livestream) - _set_arg(launcher_args, "visualizer", visualizers) - _set_arg(launcher_args, "visualizer_explicit", explicit) - _set_arg(launcher_args, "visualizer_disable_all", disable_all) + args["livestream"] = livestream + args["visualizer"] = visualizers + args["visualizer_explicit"] = explicit + args["visualizer_disable_all"] = disable_all -def _sync_visualizer_cli_settings(launcher_args: argparse.Namespace | dict) -> None: +def _sync_visualizer_cli_settings(args: dict) -> None: """Write the normalized visualizer selection and ``--max_visible_envs`` to the settings.""" - max_visible_envs = _get_arg(launcher_args, "max_visible_envs") + max_visible_envs = args.get("max_visible_envs") if max_visible_envs is not None and int(max_visible_envs) < 0: raise ValueError(f"Invalid value for --max_visible_envs: {max_visible_envs}. Expected non-negative int.") settings = get_settings_manager() - settings.set_string("/isaaclab/visualizer/types", " ".join(_get_arg(launcher_args, "visualizer") or ())) - settings.set_bool("/isaaclab/visualizer/explicit", bool(_get_arg(launcher_args, "visualizer_explicit", False))) - settings.set_bool( - "/isaaclab/visualizer/disable_all", bool(_get_arg(launcher_args, "visualizer_disable_all", False)) - ) + settings.set("/isaaclab/visualizer/types", " ".join(args.get("visualizer") or ())) + settings.set("/isaaclab/visualizer/explicit", bool(args.get("visualizer_explicit", False))) + settings.set("/isaaclab/visualizer/disable_all", bool(args.get("visualizer_disable_all", False))) # Sentinel: ``-1`` means ``--max_visible_envs`` was not passed (see ``SimulationContext``). - settings.set_int("/isaaclab/visualizer/max_visible_envs", -1 if max_visible_envs is None else int(max_visible_envs)) + settings.set("/isaaclab/visualizer/max_visible_envs", -1 if max_visible_envs is None else int(max_visible_envs)) + +def _resolve_python_logging_level(args: dict) -> int: + """Return the level for ``--verbose`` / ``--info`` (also read from ``sys.argv``), else the current root level.""" + if args.get("verbose", False) or "--verbose" in sys.argv: + return logging.DEBUG + if args.get("info", False) or "--info" in sys.argv: + return logging.INFO + level = logging.getLogger().getEffectiveLevel() + return logging.WARNING if level == logging.NOTSET else level -def _get_visualizer_intent(cfg, launcher_args: argparse.Namespace | dict | None) -> dict[str, bool]: - """Compute the visualizer intent of ``cfg.sim.visualizer_cfgs``, OR-ed with a caller's ``visualizer_intent``.""" + +def _get_visualizer_intent(cfg, args: dict) -> dict[str, bool]: + """Compute the visualizer intent of ``cfg.sim.visualizer_cfgs``, OR-ed with a caller's ``visualizer_intent``. + + An explicit ``--visualizer`` selection overrides whether the config's Kit visualizer is used. + """ # Accept both env_cfg (has .sim.visualizer_cfgs) and a bare SimulationCfg # (has .visualizer_cfgs directly). sim = getattr(cfg, "sim", None) @@ -307,9 +259,13 @@ def _get_visualizer_intent(cfg, launcher_args: argparse.Namespace | dict | None) visualizer_cfgs = [] cfgs = visualizer_cfgs if isinstance(visualizer_cfgs, list) else [visualizer_cfgs] kit_cfgs = [c for c in cfgs if getattr(c, "visualizer_type", None) == "kit"] - caller_intent = _get_arg(launcher_args, "visualizer_intent") or {} + caller_intent = args.get("visualizer_intent") or {} + if args.get("visualizer_explicit", False): + has_kit_visualizer = "kit" in (args.get("visualizer") or ()) + else: + has_kit_visualizer = bool(kit_cfgs) or bool(caller_intent.get("has_kit_visualizer")) return { - "has_kit_visualizer": bool(kit_cfgs) or bool(caller_intent.get("has_kit_visualizer")), + "has_kit_visualizer": has_kit_visualizer, "has_kit_streaming_view": any(bool(getattr(c, "streaming_view", False)) for c in kit_cfgs) or bool(caller_intent.get("has_kit_streaming_view")), } @@ -356,7 +312,7 @@ def scan(cfg, launcher_args: argparse.Namespace | dict | None = None) -> Scan: """Walk *cfg* once, collecting all launch signals and applying ``--physics``. When the ``physics`` key is present in *launcher_args*, every physics config is - replaced by the requested backend (see :func:`make_physics_cfg`): nested configs + replaced by the requested backend (see :func:`launch_simulation`): nested configs in place, a root config via :attr:`Scan.effective_cfg` (it cannot be mutated in place). Automatic PhysX configurations and RTX renderer placeholders (``renderer_type="auto_rtx"``) are also resolved @@ -366,14 +322,15 @@ def scan(cfg, launcher_args: argparse.Namespace | dict | None = None) -> Scan: a second walk of the same config observes the same signals and reaches the same launch decision. """ + args = _as_dict(launcher_args) # Livestreaming implies a Kit visualizer; make that visible to auto RTX resolution. - _normalize_launcher_args(launcher_args) + _normalize_launcher_args(args) - physics_str = _get_arg(launcher_args, "physics", None) + physics_str = args.get("physics") physics_cfgs: list[PhysicsCfg] = [] concrete_physics_cfgs: list[PhysicsCfg] = [] effective_cfg: Any = cfg - has_ovrtx = "newton_rtx" in (_get_arg(launcher_args, "visualizer") or ()) + has_ovrtx = "newton_rtx" in (args["visualizer"] or ()) has_auto_rtx = False has_auto_physx = False has_kit_camera = False @@ -388,7 +345,7 @@ def add_launcher_type(node): def visit(node, parent, key): nonlocal effective_cfg, has_ovrtx, has_auto_rtx, has_auto_physx, has_kit_camera - if _is_auto_rtx_renderer(node): + if getattr(node, "renderer_type", None) == "auto_rtx": has_auto_rtx = True auto_rtx_locations.append((parent, key, isinstance(parent, CameraCfg) and key == "renderer_cfg")) @@ -398,19 +355,21 @@ def visit(node, parent, key): if isinstance(node, PhysicsCfg): if physics_str: - node = make_physics_cfg(physics_str) + node = _make_physics_cfg(physics_str) if parent is not None: setattr(parent, key, node) else: effective_cfg = node physics_cfgs.append(node) - if _is_auto_physx_physics(node): + if isinstance(node, PhysxAutoCfg): has_auto_physx = True auto_physx_locations.append((node, parent, key, len(physics_cfgs) == 1)) return else: concrete_physics_cfgs.append(node) - elif _is_ovrtx_renderer(node) or getattr(node, "visualizer_type", None) == "newton_rtx": + elif getattr(node, "visualizer_type", None) == "newton_rtx" or ( + isinstance(node, RendererCfg) and node.renderer_type == "ovrtx" + ): has_ovrtx = True elif _is_kit_camera(node): has_kit_camera = True @@ -432,7 +391,7 @@ def visit(node, parent, key): config_scan = Scan( resolved_physics_cfg=physics_cfgs[0] if physics_cfgs else None, effective_cfg=effective_cfg, - visualizer_intent=_get_visualizer_intent(cfg, launcher_args), + visualizer_intent=_get_visualizer_intent(cfg, args), has_ovrtx=has_ovrtx, has_kit_camera=has_kit_camera, has_kit_physics=False, @@ -444,7 +403,7 @@ def visit(node, parent, key): # RTX selection depends on the resolved physics backend. if has_auto_physx: - use_isaac_sim = bool(_get_kit_runtime_sources(config_scan, launcher_args)) + use_isaac_sim = bool(_get_kit_runtime_sources(config_scan, args)) for node, parent, key, is_first_physics in auto_physx_locations: physics_cfg = _resolve_physx_auto_cfg(node, use_isaac_sim) concrete_physics_cfgs.append(physics_cfg) @@ -463,7 +422,7 @@ def visit(node, parent, key): if not has_auto_rtx: return config_scan - use_isaac_sim = bool(_get_kit_runtime_sources(config_scan, launcher_args)) + use_isaac_sim = bool(_get_kit_runtime_sources(config_scan, args)) renderer_factory = IsaacRtxRendererCfg if use_isaac_sim else OVRTXRendererCfg # Resolve every auto RTX placeholder in place, tracking camera renderers that may require Kit. @@ -491,27 +450,20 @@ def visit(node, parent, key): """ -def _has_kit_visualizer(config_scan: Scan, launcher_args: argparse.Namespace | dict | None) -> bool: - """Return whether the run uses the Kit visualizer; an explicit CLI selection overrides the config.""" - if _get_arg(launcher_args, "visualizer_explicit", False): - return "kit" in (_get_arg(launcher_args, "visualizer") or ()) - return config_scan.visualizer_intent["has_kit_visualizer"] - - -def _get_kit_runtime_sources(config_scan: Scan, launcher_args: argparse.Namespace | dict | None) -> tuple[str, ...]: - """Return the config and launcher components that require Isaac Sim / Kit.""" +def _get_kit_runtime_sources(config_scan: Scan, args: dict) -> tuple[str, ...]: + """Return the config and launcher components that require Isaac Sim / Kit; *args* are normalized by :func:`scan`.""" kit_sources = [] if config_scan.has_kit_physics: kit_sources.append("Isaac Sim PhysX physics (`PhysxCfg`)") if config_scan.has_kit_camera: kit_sources.append('a Kit-based renderer (`IsaacRtxRendererCfg`, `renderer_type="isaac_rtx"`)') - if _has_kit_visualizer(config_scan, launcher_args): + if config_scan.visualizer_intent["has_kit_visualizer"]: kit_sources.append('the Kit visualizer (`--visualizer kit` / `visualizer_type="kit"`)') - if _get_arg(launcher_args, "experience", ""): + if args.get("experience", ""): kit_sources.append("an explicit Kit experience") - if _get_livestream_mode(launcher_args) > 0: + if args.get("livestream", 0) > 0: kit_sources.append("livestreaming") - if _get_arg(launcher_args, "require_kit", False): + if args.get("require_kit", False): kit_sources.append("the caller's explicit Kit requirement") if config_scan.needs_kit and not kit_sources: kit_sources.append("the default Isaac Sim / Kit runtime") @@ -519,13 +471,6 @@ def _get_kit_runtime_sources(config_scan: Scan, launcher_args: argparse.Namespac return tuple(kit_sources) -def _format_runtime_sources(sources: tuple[str, ...]) -> str: - """Format runtime sources as a readable list.""" - if len(sources) < 3: - return " and ".join(sources) - return f"{', '.join(sources[:-1])}, and {sources[-1]}" - - def _validate_runtime(scan: Scan, kit_sources: tuple[str, ...]) -> None: """Raise if the scan and resolved Kit sources form an unsupported combination. @@ -537,7 +482,7 @@ def _validate_runtime(scan: Scan, kit_sources: tuple[str, ...]) -> None: # deep inside its own library with "Failed to initialize Carbonite and load PhysX plugins" raise ValueError( "Invalid backend combination: OvPhysX physics (`OvPhysxCfg`) is kitless and cannot be used together " - f"with Isaac Sim / Kit ({_format_runtime_sources(kit_sources)}).\n" + f"with Isaac Sim / Kit ({', '.join(kit_sources)}).\n" "\n" "To fix this, pick one of the following supported combinations:\n" " * Keep OvPhysX physics and switch to a kitless renderer/visualizer:\n" @@ -554,7 +499,7 @@ def _validate_runtime(scan: Scan, kit_sources: tuple[str, ...]) -> None: raise ValueError( "Invalid backend combination: the OVRTX runtime (`OVRTXRendererCfg` or the" " `newton_rtx` visualizer) cannot be used together" - f" with Isaac Sim / Kit ({_format_runtime_sources(kit_sources)}).\n" + f" with Isaac Sim / Kit ({', '.join(kit_sources)}).\n" "\n" "To fix this, pick one of the following supported combinations:\n" " * Keep Isaac Sim / Kit and switch the renderer:\n" @@ -563,13 +508,13 @@ def _validate_runtime(scan: Scan, kit_sources: tuple[str, ...]) -> None: ) -def _resolve_distributed_device(cfg, launcher_args: argparse.Namespace | dict | None) -> None: +def _resolve_distributed_device(cfg, args: dict) -> None: """Set ``cfg.sim.device`` and the launcher ``device`` for distributed training. When ``--distributed`` restricts each process to one GPU, ``local_rank`` may exceed the visible device count, so the process falls back to the one GPU it can see. """ - if not _get_arg(launcher_args, "distributed", False): + if not args.get("distributed", False): return import torch @@ -582,7 +527,7 @@ def _resolve_distributed_device(cfg, launcher_args: argparse.Namespace | dict | sim_cfg = getattr(cfg, "sim", None) if sim_cfg is not None: sim_cfg.device = device_str - _set_arg(launcher_args, "device", device_str) + args["device"] = device_str set_cuda_device(device_str) logger.info( "Distributed device resolved to %s (local_rank=%d, visible_gpus=%d)", @@ -618,8 +563,9 @@ def launch_simulation( arguments added by :func:`add_launcher_args`, the following keys are read when a script contributes them: - * ``physics``: Backend selector applied to every physics config in *cfg*, see - :func:`make_physics_cfg`. + * ``physics``: Backend selector applied to every physics config in *cfg*: ``"physx"`` + (Isaac Sim PhysX when Kit is required, else OvPhysX), ``"isaacsim_physx"``, + ``"newton_mjwarp"``, ``"newton_vbd"``, or ``"ovphysx"``. * ``require_kit``: Whether the caller needs Kit for a reason *cfg* cannot express, e.g. a tool that reaches a Kit-only extension API. This is additive -- it can only turn a kitless launch into a Kit one, never the reverse, so a config that already needs Kit @@ -627,33 +573,32 @@ def launch_simulation( * ``visualizer_intent``: Visualizer intent the config cannot express, e.g. ``{"has_kit_visualizer": True}``; it is combined with the intent of *cfg*. """ - if launcher_args is None: - launcher_args = {} + args = _as_dict(launcher_args) # The single walk: collect every signal, apply the --physics override, and # resolve the automatic PhysX and RTX placeholders. - config_scan = scan(cfg, launcher_args) + config_scan = scan(cfg, args) effective_cfg = config_scan.effective_cfg physics_cfg = config_scan.resolved_physics_cfg - kit_sources = _get_kit_runtime_sources(config_scan, launcher_args) + kit_sources = _get_kit_runtime_sources(config_scan, args) _validate_runtime(config_scan, kit_sources) needs_kit = bool(kit_sources) - _set_arg(launcher_args, "kit_visualizer", _has_kit_visualizer(config_scan, launcher_args)) + args["kit_visualizer"] = config_scan.visualizer_intent["has_kit_visualizer"] # Honor --verbose / --info; the Kit launcher re-applies this level once Kit has started. - apply_python_logging_level(resolve_python_logging_level(launcher_args)) + apply_python_logging_level(_resolve_python_logging_level(args)) if needs_kit and (config_scan.has_kit_camera or config_scan.visualizer_intent.get("has_kit_streaming_view")): - if not _get_arg(launcher_args, "enable_cameras", False): + if not args.get("enable_cameras", False): logger.info( "Auto-enabling camera rendering because the scene contains Kit camera sensors " "or a Kit visualizer with streaming_view=True." ) - _set_arg(launcher_args, "enable_cameras", True) + args["enable_cameras"] = True # Resolve distributed device early, before any launcher or physics init. - _resolve_distributed_device(effective_cfg, launcher_args) + _resolve_distributed_device(effective_cfg, args) # Start the launchers the resolved config names, plus Kit and OVRTX for needs that no config names # (e.g. a default-renderer camera, ``--viz kit`` or ``--viz newton_rtx``); Kit starts first. @@ -662,14 +607,14 @@ def launch_simulation( if config_scan.has_ovrtx: # validated above: OVRTX never shares the process with Kit launcher_types.append(OVRTXRendererCfg.launcher_type) - launchers = [string_to_callable(launcher_type)(launcher_args) for launcher_type in dict.fromkeys(launcher_types)] + launchers = [string_to_callable(launcher_type)(args) for launcher_type in dict.fromkeys(launcher_types)] for launcher in launchers: # the runtime may refine the device choice made by _resolve_distributed_device sim_cfg = getattr(effective_cfg, "sim", None) if sim_cfg is not None and launcher.device is not None: sim_cfg.device = launcher.device # after the launchers, so a started Kit already backs the settings - _sync_visualizer_cli_settings(launcher_args) + _sync_visualizer_cli_settings(args) exit_code = 0 try: diff --git a/source/isaaclab/isaaclab/benchmark/dispatch.py b/source/isaaclab/isaaclab/benchmark/dispatch.py index 4b24c02c3d01..2f677dc7cae2 100644 --- a/source/isaaclab/isaaclab/benchmark/dispatch.py +++ b/source/isaaclab/isaaclab/benchmark/dispatch.py @@ -12,6 +12,8 @@ import sys from typing import TYPE_CHECKING, Any +from ..app.argv import fuse_kit_args + if TYPE_CHECKING: from .api import BenchmarkLauncherConfig, BenchmarkRequest, BenchmarkResult @@ -77,7 +79,7 @@ def run_benchmark_cli(argv: list[str] | None = None) -> int: if argv is None: argv = sys.argv[1:] - argv = _fuse_kit_args(argv) + argv = fuse_kit_args(argv) multigpu_workflows = tuple(f"{name}{multigpu.MULTIGPU_SUFFIX}" for name in multigpu.MULTIGPU_WORKFLOWS) legacy_multigpu_workflows = tuple( f"{name}{multigpu.LEGACY_MULTIGPU_SUFFIX}" for name in multigpu.MULTIGPU_WORKFLOWS @@ -120,7 +122,7 @@ def run_play_cli(argv: list[str] | None = None) -> int: def _run_rl_cli(workflow: str, argv: list[str] | None) -> int: if argv is None: argv = sys.argv[1:] - argv = _fuse_kit_args(argv) + argv = fuse_kit_args(argv) backends = _RL_WORKFLOW_MODULES[workflow] parser = argparse.ArgumentParser(add_help=False) parser.add_argument("--rl_library", choices=sorted(backends)) @@ -199,7 +201,7 @@ def _request_argv(request: BenchmarkRequest) -> list[str]: if request.presets: argv.append(f"presets={','.join(request.presets)}") argv.extend(request.hydra_args) - return _fuse_kit_args(argv) + return fuse_kit_args(argv) def _output_argv(output: Any) -> list[str]: @@ -230,16 +232,9 @@ def _launcher_argv(launcher: BenchmarkLauncherConfig) -> list[str]: argv.append("--verbose") if launcher.info: argv.append("--info") - return _fuse_kit_args(argv) + return fuse_kit_args(argv) def _append_value(argv: list[str], option: str, value: object | None) -> None: if value is not None: argv.extend((option, str(value))) - - -def _fuse_kit_args(argv: list[str]) -> list[str]: - # imported here so that importing this module stays lightweight - from ..app.sim_launcher import fuse_kit_args - - return fuse_kit_args(argv) diff --git a/source/isaaclab/isaaclab/cli/commands/deploy.py b/source/isaaclab/isaaclab/cli/commands/deploy.py index e2d44f59d23b..6202aa7ab0bc 100644 --- a/source/isaaclab/isaaclab/cli/commands/deploy.py +++ b/source/isaaclab/isaaclab/cli/commands/deploy.py @@ -17,7 +17,7 @@ from isaaclab_tasks.utils.hydra import resolve_task_config from ...app import add_launcher_args, launch_simulation -from ...app.sim_launcher import fuse_kit_args +from ...app.argv import fuse_kit_args def command_deploy_leapp(argv: list[str] | None = None) -> int: diff --git a/source/isaaclab/isaaclab/devices/gamepad/se2_gamepad.py b/source/isaaclab/isaaclab/devices/gamepad/se2_gamepad.py index cc7f4a2c6e44..aac6b99d2a79 100644 --- a/source/isaaclab/isaaclab/devices/gamepad/se2_gamepad.py +++ b/source/isaaclab/isaaclab/devices/gamepad/se2_gamepad.py @@ -56,7 +56,7 @@ def __init__(self, cfg: Se2GamepadCfg): cfg: Configuration object for gamepad settings. """ # turn off simulator gamepad control - get_settings_manager().set_bool("/persistent/app/omniverse/gamepadCameraControl", False) + get_settings_manager().set("/persistent/app/omniverse/gamepadCameraControl", False) # store inputs self.v_x_sensitivity = cfg.v_x_sensitivity self.v_y_sensitivity = cfg.v_y_sensitivity diff --git a/source/isaaclab/isaaclab/devices/gamepad/se3_gamepad.py b/source/isaaclab/isaaclab/devices/gamepad/se3_gamepad.py index 13c7be89e7a9..d433b6e18fab 100644 --- a/source/isaaclab/isaaclab/devices/gamepad/se3_gamepad.py +++ b/source/isaaclab/isaaclab/devices/gamepad/se3_gamepad.py @@ -63,7 +63,7 @@ def __init__(self, cfg: Se3GamepadCfg): cfg: Configuration object for gamepad settings. """ # turn off simulator gamepad control - get_settings_manager().set_bool("/persistent/app/omniverse/gamepadCameraControl", False) + get_settings_manager().set("/persistent/app/omniverse/gamepadCameraControl", False) # store inputs self.pos_sensitivity = cfg.pos_sensitivity self.rot_sensitivity = cfg.rot_sensitivity diff --git a/source/isaaclab/isaaclab/sensors/camera/camera.py b/source/isaaclab/isaaclab/sensors/camera/camera.py index bd396e252450..5d1c98e9d1bc 100644 --- a/source/isaaclab/isaaclab/sensors/camera/camera.py +++ b/source/isaaclab/isaaclab/sensors/camera/camera.py @@ -260,15 +260,15 @@ def __init__(self, cfg: CameraCfg): from ...app.settings_manager import get_settings_manager settings = get_settings_manager() - settings.set_bool("/isaaclab/render/rtx_sensors", True) - settings.set_bool("/physics/fabricUpdateTransformations", True) + settings.set("/isaaclab/render/rtx_sensors", True) + settings.set("/physics/fabricUpdateTransformations", True) if require_hdr_output: - settings.set_bool("/rtx/rtpt/gaussian/skipTonemapping/enabled", False) + settings.set("/rtx/rtpt/gaussian/skipTonemapping/enabled", False) elif renderer_type == "ovrtx" and require_hdr_output: from ...app.settings_manager import get_settings_manager - get_settings_manager().set_bool("/rtx/rtpt/gaussian/skipTonemapping/enabled", False) - # FIXME: settings set_bool is a no-op for ovrtx + get_settings_manager().set("/rtx/rtpt/gaussian/skipTonemapping/enabled", False) + # FIXME: settings.set is a no-op for ovrtx # warning only since it affects only ParticleField3DGaussianSplat scene logger.warning( "OVRTX backend with PPISP/HDR requires /rtx/rtpt/gaussian/skipTonemapping/enabled to be false." diff --git a/source/isaaclab/isaaclab/sim/simulation_context.py b/source/isaaclab/isaaclab/sim/simulation_context.py index a839f37b5768..e79108185f91 100644 --- a/source/isaaclab/isaaclab/sim/simulation_context.py +++ b/source/isaaclab/isaaclab/sim/simulation_context.py @@ -17,7 +17,7 @@ import warp as wp from .. import sim as sim_utils -from ..app.settings_manager import SettingsManager +from ..app.settings_manager import SettingsManager, get_settings_manager from ..markers.vis_marker_registry import VisMarkerRegistry from ..physics import PhysicsCfg, PhysicsEvent, PhysicsManager from ..physics.physics_manager_cfg import _resolve_physx_auto_cfg @@ -69,15 +69,7 @@ def __init__(self, settings: SettingsManager): def set(self, name: str, value: Any) -> None: """Set a setting with automatic type routing.""" - if isinstance(value, bool): - self._settings.set_bool(name, value) - elif isinstance(value, int): - self._settings.set_int(name, value) - elif isinstance(value, float): - self._settings.set_float(name, value) - elif isinstance(value, str): - self._settings.set_string(name, value) - elif isinstance(value, (list, tuple)): + if isinstance(value, (bool, int, float, str, list, tuple)): self._settings.set(name, value) else: raise ValueError(f"Unsupported value type for setting '{name}': {type(value)}") @@ -189,7 +181,7 @@ def __init__(self, cfg: SimulationCfg | None = None): self.get_or_create_backend(KitStageBackendCfg(stage=self.stage)) # Acquire settings interface (SettingsManager: standalone dict or Omniverse when available) - self.settings = SettingsManager.instance() + self.settings = get_settings_manager() self._settings_helper = SettingsHelper(self.settings) # Initialize USD physics scene and physics manager @@ -1140,7 +1132,7 @@ def build_simulation_context( sim_cfg.device = device if visualizers: - SettingsManager.instance().set_string("/isaaclab/visualizer/types", " ".join(visualizers)) + get_settings_manager().set("/isaaclab/visualizer/types", " ".join(visualizers)) sim = SimulationContext(sim_cfg) diff --git a/source/isaaclab/test/cli/test_train_multigpu_command_building.py b/source/isaaclab/test/cli/test_train_multigpu_command_building.py index b52aeba604aa..d1a5a37d24b1 100644 --- a/source/isaaclab/test/cli/test_train_multigpu_command_building.py +++ b/source/isaaclab/test/cli/test_train_multigpu_command_building.py @@ -65,7 +65,7 @@ def _parse_as_training_script(child_argv: list[str], monkeypatch: pytest.MonkeyP class TestAddAppLauncherArgsNormalization: - """Integration tests for the ``sys.argv`` normalization in ``add_app_launcher_args``.""" + """Integration tests for the ``sys.argv`` normalization in ``add_launcher_args``.""" def test_unknown_leftovers_are_preserved(self, monkeypatch): monkeypatch.setattr(sys, "argv", ["prog", "--task", "X", "--kit_args", "--foo=/bar", "env.param=1"]) diff --git a/source/isaaclab/test/envs/test_action_state_recorder_term.py b/source/isaaclab/test/envs/test_action_state_recorder_term.py index 25d2a74edefd..9f60cdbbcc86 100644 --- a/source/isaaclab/test/envs/test_action_state_recorder_term.py +++ b/source/isaaclab/test/envs/test_action_state_recorder_term.py @@ -28,7 +28,7 @@ @pytest.fixture(scope="session", autouse=True) def setup_carb_settings(): """Set up settings to prevent simulation getting stuck.""" - get_settings_manager().set_bool("/physics/cooking/ujitsoCollisionCooking", False) + get_settings_manager().set("/physics/cooking/ujitsoCollisionCooking", False) @pytest.fixture diff --git a/source/isaaclab/test/envs/test_action_state_recorder_term_task_integration.py b/source/isaaclab/test/envs/test_action_state_recorder_term_task_integration.py index cecc54ae3c68..72aa066a5c9e 100644 --- a/source/isaaclab/test/envs/test_action_state_recorder_term_task_integration.py +++ b/source/isaaclab/test/envs/test_action_state_recorder_term_task_integration.py @@ -35,7 +35,7 @@ @pytest.fixture(scope="session", autouse=True) def setup_carb_settings(): """Set up settings to prevent simulation getting stuck.""" - get_settings_manager().set_bool("/physics/cooking/ujitsoCollisionCooking", False) + get_settings_manager().set("/physics/cooking/ujitsoCollisionCooking", False) @pytest.fixture diff --git a/source/isaaclab/test/sensors/test_outdated_sensor.py b/source/isaaclab/test/sensors/test_outdated_sensor.py index d2b0aaac6c52..5450d51b976f 100644 --- a/source/isaaclab/test/sensors/test_outdated_sensor.py +++ b/source/isaaclab/test/sensors/test_outdated_sensor.py @@ -28,7 +28,7 @@ def temp_dir(): """Fixture to create and clean up a temporary directory for test datasets.""" # this flag is necessary to prevent a bug where the simulation gets stuck randomly when running the # test on many environments. - get_settings_manager().set_bool("/physics/cooking/ujitsoCollisionCooking", False) + get_settings_manager().set("/physics/cooking/ujitsoCollisionCooking", False) # create a temporary directory to store the test datasets temp_dir = tempfile.mkdtemp() yield temp_dir diff --git a/source/isaaclab/test/sim/test_simulation_context_visualizers.py b/source/isaaclab/test/sim/test_simulation_context_visualizers.py index 0f3af5f9dbbb..518241600249 100644 --- a/source/isaaclab/test/sim/test_simulation_context_visualizers.py +++ b/source/isaaclab/test/sim/test_simulation_context_visualizers.py @@ -777,9 +777,7 @@ def test_visualizer_construction_precedes_initialization_and_happens_once(monkey monkeypatch.setattr(context_module, "has_kit", lambda: False) monkeypatch.setattr(context_module, "SceneDataProvider", lambda backend: _FakeProvider()) monkeypatch.setattr( - context_module.SettingsManager, - "instance", - lambda: SimpleNamespace(get=settings.get, set_bool=settings.__setitem__), + context_module, "get_settings_manager", lambda: SimpleNamespace(get=settings.get, set=settings.__setitem__) ) monkeypatch.setattr(SimulationContext, "_init_usd_physics_scene", lambda self: None) monkeypatch.setattr(SimulationContext, "_instance", None) diff --git a/source/isaaclab_ov/changelog.d/launch-helper-cleanup.skip b/source/isaaclab_ov/changelog.d/launch-helper-cleanup.skip new file mode 100644 index 000000000000..a3cf5c64b516 --- /dev/null +++ b/source/isaaclab_ov/changelog.d/launch-helper-cleanup.skip @@ -0,0 +1 @@ +Updated tests for the launcher and settings helper cleanup. diff --git a/source/isaaclab_ov/test/physics/test_ovphysx_scene_data_backend.py b/source/isaaclab_ov/test/physics/test_ovphysx_scene_data_backend.py index 7a1ddbe8c723..21b5137f14d0 100644 --- a/source/isaaclab_ov/test/physics/test_ovphysx_scene_data_backend.py +++ b/source/isaaclab_ov/test/physics/test_ovphysx_scene_data_backend.py @@ -746,7 +746,7 @@ def test_automatic_physx_selection_prepares_ovphysx_before_stage_creation(monkey from isaaclab_ov.physics import OvPhysxManager import isaaclab.sim.simulation_context as simulation_context_module - from isaaclab.app.sim_launcher import make_physics_cfg + from isaaclab.app.sim_launcher import _make_physics_cfg from isaaclab.sim import SimulationCfg, SimulationContext class StageCreationReached(Exception): @@ -774,7 +774,7 @@ def _stop_at_stage_creation(): monkeypatch.setattr(simulation_context_module, "create_new_stage", _stop_at_stage_creation) cfg = SimulationCfg(create_stage_in_memory=True) - physics_cfg = make_physics_cfg("physx") + physics_cfg = _make_physics_cfg("physx") physics_cfg.class_type = StubPhysxManager cfg.physics = physics_cfg diff --git a/source/isaaclab_physx/changelog.d/launch-helper-cleanup.rst b/source/isaaclab_physx/changelog.d/launch-helper-cleanup.rst new file mode 100644 index 000000000000..2617b510692c --- /dev/null +++ b/source/isaaclab_physx/changelog.d/launch-helper-cleanup.rst @@ -0,0 +1,6 @@ +Changed +^^^^^^^ + +* **Breaking:** Renamed ``KitLauncher.add_app_launcher_args`` to + :meth:`~isaaclab_physx.app.KitLauncher.add_launcher_args`, matching :func:`~isaaclab.app.add_launcher_args`. + Replace the calls, or call :func:`~isaaclab.app.add_launcher_args`. diff --git a/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py b/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py index a83bf20e2859..c718358785cc 100644 --- a/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py +++ b/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py @@ -31,10 +31,11 @@ SimulationApp = getattr(isaacsim, "SimulationApp", None) +from isaaclab.app.argv import fuse_kit_args from isaaclab.app.loading_screen import report_activity from isaaclab.app.logging_utils import apply_python_logging_level from isaaclab.app.settings_manager import get_settings_manager -from isaaclab.app.sim_launcher import SimulationLauncher, _parse_visualizer_csv, fuse_kit_args +from isaaclab.app.sim_launcher import SimulationLauncher, _parse_visualizer_csv from isaaclab.paths import ISAACLAB_ROOT from isaaclab.utils.device import set_cuda_device from isaaclab.utils.renderers import ISAAC_RTX_SHOW_ALL_PARTITIONS_BY_DEFAULT_SETTING @@ -76,7 +77,7 @@ def _sanitize_sys_argv_for_kit(argv: list[str]) -> list[str]: return [argument for index, argument in enumerate(argv) if index not in indexes_to_remove] -class ExplicitAction(argparse.Action): +class _ExplicitAction(argparse.Action): """Custom action to track if an argument was explicitly passed by the user.""" def __call__(self, parser, namespace, values, option_string=None): @@ -197,7 +198,8 @@ def __init__(self, launcher_args: argparse.Namespace | dict | None = None): hook_name="isaaclab stage context", ) ) - self._set_deferred_cuda_device() + if self._deferred_cuda_device_id is not None: + set_cuda_device(self._deferred_cuda_device_id) # Load IsaacSim extensions self._load_extensions() @@ -210,7 +212,7 @@ def __init__(self, launcher_args: argparse.Namespace | dict | None = None): _deprioritize_prebundle_paths() # Hide the stop button in the toolbar - self._hide_stop_button() + self._set_toolbar_button_visible("_stop_button", False) # Hide play button callback if the timeline is stopped import omni.timeline @@ -219,14 +221,16 @@ def __init__(self, launcher_args: argparse.Namespace | dict | None = None): omni.timeline.get_timeline_interface() .get_timeline_event_stream() .create_subscription_to_pop_by_type( - int(omni.timeline.TimelineEventType.STOP), lambda e: self._hide_play_button(True) + int(omni.timeline.TimelineEventType.STOP), + lambda e: self._set_toolbar_button_visible("_play_button", False), ) ) self._unhide_play_button_callback = ( omni.timeline.get_timeline_interface() .get_timeline_event_stream() .create_subscription_to_pop_by_type( - int(omni.timeline.TimelineEventType.PLAY), lambda e: self._hide_play_button(False) + int(omni.timeline.TimelineEventType.PLAY), + lambda e: self._set_toolbar_button_visible("_play_button", True), ) ) # the CI runner greps this marker for hang detection; __stderr__ survives @@ -259,7 +263,7 @@ def is_available(cls) -> bool: return SimulationApp is not None @staticmethod - def add_app_launcher_args(parser: argparse.ArgumentParser) -> None: + def add_launcher_args(parser: argparse.ArgumentParser) -> None: """Utility function to configure KitLauncher arguments with an existing argument parser object. This function appends the command-line arguments relevant to the SimulationApp to the input @@ -349,7 +353,7 @@ def add_app_launcher_args(parser: argparse.ArgumentParser) -> None: arg_group.add_argument( "--device", type=str, - action=ExplicitAction, + action=_ExplicitAction, default="cuda:0", help='The device to run the simulation on. Can be "cpu", "cuda", "cuda:N", where N is the device ID', ) @@ -357,7 +361,7 @@ def add_app_launcher_args(parser: argparse.ArgumentParser) -> None: "--visualizer", "--viz", type=_parse_visualizer_csv, - action=ExplicitAction, + action=_ExplicitAction, default=None, help="Visualizer backends to enable as CSV (e.g., kit,newton,rerun,viser).", ) @@ -614,13 +618,6 @@ def _initialize_preloaded_torch_cuda(self) -> None: if torch is not None and not torch.cuda.is_initialized(): torch.cuda.init() - def _set_deferred_cuda_device(self) -> None: - """Set the current CUDA device after Kit startup.""" - if self._deferred_cuda_device_id is None: - return - - set_cuda_device(self._deferred_cuda_device_id) - def _resolve_experience_file(self, launcher_args: dict): """Resolve experience file related settings.""" # Check if input keywords contain an 'experience' file setting @@ -690,21 +687,18 @@ def _resolve_experience_file(self, launcher_args: dict): # Resolve the absolute path of the experience file self._sim_experience_file = os.path.abspath(self._sim_experience_file) # Detect a known incompatibility between Isaac Lab and Isaac Sim full-app experiences. - if self._livestream in {1, 2} and self._experience_depends_on_isaacsim_exp_full(self._sim_experience_file): - raise ValueError( - "The experience file depends on 'isaacsim.exp.full', which is known to hang or invalidate PhysX " - "tensor views when launched through Isaac Lab with livestreaming enabled. Omit '--experience' so " - "KitLauncher can select an Isaac Lab experience file, or remove the 'isaacsim.exp.full' dependency." - ) + if self._livestream in {1, 2}: + with open(self._sim_experience_file, encoding="utf-8") as file: + experience = file.read() + if re.search(r'^\s*["\']isaacsim\.exp\.full["\']\s*=', experience, re.MULTILINE): + raise ValueError( + "The experience file depends on 'isaacsim.exp.full', which is known to hang or invalidate PhysX " + "tensor views when launched through Isaac Lab with livestreaming enabled. Omit '--experience' so " + "KitLauncher can select an Isaac Lab experience file, or remove the 'isaacsim.exp.full' dependency." + ) self._deterministic_rendering = bool(deterministic_mode) logger.info("Loading experience file: %s", self._sim_experience_file) - @staticmethod - def _experience_depends_on_isaacsim_exp_full(experience_file: str) -> bool: - """Return whether a Kit experience directly depends on ``isaacsim.exp.full``.""" - with open(experience_file, encoding="utf-8") as file: - return re.search(r'^\s*["\']isaacsim\.exp\.full["\']\s*=', file.read(), re.MULTILINE) is not None - def _resolve_anim_recording_settings(self, launcher_args: dict): """Resolve animation recording settings; recording needs the ``omni.physx.pvd`` extension.""" self._anim_recording = None @@ -721,10 +715,6 @@ def _resolve_anim_recording_settings(self, launcher_args: dict): self._anim_recording = (start_time, stop_time) sys.argv += ["--enable", "omni.physx.pvd"] - def _requires_all_partitions_spectator_view(self) -> bool: - """Return whether the launch needs an unpartitioned all-environment view.""" - return self._kit_visualizer or self._render_viewport or self._video_enabled or self._livestream > 0 or self._xr - def _resolve_kit_args(self, launcher_args: dict): """Resolve additional arguments passed to Kit.""" self._kit_args = launcher_args.get("kit_args", "").split() @@ -746,9 +736,9 @@ def _resolve_kit_args(self, launcher_args: dict): if not any(arg.partition("=")[0] == setting for arg in sys.argv + self._kit_args): self._kit_args.append(argument) - # RTX allocates spectator support during startup, so visual output intent - # must become a Kit argument before SimulationApp is created. - if self._requires_all_partitions_spectator_view(): + # RTX allocates spectator support during startup, so visual output intent (which needs an + # unpartitioned all-environment view) must become a Kit argument before SimulationApp is created. + if self._kit_visualizer or self._render_viewport or self._video_enabled or self._livestream > 0 or self._xr: argument = f"--{ISAAC_RTX_SHOW_ALL_PARTITIONS_BY_DEFAULT_SETTING}=true" setting = argument.partition("=")[0] if not any(arg.partition("=")[0] == setting for arg in sys.argv + self._kit_args): @@ -822,61 +812,50 @@ def _load_extensions(self): # Publish whether Kit has an interactive GUI (local window, livestream, or XR). # SimulationContext and renderers consume this setting during their initialization. - settings.set_bool("/isaaclab/has_gui", not self._headless or self._livestream >= 1 or self._xr) - settings.set_bool("/isaaclab/render/offscreen", self._offscreen_render) - settings.set_bool("/isaaclab/xr/enabled", self._xr) + settings.set("/isaaclab/has_gui", not self._headless or self._livestream >= 1 or self._xr) + settings.set("/isaaclab/render/offscreen", self._offscreen_render) + settings.set("/isaaclab/xr/enabled", self._xr) # set setting to indicate XR auto-start mode -- when running headless # (no Kit GUI) the AR profile must be enabled programmatically so that # the OpenXR session starts without user interaction - settings.set_bool("/isaaclab/xr/auto_start", self._headless and self._xr) - settings.set_bool("/isaaclab/video/enabled", self._video_enabled) + settings.set("/isaaclab/xr/auto_start", self._headless and self._xr) + settings.set("/isaaclab/video/enabled", self._video_enabled) # publish the reproducible-rendering intent; rendering backends read this on initialization - settings.set_bool("/isaaclab/render/deterministic", self._deterministic_rendering) + settings.set("/isaaclab/render/deterministic", self._deterministic_rendering) # use fixed time stepping disabled; custom loop runner from Isaac Sim is used instead - settings.set_bool("/app/player/useFixedTimeStepping", False) + settings.set("/app/player/useFixedTimeStepping", False) if self._anim_recording is not None: - settings.set_bool("/isaaclab/anim_recording/enabled", True) - settings.set_float("/isaaclab/anim_recording/start_time", self._anim_recording[0]) - settings.set_float("/isaaclab/anim_recording/stop_time", self._anim_recording[1]) + settings.set("/isaaclab/anim_recording/enabled", True) + settings.set("/isaaclab/anim_recording/start_time", self._anim_recording[0]) + settings.set("/isaaclab/anim_recording/stop_time", self._anim_recording[1]) - def _hide_stop_button(self): - """Hide the stop button in the toolbar. + def _set_toolbar_button_visible(self, button_name: str, visible: bool): + """Show and enable, or hide and disable, a button of the toolbar's play button group. - For standalone executions, having a stop button is confusing since it invalidates the whole simulation. - Thus, we hide the button so that users don't accidentally click it. - """ - # when we are truly headless, then we can't import the widget toolbar - # thus, we only hide the stop button when we are not headless (i.e. GUI is enabled) - if self._livestream >= 1 or not self._headless: - import omni.kit.widget.toolbar - - # grey out the stop button because we don't want to stop the simulation manually in standalone mode - toolbar = omni.kit.widget.toolbar.get_instance() - play_button_group = toolbar._builtin_tools._play_button_group # type: ignore - if play_button_group is not None: - play_button_group._stop_button.visible = False # type: ignore - play_button_group._stop_button.enabled = False # type: ignore - play_button_group._stop_button = None # type: ignore - - def _hide_play_button(self, flag): - """Hide/Unhide the play button in the toolbar. - - This is used if the timeline is stopped by a GUI action like "save as" to not allow the user to - resume the timeline afterwards. + Standalone runs hide the stop button for good, since stopping invalidates the whole simulation. They hide + the play button while a GUI action like "save as" has stopped the timeline, so the user cannot resume it. + + Args: + button_name: Attribute of the play button group, ``"_stop_button"`` or ``"_play_button"``. + visible: Whether to show and enable the button. """ - # when we are truly headless, then we can't import the widget toolbar - # thus, we only hide the play button when we are not headless (i.e. GUI is enabled) - if self._livestream >= 1 or not self._headless: - import omni.kit.widget.toolbar - - toolbar = omni.kit.widget.toolbar.get_instance() - play_button_group = toolbar._builtin_tools._play_button_group # type: ignore - if play_button_group is not None: - play_button_group._play_button.visible = not flag # type: ignore - play_button_group._play_button.enabled = not flag # type: ignore + # a truly headless app has no toolbar widget to import + if self._livestream < 1 and self._headless: + return + import omni.kit.widget.toolbar + + play_button_group = omni.kit.widget.toolbar.get_instance()._builtin_tools._play_button_group # type: ignore + if play_button_group is None: + return + button = getattr(play_button_group, button_name) + button.visible = visible + button.enabled = visible + if button_name == "_stop_button": + # detach the stop button so the group never shows it again + setattr(play_button_group, button_name, None) def _share_stage_context() -> None: diff --git a/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py b/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py index bab4a9213b08..df6b40130987 100644 --- a/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py +++ b/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py @@ -323,7 +323,7 @@ def create_render_data(self, spec: CameraRenderSpec) -> IsaacRtxRenderData: ) has_gui = settings.get("/isaaclab/has_gui") if simple_shading_mode is None and (not needs_color_render or has_gui): - settings.set_bool("/rtx/sdg/force/disableColorRender", not needs_color_render and not has_gui) + settings.set("/rtx/sdg/force/disableColorRender", not needs_color_render and not has_gui) else: unsupported = [] if "albedo" in spec.cfg.data_types: diff --git a/source/isaaclab_physx/isaaclab_physx/sensors/contact_sensor/contact_sensor.py b/source/isaaclab_physx/isaaclab_physx/sensors/contact_sensor/contact_sensor.py index d26d621c9f38..1aeb71a2daeb 100644 --- a/source/isaaclab_physx/isaaclab_physx/sensors/contact_sensor/contact_sensor.py +++ b/source/isaaclab_physx/isaaclab_physx/sensors/contact_sensor/contact_sensor.py @@ -92,7 +92,7 @@ def __init__(self, cfg: ContactSensorCfg): super().__init__(cfg) # Enable contact processing - get_settings_manager().set_bool("/physics/disableContactProcessing", False) + get_settings_manager().set("/physics/disableContactProcessing", False) # Create empty variables for storing output data self._data: ContactSensorData = ContactSensorData() diff --git a/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py b/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py index 4ff6c17bf2ec..590cefef91a3 100644 --- a/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py +++ b/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py @@ -16,7 +16,7 @@ from pxr import Gf, Usd, UsdGeom -from isaaclab.app.settings_manager import SettingsManager +from isaaclab.app.settings_manager import get_settings_manager from isaaclab.sim.views.base_frame_view import BaseFrameView from isaaclab.sim.views.fabric_xform_selection import FabricXformSelection from isaaclab.sim.views.usd_frame_view import UsdFrameView @@ -188,7 +188,7 @@ def __init__( self._usd_view = UsdFrameView(prim_path, device=device, validate_xform_ops=validate_xform_ops, stage=stage) self._device = device - settings = SettingsManager.instance() + settings = get_settings_manager() self._use_fabric = bool(settings.get("/physics/fabricEnabled", False)) # TODO(pv): Misleading abstraction -- FabricFrameView can fall back to USD internally; diff --git a/source/isaaclab_physx/test/app/test_kit_launcher.py b/source/isaaclab_physx/test/app/test_kit_launcher.py index 9ea5c9a7739e..18d92a95fc7b 100644 --- a/source/isaaclab_physx/test/app/test_kit_launcher.py +++ b/source/isaaclab_physx/test/app/test_kit_launcher.py @@ -18,6 +18,7 @@ import isaaclab.app.sim_launcher as sim_launcher import isaaclab.utils as utils_module from isaaclab.app import SimulationLauncher, add_launcher_args +from isaaclab.app.argv import fuse_kit_args from isaaclab.app.sim_launcher import Scan, _get_kit_runtime_sources, _normalize_launcher_args from isaaclab.utils.renderers import ISAAC_RTX_SHOW_ALL_PARTITIONS_BY_DEFAULT_SETTING @@ -199,10 +200,10 @@ def test_explicit_spectator_setting_overrides_visualizer_default(monkeypatch): ) def test_fuse_kit_args(argv: list[str], expected: list[str]): """Fuse only ``--kit_args`` pairs whose value argparse would mistake for an option.""" - assert sim_launcher.fuse_kit_args(argv) == expected + assert fuse_kit_args(argv) == expected -def test_add_app_launcher_args_registers_every_launcher_option(): +def test_add_launcher_args_registers_every_launcher_option(): """Every launcher option is a command-line option.""" parser = argparse.ArgumentParser() add_launcher_args(parser) @@ -236,7 +237,7 @@ def test_deprecated_render_flags_are_rejected(deprecated_arg: str): def test_help_on_parser_with_required_positionals(monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture): """Launcher arguments reach the help output of a script that takes required positionals. - ``add_app_launcher_args`` probes the command line to check for name collisions. That probe + ``add_launcher_args`` probes the command line to check for name collisions. That probe exits when a required argument is missing, which is the case for every tool script invoked with ``--help``, so it must not take the parser down before the arguments are added. """ @@ -375,7 +376,7 @@ def test_explicit_simulation_manager_callback_setting_takes_precedence(monkeypat def test_make_physics_cfg_builds_core_vbd(): from isaaclab_newton.physics import NewtonCfg, VBDSolverCfg - physics_cfg = sim_launcher.make_physics_cfg("newton_vbd") + physics_cfg = sim_launcher._make_physics_cfg("newton_vbd") assert isinstance(physics_cfg, NewtonCfg) assert isinstance(physics_cfg.solver_cfg, VBDSolverCfg) @@ -384,7 +385,7 @@ def test_make_physics_cfg_builds_core_vbd(): def test_livestream_injects_kit_visualizer_when_missing(): args = argparse.Namespace(livestream=2, visualizer=None, visualizer_explicit=False) - _normalize_launcher_args(args) + _normalize_launcher_args(vars(args)) assert args.visualizer == ["kit"] assert args.visualizer_explicit is True @@ -394,7 +395,7 @@ def test_livestream_rejects_disabled_visualizers(): args = argparse.Namespace(livestream=2, visualizer=None, visualizer_explicit=True) with pytest.raises(ValueError, match="Livestreaming requires the Kit visualizer"): - _normalize_launcher_args(args) + _normalize_launcher_args(vars(args)) def test_livestream_rejects_invalid_environment_value(monkeypatch: pytest.MonkeyPatch): @@ -416,7 +417,8 @@ def test_explicit_experience_requires_isaac_sim_runtime(): has_ovphysx_physics=False, needs_kit=False, ) - args = argparse.Namespace(experience="isaaclab.python.kit", visualizer=None) + args = {"experience": "isaaclab.python.kit", "visualizer": None} + _normalize_launcher_args(args) assert _get_kit_runtime_sources(scan, args) @@ -431,8 +433,8 @@ def _resolve_headless_for_case( monkeypatch.delenv("XR", raising=False) monkeypatch.delenv("LIVESTREAM", raising=False) _normalize_launcher_args(launcher_args) - config_scan = SimpleNamespace(visualizer_intent={"has_kit_visualizer": cfg_has_kit}) - launcher_args["kit_visualizer"] = sim_launcher._has_kit_visualizer(config_scan, launcher_args) + cfg = SimpleNamespace(visualizer_cfgs=[SimpleNamespace(visualizer_type="kit")] if cfg_has_kit else []) + launcher_args["kit_visualizer"] = sim_launcher._get_visualizer_intent(cfg, launcher_args)["has_kit_visualizer"] launcher = KitLauncher.__new__(KitLauncher) launcher._kit_visualizer = launcher_args["kit_visualizer"] launcher._livestream = launcher_args["livestream"] @@ -542,18 +544,6 @@ def _scan(_cfg, launcher_args): assert received_args["enable_cameras"] is True -def test_deferred_cuda_device_synchronizes_torch_and_warp(monkeypatch: pytest.MonkeyPatch): - """The post-Kit device hook must synchronize both CUDA runtimes.""" - devices = [] - monkeypatch.setattr(kit_launcher_module, "set_cuda_device", devices.append) - launcher = KitLauncher.__new__(KitLauncher) - launcher._deferred_cuda_device_id = 2 - - launcher._set_deferred_cuda_device() - - assert devices == [2] - - def test_preloaded_torch_cuda_is_initialized_before_kit(monkeypatch: pytest.MonkeyPatch): """Drain PyTorch's queued CUDA checks before Kit can change visible device indices.""" init_calls = [] @@ -601,13 +591,7 @@ class _DummySettings: def __init__(self): self.values = {} - def set_string(self, path: str, value: str) -> None: - self.values[path] = value - - def set_int(self, path: str, value: int) -> None: - self.values[path] = value - - def set_bool(self, path: str, value: bool) -> None: + def set(self, path: str, value) -> None: self.values[path] = value diff --git a/source/isaaclab_physx/test/envs/test_manager_based_rl_env_ui.py b/source/isaaclab_physx/test/envs/test_manager_based_rl_env_ui.py index 6776114631b9..fd9c5560657d 100644 --- a/source/isaaclab_physx/test/envs/test_manager_based_rl_env_ui.py +++ b/source/isaaclab_physx/test/envs/test_manager_based_rl_env_ui.py @@ -60,7 +60,7 @@ def test_ui_window_follows_has_gui_gate(has_gui: bool): TrackingManagerBasedRLEnvWindow.construction_count = 0 try: - settings.set_bool(_HAS_GUI_SETTING, has_gui) + settings.set(_HAS_GUI_SETTING, has_gui) sim_utils.create_new_stage() env = ManagerBasedRLEnv(cfg=make_empty_manager_based_rl_env_ui_cfg()) @@ -69,4 +69,4 @@ def test_ui_window_follows_has_gui_gate(has_gui: bool): finally: if env is not None: env.close() - settings.set_bool(_HAS_GUI_SETTING, previous_has_gui) + settings.set(_HAS_GUI_SETTING, previous_has_gui) diff --git a/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py b/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py index c7533bf16e06..319ce055b8cb 100644 --- a/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py +++ b/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py @@ -280,11 +280,7 @@ def _create_attribute(name, value_type): # The shading level must not leak into process-wide state, where the last camera would win. global_setting_calls = [ setting_call - for setting_call in ( - *settings.set_int.call_args_list, - *settings.set.call_args_list, - *settings.set_bool.call_args_list, - ) + for setting_call in settings.set.call_args_list if setting_call.args and setting_call.args[0] in ("/rtx/minimal/mode", "/rtx/rendermode", "/rtx/sdg/force/disableColorRender") ] @@ -338,7 +334,7 @@ def test_depth_only_camera_color_render_setting(monkeypatch, has_gui, expected_d color_render_calls = [ setting_call - for setting_call in settings.set_bool.call_args_list + for setting_call in settings.set.call_args_list if setting_call.args[0] == "/rtx/sdg/force/disableColorRender" ] assert color_render_calls[-1] == call("/rtx/sdg/force/disableColorRender", expected_disable_color_render) diff --git a/source/isaaclab_physx/test/sensors/test_contact_sensor.py b/source/isaaclab_physx/test/sensors/test_contact_sensor.py index d82b7ad603a6..dd7fe402310c 100644 --- a/source/isaaclab_physx/test/sensors/test_contact_sensor.py +++ b/source/isaaclab_physx/test/sensors/test_contact_sensor.py @@ -633,7 +633,7 @@ def test_contact_sensor_no_stale_data_after_reset(setup_simulation, device): PhysX's stale post-step buffer for the reset env — it must reflect the freshly reset state. """ sim_dt, _, settings = setup_simulation - settings.set_bool("/physics/disableContactProcessing", False) + settings.set("/physics/disableContactProcessing", False) with build_simulation_context(device=device, dt=sim_dt, add_lighting=True) as sim: sim._app_control_on_stop_handle = None scene_cfg = ContactSensorSceneCfg(num_envs=1, env_spacing=2.0, lazy_sensor_update=False) @@ -692,7 +692,7 @@ def test_contact_history_updates_at_sensor_period( timestamps) on data access. """ sim_dt, _, settings = setup_simulation - settings.set_bool("/physics/disableContactProcessing", False) + settings.set("/physics/disableContactProcessing", False) with build_simulation_context(device=device, dt=sim_dt, add_lighting=False) as sim: sim._app_control_on_stop_handle = None scene_cfg = ContactSensorSceneCfg(num_envs=1, env_spacing=2.0, lazy_sensor_update=True) @@ -752,7 +752,7 @@ def test_lazy_sensor_reports_contact_loss(setup_simulation, device): (regression for #3498, where the spawner passed ``activate_contact_sensors`` as the threshold). """ sim_dt, _, settings = setup_simulation - settings.set_bool("/physics/disableContactProcessing", False) + settings.set("/physics/disableContactProcessing", False) with build_simulation_context(device=device, dt=sim_dt, add_lighting=False) as sim: sim._app_control_on_stop_handle = None scene_cfg = ContactSensorSceneCfg(num_envs=1, env_spacing=1.0, lazy_sensor_update=True) @@ -814,7 +814,7 @@ def test_friction_reporting(setup_simulation): # minor gravity force in -z to ensure object stays on ground plane grav_dir = (-10.0, 0.0, -0.1) sim_dt, _, settings = setup_simulation - settings.set_bool("/physics/disableContactProcessing", True) + settings.set("/physics/disableContactProcessing", True) device = "cuda:0" sim_cfg = SimulationCfg(dt=sim_dt, device=device, gravity=grav_dir) with build_simulation_context(sim_cfg=sim_cfg, add_lighting=False) as sim: @@ -871,7 +871,7 @@ def test_friction_reporting(setup_simulation): def test_invalid_contact_data_config(setup_simulation, use_filter, max_contact_data_count_per_prim, match): """Tracking friction forces requires a filter pattern and a positive contact data budget.""" sim_dt, _, settings = setup_simulation - settings.set_bool("/physics/disableContactProcessing", True) + settings.set("/physics/disableContactProcessing", True) device = "cuda:0" sim_cfg = SimulationCfg(dt=sim_dt, device=device) with build_simulation_context(sim_cfg=sim_cfg, add_lighting=False) as sim: diff --git a/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip b/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip new file mode 100644 index 000000000000..fde46a0c6757 --- /dev/null +++ b/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip @@ -0,0 +1 @@ +Imported fuse_kit_args from isaaclab.app.argv and switched tests to SettingsManager.set; no behavior change. diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py index a22692838f53..bb41c7a71618 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py @@ -1004,7 +1004,7 @@ def _resolve_video_source(env_cfg: Any, args_cli: argparse.Namespace) -> str: ValueError: If ``--viz none`` or only streaming visualizers were requested. """ # _parse_visualizer_csv("none") yields None rather than ["none"], so an explicitly disabled - # visualizer is only recognizable through the ExplicitAction sentinel + # visualizer is only recognizable through the _ExplicitAction sentinel if getattr(args_cli, "visualizer_explicit", False) and getattr(args_cli, "visualizer", "not_none") is None: raise ValueError( "--video is not compatible with --viz none: there is no active visualizer to record from. " diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py index 36d882f66731..50ad31cae7b9 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py @@ -14,6 +14,8 @@ import gymnasium as gym +from isaaclab.app.argv import fuse_kit_args + if TYPE_CHECKING: from .simple_agents import PolicyName @@ -130,9 +132,6 @@ def _normalize_argv(argv: list[str] | None) -> list[str]: The backends parse this explicit list rather than ``sys.argv``, so the fusing that :func:`~isaaclab.app.add_launcher_args` applies to ``sys.argv`` never reaches it. """ - # imported here so that importing this module stays lightweight - from isaaclab.app.sim_launcher import fuse_kit_args - if argv is None: argv = sys.argv[1:] return fuse_kit_args(argv) diff --git a/source/isaaclab_rl/test/export/leapp_initialized_checkpoints.py b/source/isaaclab_rl/test/export/leapp_initialized_checkpoints.py index 7dc85f264b0f..04dc96dd8a7c 100644 --- a/source/isaaclab_rl/test/export/leapp_initialized_checkpoints.py +++ b/source/isaaclab_rl/test/export/leapp_initialized_checkpoints.py @@ -339,8 +339,8 @@ def main(argv: Sequence[str] | None = None) -> None: try: env_cfg, agent_cfg = resolve_task_config(task_name, _agent_cfg_entry_point(backend_id)) with launch_simulation(env_cfg, vars(cli_args)): - get_settings_manager().set_bool("/physics/cooking/ujitsoCollisionCooking", False) - get_settings_manager().set_bool("/isaaclab/render/rtx_sensors", False) + get_settings_manager().set("/physics/cooking/ujitsoCollisionCooking", False) + get_settings_manager().set("/isaaclab/render/rtx_sensors", False) checkpoint_path = create_initialized_checkpoint( backend_id, task_name, diff --git a/source/isaaclab_tasks/changelog.d/launch-helper-cleanup.skip b/source/isaaclab_tasks/changelog.d/launch-helper-cleanup.skip new file mode 100644 index 000000000000..a3cf5c64b516 --- /dev/null +++ b/source/isaaclab_tasks/changelog.d/launch-helper-cleanup.skip @@ -0,0 +1 @@ +Updated tests for the launcher and settings helper cleanup. diff --git a/source/isaaclab_tasks/test/core/test_distributed_device_resolution.py b/source/isaaclab_tasks/test/core/test_distributed_device_resolution.py index 5759653d7a5f..cc768de7c31d 100644 --- a/source/isaaclab_tasks/test/core/test_distributed_device_resolution.py +++ b/source/isaaclab_tasks/test/core/test_distributed_device_resolution.py @@ -27,8 +27,6 @@ import types from unittest.mock import patch -import pytest - import isaaclab.app.sim_launcher as sim_launcher # --------------------------------------------------------------------------- @@ -106,7 +104,7 @@ def test_normal_multi_gpu_rank3(self, mock_count, mock_set_device): env = _make_env_vars(local_rank=3, world_size=4) with patch.dict(os.environ, env, clear=False): - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, vars(args)) assert env_cfg.sim.device == "cuda:3" # the Kit launcher reads the resolved device from the launcher args @@ -122,7 +120,7 @@ def test_cuda_visible_devices_restricted_rank1(self, mock_count, mock_set_device env = _make_env_vars(local_rank=1, world_size=2) with patch.dict(os.environ, env, clear=False): - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, vars(args)) assert env_cfg.sim.device == "cuda:0" mock_set_device.assert_called_once_with("cuda:0") @@ -137,7 +135,7 @@ def test_jax_local_rank_added(self, mock_count, mock_set_device): env = _make_env_vars(local_rank=0, world_size=2, jax_local_rank=1) with patch.dict(os.environ, env, clear=False): - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, vars(args)) assert env_cfg.sim.device == "cuda:1" mock_set_device.assert_called_once_with("cuda:1") @@ -174,13 +172,12 @@ def test_dict_args_distributed(self, mock_count, mock_set_device): class TestResolveDistributedDeviceNoop: """Tests that non-distributed runs skip device resolution.""" - @pytest.mark.parametrize("args", [argparse.Namespace(distributed=False), {"distributed": False}]) @patch.object(sim_launcher, "set_cuda_device") - def test_not_distributed(self, mock_set_device, args): - """distributed=False as a namespace or dict → device unchanged, set_device not called.""" + def test_not_distributed(self, mock_set_device): + """distributed=False → device unchanged, set_device not called.""" env_cfg = _DummyEnvCfg(device="cuda:0") - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, {"distributed": False}) assert env_cfg.sim.device == "cuda:0" mock_set_device.assert_not_called() @@ -196,16 +193,6 @@ def test_no_distributed_key(self, mock_set_device): assert env_cfg.sim.device == "cuda:0" mock_set_device.assert_not_called() - @patch.object(sim_launcher, "set_cuda_device") - def test_none_launcher_args(self, mock_set_device): - """launcher_args=None → no-op.""" - env_cfg = _DummyEnvCfg(device="cuda:0") - - sim_launcher._resolve_distributed_device(env_cfg, None) - - assert env_cfg.sim.device == "cuda:0" - mock_set_device.assert_not_called() - # --------------------------------------------------------------------------- # _resolve_distributed_device — edge cases @@ -228,7 +215,7 @@ class _BareEnvCfg: env = _make_env_vars(local_rank=1, world_size=2) with patch.dict(os.environ, env, clear=False): - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, vars(args)) # Should still call set_device even without sim_cfg mock_set_device.assert_called_once_with("cuda:1") @@ -248,7 +235,7 @@ def test_missing_env_vars_default_to_zero(self, mock_count, mock_set_device): } with patch.dict(os.environ, clean_env, clear=True): - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, vars(args)) # local_rank=0, 0 < 4 → cuda:0 assert env_cfg.sim.device == "cuda:0" @@ -275,7 +262,7 @@ def test_multi_node_rank3_sees_4_gpus(self, mock_count, mock_set_device): env = _make_env_vars(local_rank=3, world_size=8, rank=7) with patch.dict(os.environ, env, clear=False): - sim_launcher._resolve_distributed_device(env_cfg, args) + sim_launcher._resolve_distributed_device(env_cfg, vars(args)) assert env_cfg.sim.device == "cuda:3" mock_set_device.assert_called_once_with("cuda:3") diff --git a/source/isaaclab_tasks/test/core/test_environment_determinism.py b/source/isaaclab_tasks/test/core/test_environment_determinism.py index bbc987f36b47..492fea6ddeb5 100644 --- a/source/isaaclab_tasks/test/core/test_environment_determinism.py +++ b/source/isaaclab_tasks/test/core/test_environment_determinism.py @@ -25,7 +25,7 @@ def setup_environment(): # test on many environments. from isaaclab.app.settings_manager import get_settings_manager - get_settings_manager().set_bool("/physics/cooking/ujitsoCollisionCooking", False) + get_settings_manager().set("/physics/cooking/ujitsoCollisionCooking", False) @pytest.mark.parametrize( diff --git a/source/isaaclab_tasks/test/core/test_rl_device_separation.py b/source/isaaclab_tasks/test/core/test_rl_device_separation.py index e71184051c9d..c79b4c2a7c22 100644 --- a/source/isaaclab_tasks/test/core/test_rl_device_separation.py +++ b/source/isaaclab_tasks/test/core/test_rl_device_separation.py @@ -66,7 +66,7 @@ def _create_env(sim_device: str): # Reset the rtx sensors setting to False from isaaclab.app.settings_manager import get_settings_manager - get_settings_manager().set_bool("/isaaclab/render/rtx_sensors", False) + get_settings_manager().set("/isaaclab/render/rtx_sensors", False) try: env_cfg = parse_env_cfg(TEST_ENV, device=sim_device, num_envs=NUM_ENVS) diff --git a/source/isaaclab_tasks/test/core/test_runtime_compatibility.py b/source/isaaclab_tasks/test/core/test_runtime_compatibility.py index 17fb821d0e8f..c5a83ac9e2d6 100644 --- a/source/isaaclab_tasks/test/core/test_runtime_compatibility.py +++ b/source/isaaclab_tasks/test/core/test_runtime_compatibility.py @@ -34,8 +34,9 @@ def validate_runtime_compatibility(env_cfg, launcher_args=None): """Run the single-scan runtime validation for *env_cfg* (test adapter).""" - config_scan = scan(env_cfg, launcher_args) - kit_sources = _get_kit_runtime_sources(config_scan, launcher_args) + args = vars(launcher_args) if isinstance(launcher_args, argparse.Namespace) else launcher_args or {} + config_scan = scan(env_cfg, args) + kit_sources = _get_kit_runtime_sources(config_scan, args) _validate_runtime(config_scan, kit_sources) return config_scan diff --git a/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py b/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py index 28a0d80d847d..69626e2d71ed 100644 --- a/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py +++ b/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py @@ -8,6 +8,7 @@ from __future__ import annotations import argparse +import logging import sys import types @@ -75,9 +76,9 @@ def close(self, exit_code=0): with sim_launcher.launch_simulation(env_cfg, launcher_args): pass - forwarded_args = captured["launcher_args"] - assert isinstance(forwarded_args, argparse.Namespace) - assert forwarded_args.kit_visualizer is True + # the launcher gets the caller's namespace as a dict, so resolved values reach both + assert captured["launcher_args"]["kit_visualizer"] is True + assert launcher_args.kit_visualizer is True assert captured["closed"] is True @@ -119,22 +120,21 @@ def test_cli_visualizer_selection_overrides_config_kit_visualizer(kit_launcher_a def test_launch_simulation_writes_max_visible_envs_without_visualizer(kit_launcher_args, require_kit): """``--max_visible_envs`` reaches the settings with or without Kit, even when no visualizer is selected.""" settings = get_settings_manager() - settings.set_int("/isaaclab/visualizer/max_visible_envs", -1) + settings.set("/isaaclab/visualizer/max_visible_envs", -1) with sim_launcher.launch_simulation(NewtonCfg(), {"require_kit": require_kit, "max_visible_envs": 3}): pass assert len(kit_launcher_args) == int(require_kit) assert settings.get("/isaaclab/visualizer/max_visible_envs") == 3 - settings.set_int("/isaaclab/visualizer/max_visible_envs", -1) + settings.set("/isaaclab/visualizer/max_visible_envs", -1) def test_launch_simulation_kitless_viz_none_sets_disable_all(monkeypatch): """Kitless mode should persist explicit disable-all semantics for --viz none.""" captured = {"types": None, "explicit": None, "disable_all": None} - def fake_sync(launcher_args: argparse.Namespace) -> None: - launcher_args = vars(launcher_args) + def fake_sync(launcher_args: dict) -> None: captured["types"] = " ".join(launcher_args["visualizer"]) if launcher_args.get("visualizer") else "" captured["explicit"] = launcher_args["visualizer_explicit"] captured["disable_all"] = launcher_args["visualizer_disable_all"] @@ -154,21 +154,15 @@ def test_launch_simulation_kitless_applies_python_logging_level(monkeypatch): """Kitless mode should apply the resolved Python logging level before yielding.""" captured: dict[str, object] = {} - def fake_resolve(launcher_args): - captured["resolve_args"] = launcher_args - return 42 - def fake_apply(level): captured["applied_level"] = level _force_kitless(monkeypatch) - monkeypatch.setattr(sim_launcher, "resolve_python_logging_level", fake_resolve) monkeypatch.setattr(sim_launcher, "apply_python_logging_level", fake_apply) env_cfg = _DummyEnvCfg(_DummySimCfg(None)) - launcher_args = argparse.Namespace(visualizer=None, visualizer_explicit=True) + launcher_args = argparse.Namespace(visualizer=None, visualizer_explicit=True, verbose=True) with sim_launcher.launch_simulation(env_cfg, launcher_args): pass - assert captured["resolve_args"] is launcher_args - assert captured["applied_level"] == 42 + assert captured["applied_level"] == logging.DEBUG diff --git a/source/isaaclab_tasks/test/env_test_utils.py b/source/isaaclab_tasks/test/env_test_utils.py index d72a998ffb8e..63fad921d720 100644 --- a/source/isaaclab_tasks/test/env_test_utils.py +++ b/source/isaaclab_tasks/test/env_test_utils.py @@ -123,7 +123,7 @@ def setup_environment( registered_tasks.sort() # this flag is necessary to prevent a bug where the simulation gets stuck randomly when running many environments - get_settings_manager().set_bool("/physics/cooking/ujitsoCollisionCooking", False) + get_settings_manager().set("/physics/cooking/ujitsoCollisionCooking", False) print(">>> All registered environments:", registered_tasks) @@ -310,7 +310,7 @@ def _check_random_actions( sim_utils.create_new_stage() # reset the rtx sensors setting to False - get_settings_manager().set_bool("/isaaclab/render/rtx_sensors", False) + get_settings_manager().set("/isaaclab/render/rtx_sensors", False) env = None try: # Parse the requested physics preset before resolving the config. ``parse_env_cfg`` resolves every preset to diff --git a/source/isaaclab_visualizers/changelog.d/launch-helper-cleanup.skip b/source/isaaclab_visualizers/changelog.d/launch-helper-cleanup.skip new file mode 100644 index 000000000000..d992140bd68b --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/launch-helper-cleanup.skip @@ -0,0 +1 @@ +Switched the Kit visualizer to SettingsManager.set; no behavior change. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py index 2ef4f921874c..849e8db69166 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py @@ -204,9 +204,9 @@ def step(self, dt: float) -> None: # Keep app pumping for viewport/UI updates only; physics is owned by SimulationContext. # Disable playSimulations around app.update() so Kit does not advance its own physics here. settings = get_settings_manager() - settings.set_bool("/app/player/playSimulations", False) + settings.set("/app/player/playSimulations", False) app.update() - settings.set_bool("/app/player/playSimulations", True) + settings.set("/app/player/playSimulations", True) self._app_pumped_this_step = True except (ImportError, AttributeError) as exc: logger.debug("[KitVisualizer] App update skipped: %s", exc) @@ -288,9 +288,9 @@ def render_rgb_array(self) -> np.ndarray: if not self._app_pumped_this_step: settings = get_settings_manager() play_flag = settings.get("/app/player/playSimulations") - settings.set_bool("/app/player/playSimulations", False) + settings.set("/app/player/playSimulations", False) omni.kit.app.get_app().update() - settings.set_bool("/app/player/playSimulations", bool(play_flag)) + settings.set("/app/player/playSimulations", bool(play_flag)) raw = self._rgb_annotator.get_data() if isinstance(raw, dict): From baca09d279cb698a4387588ccee93d2afb256ef0 Mon Sep 17 00:00:00 2001 From: Octi Zhang Date: Mon, 28 Sep 2026 08:37:57 -0700 Subject: [PATCH 2/3] Keep fuse_kit_args in sim_launcher instead of a new module Importing sim_launcher takes about a second (it loads the backend configs), so the benchmark dispatcher keeps its lazy-import wrapper and no new module is needed. --- docs/source/api/lab/isaaclab.app.rst | 3 -- .../launch-helper-cleanup.major.rst | 6 --- source/isaaclab/isaaclab/app/__init__.pyi | 2 - source/isaaclab/isaaclab/app/argv.py | 39 ------------------- source/isaaclab/isaaclab/app/sim_launcher.py | 28 +++++++++++++ .../isaaclab/isaaclab/benchmark/dispatch.py | 17 +++++--- .../isaaclab/isaaclab/cli/commands/deploy.py | 2 +- .../isaaclab_physx/app/kit_launcher.py | 3 +- .../test/app/test_kit_launcher.py | 3 +- .../changelog.d/launch-helper-cleanup.skip | 2 +- .../isaaclab_rl/entrypoints/dispatch.py | 5 ++- 11 files changed, 46 insertions(+), 64 deletions(-) delete mode 100644 source/isaaclab/isaaclab/app/argv.py diff --git a/docs/source/api/lab/isaaclab.app.rst b/docs/source/api/lab/isaaclab.app.rst index 46e34a320dff..5dc613433c63 100644 --- a/docs/source/api/lab/isaaclab.app.rst +++ b/docs/source/api/lab/isaaclab.app.rst @@ -17,7 +17,6 @@ .. autosummary:: add_launcher_args - fuse_kit_args launch_simulation report_activity scan @@ -36,8 +35,6 @@ Simulation Launcher .. autoclass:: AppLauncher :members: -.. autofunction:: fuse_kit_args - .. autofunction:: scan .. autoclass:: Scan diff --git a/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst b/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst index 2a5390ed44f5..7b77ecef974d 100644 --- a/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst +++ b/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst @@ -1,9 +1,3 @@ -Added -^^^^^ - -* Added :func:`~isaaclab.app.fuse_kit_args` to the :mod:`isaaclab.app` exports. It moved to the lightweight - ``isaaclab.app.argv`` module; import it from :mod:`isaaclab.app` instead of ``isaaclab.app.sim_launcher``. - Removed ^^^^^^^ diff --git a/source/isaaclab/isaaclab/app/__init__.pyi b/source/isaaclab/isaaclab/app/__init__.pyi index ab9d8cc30013..59e1da56024e 100644 --- a/source/isaaclab/isaaclab/app/__init__.pyi +++ b/source/isaaclab/isaaclab/app/__init__.pyi @@ -9,7 +9,6 @@ __all__ = [ "SettingsManager", "get_settings_manager", "add_launcher_args", - "fuse_kit_args", "launch_simulation", "scan", "Scan", @@ -18,7 +17,6 @@ __all__ = [ ] from .app_launcher import AppLauncher -from .argv import fuse_kit_args from .loading_screen import LoadingScreen, report_activity from .settings_manager import SettingsManager, get_settings_manager from .sim_launcher import ( diff --git a/source/isaaclab/isaaclab/app/argv.py b/source/isaaclab/isaaclab/app/argv.py deleted file mode 100644 index efbc422d0e86..000000000000 --- a/source/isaaclab/isaaclab/app/argv.py +++ /dev/null @@ -1,39 +0,0 @@ -# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). -# All rights reserved. -# -# SPDX-License-Identifier: BSD-3-Clause - -"""Command-line token helpers for the launcher arguments. - -This module imports nothing heavy, so CLI dispatchers can use it before choosing a backend. -""" - -from __future__ import annotations - - -def fuse_kit_args(argv: list[str]) -> list[str]: - """Fuse ``["--kit_args", ""]`` pairs into single ``--kit_args=`` tokens. - - Argparse rejects a value token that itself looks like an option (starts with ``-`` and contains - no space) with "expected one argument", and Kit arguments always start with ``--``. Fusing the - pair into the ``=``-attached form before parsing makes the documented space-separated form work - for a single Kit argument. All other forms pass through unchanged. - - Args: - argv: Command-line tokens, excluding the program name. - - Returns: - Tokens with any affected pair replaced by one fused token. - """ - fused: list[str] = [] - index = 0 - while index < len(argv): - token = argv[index] - next_token = argv[index + 1] if index + 1 < len(argv) else None - if token == "--kit_args" and next_token is not None and next_token.startswith("-") and " " not in next_token: - fused.append(f"--kit_args={next_token}") - index += 2 - else: - fused.append(token) - index += 1 - return fused diff --git a/source/isaaclab/isaaclab/app/sim_launcher.py b/source/isaaclab/isaaclab/app/sim_launcher.py index 2ee9c3e5d9d6..119ca54b833f 100644 --- a/source/isaaclab/isaaclab/app/sim_launcher.py +++ b/source/isaaclab/isaaclab/app/sim_launcher.py @@ -80,6 +80,34 @@ def add_launcher_args(parser: argparse.ArgumentParser) -> None: string_to_callable(_KIT_LAUNCHER).add_launcher_args(parser) +def fuse_kit_args(argv: list[str]) -> list[str]: + """Fuse ``["--kit_args", ""]`` pairs into single ``--kit_args=`` tokens. + + Argparse rejects a value token that itself looks like an option (starts with ``-`` and contains + no space) with "expected one argument", and Kit arguments always start with ``--``. Fusing the + pair into the ``=``-attached form before parsing makes the documented space-separated form work + for a single Kit argument. All other forms pass through unchanged. + + Args: + argv: Command-line tokens, excluding the program name. + + Returns: + Tokens with any affected pair replaced by one fused token. + """ + fused: list[str] = [] + index = 0 + while index < len(argv): + token = argv[index] + next_token = argv[index + 1] if index + 1 < len(argv) else None + if token == "--kit_args" and next_token is not None and next_token.startswith("-") and " " not in next_token: + fused.append(f"--kit_args={next_token}") + index += 2 + else: + fused.append(token) + index += 1 + return fused + + def _make_physics_cfg(physics_cfg_str: str) -> PhysicsCfg: """Build a physics config for the requested backend. diff --git a/source/isaaclab/isaaclab/benchmark/dispatch.py b/source/isaaclab/isaaclab/benchmark/dispatch.py index 2f677dc7cae2..4b24c02c3d01 100644 --- a/source/isaaclab/isaaclab/benchmark/dispatch.py +++ b/source/isaaclab/isaaclab/benchmark/dispatch.py @@ -12,8 +12,6 @@ import sys from typing import TYPE_CHECKING, Any -from ..app.argv import fuse_kit_args - if TYPE_CHECKING: from .api import BenchmarkLauncherConfig, BenchmarkRequest, BenchmarkResult @@ -79,7 +77,7 @@ def run_benchmark_cli(argv: list[str] | None = None) -> int: if argv is None: argv = sys.argv[1:] - argv = fuse_kit_args(argv) + argv = _fuse_kit_args(argv) multigpu_workflows = tuple(f"{name}{multigpu.MULTIGPU_SUFFIX}" for name in multigpu.MULTIGPU_WORKFLOWS) legacy_multigpu_workflows = tuple( f"{name}{multigpu.LEGACY_MULTIGPU_SUFFIX}" for name in multigpu.MULTIGPU_WORKFLOWS @@ -122,7 +120,7 @@ def run_play_cli(argv: list[str] | None = None) -> int: def _run_rl_cli(workflow: str, argv: list[str] | None) -> int: if argv is None: argv = sys.argv[1:] - argv = fuse_kit_args(argv) + argv = _fuse_kit_args(argv) backends = _RL_WORKFLOW_MODULES[workflow] parser = argparse.ArgumentParser(add_help=False) parser.add_argument("--rl_library", choices=sorted(backends)) @@ -201,7 +199,7 @@ def _request_argv(request: BenchmarkRequest) -> list[str]: if request.presets: argv.append(f"presets={','.join(request.presets)}") argv.extend(request.hydra_args) - return fuse_kit_args(argv) + return _fuse_kit_args(argv) def _output_argv(output: Any) -> list[str]: @@ -232,9 +230,16 @@ def _launcher_argv(launcher: BenchmarkLauncherConfig) -> list[str]: argv.append("--verbose") if launcher.info: argv.append("--info") - return fuse_kit_args(argv) + return _fuse_kit_args(argv) def _append_value(argv: list[str], option: str, value: object | None) -> None: if value is not None: argv.extend((option, str(value))) + + +def _fuse_kit_args(argv: list[str]) -> list[str]: + # imported here so that importing this module stays lightweight + from ..app.sim_launcher import fuse_kit_args + + return fuse_kit_args(argv) diff --git a/source/isaaclab/isaaclab/cli/commands/deploy.py b/source/isaaclab/isaaclab/cli/commands/deploy.py index 6202aa7ab0bc..e2d44f59d23b 100644 --- a/source/isaaclab/isaaclab/cli/commands/deploy.py +++ b/source/isaaclab/isaaclab/cli/commands/deploy.py @@ -17,7 +17,7 @@ from isaaclab_tasks.utils.hydra import resolve_task_config from ...app import add_launcher_args, launch_simulation -from ...app.argv import fuse_kit_args +from ...app.sim_launcher import fuse_kit_args def command_deploy_leapp(argv: list[str] | None = None) -> int: diff --git a/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py b/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py index c718358785cc..8cd658997613 100644 --- a/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py +++ b/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py @@ -31,11 +31,10 @@ SimulationApp = getattr(isaacsim, "SimulationApp", None) -from isaaclab.app.argv import fuse_kit_args from isaaclab.app.loading_screen import report_activity from isaaclab.app.logging_utils import apply_python_logging_level from isaaclab.app.settings_manager import get_settings_manager -from isaaclab.app.sim_launcher import SimulationLauncher, _parse_visualizer_csv +from isaaclab.app.sim_launcher import SimulationLauncher, _parse_visualizer_csv, fuse_kit_args from isaaclab.paths import ISAACLAB_ROOT from isaaclab.utils.device import set_cuda_device from isaaclab.utils.renderers import ISAAC_RTX_SHOW_ALL_PARTITIONS_BY_DEFAULT_SETTING diff --git a/source/isaaclab_physx/test/app/test_kit_launcher.py b/source/isaaclab_physx/test/app/test_kit_launcher.py index 18d92a95fc7b..691b26352d0f 100644 --- a/source/isaaclab_physx/test/app/test_kit_launcher.py +++ b/source/isaaclab_physx/test/app/test_kit_launcher.py @@ -18,7 +18,6 @@ import isaaclab.app.sim_launcher as sim_launcher import isaaclab.utils as utils_module from isaaclab.app import SimulationLauncher, add_launcher_args -from isaaclab.app.argv import fuse_kit_args from isaaclab.app.sim_launcher import Scan, _get_kit_runtime_sources, _normalize_launcher_args from isaaclab.utils.renderers import ISAAC_RTX_SHOW_ALL_PARTITIONS_BY_DEFAULT_SETTING @@ -200,7 +199,7 @@ def test_explicit_spectator_setting_overrides_visualizer_default(monkeypatch): ) def test_fuse_kit_args(argv: list[str], expected: list[str]): """Fuse only ``--kit_args`` pairs whose value argparse would mistake for an option.""" - assert fuse_kit_args(argv) == expected + assert sim_launcher.fuse_kit_args(argv) == expected def test_add_launcher_args_registers_every_launcher_option(): diff --git a/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip b/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip index fde46a0c6757..cee64c9ed00f 100644 --- a/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip +++ b/source/isaaclab_rl/changelog.d/launch-helper-cleanup.skip @@ -1 +1 @@ -Imported fuse_kit_args from isaaclab.app.argv and switched tests to SettingsManager.set; no behavior change. +Switched a test to SettingsManager.set; no behavior change. diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py index 50ad31cae7b9..36d882f66731 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/dispatch.py @@ -14,8 +14,6 @@ import gymnasium as gym -from isaaclab.app.argv import fuse_kit_args - if TYPE_CHECKING: from .simple_agents import PolicyName @@ -132,6 +130,9 @@ def _normalize_argv(argv: list[str] | None) -> list[str]: The backends parse this explicit list rather than ``sys.argv``, so the fusing that :func:`~isaaclab.app.add_launcher_args` applies to ``sys.argv`` never reaches it. """ + # imported here so that importing this module stays lightweight + from isaaclab.app.sim_launcher import fuse_kit_args + if argv is None: argv = sys.argv[1:] return fuse_kit_args(argv) From 012d277d4733317a106918df0964be4f98b84c2f Mon Sep 17 00:00:00 2001 From: Octi Zhang Date: Mon, 28 Sep 2026 16:51:27 -0700 Subject: [PATCH 3/3] Address review: plain attribute access and one visualizer signal - _is_kit_camera and scan read typed attributes behind isinstance checks instead of probing with getattr; launcher_type is declared on the base physics and renderer configs. - scan collects visualizer configs during its tree walk, so the visualizer intent no longer digs through env and sim config shapes. - Inline the launcher-args conversion at its two call sites. - --viz none parses to an empty list, so "passed" is visualizer is not None and "disable all" is visualizer == []; the --viz explicit-flag action and the visualizer_explicit/visualizer_disable_all keys are gone. --device keeps a small action because its default must stay a string and XR needs to know whether it was passed. - Add unit tests for the toolbar button helper. --- .../launch-helper-cleanup.major.rst | 15 ++++ source/isaaclab/isaaclab/app/sim_launcher.py | 90 +++++++++---------- .../isaaclab/physics/physics_manager_cfg.py | 5 +- .../isaaclab/renderers/renderer_cfg.py | 5 +- .../app/test_launch_simulation_require_kit.py | 5 +- .../isaaclab_physx/app/kit_launcher.py | 17 ++-- .../test/app/test_kit_launcher.py | 83 ++++++++++++----- .../isaaclab_rl/entrypoints/common.py | 5 +- .../test/test_apply_video_recording.py | 2 +- source/isaaclab_rl/test/test_wrappers.py | 2 +- .../test/core/test_runtime_compatibility.py | 8 +- .../test_sim_launcher_visualizer_intent.py | 22 +++-- 12 files changed, 151 insertions(+), 108 deletions(-) diff --git a/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst b/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst index 7b77ecef974d..4850251d84b8 100644 --- a/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst +++ b/source/isaaclab/changelog.d/launch-helper-cleanup.major.rst @@ -1,3 +1,18 @@ +Changed +^^^^^^^ + +* **Breaking:** Changed ``--viz none`` to parse to an empty list instead of ``None``, so ``args_cli.visualizer is None`` + means no ``--viz`` was passed and ``[]`` means all visualizers are disabled. The ``visualizer_explicit`` and + ``visualizer_disable_all`` launcher arguments are removed; check ``args_cli.visualizer`` instead. +* Changed :func:`~isaaclab.app.launch_simulation` to also detect a ``newton_rtx`` visualizer listed in + ``visualizer_cfgs``, so the OVRTX runtime starts and the Kit conflict check applies. + +Added +^^^^^ + +* Added ``launcher_type`` to :class:`~isaaclab.physics.PhysicsCfg` and :class:`~isaaclab.renderers.RendererCfg`, + defaulting to ``None`` for configs whose runtime needs no launcher. + Removed ^^^^^^^ diff --git a/source/isaaclab/isaaclab/app/sim_launcher.py b/source/isaaclab/isaaclab/app/sim_launcher.py index 119ca54b833f..be0b47448e4a 100644 --- a/source/isaaclab/isaaclab/app/sim_launcher.py +++ b/source/isaaclab/isaaclab/app/sim_launcher.py @@ -33,6 +33,7 @@ from ..sensors.camera.camera_cfg import CameraCfg from ..utils.device import set_cuda_device from ..utils.string import string_to_callable +from ..visualizers.visualizer_cfg import VisualizerCfg from .logging_utils import apply_python_logging_level from .settings_manager import get_settings_manager @@ -148,17 +149,17 @@ def _is_kit_camera(node) -> bool: """True for a CameraCfg whose renderer requires Kit (not Newton).""" if not isinstance(node, CameraCfg): return False - renderer_cfg = getattr(node, "renderer_cfg", None) + renderer_cfg = node.renderer_cfg if renderer_cfg is None: return True - if getattr(renderer_cfg, "renderer_type", None) == "auto_rtx": - # ``auto_rtx`` is resolved after the initial scan once physics and - # visualizer intent are known; ie. it may become OVRTX for a kitless run. - return False if not isinstance(renderer_cfg, RendererCfg): raise TypeError( f"CameraCfg.renderer_cfg must be a concrete RendererCfg or None, got {type(renderer_cfg).__name__}." ) + if renderer_cfg.renderer_type == "auto_rtx": + # ``auto_rtx`` is resolved after the initial scan once physics and + # visualizer intent are known; ie. it may become OVRTX for a kitless run. + return False return renderer_cfg.renderer_type in ("default", "isaac_rtx") @@ -167,15 +168,8 @@ def _is_kit_camera(node) -> bool: """ -def _as_dict(launcher_args: argparse.Namespace | dict | None) -> dict: - """Return launcher args as a dict; writes to it reach the caller's namespace or dict.""" - if isinstance(launcher_args, argparse.Namespace): - return vars(launcher_args) - return {} if launcher_args is None else launcher_args - - -def _parse_visualizer_csv(value: str) -> list[str] | None: - """Parse the ``--visualizer`` comma-separated list into canonical names; ``none`` yields None.""" +def _parse_visualizer_csv(value: str) -> list[str]: + """Parse the ``--visualizer`` comma-separated list into canonical names; ``none`` yields an empty list.""" token = (value or "").strip() if not token: raise argparse.ArgumentTypeError( @@ -211,7 +205,7 @@ def _parse_visualizer_csv(value: str) -> list[str] | None: raise argparse.ArgumentTypeError( "Invalid --visualizer value: 'none' cannot be combined with other visualizer types." ) - return None + return [] return list(dict.fromkeys(names)) @@ -219,8 +213,8 @@ def _normalize_launcher_args(args: dict) -> None: """Resolve the livestream mode and the visualizer selection in place, once for every consumer. Writes ``livestream`` (the effective mode: ``--livestream`` when set (>= 0), else the ``LIVESTREAM`` - environment variable), ``visualizer`` (canonical names, or None when all are disabled), - ``visualizer_explicit`` and ``visualizer_disable_all``. Livestreaming adds the Kit visualizer, whose + environment variable) and ``visualizer``: canonical names, an empty list when ``--viz none`` disabled + all visualizers, or None when no visualizer was requested. Livestreaming adds the Kit visualizer, whose viewport produces the stream. Normalizing twice gives the same result. """ livestream = args.get("livestream", -1) @@ -230,7 +224,6 @@ def _normalize_launcher_args(args: dict) -> None: if livestream not in (0, 1, 2): raise ValueError(f"Invalid livestream mode: {livestream}. Expected 0 (disabled), 1, or 2.") visualizers = args.get("visualizer") - explicit = bool(args.get("visualizer_explicit", False)) or visualizers is not None if visualizers and not isinstance(visualizers, str): visualizers = ",".join(str(visualizer).strip() for visualizer in visualizers) if visualizers: @@ -238,17 +231,13 @@ def _normalize_launcher_args(args: dict) -> None: visualizers = _parse_visualizer_csv(visualizers) except argparse.ArgumentTypeError as error: raise ValueError(str(error)) from error - disable_all = explicit and visualizers is None if livestream > 0: - if disable_all: + if visualizers == []: raise ValueError("Livestreaming requires the Kit visualizer. Remove '--viz none' or pass '--viz kit'.") if "kit" not in (visualizers or []): visualizers = [*(visualizers or []), "kit"] - explicit = True args["livestream"] = livestream args["visualizer"] = visualizers - args["visualizer_explicit"] = explicit - args["visualizer_disable_all"] = disable_all def _sync_visualizer_cli_settings(args: dict) -> None: @@ -256,10 +245,11 @@ def _sync_visualizer_cli_settings(args: dict) -> None: max_visible_envs = args.get("max_visible_envs") if max_visible_envs is not None and int(max_visible_envs) < 0: raise ValueError(f"Invalid value for --max_visible_envs: {max_visible_envs}. Expected non-negative int.") + visualizers = args.get("visualizer") settings = get_settings_manager() - settings.set("/isaaclab/visualizer/types", " ".join(args.get("visualizer") or ())) - settings.set("/isaaclab/visualizer/explicit", bool(args.get("visualizer_explicit", False))) - settings.set("/isaaclab/visualizer/disable_all", bool(args.get("visualizer_disable_all", False))) + settings.set("/isaaclab/visualizer/types", " ".join(visualizers or ())) + settings.set("/isaaclab/visualizer/explicit", visualizers is not None) + settings.set("/isaaclab/visualizer/disable_all", visualizers == []) # Sentinel: ``-1`` means ``--max_visible_envs`` was not passed (see ``SimulationContext``). settings.set("/isaaclab/visualizer/max_visible_envs", -1 if max_visible_envs is None else int(max_visible_envs)) @@ -274,28 +264,21 @@ def _resolve_python_logging_level(args: dict) -> int: return logging.WARNING if level == logging.NOTSET else level -def _get_visualizer_intent(cfg, args: dict) -> dict[str, bool]: - """Compute the visualizer intent of ``cfg.sim.visualizer_cfgs``, OR-ed with a caller's ``visualizer_intent``. +def _get_visualizer_intent(visualizer_cfgs: list[VisualizerCfg], args: dict) -> dict[str, bool]: + """Compute the intent of the config's visualizers, OR-ed with a caller's ``visualizer_intent``. An explicit ``--visualizer`` selection overrides whether the config's Kit visualizer is used. """ - # Accept both env_cfg (has .sim.visualizer_cfgs) and a bare SimulationCfg - # (has .visualizer_cfgs directly). - sim = getattr(cfg, "sim", None) - visualizer_cfgs = getattr(sim, "visualizer_cfgs", None) or getattr(cfg, "visualizer_cfgs", None) - if visualizer_cfgs is None: - visualizer_cfgs = [] - cfgs = visualizer_cfgs if isinstance(visualizer_cfgs, list) else [visualizer_cfgs] - kit_cfgs = [c for c in cfgs if getattr(c, "visualizer_type", None) == "kit"] + kit_cfgs = [cfg for cfg in visualizer_cfgs if cfg.visualizer_type == "kit"] caller_intent = args.get("visualizer_intent") or {} - if args.get("visualizer_explicit", False): - has_kit_visualizer = "kit" in (args.get("visualizer") or ()) + if args["visualizer"] is not None: + has_kit_visualizer = "kit" in args["visualizer"] else: has_kit_visualizer = bool(kit_cfgs) or bool(caller_intent.get("has_kit_visualizer")) + has_kit_streaming_view = any(cfg.streaming_view for cfg in kit_cfgs) return { "has_kit_visualizer": has_kit_visualizer, - "has_kit_streaming_view": any(bool(getattr(c, "streaming_view", False)) for c in kit_cfgs) - or bool(caller_intent.get("has_kit_streaming_view")), + "has_kit_streaming_view": has_kit_streaming_view or bool(caller_intent.get("has_kit_streaming_view")), } @@ -350,7 +333,8 @@ def scan(cfg, launcher_args: argparse.Namespace | dict | None = None) -> Scan: a second walk of the same config observes the same signals and reaches the same launch decision. """ - args = _as_dict(launcher_args) + args = vars(launcher_args) if isinstance(launcher_args, argparse.Namespace) else launcher_args + args = {} if args is None else args # Livestreaming implies a Kit visualizer; make that visible to auto RTX resolution. _normalize_launcher_args(args) @@ -365,15 +349,16 @@ def scan(cfg, launcher_args: argparse.Namespace | dict | None = None) -> Scan: auto_rtx_locations: list[tuple[Any, Any, bool]] = [] # (parent, key, is_cam_renderer) for each auto RTX placeholder auto_physx_locations: list[tuple[PhysicsCfg, Any, Any, bool]] = [] # (node, parent, key, is_first_physics) launcher_types: list[str] = [] + visualizer_cfgs: list[VisualizerCfg] = [] visited: set[int] = set() - def add_launcher_type(node): - if getattr(node, "launcher_type", None): + def add_launcher_type(node: PhysicsCfg | RendererCfg): + if node.launcher_type: launcher_types.append(node.launcher_type) def visit(node, parent, key): nonlocal effective_cfg, has_ovrtx, has_auto_rtx, has_auto_physx, has_kit_camera - if getattr(node, "renderer_type", None) == "auto_rtx": + if isinstance(node, RendererCfg) and node.renderer_type == "auto_rtx": has_auto_rtx = True auto_rtx_locations.append((parent, key, isinstance(parent, CameraCfg) and key == "renderer_cfg")) @@ -395,9 +380,9 @@ def visit(node, parent, key): return else: concrete_physics_cfgs.append(node) - elif getattr(node, "visualizer_type", None) == "newton_rtx" or ( - isinstance(node, RendererCfg) and node.renderer_type == "ovrtx" - ): + elif isinstance(node, VisualizerCfg): + visualizer_cfgs.append(node) + elif isinstance(node, RendererCfg) and node.renderer_type == "ovrtx": has_ovrtx = True elif _is_kit_camera(node): has_kit_camera = True @@ -411,15 +396,20 @@ def visit(node, parent, key): for name, child in children.items(): if child is None or isinstance(child, (int, float, str, bool)): continue + if isinstance(child, (list, tuple)): + # sequences are not walked, except to collect visualizers, e.g. ``SimulationCfg.visualizer_cfgs`` + visualizer_cfgs.extend(item for item in child if isinstance(item, VisualizerCfg)) + continue visit(child, node, name) visit(cfg, None, None) + has_ovrtx = has_ovrtx or any(visualizer_cfg.visualizer_type == "newton_rtx" for visualizer_cfg in visualizer_cfgs) has_physics = bool(physics_cfgs) config_scan = Scan( resolved_physics_cfg=physics_cfgs[0] if physics_cfgs else None, effective_cfg=effective_cfg, - visualizer_intent=_get_visualizer_intent(cfg, args), + visualizer_intent=_get_visualizer_intent(visualizer_cfgs, args), has_ovrtx=has_ovrtx, has_kit_camera=has_kit_camera, has_kit_physics=False, @@ -601,7 +591,9 @@ def launch_simulation( * ``visualizer_intent``: Visualizer intent the config cannot express, e.g. ``{"has_kit_visualizer": True}``; it is combined with the intent of *cfg*. """ - args = _as_dict(launcher_args) + # writes to ``args`` reach the caller's namespace or dict + args = vars(launcher_args) if isinstance(launcher_args, argparse.Namespace) else launcher_args + args = {} if args is None else args # The single walk: collect every signal, apply the --physics override, and # resolve the automatic PhysX and RTX placeholders. diff --git a/source/isaaclab/isaaclab/physics/physics_manager_cfg.py b/source/isaaclab/isaaclab/physics/physics_manager_cfg.py index 60249f14cda3..b259d9511b2b 100644 --- a/source/isaaclab/isaaclab/physics/physics_manager_cfg.py +++ b/source/isaaclab/isaaclab/physics/physics_manager_cfg.py @@ -8,7 +8,7 @@ from __future__ import annotations from dataclasses import MISSING -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, ClassVar from ..utils import configclass @@ -34,6 +34,9 @@ class PhysicsCfg: class_type: type[PhysicsManager] | Any = MISSING """The physics manager class to use. Must be set by subclasses.""" + launcher_type: ClassVar[str | None] = None + """The launcher that starts the runtime this backend runs in, as ``"module:Class"``, or None if none is needed.""" + deterministic: bool = False """Whether to request reproducible physics from the backend. Defaults to False. diff --git a/source/isaaclab/isaaclab/renderers/renderer_cfg.py b/source/isaaclab/isaaclab/renderers/renderer_cfg.py index 2ba9d495018d..8ed1d9f73204 100644 --- a/source/isaaclab/isaaclab/renderers/renderer_cfg.py +++ b/source/isaaclab/isaaclab/renderers/renderer_cfg.py @@ -7,7 +7,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, ClassVar from ..sim.simulation_cfg import BackendCfg from ..utils import configclass @@ -26,6 +26,9 @@ class RendererCfg(BackendCfg): renderer_type: str = "default" + launcher_type: ClassVar[str | None] = None + """The launcher that starts the runtime this renderer needs, as ``"module:Class"``, or None if none is needed.""" + cloning_contexts: tuple[type | str, ...] = () """Clone contexts that build this renderer's scene representation from the asset plan.""" diff --git a/source/isaaclab/test/app/test_launch_simulation_require_kit.py b/source/isaaclab/test/app/test_launch_simulation_require_kit.py index fa88ee8b9f12..ef7ce8503801 100644 --- a/source/isaaclab/test/app/test_launch_simulation_require_kit.py +++ b/source/isaaclab/test/app/test_launch_simulation_require_kit.py @@ -26,6 +26,7 @@ from isaaclab.app import SimulationLauncher, launch_simulation from isaaclab.physics import PhysicsCfg from isaaclab.renderers import RendererCfg +from isaaclab.visualizers import VisualizerCfg @pytest.fixture @@ -135,7 +136,7 @@ def test_kitless_ovrtx_registers_before_user_code(monkeypatch: pytest.MonkeyPatc monkeypatch.setattr(assets_utils, "configure_storage_profile", lambda: calls.append("storage")) cfg = argparse.Namespace( physics=sim_launcher.NewtonCfg(), - visualizer_cfgs=argparse.Namespace(visualizer_type="newton_rtx"), + visualizer_cfgs=VisualizerCfg(visualizer_type="newton_rtx"), ) with launch_simulation(cfg): @@ -169,7 +170,7 @@ class CustomRendererCfg(RendererCfg): def test_require_kit_false_does_not_suppress_a_kit_config(kit_branch_taken): # '--viz kit' requires Kit on its own; require_kit=False must not override that - launcher_args = {"visualizer": ["kit"], "visualizer_explicit": True, "require_kit": False} + launcher_args = {"visualizer": ["kit"], "require_kit": False} with launch_simulation(cfg=PhysicsCfg(), launcher_args=launcher_args): pass diff --git a/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py b/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py index 8cd658997613..8f697f964a96 100644 --- a/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py +++ b/source/isaaclab_physx/isaaclab_physx/app/kit_launcher.py @@ -76,13 +76,15 @@ def _sanitize_sys_argv_for_kit(argv: list[str]) -> list[str]: return [argument for index, argument in enumerate(argv) if index not in indexes_to_remove] -class _ExplicitAction(argparse.Action): - """Custom action to track if an argument was explicitly passed by the user.""" +class _StoreAndMarkExplicit(argparse.Action): + """Store the value and set ``_explicit``, for options whose default must stay a real value. + + ``--device`` defaults to ``"cuda:0"`` because scripts read ``args_cli.device`` before launch, yet XR + needs to know whether it was passed (see :meth:`KitLauncher._resolve_device_settings`). + """ def __call__(self, parser, namespace, values, option_string=None): - # Set the parameter value setattr(namespace, self.dest, values) - # Set a flag indicating the parameter was explicitly passed setattr(namespace, f"{self.dest}_explicit", True) @@ -352,7 +354,7 @@ def add_launcher_args(parser: argparse.ArgumentParser) -> None: arg_group.add_argument( "--device", type=str, - action=_ExplicitAction, + action=_StoreAndMarkExplicit, default="cuda:0", help='The device to run the simulation on. Can be "cpu", "cuda", "cuda:N", where N is the device ID', ) @@ -360,7 +362,6 @@ def add_launcher_args(parser: argparse.ArgumentParser) -> None: "--visualizer", "--viz", type=_parse_visualizer_csv, - action=_ExplicitAction, default=None, help="Visualizer backends to enable as CSV (e.g., kit,newton,rerun,viser).", ) @@ -529,9 +530,7 @@ def _resolve_xr_settings(self, launcher_args: dict): # CLI, we auto-inject one so that app.update() and forward() are pumped # each frame -- the XR runtime needs both to receive updated hand/joint # transforms. - self._xr_auto_start = self._xr and not ( - launcher_args.get("visualizer_explicit", False) and "kit" in (launcher_args.get("visualizer") or ()) - ) + self._xr_auto_start = self._xr and "kit" not in (launcher_args.get("visualizer") or ()) def _resolve_viewport_settings(self, launcher_args: dict): """Resolve viewport related settings.""" diff --git a/source/isaaclab_physx/test/app/test_kit_launcher.py b/source/isaaclab_physx/test/app/test_kit_launcher.py index 691b26352d0f..b6381aaa45d4 100644 --- a/source/isaaclab_physx/test/app/test_kit_launcher.py +++ b/source/isaaclab_physx/test/app/test_kit_launcher.py @@ -20,6 +20,7 @@ from isaaclab.app import SimulationLauncher, add_launcher_args from isaaclab.app.sim_launcher import Scan, _get_kit_runtime_sources, _normalize_launcher_args from isaaclab.utils.renderers import ISAAC_RTX_SHOW_ALL_PARTITIONS_BY_DEFAULT_SETTING +from isaaclab.visualizers import VisualizerCfg def _launcher_after_output_resolution(**state) -> KitLauncher: @@ -220,7 +221,6 @@ def test_visualizer_alias_parsing(): with pytest.warns(DeprecationWarning, match="--viz 'newton' is deprecated"): args = parser.parse_args(["--viz", "kit,newton"]) assert args.visualizer == ["kit", "newton_gl"] - assert args.visualizer_explicit is True @pytest.mark.parametrize("deprecated_arg", ["--headless", "--enable_cameras", "--cpu"]) @@ -263,8 +263,7 @@ def test_visualizer_none_parsing(value: str): parser = argparse.ArgumentParser() add_launcher_args(parser) args = parser.parse_args(["--viz", value]) - assert args.visualizer is None - assert args.visualizer_explicit is True + assert args.visualizer == [] @pytest.mark.parametrize( @@ -382,16 +381,15 @@ def test_make_physics_cfg_builds_core_vbd(): def test_livestream_injects_kit_visualizer_when_missing(): - args = argparse.Namespace(livestream=2, visualizer=None, visualizer_explicit=False) + args = argparse.Namespace(livestream=2, visualizer=None) _normalize_launcher_args(vars(args)) assert args.visualizer == ["kit"] - assert args.visualizer_explicit is True def test_livestream_rejects_disabled_visualizers(): - args = argparse.Namespace(livestream=2, visualizer=None, visualizer_explicit=True) + args = argparse.Namespace(livestream=2, visualizer=[]) with pytest.raises(ValueError, match="Livestreaming requires the Kit visualizer"): _normalize_launcher_args(vars(args)) @@ -432,8 +430,9 @@ def _resolve_headless_for_case( monkeypatch.delenv("XR", raising=False) monkeypatch.delenv("LIVESTREAM", raising=False) _normalize_launcher_args(launcher_args) - cfg = SimpleNamespace(visualizer_cfgs=[SimpleNamespace(visualizer_type="kit")] if cfg_has_kit else []) - launcher_args["kit_visualizer"] = sim_launcher._get_visualizer_intent(cfg, launcher_args)["has_kit_visualizer"] + visualizer_cfgs = [VisualizerCfg(visualizer_type="kit")] if cfg_has_kit else [] + intent = sim_launcher._get_visualizer_intent(visualizer_cfgs, launcher_args) + launcher_args["kit_visualizer"] = intent["has_kit_visualizer"] launcher = KitLauncher.__new__(KitLauncher) launcher._kit_visualizer = launcher_args["kit_visualizer"] launcher._livestream = launcher_args["livestream"] @@ -442,7 +441,7 @@ def _resolve_headless_for_case( return launcher._headless, launcher_args -_XR_KIT = {"xr": True, "visualizer": ["kit"], "visualizer_explicit": True} +_XR_KIT = {"xr": True, "visualizer": ["kit"]} @pytest.mark.parametrize( @@ -452,7 +451,7 @@ def _resolve_headless_for_case( ({"xr": True}, 0, 0, True), # An explicit '--viz kit' is the only way to get a viewport alongside XR. (_XR_KIT, 0, 0, False), - ({"xr": True, "visualizer": ["none"], "visualizer_explicit": True}, 0, 0, True), + ({"xr": True, "visualizer": ["none"]}, 0, 0, True), # ...but an explicit '--viz kit' cannot override HEADLESS=1. (_XR_KIT, 1, 0, True), # Livestreaming forces headless. @@ -666,6 +665,49 @@ def test_load_extensions_publishes_has_gui_setting( assert settings.values["/isaaclab/xr/auto_start"] is expected_xr_auto_start +@pytest.mark.parametrize("visible", [True, False]) +def test_toolbar_play_button_toggles_and_stop_button_is_detached(monkeypatch: pytest.MonkeyPatch, visible: bool): + """The play button follows *visible*; hiding the stop button also removes it from its group for good.""" + play_button = SimpleNamespace(visible=None, enabled=None) + stop_button = SimpleNamespace(visible=None, enabled=None) + group = SimpleNamespace(_play_button=play_button, _stop_button=stop_button) + toolbar = SimpleNamespace( + get_instance=lambda: SimpleNamespace(_builtin_tools=SimpleNamespace(_play_button_group=group)) + ) + # ``import omni.kit.widget.toolbar`` binds the top-level fake and reaches the toolbar through its attributes + widget = SimpleNamespace(toolbar=toolbar) + kit = SimpleNamespace(widget=widget) + omni = SimpleNamespace(kit=kit) + for name, module in { + "omni": omni, + "omni.kit": kit, + "omni.kit.widget": widget, + "omni.kit.widget.toolbar": toolbar, + }.items(): + monkeypatch.setitem(sys.modules, name, module) + launcher = KitLauncher.__new__(KitLauncher) + launcher._headless = False + launcher._livestream = 0 + + launcher._set_toolbar_button_visible("_play_button", visible) + launcher._set_toolbar_button_visible("_stop_button", False) + + assert (play_button.visible, play_button.enabled) == (visible, visible) + assert (stop_button.visible, stop_button.enabled) == (False, False) + assert group._play_button is play_button + assert group._stop_button is None + + +def test_toolbar_is_untouched_when_headless_without_livestream(monkeypatch: pytest.MonkeyPatch): + """A truly headless app has no toolbar widget, so the toolbar module is never imported.""" + monkeypatch.setitem(sys.modules, "omni.kit.widget.toolbar", None) # importing it would raise + launcher = KitLauncher.__new__(KitLauncher) + launcher._headless = True + launcher._livestream = 0 + + launcher._set_toolbar_button_visible("_stop_button", False) + + def test_sync_visualizer_settings_stores_values(monkeypatch: pytest.MonkeyPatch): settings = _DummySettings() monkeypatch.setattr(sim_launcher, "get_settings_manager", lambda: settings) @@ -674,7 +716,7 @@ def test_sync_visualizer_settings_stores_values(monkeypatch: pytest.MonkeyPatch) assert settings.values == { "/isaaclab/visualizer/types": "viser rerun", - "/isaaclab/visualizer/explicit": False, + "/isaaclab/visualizer/explicit": True, "/isaaclab/visualizer/disable_all": False, "/isaaclab/visualizer/max_visible_envs": 0, } @@ -699,7 +741,7 @@ def test_parse_visualizer_csv_rejects_spaces_between_entries(): def test_normalize_visualizers_rejects_none_with_others(): with pytest.raises(ValueError, match="'none' cannot be combined"): - _normalize_launcher_args({"visualizer": ["none", "kit"], "visualizer_explicit": True}) + _normalize_launcher_args({"visualizer": ["none", "kit"]}) def test_visualizer_csv_does_not_swallow_hydra_overrides(): @@ -715,17 +757,13 @@ def test_visualizer_csv_does_not_swallow_hydra_overrides(): def test_matrix_cli_kit_newton_gl_with_custom_kit_cfg_intent_non_headless(monkeypatch: pytest.MonkeyPatch): - headless, args = _resolve_headless_for_case( - monkeypatch, {"visualizer": ["kit", "newton_gl"], "visualizer_explicit": True}, cfg_has_kit=True - ) + headless, args = _resolve_headless_for_case(monkeypatch, {"visualizer": ["kit", "newton_gl"]}, cfg_has_kit=True) assert headless is False assert args["visualizer"] == ["kit", "newton_gl"] def test_matrix_cli_rerun_with_custom_kit_cfg_intent_headless(monkeypatch: pytest.MonkeyPatch): - headless, args = _resolve_headless_for_case( - monkeypatch, {"visualizer": ["rerun"], "visualizer_explicit": True}, cfg_has_kit=True - ) + headless, args = _resolve_headless_for_case(monkeypatch, {"visualizer": ["rerun"]}, cfg_has_kit=True) assert headless is True assert args["visualizer"] == ["rerun"] @@ -743,14 +781,11 @@ def test_matrix_viz_kit_dict_resolves_windowed(monkeypatch: pytest.MonkeyPatch): assert args["visualizer"] == ["kit"] -@pytest.mark.parametrize("visualizer", [None, ["none"]]) +@pytest.mark.parametrize("visualizer", [[], ["none"]]) def test_matrix_viz_none_disables_all_and_headless(monkeypatch: pytest.MonkeyPatch, visualizer): - headless, args = _resolve_headless_for_case( - monkeypatch, {"visualizer": visualizer, "visualizer_explicit": True}, cfg_has_kit=True - ) + headless, args = _resolve_headless_for_case(monkeypatch, {"visualizer": visualizer}, cfg_has_kit=True) assert headless is True - assert args["visualizer_disable_all"] is True - assert args["visualizer"] is None + assert args["visualizer"] == [] def test_matrix_headless_flag_deprecated_takes_precedence(monkeypatch: pytest.MonkeyPatch): diff --git a/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py b/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py index bb41c7a71618..8675c422e198 100644 --- a/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py +++ b/source/isaaclab_rl/isaaclab_rl/entrypoints/common.py @@ -1003,9 +1003,8 @@ def _resolve_video_source(env_cfg: Any, args_cli: argparse.Namespace) -> str: Raises: ValueError: If ``--viz none`` or only streaming visualizers were requested. """ - # _parse_visualizer_csv("none") yields None rather than ["none"], so an explicitly disabled - # visualizer is only recognizable through the _ExplicitAction sentinel - if getattr(args_cli, "visualizer_explicit", False) and getattr(args_cli, "visualizer", "not_none") is None: + # ``--viz none`` parses to an empty list + if getattr(args_cli, "visualizer", None) == []: raise ValueError( "--video is not compatible with --viz none: there is no active visualizer to record from. " "Remove --viz none so that video recording can auto-create a visualizer, " diff --git a/source/isaaclab_rl/test/test_apply_video_recording.py b/source/isaaclab_rl/test/test_apply_video_recording.py index 415b0122681f..10317dab8472 100644 --- a/source/isaaclab_rl/test/test_apply_video_recording.py +++ b/source/isaaclab_rl/test/test_apply_video_recording.py @@ -105,7 +105,7 @@ def test_apply_video_recording_patches_existing_recorders(): @pytest.mark.parametrize( ("visualizer_args", "message"), [ - (dict(visualizer=None, visualizer_explicit=True), "--video is not compatible with --viz none"), + (dict(visualizer=[]), "--video is not compatible with --viz none"), (dict(visualizer=["rerun"]), "--video is not supported"), (dict(visualizer=["viser"]), "--video is not supported"), ], diff --git a/source/isaaclab_rl/test/test_wrappers.py b/source/isaaclab_rl/test/test_wrappers.py index ae97b4c862c6..331875a9e621 100644 --- a/source/isaaclab_rl/test/test_wrappers.py +++ b/source/isaaclab_rl/test/test_wrappers.py @@ -60,7 +60,7 @@ def raw_env(task: str, library: str, finite_horizon: bool) -> Iterator[Any]: cfg.seed = 42 cfg.is_finite_horizon = finite_horizon cfg.compute_final_obs = library == "torchrl" - with launch_simulation(cfg, {"headless": True, "visualizer": None, "visualizer_explicit": True}): + with launch_simulation(cfg, {"headless": True, "visualizer": ["none"]}): env = gym.make(task, cfg=cfg) try: yield env diff --git a/source/isaaclab_tasks/test/core/test_runtime_compatibility.py b/source/isaaclab_tasks/test/core/test_runtime_compatibility.py index c5a83ac9e2d6..6a83c9f31526 100644 --- a/source/isaaclab_tasks/test/core/test_runtime_compatibility.py +++ b/source/isaaclab_tasks/test/core/test_runtime_compatibility.py @@ -25,6 +25,7 @@ from isaaclab.app import SimulationLauncher, scan from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, launch_simulation from isaaclab.physics import PhysxAutoCfg +from isaaclab.visualizers import VisualizerCfg import isaaclab_tasks # noqa: F401 from isaaclab_tasks.utils import resolve_task_config @@ -223,11 +224,8 @@ def test_physx_plus_isaacsim_rtx_is_valid(): def test_auto_physx_configured_kit_visualizer_resolves_to_isaac_sim_backends(): """Config-declared Kit visualizers should drive automatic PhysX and RTX resolution.""" - class KitVisualizerCfg: - visualizer_type = "kit" - env_cfg = _resolve_with_args("physics=physx", "renderer=rtx") - env_cfg.sim.visualizer_cfgs = KitVisualizerCfg() + env_cfg.sim.visualizer_cfgs = VisualizerCfg(visualizer_type="kit") config_scan = validate_runtime_compatibility(env_cfg) assert isinstance(env_cfg.sim.physics, PhysxCfg) @@ -343,7 +341,7 @@ def test_rtx_with_kit_visualizer_is_valid_and_resolves_to_isaac_rtx(): def test_livestream_rtx_injects_kit_before_auto_rtx_resolution(monkeypatch: pytest.MonkeyPatch): """Livestreaming should make ``presets=newton_mjwarp,rtx`` choose Isaac RTX.""" env_cfg = _resolve_with_presets("newton_mjwarp,rtx") - launcher_args = argparse.Namespace(livestream=2, visualizer=None, visualizer_explicit=False) + launcher_args = argparse.Namespace(livestream=2, visualizer=None) monkeypatch.setattr(physx_app, "KitLauncher", SimulationLauncher) with launch_simulation(env_cfg, launcher_args) as physics_cfg: diff --git a/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py b/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py index 69626e2d71ed..07cd249625c3 100644 --- a/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py +++ b/source/isaaclab_tasks/test/core/test_sim_launcher_visualizer_intent.py @@ -19,6 +19,7 @@ import isaaclab.app.sim_launcher as sim_launcher from isaaclab.app import SimulationLauncher, get_settings_manager +from isaaclab.visualizers import VisualizerCfg def _force_kitless(monkeypatch): @@ -33,11 +34,6 @@ def fake_scan(*args, **kwargs): monkeypatch.setattr(sim_launcher, "scan", fake_scan) -class _DummyVizCfg: - def __init__(self, visualizer_type: str): - self.visualizer_type = visualizer_type - - class _DummySimCfg: def __init__(self, visualizer_cfgs): self.visualizer_cfgs = visualizer_cfgs @@ -70,7 +66,9 @@ def close(self, exit_code=0): ) monkeypatch.setitem(sys.modules, "isaaclab_physx.app", types.SimpleNamespace(KitLauncher=_FakeKitLauncher)) - env_cfg = _DummyEnvCfg(_DummySimCfg([_DummyVizCfg("kit"), _DummyVizCfg("newton")])) + env_cfg = _DummyEnvCfg( + _DummySimCfg([VisualizerCfg(visualizer_type="kit"), VisualizerCfg(visualizer_type="newton_gl")]) + ) launcher_args = argparse.Namespace() with sim_launcher.launch_simulation(env_cfg, launcher_args): @@ -106,8 +104,8 @@ def test_caller_visualizer_intent_requests_kit_visualizer(kit_launcher_args): @pytest.mark.parametrize("physics_cfg", [NewtonCfg(), OvPhysxCfg()], ids=["newton", "ovphysx"]) def test_cli_visualizer_selection_overrides_config_kit_visualizer(kit_launcher_args, physics_cfg): """``--viz rerun`` drops a configured Kit visualizer, so the run stays kitless.""" - cfg = argparse.Namespace(physics=physics_cfg, visualizer_cfgs=[_DummyVizCfg("kit")]) - launcher_args = argparse.Namespace(visualizer=["rerun"], visualizer_explicit=True) + cfg = argparse.Namespace(physics=physics_cfg, visualizer_cfgs=[VisualizerCfg(visualizer_type="kit")]) + launcher_args = argparse.Namespace(visualizer=["rerun"]) with sim_launcher.launch_simulation(cfg, launcher_args): pass @@ -136,14 +134,14 @@ def test_launch_simulation_kitless_viz_none_sets_disable_all(monkeypatch): def fake_sync(launcher_args: dict) -> None: captured["types"] = " ".join(launcher_args["visualizer"]) if launcher_args.get("visualizer") else "" - captured["explicit"] = launcher_args["visualizer_explicit"] - captured["disable_all"] = launcher_args["visualizer_disable_all"] + captured["explicit"] = launcher_args["visualizer"] is not None + captured["disable_all"] = launcher_args["visualizer"] == [] _force_kitless(monkeypatch) monkeypatch.setattr(sim_launcher, "_sync_visualizer_cli_settings", fake_sync) env_cfg = _DummyEnvCfg(_DummySimCfg(None)) - launcher_args = argparse.Namespace(visualizer=None, visualizer_explicit=True) + launcher_args = argparse.Namespace(visualizer=["none"]) with sim_launcher.launch_simulation(env_cfg, launcher_args): pass @@ -161,7 +159,7 @@ def fake_apply(level): monkeypatch.setattr(sim_launcher, "apply_python_logging_level", fake_apply) env_cfg = _DummyEnvCfg(_DummySimCfg(None)) - launcher_args = argparse.Namespace(visualizer=None, visualizer_explicit=True, verbose=True) + launcher_args = argparse.Namespace(visualizer=["none"], verbose=True) with sim_launcher.launch_simulation(env_cfg, launcher_args): pass