Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fixed
^^^^^

* Fixed an issue where script-defined default visualizers were incorrectly promoted to explicit user intent, causing ``RuntimeError`` during headless execution.
14 changes: 11 additions & 3 deletions source/isaaclab/isaaclab/app/app_launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ def add_app_launcher_args(parser: argparse.ArgumentParser) -> None:
default=AppLauncher._APPLAUNCHER_CFG_INFO["device"][1],
help='The device to run the simulation on. Can be "cpu", "cuda", "cuda:N", where N is the device ID',
)
parser.set_defaults(visualizer_explicit=False)
arg_group.add_argument(
"--visualizer",
"--viz",
Expand Down Expand Up @@ -962,9 +963,11 @@ def _resolve_visualizer_settings(self, launcher_args: dict) -> None:
)
self._cfg_has_any_visualizers = cfg_has_any
self._cfg_has_kit_visualizer = cfg_has_kit
visualizer_explicit = bool(launcher_args.pop("visualizer_explicit", False))
if not visualizer_explicit and "visualizer" in launcher_args:
visualizer_explicit = raw_visualizers is not None
visualizer_explicit = launcher_args.pop("visualizer_explicit", None)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Warning · Design Architecture — Script default visualizers now silently force headless

With visualizer_explicit=False always present in the parsed namespace, a script using parser.set_defaults(visualizer=["kit"]) takes the else branch of _resolve_headless_settings (lines 938-954). That branch consults only visualizer_intent, which such scripts do not set, so _headless becomes True even on a plain non-headless run, while _cli_visualizer_types still holds kit. Map non-explicit argparse visualizer defaults into _cfg_has_any_visualizers/_cfg_has_kit_visualizer instead of discarding them.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-explicit argparse visualizer defaults are now properly mapped into _cfg_has_any_visualizers and _cfg_has_kit_visualizer

if visualizer_explicit is None:
visualizer_explicit = "visualizer" in launcher_args and raw_visualizers is not None
else:
visualizer_explicit = bool(visualizer_explicit)

visualizer_types: list[str] = []
if raw_visualizers is not None:
Expand Down Expand Up @@ -1010,6 +1013,11 @@ def _resolve_visualizer_settings(self, launcher_args: dict) -> None:
raw_visualizers is None or "none" in visualizer_types
)
self._cli_visualizer_types = [] if self._cli_visualizer_disable_all else visualizer_types

if not self._cli_visualizer_explicit and self._cli_visualizer_types:
self._cfg_has_any_visualizers = True
if "kit" in self._cli_visualizer_types:
self._cfg_has_kit_visualizer = True
launcher_args["visualizer"] = self._cli_visualizer_types

def _resolve_camera_settings(self, launcher_args: dict):
Expand Down
45 changes: 40 additions & 5 deletions source/isaaclab/test/app/test_kwarg_launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ def _raise_settings_error():

def test_parse_visualizer_csv_accepts_comma_delimited_values():
parsed = app_launcher_module.AppLauncher._parse_visualizer_csv("kit,newton,rerun,viser")
assert parsed == ["kit", "newton", "rerun", "viser"]
assert parsed == ["kit", "newton_gl", "rerun", "viser"]


def test_parse_visualizer_csv_rejects_spaces_between_entries():
Expand All @@ -380,7 +380,7 @@ def test_visualizer_csv_does_not_swallow_hydra_overrides():
["--visualizer", "kit,newton,rerun", "presets=newton_mjwarp", "env.episode_length=10"]
)

assert args.visualizer == ["kit", "newton", "rerun"]
assert args.visualizer == ["kit", "newton_gl", "rerun"]
assert hydra_args == ["presets=newton_mjwarp", "env.episode_length=10"]


Expand All @@ -403,7 +403,7 @@ def test_matrix_cli_kit_newton_with_custom_kit_cfg_intent_non_headless(monkeypat
},
)
assert headless is False
assert launcher._cli_visualizer_types == ["kit", "newton"]
assert launcher._cli_visualizer_types == ["kit", "newton_gl"]


def test_matrix_cli_rerun_with_custom_kit_cfg_intent_headless(monkeypatch: pytest.MonkeyPatch):
Expand Down Expand Up @@ -483,8 +483,8 @@ def test_matrix_headless_with_viz_names_takes_precedence(monkeypatch: pytest.Mon
},
)
assert headless is True
assert launcher._cli_visualizer_disable_all is True
assert launcher._cli_visualizer_types == []
assert launcher._cli_visualizer_disable_all is False
assert launcher._cli_visualizer_types == ["kit", "newton_gl"]


def test_no_cli_and_no_cfg_visualizers_defaults_headless(monkeypatch: pytest.MonkeyPatch):
Expand Down Expand Up @@ -581,3 +581,38 @@ def test_has_gui_reads_published_setting():
assert AppLauncher.has_gui() is False
finally:
settings.set_bool("/isaaclab/has_gui", bool(original))


def test_visualizer_argparse_set_defaults_is_not_explicit():
parser = argparse.ArgumentParser(add_help=False)
app_launcher_module.AppLauncher.add_app_launcher_args(parser)
parser.set_defaults(visualizer=["kit"])

args, _ = parser.parse_known_args([])
launcher = AppLauncher.__new__(AppLauncher)
launcher._resolve_visualizer_settings(vars(args))

assert launcher._cli_visualizer_explicit is False
assert launcher._cfg_has_any_visualizers is True
assert launcher._cfg_has_kit_visualizer is True


def test_visualizer_direct_kwargs_is_explicit():
launcher = AppLauncher.__new__(AppLauncher)
launcher._resolve_visualizer_settings({"visualizer": ["kit"]})

assert launcher._cli_visualizer_explicit is True


def test_matrix_headless_env_with_kit_visualizer(monkeypatch: pytest.MonkeyPatch):
monkeypatch.setenv("HEADLESS", "1")
launcher = AppLauncher.__new__(AppLauncher)
launcher._livestream = 0
launcher._resolve_visualizer_settings({"visualizer": ["kit"], "visualizer_explicit": True})
launcher._resolve_headless_settings(
{"visualizer": ["kit"], "visualizer_explicit": True}, livestream_arg=-1, livestream_env=0
)

assert launcher._headless is True
# Kit visualizer shouldn't be disabled just because of HEADLESS=1
assert launcher._cli_visualizer_disable_all is False