From 90fcf2fd18c2fc9d416b1d82e3c5a1235bc214b5 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 12 Aug 2026 14:33:18 +0200 Subject: [PATCH 01/10] Add a task discovery API for backend combinations The Gym registry is walked in two places with two different answers. tools/environ_docs.py reads what a task declares, which is what the environment tables publish. Nothing reports what actually resolves, so a combination can be documented while being impossible to run: that is how the AnymalC-Direct rows in environments.rst came to advertise presets the task does not have. Add tools/task_discovery.py. discover_tasks(resolve=False) reports the declared view; resolve=True additionally builds each combination and runs the runtime validator, keeping only combinations that can run. Automatic selectors are reported separately from concrete backends. physics=physx resolves to OvPhysX kitless and to Isaac Sim PhysX under Kit, and renderer=rtx behaves the same way, so a selector and its target are the same run. Which of the two to drop depends on how the caller launches, so discovery reports both and lets callers decide. They are detected by config type rather than by name, so a new selector upstream needs no edit here. --- .../changelog.d/task-discovery-api.rst | 10 + tools/task_discovery.py | 350 ++++++++++++++++++ tools/test/test_task_discovery.py | 103 ++++++ 3 files changed, 463 insertions(+) create mode 100644 source/isaaclab/changelog.d/task-discovery-api.rst create mode 100644 tools/task_discovery.py create mode 100644 tools/test/test_task_discovery.py diff --git a/source/isaaclab/changelog.d/task-discovery-api.rst b/source/isaaclab/changelog.d/task-discovery-api.rst new file mode 100644 index 000000000000..c016248f6cad --- /dev/null +++ b/source/isaaclab/changelog.d/task-discovery-api.rst @@ -0,0 +1,10 @@ +Added +^^^^^ + +* Added ``tools/task_discovery.py``, which enumerates registered training tasks and the + backend combinations they support. ``discover_tasks(resolve=False)`` reports what each + task declares; ``resolve=True`` additionally builds each combination and runs the + runtime validator, so combinations that are declared but cannot run are excluded. + Automatic selectors such as ``physics=physx`` and ``renderer=rtx`` are reported + separately from concrete backends, letting callers exclude aliases without hardcoding + their names. diff --git a/tools/task_discovery.py b/tools/task_discovery.py new file mode 100644 index 000000000000..c5dbaa08bfce --- /dev/null +++ b/tools/task_discovery.py @@ -0,0 +1,350 @@ +# 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 + +"""Enumerate registered training tasks and the backend combinations they support. + +Two questions get asked of the Gym registry, and they do not have the same answer: + +* What does a task **declare**? Reading :func:`~isaaclab_tasks.utils.preset_cli.enumerate_task_presets` + is fast and is what the environment documentation reports. +* What does a task actually **resolve**? Building the config and running the runtime + validator is slow, but it is the only way to know a combination can run. The cross + product is not all legal: OVRTX is kitless and cannot share a process with Kit + physics, so ``isaacsim_physx + ovrtx`` is declared yet unusable. + +:func:`discover_tasks` answers either, selected with ``resolve``. Declared mode costs +one registry walk; resolved mode additionally costs one config resolution per +combination, which is minutes for the full registry but far cheaper than finding out +on a GPU. + +The gap between the two is itself useful: a combination that is declared but does not +resolve is documentation drift. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import Any + +__all__ = [ + "RL_LIBRARY_PRIORITY", + "DiscoveredTask", + "DiscoveryError", + "discover_tasks", +] + +# Stable ordering for the RL library axis. +RL_LIBRARY_PRIORITY: tuple[str, ...] = ("rsl_rl", "rl_games", "skrl", "sb3") + +# Physics presets that are proxy variants rather than backends under test. +_SKIP_PHYSICS = frozenset({"newton_mjwarp_vbd_proxy"}) + +# Backend names that also appear under ``PresetTarget.DOMAIN`` on some tasks. +# They are selected with ``physics=`` / ``renderer=``, so reporting them as a +# ``presets=`` token would be a duplicate at best and wrong at worst. A per-task +# check is not enough: a task can list ``newton_mjwarp`` under DOMAIN without +# declaring it under PHYSICS. +_BACKEND_MIRROR_NAMES = frozenset( + { + "newton_kamino", + "newton_mjwarp", + "newton_mjwarp_vbd", + "newton_mjwarp_vbd_proxy", + "ovphysx", + "physx", + "isaacsim_physx", + "newton", + "kamino", + "isaacsim_rtx", + "isaacsim_rtx_renderer", + "newton_renderer", + "ovrtx", + "ovrtx_renderer", + "rtx", + } +) + +# Errors that mean the validator itself could not run, rather than that the +# combination under test was rejected. Swallowing these marks every combination +# illegal and leaves callers blaming their filters for an empty result. +# ``TypeError`` is included because calling the validator with the wrong argument +# type is otherwise indistinguishable from a rejected combination. +_INFRASTRUCTURE_ERRORS = (ImportError, AttributeError, NameError, SyntaxError, TypeError) + + +class DiscoveryError(RuntimeError): + """Raised when the registry cannot be walked, or validation cannot run.""" + + +@dataclass(frozen=True) +class DiscoveredTask: + """One registered training task. + + Args: + task_id: Gym task id. + scope: ``core`` or ``contrib``. + rl_libraries: RL libraries the task declares, in :data:`RL_LIBRARY_PRIORITY` + order. Empty for registered environments with no RL entry point, such as + IK, teleop and mimic tasks. + declared: Preset names the task declares, keyed by axis (``physics``, + ``renderer``, ``presets``). Backend mirrors are already removed from + ``presets``. + selectors: Declared names that are automatic selectors rather than concrete + backends, keyed by axis. ``physics=physx`` and ``renderer=rtx`` resolve to + a backend at launch, so a selector and its target are the same run. + Consumers that must not double-count — a benchmark dispatcher — subtract + these; the environment tables exclude them for the same reason. + modes: Backend combinations. In resolved mode these are the combinations that + passed the runtime validator; in declared mode they are the full cross + product, unverified. + resolved: Whether ``modes`` was filtered by the runtime validator. + """ + + @dataclass(frozen=True) + class Mode: + """One way to run a task. + + Args: + physics: Physics preset token, or ``None`` for tasks that declare none + and reject any ``physics=`` selector. + renderer: Renderer preset token, or ``None`` to run headless. + presets: Domain preset token passed as ``presets=``, or ``None``. + Exactly one at a time: domain presets targeting the same field + conflict outright, e.g. ``presets=depth,rgb`` is rejected. + """ + + physics: str | None + renderer: str | None + presets: str | None + + task_id: str + scope: str + rl_libraries: tuple[str, ...] + declared: dict[str, tuple[str, ...]] = field(default_factory=dict) + selectors: dict[str, tuple[str, ...]] = field(default_factory=dict) + modes: tuple[DiscoveredTask.Mode, ...] = () + resolved: bool = False + + +def _selector_names(task_id: str) -> dict[str, tuple[str, ...]]: + """Return the declared preset names that are automatic selectors, by axis. + + A selector is an alias rather than a backend: ``physics=physx`` resolves to + OvPhysX kitless and to Isaac Sim PhysX under Kit, and ``renderer=rtx`` picks an + RTX backend the same way. Because the target depends on how the run is + launched, a selector and the backend it resolves to are the same run — which is + why the environment tables list only concrete backends. + + Detected by config type rather than by name so that adding a selector upstream + does not require editing a hardcoded list here. + """ + from isaaclab.physics.physics_manager_cfg import PhysxAutoCfg + + from isaaclab_tasks.utils.hydra import collect_presets + from isaaclab_tasks.utils.parse_cfg import load_cfg_from_registry + + try: + walked = collect_presets(load_cfg_from_registry(task_id, "env_cfg_entry_point")) + except _INFRASTRUCTURE_ERRORS as exc: + # A structural failure here returns no selectors, which is indistinguishable + # from a task that genuinely has none — so it would silently disable + # selector-aware filtering for every task. Fail loudly instead. + raise DiscoveryError(f"selector detection for {task_id!r} could not run: {type(exc).__name__}: {exc}") from exc + except Exception: # noqa: BLE001 - a config that cannot load declares no selectors + return {} + + # ``collect_presets`` maps dotted config paths to ``{preset name: cfg}``, so the + # variants live one level in. Iterating the outer mapping yields dicts, never + # configs, and silently finds nothing. + physics: set[str] = set() + renderer: set[str] = set() + for variants in walked.values(): + for name, cfg in variants.items(): + if isinstance(cfg, PhysxAutoCfg): + physics.add(name) + elif getattr(cfg, "renderer_type", None) == "auto_rtx": + renderer.add(name) + return {"physics": tuple(sorted(physics)), "renderer": tuple(sorted(renderer))} + + +def _canonical_physics(names: tuple[str, ...]) -> tuple[str, ...]: + """Drop proxy physics variants that are never a backend under test. + + ``physx`` is deliberately kept even when ``ovphysx`` is also declared. It is a + real selector that resolves to a concrete backend, so a task matrix should + report it. Callers that would run both and consider the pair redundant — a + benchmark dispatcher, say — should apply that policy themselves. + """ + return tuple(name for name in names if name not in _SKIP_PHYSICS) + + +def _domain_presets(names: list[str]) -> tuple[str, ...]: + """Return domain presets, dropping names that mirror a backend selector.""" + return tuple(sorted(name for name in names if name not in _BACKEND_MIRROR_NAMES)) + + +def _is_training_task(task_id: str) -> bool: + """Return whether *task_id* is a trainable Isaac task.""" + if "Isaac" not in task_id: + return False + return not task_id.endswith("-Eval") and "-Benchmark-" not in task_id + + +def _rl_libraries_from_kwargs(kwargs: dict[str, Any]) -> tuple[str, ...]: + """Return the RL libraries a registration declares an agent config for. + + Entry points are matched on the stem before ``_cfg_entry_point`` so that + variants such as ``rsl_rl_recurrent_cfg_entry_point`` count towards their + library rather than being dropped. + """ + declared = set() + for key in kwargs: + if not key.endswith("_cfg_entry_point") or key == "env_cfg_entry_point": + continue + stem = key[: -len("_cfg_entry_point")] + for candidate in RL_LIBRARY_PRIORITY: + if stem == candidate or stem.startswith(f"{candidate}_"): + declared.add(candidate) + break + return tuple(name for name in RL_LIBRARY_PRIORITY if name in declared) + + +def _mode_resolves(task_id: str, physics: str | None, renderer: str | None, presets: str | None = None) -> bool: + """Return whether a physics/renderer/preset combination resolves and validates. + + An unknown preset, an unloadable config, or a rejected backend combination all + mean the same thing — the combination cannot run — so they return ``False`` alike. + + Raises: + DiscoveryError: If validation could not run at all, e.g. because an Isaac Lab + import or API it depends on has changed. + """ + import argparse + import sys + + from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, scan + + from isaaclab_tasks.utils import resolve_task_config, setup_preset_cli + + parser = argparse.ArgumentParser() + parser.add_argument("--task") + parser.add_argument("--agent", default=None) + argv = ["--task", task_id] + if physics is not None: + argv.append(f"physics={physics}") + if renderer is not None: + argv.append(f"renderer={renderer}") + if presets is not None: + argv.append(f"presets={presets}") + + original_argv = list(sys.argv) + try: + args, remaining = setup_preset_cli(parser, argv) + sys.argv = [sys.argv[0]] + remaining + env_cfg, _ = resolve_task_config(args.task, args.agent) + # ``_validate_runtime`` takes the resolved Kit sources, not the parsed args. + # Passing args makes every scan look Kit-backed, which fires the OvPhysX + # guard for every OvPhysX combination and marks them all unusable. + config_scan = scan(env_cfg, args) + _validate_runtime(config_scan, _get_kit_runtime_sources(config_scan, args)) + return True + except _INFRASTRUCTURE_ERRORS as exc: + raise DiscoveryError( + f"preset validation for {task_id!r} could not run: {type(exc).__name__}: {exc}. This is an Isaac Lab" + " import or API failure, not a rejected preset combination." + ) from exc + except Exception: # noqa: BLE001 - any other failure means the combination cannot run + return False + finally: + sys.argv = original_argv + + +def _build_modes( + task_id: str, + physics: tuple[str, ...], + renderers: tuple[str, ...], + domains: tuple[str, ...], + *, + resolve: bool, +) -> tuple[DiscoveredTask.Mode, ...]: + """Return the backend combinations for one task. + + A task declaring renderers is expanded across them: reporting a camera task as + headless-only omits the thing under test. Domain presets are expanded one at a + time, never combined, because presets targeting the same field conflict. + """ + physics_options: tuple[str | None, ...] = physics or (None,) + renderer_options: tuple[str | None, ...] = renderers or (None,) + # ``None`` keeps the task's own default alongside each explicit preset. + domain_options: tuple[str | None, ...] = (None, *domains) if domains else (None,) + + modes: list[DiscoveredTask.Mode] = [] + for physics_name in physics_options: + for renderer in renderer_options: + for domain in domain_options: + if resolve and not _mode_resolves(task_id, physics_name, renderer, domain): + continue + modes.append(DiscoveredTask.Mode(physics=physics_name, renderer=renderer, presets=domain)) + return tuple(modes) + + +def discover_tasks(*, resolve: bool = True) -> list[DiscoveredTask]: + """Walk the Gym registry and return every registered training task. + + Imports Isaac Lab, so it needs the project environment. Contrib tasks are included + when ``isaaclab_tasks_experimental`` is importable. + + Args: + resolve: When ``True``, every backend combination is built and checked against + the runtime validator, and only combinations that can actually run are + returned. When ``False``, combinations are reported as declared, which is + fast but unverified. + + Returns: + Discovered tasks sorted by ``task_id``. + + Raises: + DiscoveryError: If the task packages cannot be imported. + """ + import contextlib + + try: + import gymnasium as gym + + import isaaclab_tasks # noqa: F401 + from isaaclab_tasks.utils.preset_cli import enumerate_task_presets + from isaaclab_tasks.utils.preset_target import PresetTarget + except ImportError as exc: + raise DiscoveryError(f"could not import the Isaac Lab task packages: {exc}") from exc + + with contextlib.suppress(ImportError): + import isaaclab_tasks_experimental # noqa: F401 + + tasks: list[DiscoveredTask] = [] + for spec in gym.registry.values(): + if not _is_training_task(spec.id) or spec.kwargs.get("deprecated"): + continue + # Tasks without an RL entry point (IK, teleop, mimic) are still registered + # environments and are reported with an empty ``rl_libraries``. Callers that + # need a trainable task — a dispatcher, say — filter on it themselves rather + # than having that policy baked in here. + libraries = _rl_libraries_from_kwargs(spec.kwargs) + preset_map = enumerate_task_presets(spec.id) + physics = _canonical_physics(tuple(sorted(preset_map.get(PresetTarget.PHYSICS, [])))) if preset_map else () + renderers = tuple(sorted(preset_map.get(PresetTarget.RENDERER, []))) if preset_map else () + domains = _domain_presets(preset_map.get(PresetTarget.DOMAIN, [])) if preset_map else () + tasks.append( + DiscoveredTask( + task_id=spec.id, + scope="contrib" if spec.id.startswith("IsaacContrib-") else "core", + rl_libraries=libraries, + declared={"physics": physics, "renderer": renderers, "presets": domains}, + selectors=_selector_names(spec.id), + modes=_build_modes(spec.id, physics, renderers, domains, resolve=resolve), + resolved=resolve, + ) + ) + tasks.sort(key=lambda task: task.task_id) + return tasks diff --git a/tools/test/test_task_discovery.py b/tools/test/test_task_discovery.py new file mode 100644 index 000000000000..30d3fbcd7b28 --- /dev/null +++ b/tools/test/test_task_discovery.py @@ -0,0 +1,103 @@ +# 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 + +"""Tests for the registry-independent parts of task discovery. + +The registry walk and selector detection need Isaac Lab importable and are +exercised by running the tool; everything here is pure and runs offline. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + + +def _bootstrap_paths() -> None: + """Prepend ``tools/`` so the module imports the same way the tool does.""" + tools_dir = Path(__file__).resolve().parents[1] + if str(tools_dir) not in sys.path: + sys.path.insert(0, str(tools_dir)) + + +_bootstrap_paths() + +from task_discovery import ( # noqa: E402 + DiscoveredTask, + _build_modes, + _canonical_physics, + _domain_presets, + _is_training_task, + _rl_libraries_from_kwargs, +) + +Mode = DiscoveredTask.Mode + + +@pytest.mark.parametrize( + ("kwargs", "expected"), + [ + ({"rsl_rl_cfg_entry_point": "x"}, ("rsl_rl",)), + # Variant entry points belong to their library; matching the exact name + # would drop every recurrent, distillation and per-terrain config. + ({"rsl_rl_recurrent_cfg_entry_point": "x"}, ("rsl_rl",)), + ({"skrl_flat_ppo_cfg_entry_point": "x"}, ("skrl",)), + # Ordering follows RL_LIBRARY_PRIORITY, not registration order. + ({"skrl_cfg_entry_point": "x", "rsl_rl_cfg_entry_point": "x"}, ("rsl_rl", "skrl")), + # The env config is not an agent config. + ({"env_cfg_entry_point": "x"}, ()), + ({}, ()), + ], +) +def test_rl_libraries_are_read_from_entry_point_stems(kwargs: dict, expected: tuple[str, ...]) -> None: + assert _rl_libraries_from_kwargs(kwargs) == expected + + +@pytest.mark.parametrize( + ("task_id", "expected"), + [ + ("Isaac-Ant", True), + ("IsaacContrib-Walk", True), + ("Isaac-Ant-Eval", False), + ("Isaac-Benchmark-Cartpole", False), + ("Some-Other-Env", False), + ], +) +def test_only_trainable_isaac_tasks_are_walked(task_id: str, expected: bool) -> None: + assert _is_training_task(task_id) is expected + + +def test_proxy_physics_variants_are_dropped() -> None: + assert _canonical_physics(("newton_mjwarp", "newton_mjwarp_vbd_proxy")) == ("newton_mjwarp",) + + +def test_the_physx_selector_is_reported_alongside_concrete_backends() -> None: + # Whether a selector duplicates a concrete backend depends on how the run is + # launched, so the decision belongs to the caller, not to discovery. + assert _canonical_physics(("isaacsim_physx", "ovphysx", "physx")) == ("isaacsim_physx", "ovphysx", "physx") + + +def test_domain_presets_drop_names_that_mirror_a_backend_selector() -> None: + assert _domain_presets(["rgb", "ovphysx", "depth", "newton_mjwarp"]) == ("depth", "rgb") + + +def test_modes_are_the_cross_product_when_resolution_is_skipped() -> None: + modes = _build_modes("Isaac-X", ("physx", "newton_mjwarp"), ("ovrtx",), (), resolve=False) + + assert modes == (Mode("physx", "ovrtx", None), Mode("newton_mjwarp", "ovrtx", None)) + + +def test_a_task_without_presets_gets_one_mode_carrying_no_tokens() -> None: + assert _build_modes("Isaac-X", (), (), (), resolve=False) == (Mode(None, None, None),) + + +def test_domain_presets_are_expanded_one_at_a_time_beside_the_default() -> None: + # Presets targeting the same field conflict, so they are never combined; the + # ``None`` entry keeps the task's own default reachable. + modes = _build_modes("Isaac-X", (), (), ("rgb", "depth"), resolve=False) + + assert modes == (Mode(None, None, None), Mode(None, None, "rgb"), Mode(None, None, "depth")) From c728bdb7081c8cd0fa4b39c76711a4e14dfc6003 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 12 Aug 2026 14:58:52 +0200 Subject: [PATCH 02/10] Survive partial installs and report unloadable configs distinctly Two defects in the first cut. ModuleNotFoundError subclasses ImportError, which was listed as a structural failure, so one uninstalled extra aborted the whole walk rather than skipping that task. A task whose config needs teleop or mimic simply cannot be inspected from a partial install; that is a property of the environment, not a broken registry. ``declared`` also collapsed two different answers into one. A config that cannot be loaded is unknown; a config that loads and declares nothing means the task runs on a fixed backend with no selectable alternative. 37 of 127 tasks are in the second group, and the environment tables render the two differently, so ``declared`` is now ``None`` only for the first. ``declared`` reports raw registry names, with BACKEND_MIRROR_NAMES exported so callers filter to suit what they are reporting. --- tools/task_discovery.py | 49 ++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/tools/task_discovery.py b/tools/task_discovery.py index c5dbaa08bfce..09b2d3f11fa2 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -29,6 +29,7 @@ from typing import Any __all__ = [ + "BACKEND_MIRROR_NAMES", "RL_LIBRARY_PRIORITY", "DiscoveredTask", "DiscoveryError", @@ -46,7 +47,7 @@ # ``presets=`` token would be a duplicate at best and wrong at worst. A per-task # check is not enough: a task can list ``newton_mjwarp`` under DOMAIN without # declaring it under PHYSICS. -_BACKEND_MIRROR_NAMES = frozenset( +BACKEND_MIRROR_NAMES = frozenset( { "newton_kamino", "newton_mjwarp", @@ -89,8 +90,14 @@ class DiscoveredTask: order. Empty for registered environments with no RL entry point, such as IK, teleop and mimic tasks. declared: Preset names the task declares, keyed by axis (``physics``, - ``renderer``, ``presets``). Backend mirrors are already removed from - ``presets``. + ``renderer``, ``presets``), exactly as the registry reports them, or + ``None`` when the config could not be loaded at all. ``None`` and an + all-empty mapping are different answers: the first means unknown, the + second means the task declares nothing and runs on a fixed backend. + Nothing is filtered out: names that mirror a backend selector, and + selectors themselves, are all present. Consumers narrow this with + :data:`BACKEND_MIRROR_NAMES` and ``selectors`` according to what they + are reporting. selectors: Declared names that are automatic selectors rather than concrete backends, keyed by axis. ``physics=physx`` and ``renderer=rtx`` resolve to a backend at launch, so a selector and its target are the same run. @@ -122,7 +129,7 @@ class Mode: task_id: str scope: str rl_libraries: tuple[str, ...] - declared: dict[str, tuple[str, ...]] = field(default_factory=dict) + declared: dict[str, tuple[str, ...]] | None = field(default_factory=dict) selectors: dict[str, tuple[str, ...]] = field(default_factory=dict) modes: tuple[DiscoveredTask.Mode, ...] = () resolved: bool = False @@ -147,8 +154,13 @@ def _selector_names(task_id: str) -> dict[str, tuple[str, ...]]: try: walked = collect_presets(load_cfg_from_registry(task_id, "env_cfg_entry_point")) + except ImportError: + # A task whose config needs an extra that is not installed (teleop, mimic) + # simply cannot be inspected here. That is a property of the environment, + # not a broken registry, so report no selectors and carry on. + return {} except _INFRASTRUCTURE_ERRORS as exc: - # A structural failure here returns no selectors, which is indistinguishable + # Anything else structural returns no selectors, which is indistinguishable # from a task that genuinely has none — so it would silently disable # selector-aware filtering for every task. Fail loudly instead. raise DiscoveryError(f"selector detection for {task_id!r} could not run: {type(exc).__name__}: {exc}") from exc @@ -182,7 +194,7 @@ def _canonical_physics(names: tuple[str, ...]) -> tuple[str, ...]: def _domain_presets(names: list[str]) -> tuple[str, ...]: """Return domain presets, dropping names that mirror a backend selector.""" - return tuple(sorted(name for name in names if name not in _BACKEND_MIRROR_NAMES)) + return tuple(sorted(name for name in names if name not in BACKEND_MIRROR_NAMES)) def _is_training_task(task_id: str) -> bool: @@ -250,10 +262,15 @@ def _mode_resolves(task_id: str, physics: str | None, renderer: str | None, pres config_scan = scan(env_cfg, args) _validate_runtime(config_scan, _get_kit_runtime_sources(config_scan, args)) return True + except ImportError: + # The task's config needs an extra that is not installed, so this + # combination cannot run in this environment. That is the same answer as a + # rejected combination, and it keeps discovery usable from a partial install. + return False except _INFRASTRUCTURE_ERRORS as exc: raise DiscoveryError( f"preset validation for {task_id!r} could not run: {type(exc).__name__}: {exc}. This is an Isaac Lab" - " import or API failure, not a rejected preset combination." + " API failure, not a rejected preset combination." ) from exc except Exception: # noqa: BLE001 - any other failure means the combination cannot run return False @@ -290,13 +307,14 @@ def _build_modes( return tuple(modes) -def discover_tasks(*, resolve: bool = True) -> list[DiscoveredTask]: +def discover_tasks(specs: list[Any] | None = None, *, resolve: bool = True) -> list[DiscoveredTask]: """Walk the Gym registry and return every registered training task. Imports Isaac Lab, so it needs the project environment. Contrib tasks are included when ``isaaclab_tasks_experimental`` is importable. Args: + specs: Gym specs to walk. When ``None``, the whole registry is scanned. resolve: When ``True``, every backend combination is built and checked against the runtime validator, and only combinations that can actually run are returned. When ``False``, combinations are reported as declared, which is @@ -322,8 +340,11 @@ def discover_tasks(*, resolve: bool = True) -> list[DiscoveredTask]: with contextlib.suppress(ImportError): import isaaclab_tasks_experimental # noqa: F401 + if specs is None: + specs = list(gym.registry.values()) + tasks: list[DiscoveredTask] = [] - for spec in gym.registry.values(): + for spec in specs: if not _is_training_task(spec.id) or spec.kwargs.get("deprecated"): continue # Tasks without an RL entry point (IK, teleop, mimic) are still registered @@ -340,7 +361,15 @@ def discover_tasks(*, resolve: bool = True) -> list[DiscoveredTask]: task_id=spec.id, scope="contrib" if spec.id.startswith("IsaacContrib-") else "core", rl_libraries=libraries, - declared={"physics": physics, "renderer": renderers, "presets": domains}, + declared=( + None + if preset_map is None + else { + "physics": tuple(sorted(preset_map.get(PresetTarget.PHYSICS, []))), + "renderer": renderers, + "presets": tuple(sorted(preset_map.get(PresetTarget.DOMAIN, []))), + } + ), selectors=_selector_names(spec.id), modes=_build_modes(spec.id, physics, renderers, domains, resolve=resolve), resolved=resolve, From f26f92dbef60835d6cec872594fd6c02401c8272 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 12 Aug 2026 15:05:51 +0200 Subject: [PATCH 03/10] Read the task matrix from task_discovery in the docs generator The environment tables and the discovery API walked the Gym registry separately, so the same knowledge - which tasks are trainable, which presets they declare, which names mirror a backend selector - was maintained twice and had already drifted. Delegate the walk. The documentation policies stay here, because they are about how the tables read rather than what the registry contains: the implicit-PhysX inference, the manual RL-library overrides, and the render-time preset filtering are unchanged. Verified by generating environments.rst and environment-browser.js twice in one environment, once per implementation, in separate processes: both files are byte-identical. Note that `update_environments_rst.py --check` is not a valid test for this, because the generator's output depends on which optional backends are installed - a task shows `ovphysx` only when ovphysx is importable - so it conflates a code change with environment drift. The A/B comparison isolates the code change. Run with teleop and mimic installed so that all 127 tasks load; without them 31 configs are unloadable and never exercise the comparison. --- tools/environ_docs.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tools/environ_docs.py b/tools/environ_docs.py index 7096b853b51e..6ab6f047e20f 100644 --- a/tools/environ_docs.py +++ b/tools/environ_docs.py @@ -22,8 +22,8 @@ from typing import TYPE_CHECKING import gymnasium as gym +from task_discovery import discover_tasks -from isaaclab_tasks.utils.preset_cli import enumerate_task_presets from isaaclab_tasks.utils.preset_target import PresetTarget if TYPE_CHECKING: @@ -527,13 +527,17 @@ def collect_environment_doc_rows( rows: list[EnvironmentDocRow] = [] - for spec in specs: - if not is_training_task(spec.id) or spec.kwargs.get("deprecated"): - continue - - preset_map = enumerate_task_presets(spec.id) - if preset_map is not None: - preset_map = dict(preset_map) + specs_by_id = {spec.id: spec for spec in specs} + for task in discover_tasks(specs, resolve=False): + spec = specs_by_id[task.task_id] + + preset_map = None + if task.declared is not None: + preset_map = { + PresetTarget.PHYSICS: list(task.declared["physics"]), + PresetTarget.RENDERER: list(task.declared["renderer"]), + PresetTarget.DOMAIN: list(task.declared["presets"]), + } preset_map[PresetTarget.PHYSICS] = _physics_names_for_docs(spec.id, preset_map) agents = apply_rl_library_overrides(spec.id, parse_rl_libraries_from_kwargs(spec.kwargs)) From 861875d4c91460d76a3a00ac4f09ef81403af6c2 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 19 Aug 2026 10:57:38 +0200 Subject: [PATCH 04/10] Report distinct runs from task discovery, not distinct spellings Resolving a combination already builds its env config, so combinations can be compared on what they produce rather than on how they were spelled. Fingerprint the resolved config and collapse the matches: aliases fold into the backend they resolve to, and the no-token run folds into whichever preset the config already defaults to. Across the registry that turns 2024 declared combinations into 1045 distinct runs. Collapsing on the resolved config rather than on the backend is deliberate -- the Reach controller presets share a backend and are four different runs. What a task does with no tokens is reported separately as ``default``, since the collapse would otherwise hide it. This subsumes three name-based filters, all of which were wrong per task: * ``BACKEND_MIRROR_NAMES`` dropped every DOMAIN name matching a backend. On a task that exposes its backends only as ``presets=`` tokens, that was the sole handle: ``Isaac-Open-Drawer-Franka`` reported one mode instead of four. A DOMAIN name is now a duplicate only when the task also declares it on a typed axis. * ``_SKIP_PHYSICS`` dropped ``newton_mjwarp_vbd_proxy`` as a proxy variant. It is a real backend, and on ``Isaac-Lift-Cloth-Franka`` the only one that resolves. * ``_selector_names`` existed to let callers subtract aliases. It also missed selectors nested inside a variant, so it reported none for the cabinet tasks. Also: * An Isaac Lab API failure now drops the combination it was judging and logs it, instead of ending the whole walk; pass ``strict=True`` for the old behaviour. * ``collapse=False`` keeps every validated spelling, which is what documentation needs -- a preset naming the default of its own axis is still a token a reader can type. * ``RL_LIBRARY_PRIORITY`` and ``is_training_task`` are now owned here and imported by the docs generator, so the tables and the matrix cannot drift. ``rlinf`` was missing from the discovery copy. * ``Mode.presets`` documented that ``presets=`` composes across config paths and that discovery validates each name alone, so ``modes`` under-approximates a task with several independent preset axes. * ``modes`` documented as assuming a headless launch, since a visualizer, livestream or explicit experience is a Kit source that narrows the legal set. The changelog fragment is dropped: no source package changed, and nothing under tools/ is released. --- .../changelog.d/task-discovery-api.rst | 10 - tools/environ_docs.py | 25 +- tools/task_discovery.py | 380 +++++++++++------- tools/test/test_task_discovery.py | 188 ++++++++- 4 files changed, 423 insertions(+), 180 deletions(-) delete mode 100644 source/isaaclab/changelog.d/task-discovery-api.rst diff --git a/source/isaaclab/changelog.d/task-discovery-api.rst b/source/isaaclab/changelog.d/task-discovery-api.rst deleted file mode 100644 index c016248f6cad..000000000000 --- a/source/isaaclab/changelog.d/task-discovery-api.rst +++ /dev/null @@ -1,10 +0,0 @@ -Added -^^^^^ - -* Added ``tools/task_discovery.py``, which enumerates registered training tasks and the - backend combinations they support. ``discover_tasks(resolve=False)`` reports what each - task declares; ``resolve=True`` additionally builds each combination and runs the - runtime validator, so combinations that are declared but cannot run are excluded. - Automatic selectors such as ``physics=physx`` and ``renderer=rtx`` are reported - separately from concrete backends, letting callers exclude aliases without hardcoding - their names. diff --git a/tools/environ_docs.py b/tools/environ_docs.py index 6ab6f047e20f..0042d6901341 100644 --- a/tools/environ_docs.py +++ b/tools/environ_docs.py @@ -22,7 +22,9 @@ from typing import TYPE_CHECKING import gymnasium as gym -from task_discovery import discover_tasks + +# ``is_training_task`` is re-exported for callers that import it from here. +from task_discovery import RL_LIBRARY_PRIORITY, discover_tasks, is_training_task # noqa: F401 from isaaclab_tasks.utils.preset_target import PresetTarget @@ -48,13 +50,9 @@ } ) -# RL libraries listed in a stable order across generated docs. -_RL_LIBRARY_ORDER = ("rl_games", "rsl_rl", "skrl", "sb3", "rlinf") - -# Gym IDs excluded from the training list. The ``-Eval`` suffix marks dedicated -# evaluation variants (e.g. ``IsaacContrib-Assemble-Trocar-G129-Dex3-Eval``, an alias -# registered for RLinf eval configs) that should not appear as their own training row. -_EVAL_TASK_SUFFIXES = ("-Eval",) +# RL libraries listed in a stable order across generated docs. Owned by +# ``task_discovery`` so the tables and the task matrix cannot drift apart. +_RL_LIBRARY_ORDER = RL_LIBRARY_PRIORITY # RL libraries not discoverable from Gym ``kwargs`` (e.g. RLinf YAML-based workflows). RL_LIBRARY_OVERRIDES: dict[str, dict[str, list[str]]] = { @@ -117,17 +115,6 @@ def _supports_warp_frontend(task_name: str, workflow: str, presets: dict[PresetT return False -def is_training_task(task_id: str) -> bool: - """Return ``True`` when *task_id* is a training (non-inference) Isaac task.""" - if "Isaac" not in task_id: - return False - if any(task_id.endswith(suffix) for suffix in _EVAL_TASK_SUFFIXES): - return False - if "-Benchmark-" in task_id: - return False - return True - - def parse_rl_libraries_from_kwargs(kwargs: dict) -> dict[str, list[str]]: """Parse RL-library and algorithm labels from Gym registry kwargs. diff --git a/tools/task_discovery.py b/tools/task_discovery.py index 09b2d3f11fa2..a66fdf26e9cc 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -19,57 +19,49 @@ combination, which is minutes for the full registry but far cheaper than finding out on a GPU. -The gap between the two is itself useful: a combination that is declared but does not -resolve is documentation drift. +Resolving buys more than a legality check. Because each combination is resolved all +the way to an env config, combinations can be compared on what they *produce* rather +than on how they were spelled, and the ones that produce the same run collapse. That +is what makes ``modes`` a list of distinct runs: ``physics=physx`` folds into whatever +concrete backend it resolves to, and passing no tokens at all folds into whichever +preset the config already defaults to. Aliases need no table of names, and a +dispatcher can run every mode without repeating work. What the task does when given +no tokens is kept separately as ``default`` — the collapse would otherwise hide it. + +The gap between declared and resolved is itself useful: a combination that is declared +but does not resolve is documentation drift. """ from __future__ import annotations +import logging from dataclasses import dataclass, field from typing import Any +logger = logging.getLogger(__name__) + __all__ = [ - "BACKEND_MIRROR_NAMES", "RL_LIBRARY_PRIORITY", + "is_training_task", "DiscoveredTask", "DiscoveryError", "discover_tasks", ] -# Stable ordering for the RL library axis. -RL_LIBRARY_PRIORITY: tuple[str, ...] = ("rsl_rl", "rl_games", "skrl", "sb3") - -# Physics presets that are proxy variants rather than backends under test. -_SKIP_PHYSICS = frozenset({"newton_mjwarp_vbd_proxy"}) - -# Backend names that also appear under ``PresetTarget.DOMAIN`` on some tasks. -# They are selected with ``physics=`` / ``renderer=``, so reporting them as a -# ``presets=`` token would be a duplicate at best and wrong at worst. A per-task -# check is not enough: a task can list ``newton_mjwarp`` under DOMAIN without -# declaring it under PHYSICS. -BACKEND_MIRROR_NAMES = frozenset( - { - "newton_kamino", - "newton_mjwarp", - "newton_mjwarp_vbd", - "newton_mjwarp_vbd_proxy", - "ovphysx", - "physx", - "isaacsim_physx", - "newton", - "kamino", - "isaacsim_rtx", - "isaacsim_rtx_renderer", - "newton_renderer", - "ovrtx", - "ovrtx_renderer", - "rtx", - } -) +# Stable ordering for the RL library axis, shared with the environment tables so the +# two never disagree. ``rlinf`` has no discoverable entry point but is listed for +# ordering, since :data:`~environ_docs.RL_LIBRARY_OVERRIDES` supplies it. +RL_LIBRARY_PRIORITY: tuple[str, ...] = ("rl_games", "rsl_rl", "skrl", "sb3", "rlinf") + +# Gym IDs excluded from the training list. The ``-Eval`` suffix marks dedicated +# evaluation variants (e.g. ``IsaacContrib-Assemble-Trocar-G129-Dex3-Eval``, an alias +# registered for RLinf eval configs) that should not appear as their own training row. +_EVAL_TASK_SUFFIXES = ("-Eval",) # Errors that mean the validator itself could not run, rather than that the -# combination under test was rejected. Swallowing these marks every combination -# illegal and leaves callers blaming their filters for an empty result. +# combination under test was rejected. They are logged and the combination is +# dropped, so one broken task costs the caller that task and not the whole walk; +# ``strict=True`` re-raises instead, for callers policing Isaac Lab API drift. # ``TypeError`` is included because calling the validator with the wrong argument # type is otherwise indistinguishable from a rejected combination. _INFRASTRUCTURE_ERRORS = (ImportError, AttributeError, NameError, SyntaxError, TypeError) @@ -94,19 +86,26 @@ class DiscoveredTask: ``None`` when the config could not be loaded at all. ``None`` and an all-empty mapping are different answers: the first means unknown, the second means the task declares nothing and runs on a fixed backend. - Nothing is filtered out: names that mirror a backend selector, and - selectors themselves, are all present. Consumers narrow this with - :data:`BACKEND_MIRROR_NAMES` and ``selectors`` according to what they - are reporting. - selectors: Declared names that are automatic selectors rather than concrete - backends, keyed by axis. ``physics=physx`` and ``renderer=rtx`` resolve to - a backend at launch, so a selector and its target are the same run. - Consumers that must not double-count — a benchmark dispatcher — subtract - these; the environment tables exclude them for the same reason. - modes: Backend combinations. In resolved mode these are the combinations that - passed the runtime validator; in declared mode they are the full cross - product, unverified. - resolved: Whether ``modes`` was filtered by the runtime validator. + Nothing is filtered out: a backend a task exposes on both a typed axis + and as a ``presets=`` token appears under both, and aliases such as + ``physx`` are present alongside what they resolve to. Use ``modes`` for + a deduplicated answer; ``declared`` is the unreconciled registry view. + modes: Ways to run the task. In resolved mode these passed the runtime + validator; with ``collapse`` they are further reduced so that two token + spellings producing the same resolved config appear once — no aliases, no + double-counting. Without it every validated spelling is kept, which is what + a table of "what can I pass?" needs. In declared mode they are the raw + cross product, unverified and uncollapsed. + + Validation assumes a **headless launch**: no ``--visualizer``, no + ``--livestream``, no ``--experience`` and no ``--require_kit``. Each of + those is a Kit source, so adding one narrows the legal set — a kitless + OvPhysX combination that passes here is rejected under ``--visualizer kit``. + default: What the task does when the user passes no preset tokens, or ``None`` + in declared mode. Reported separately because the collapse folds the + no-token run into whichever named mode it matches, and a table still wants + to say what you get if you change nothing. + resolved: Whether ``modes`` was resolved and collapsed, or merely declared. """ @dataclass(frozen=True) @@ -118,90 +117,80 @@ class Mode: and reject any ``physics=`` selector. renderer: Renderer preset token, or ``None`` to run headless. presets: Domain preset token passed as ``presets=``, or ``None``. - Exactly one at a time: domain presets targeting the same field - conflict outright, e.g. ``presets=depth,rgb`` is rejected. + Never more than one. ``presets=`` does accept a comma-separated list, + and names on *different* config paths compose fine — on + ``Isaac-Lift-KukaAllegro-Camera``, ``presets=duo_camera,depth128,cube`` + sets the camera count, the modality and the object independently. Only + names sharing a path conflict, e.g. ``presets=depth,rgb``. Discovery + validates each name on its own and never tries a pair, so ``modes`` + under-approximates a task with several independent preset axes. """ physics: str | None renderer: str | None presets: str | None + @dataclass(frozen=True) + class Default: + """The run a task performs when given no preset tokens. + + Args: + backend: Concrete physics config the run resolves to, e.g. ``PhysxCfg`` + or ``NewtonCfg(MJWarpSolverCfg)``. Reported as the config class rather + than a preset name because a task's default need not have one. + mode: The entry in ``modes`` this run collapsed into — the explicit way to + ask for the same thing. + """ + + backend: str | None + mode: DiscoveredTask.Mode + task_id: str scope: str rl_libraries: tuple[str, ...] declared: dict[str, tuple[str, ...]] | None = field(default_factory=dict) - selectors: dict[str, tuple[str, ...]] = field(default_factory=dict) modes: tuple[DiscoveredTask.Mode, ...] = () + default: DiscoveredTask.Default | None = None resolved: bool = False -def _selector_names(task_id: str) -> dict[str, tuple[str, ...]]: - """Return the declared preset names that are automatic selectors, by axis. +def _domain_presets(names: list[str], typed_names: tuple[str, ...]) -> tuple[str, ...]: + """Return domain presets, dropping the ones that mirror a typed selector. - A selector is an alias rather than a backend: ``physics=physx`` resolves to - OvPhysX kitless and to Isaac Sim PhysX under Kit, and ``renderer=rtx`` picks an - RTX backend the same way. Because the target depends on how the run is - launched, a selector and the backend it resolves to are the same run — which is - why the environment tables list only concrete backends. + Whether a backend lands under ``PresetTarget.DOMAIN`` or under ``PHYSICS`` / + ``RENDERER`` depends on whether its cfg class subclasses ``PhysicsCfg`` / + ``RendererCfg``, so the same name means different things on different tasks: - Detected by config type rather than by name so that adding a selector upstream - does not require editing a hardcoded list here. - """ - from isaaclab.physics.physics_manager_cfg import PhysxAutoCfg + * Also declared on a typed axis — reachable as ``physics=NAME``, so reporting it + again as ``presets=NAME`` double-counts one run. Dropped. + * Not declared on a typed axis — ``presets=NAME`` is the *only* way to select + that backend, and ``physics=NAME`` is rejected outright. Kept. - from isaaclab_tasks.utils.hydra import collect_presets - from isaaclab_tasks.utils.parse_cfg import load_cfg_from_registry + Deciding by name instead of per task gets the second case backwards and silently + hides every backend such a task has (``Isaac-Open-Drawer-Franka`` has five). - try: - walked = collect_presets(load_cfg_from_registry(task_id, "env_cfg_entry_point")) - except ImportError: - # A task whose config needs an extra that is not installed (teleop, mimic) - # simply cannot be inspected here. That is a property of the environment, - # not a broken registry, so report no selectors and carry on. - return {} - except _INFRASTRUCTURE_ERRORS as exc: - # Anything else structural returns no selectors, which is indistinguishable - # from a task that genuinely has none — so it would silently disable - # selector-aware filtering for every task. Fail loudly instead. - raise DiscoveryError(f"selector detection for {task_id!r} could not run: {type(exc).__name__}: {exc}") from exc - except Exception: # noqa: BLE001 - a config that cannot load declares no selectors - return {} - - # ``collect_presets`` maps dotted config paths to ``{preset name: cfg}``, so the - # variants live one level in. Iterating the outer mapping yields dicts, never - # configs, and silently finds nothing. - physics: set[str] = set() - renderer: set[str] = set() - for variants in walked.values(): - for name, cfg in variants.items(): - if isinstance(cfg, PhysxAutoCfg): - physics.add(name) - elif getattr(cfg, "renderer_type", None) == "auto_rtx": - renderer.add(name) - return {"physics": tuple(sorted(physics)), "renderer": tuple(sorted(renderer))} - - -def _canonical_physics(names: tuple[str, ...]) -> tuple[str, ...]: - """Drop proxy physics variants that are never a backend under test. - - ``physx`` is deliberately kept even when ``ovphysx`` is also declared. It is a - real selector that resolves to a concrete backend, so a task matrix should - report it. Callers that would run both and consider the pair redundant — a - benchmark dispatcher, say — should apply that policy themselves. + A backend name surviving here can also mean the task is inconsistent. Shared + configs pair a backend with *companion* overrides under the same name -- + ``velocity_env_cfg`` sets ``events.base_com=None`` under ``newton_mjwarp``, + because Newton does not support that randomization. The companion is normally + invisible: it rides along with the ``newton_mjwarp`` already on the physics axis + and is dropped as a mirror. It only shows up as a standalone ``presets=`` token + on a task that inherited the companion without offering the backend, where + selecting it applies a Newton workaround to a PhysX run. Reporting it is correct + -- the token is real and does change the config -- and it is worth reading as a + signal to fix the task. """ - return tuple(name for name in names if name not in _SKIP_PHYSICS) - + typed = set(typed_names) + return tuple(sorted(name for name in names if name not in typed)) -def _domain_presets(names: list[str]) -> tuple[str, ...]: - """Return domain presets, dropping names that mirror a backend selector.""" - return tuple(sorted(name for name in names if name not in BACKEND_MIRROR_NAMES)) - -def _is_training_task(task_id: str) -> bool: - """Return whether *task_id* is a trainable Isaac task.""" +def is_training_task(task_id: str) -> bool: + """Return whether *task_id* is a trainable (non-inference) Isaac task.""" if "Isaac" not in task_id: return False - return not task_id.endswith("-Eval") and "-Benchmark-" not in task_id + if any(task_id.endswith(suffix) for suffix in _EVAL_TASK_SUFFIXES): + return False + return "-Benchmark-" not in task_id def _rl_libraries_from_kwargs(kwargs: dict[str, Any]) -> tuple[str, ...]: @@ -223,17 +212,28 @@ def _rl_libraries_from_kwargs(kwargs: dict[str, Any]) -> tuple[str, ...]: return tuple(name for name in RL_LIBRARY_PRIORITY if name in declared) -def _mode_resolves(task_id: str, physics: str | None, renderer: str | None, presets: str | None = None) -> bool: - """Return whether a physics/renderer/preset combination resolves and validates. +def _mode_resolves( + task_id: str, physics: str | None, renderer: str | None, presets: str | None = None +) -> tuple[str, str | None] | None: + """Resolve one physics/renderer/preset combination and identify the run it produces. An unknown preset, an unloadable config, or a rejected backend combination all - mean the same thing — the combination cannot run — so they return ``False`` alike. + mean the same thing — the combination cannot run — so they return ``None`` alike. + + Returns: + ``None`` when the combination cannot run, else ``(fingerprint, backend)``. + *fingerprint* digests the fully resolved env config, so two token spellings + that produce the same run share it — ``presets=physx`` and ``presets=ovphysx`` + on the cabinet tasks, or passing nothing at all and naming the preset the + config already defaults to. *backend* names the concrete physics config the + run ends up with, e.g. ``PhysxCfg`` or ``NewtonCfg(MJWarpSolverCfg)``. Raises: DiscoveryError: If validation could not run at all, e.g. because an Isaac Lab import or API it depends on has changed. """ import argparse + import hashlib import sys from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, scan @@ -261,23 +261,37 @@ def _mode_resolves(task_id: str, physics: str | None, renderer: str | None, pres # guard for every OvPhysX combination and marks them all unusable. config_scan = scan(env_cfg, args) _validate_runtime(config_scan, _get_kit_runtime_sources(config_scan, args)) - return True + fingerprint = hashlib.sha256(repr(env_cfg.to_dict()).encode()).hexdigest() + return fingerprint, _backend_name(config_scan.resolved_physics_cfg) except ImportError: # The task's config needs an extra that is not installed, so this # combination cannot run in this environment. That is the same answer as a # rejected combination, and it keeps discovery usable from a partial install. - return False + return None except _INFRASTRUCTURE_ERRORS as exc: raise DiscoveryError( f"preset validation for {task_id!r} could not run: {type(exc).__name__}: {exc}. This is an Isaac Lab" " API failure, not a rejected preset combination." ) from exc except Exception: # noqa: BLE001 - any other failure means the combination cannot run - return False + return None finally: sys.argv = original_argv +def _backend_name(physics_cfg: Any) -> str | None: + """Name the concrete physics config a run resolved to, solver included. + + Newton's solver lives on ``solver_cfg`` and is what separates ``newton_mjwarp`` + from ``newton_kamino``; the class name alone reports both as ``NewtonCfg``. + """ + if physics_cfg is None: + return None + solver = getattr(physics_cfg, "solver_cfg", None) + name = type(physics_cfg).__name__ + return f"{name}({type(solver).__name__})" if solver is not None else name + + def _build_modes( task_id: str, physics: tuple[str, ...], @@ -285,29 +299,108 @@ def _build_modes( domains: tuple[str, ...], *, resolve: bool, -) -> tuple[DiscoveredTask.Mode, ...]: - """Return the backend combinations for one task. + strict: bool = False, + collapse: bool = True, +) -> tuple[tuple[DiscoveredTask.Mode, ...], DiscoveredTask.Default | None]: + """Return the runs for one task, and what it does when given no tokens. A task declaring renderers is expanded across them: reporting a camera task as headless-only omits the thing under test. Domain presets are expanded one at a - time, never combined, because presets targeting the same field conflict. + time and never combined, so every preset name is validated on its own but no pair + is ever tried (see :class:`DiscoveredTask.Mode`). + + With ``collapse``, the cross product is deduplicated on the resolved config, so + each returned mode is a distinct run rather than a distinct spelling. That is what + removes selector double-counting without a table of alias names: ``physics=physx`` + and ``physics=ovphysx`` collapse wherever they resolve alike and stay separate + wherever they do not. Collapsing on the *backend* instead would be wrong — the + Reach controller presets share a backend and are four different runs. + + Without ``collapse``, every combination that validated is returned, duplicate + spellings included. That is what documentation wants: ``presets=shapes`` is a real + token a reader can type even on a task where it happens to name the default of its + own axis, and collapsing would delete it from the table. + + Declared mode neither validates nor collapses, because nothing has been resolved: + it returns the raw cross product and no default. + + Returns: + ``(modes, default)``. *default* is the run the task performs when the user + passes nothing, or ``None`` in declared mode / when that run cannot resolve. + It names an explicit spelling even when ``collapse`` is off. + + Raises: + DiscoveryError: If ``strict`` and the validator could not judge a combination. """ physics_options: tuple[str | None, ...] = physics or (None,) renderer_options: tuple[str | None, ...] = renderers or (None,) - # ``None`` keeps the task's own default alongside each explicit preset. + # ``None`` is the task's own default. It survives the collapse only when it is a + # run of its own; usually it folds into whichever preset the config defaults to. domain_options: tuple[str | None, ...] = (None, *domains) if domains else (None,) - modes: list[DiscoveredTask.Mode] = [] - for physics_name in physics_options: - for renderer in renderer_options: - for domain in domain_options: - if resolve and not _mode_resolves(task_id, physics_name, renderer, domain): - continue - modes.append(DiscoveredTask.Mode(physics=physics_name, renderer=renderer, presets=domain)) - return tuple(modes) - + if not resolve: + return ( + tuple( + DiscoveredTask.Mode(physics=p, renderer=r, presets=d) + for p in physics_options + for r in renderer_options + for d in domain_options + ), + None, + ) -def discover_tasks(specs: list[Any] | None = None, *, resolve: bool = True) -> list[DiscoveredTask]: + combinations = [(p, r, d) for p in physics_options for r in renderer_options for d in domain_options] + # The no-token run is what a user gets by changing nothing, so it always has to be + # probed. It is not always in the cross product: a task declaring physics presets + # has no ``physics=None`` column, only its declared backends. + if (None, None, None) not in combinations: + combinations.insert(0, (None, None, None)) + + # ``unique`` is built either way: even when every validated mode is returned, it is + # what identifies the explicit spelling of the no-token run. + unique: dict[str, DiscoveredTask.Mode] = {} + validated: list[DiscoveredTask.Mode] = [] + default_key: str | None = None + default_backend: str | None = None + for physics_name, renderer, domain in combinations: + try: + resolution = _mode_resolves(task_id, physics_name, renderer, domain) + except DiscoveryError as exc: + if strict: + raise + # Unknown is not the same answer as legal, so the combination is dropped + # -- but loudly, because it is a gap in the matrix. + logger.warning( + "%s: dropping physics=%s renderer=%s presets=%s: %s", task_id, physics_name, renderer, domain, exc + ) + continue + if resolution is None: + continue + key, backend = resolution + mode = DiscoveredTask.Mode(physics=physics_name, renderer=renderer, presets=domain) + validated.append(mode) + if (physics_name, renderer, domain) == (None, None, None): + default_key, default_backend = key, backend + # Keep the most explicit spelling of each run: naming the presets reproduces + # it even if the config's own defaults move later. + incumbent = unique.get(key) + if incumbent is None or _explicitness(mode) > _explicitness(incumbent): + unique[key] = mode + + default = None + if default_key is not None: + default = DiscoveredTask.Default(backend=default_backend, mode=unique[default_key]) + return tuple(unique.values()) if collapse else tuple(validated), default + + +def _explicitness(mode: DiscoveredTask.Mode) -> int: + """Return how many selector tokens *mode* spells out.""" + return sum(token is not None for token in (mode.physics, mode.renderer, mode.presets)) + + +def discover_tasks( + specs: list[Any] | None = None, *, resolve: bool = True, strict: bool = False, collapse: bool = True +) -> list[DiscoveredTask]: """Walk the Gym registry and return every registered training task. Imports Isaac Lab, so it needs the project environment. Contrib tasks are included @@ -319,12 +412,22 @@ def discover_tasks(specs: list[Any] | None = None, *, resolve: bool = True) -> l the runtime validator, and only combinations that can actually run are returned. When ``False``, combinations are reported as declared, which is fast but unverified. + strict: When ``True``, a task the validator cannot run against at all raises + instead of being logged and skipped. Off by default so that one broken + task costs the caller that task rather than the whole registry; turn it + on to police Isaac Lab API drift. + collapse: When ``True`` (the default), spellings that resolve to the same + config are reduced to one, so ``modes`` is the list of distinct runs a + dispatcher should schedule. Turn it off to keep every validated spelling — + what documentation needs, since a preset that names the default of its own + axis is still a token a reader can type. Ignored when ``resolve`` is off. Returns: Discovered tasks sorted by ``task_id``. Raises: - DiscoveryError: If the task packages cannot be imported. + DiscoveryError: If the task packages cannot be imported, or, when ``strict``, + if any task could not be inspected. """ import contextlib @@ -345,7 +448,7 @@ def discover_tasks(specs: list[Any] | None = None, *, resolve: bool = True) -> l tasks: list[DiscoveredTask] = [] for spec in specs: - if not _is_training_task(spec.id) or spec.kwargs.get("deprecated"): + if not is_training_task(spec.id) or spec.kwargs.get("deprecated"): continue # Tasks without an RL entry point (IK, teleop, mimic) are still registered # environments and are reported with an empty ``rl_libraries``. Callers that @@ -353,9 +456,14 @@ def discover_tasks(specs: list[Any] | None = None, *, resolve: bool = True) -> l # than having that policy baked in here. libraries = _rl_libraries_from_kwargs(spec.kwargs) preset_map = enumerate_task_presets(spec.id) - physics = _canonical_physics(tuple(sorted(preset_map.get(PresetTarget.PHYSICS, [])))) if preset_map else () + declared_physics = tuple(sorted(preset_map.get(PresetTarget.PHYSICS, []))) if preset_map else () renderers = tuple(sorted(preset_map.get(PresetTarget.RENDERER, []))) if preset_map else () - domains = _domain_presets(preset_map.get(PresetTarget.DOMAIN, [])) if preset_map else () + domains = ( + _domain_presets(preset_map.get(PresetTarget.DOMAIN, []), declared_physics + renderers) if preset_map else () + ) + modes, default = _build_modes( + spec.id, declared_physics, renderers, domains, resolve=resolve, strict=strict, collapse=collapse + ) tasks.append( DiscoveredTask( task_id=spec.id, @@ -365,13 +473,13 @@ def discover_tasks(specs: list[Any] | None = None, *, resolve: bool = True) -> l None if preset_map is None else { - "physics": tuple(sorted(preset_map.get(PresetTarget.PHYSICS, []))), + "physics": declared_physics, "renderer": renderers, "presets": tuple(sorted(preset_map.get(PresetTarget.DOMAIN, []))), } ), - selectors=_selector_names(spec.id), - modes=_build_modes(spec.id, physics, renderers, domains, resolve=resolve), + modes=modes, + default=default, resolved=resolve, ) ) diff --git a/tools/test/test_task_discovery.py b/tools/test/test_task_discovery.py index 30d3fbcd7b28..a87d0c2e58c8 100644 --- a/tools/test/test_task_discovery.py +++ b/tools/test/test_task_discovery.py @@ -26,13 +26,14 @@ def _bootstrap_paths() -> None: _bootstrap_paths() +import task_discovery # noqa: E402 from task_discovery import ( # noqa: E402 DiscoveredTask, + DiscoveryError, _build_modes, - _canonical_physics, _domain_presets, - _is_training_task, _rl_libraries_from_kwargs, + is_training_task, ) Mode = DiscoveredTask.Mode @@ -68,36 +69,193 @@ def test_rl_libraries_are_read_from_entry_point_stems(kwargs: dict, expected: tu ], ) def test_only_trainable_isaac_tasks_are_walked(task_id: str, expected: bool) -> None: - assert _is_training_task(task_id) is expected + assert is_training_task(task_id) is expected -def test_proxy_physics_variants_are_dropped() -> None: - assert _canonical_physics(("newton_mjwarp", "newton_mjwarp_vbd_proxy")) == ("newton_mjwarp",) +def test_domain_presets_drop_names_the_task_also_declares_as_a_typed_selector() -> None: + # Reachable as ``physics=ovphysx`` / ``renderer=ovrtx``, so reporting them again + # as ``presets=`` tokens would double-count the same run. + names = ["rgb", "ovphysx", "depth", "ovrtx"] + assert _domain_presets(names, ("ovphysx", "ovrtx")) == ("depth", "rgb") -def test_the_physx_selector_is_reported_alongside_concrete_backends() -> None: - # Whether a selector duplicates a concrete backend depends on how the run is - # launched, so the decision belongs to the caller, not to discovery. - assert _canonical_physics(("isaacsim_physx", "ovphysx", "physx")) == ("isaacsim_physx", "ovphysx", "physx") +def test_a_backend_exposed_only_as_a_domain_preset_is_kept() -> None: + # Isaac-Open-Drawer-Franka buckets its backends under DOMAIN because their cfg + # classes do not subclass PhysicsCfg, so ``presets=newton_mjwarp`` is the only + # way to select them and ``physics=newton_mjwarp`` is rejected. Dropping them by + # name left the task reporting one mode carrying no tokens, hiding four runs. + names = ["isaacsim_physx", "newton_kamino", "newton_mjwarp", "ovphysx", "physx"] -def test_domain_presets_drop_names_that_mirror_a_backend_selector() -> None: - assert _domain_presets(["rgb", "ovphysx", "depth", "newton_mjwarp"]) == ("depth", "rgb") + assert _domain_presets(names, ()) == tuple(sorted(names)) -def test_modes_are_the_cross_product_when_resolution_is_skipped() -> None: - modes = _build_modes("Isaac-X", ("physx", "newton_mjwarp"), ("ovrtx",), (), resolve=False) +def test_modes_are_the_raw_cross_product_when_resolution_is_skipped() -> None: + # Nothing has been resolved, so nothing can be collapsed and there is no default. + modes, default = _build_modes("Isaac-X", ("physx", "newton_mjwarp"), ("ovrtx",), (), resolve=False) assert modes == (Mode("physx", "ovrtx", None), Mode("newton_mjwarp", "ovrtx", None)) + assert default is None def test_a_task_without_presets_gets_one_mode_carrying_no_tokens() -> None: - assert _build_modes("Isaac-X", (), (), (), resolve=False) == (Mode(None, None, None),) + assert _build_modes("Isaac-X", (), (), (), resolve=False)[0] == (Mode(None, None, None),) def test_domain_presets_are_expanded_one_at_a_time_beside_the_default() -> None: # Presets targeting the same field conflict, so they are never combined; the # ``None`` entry keeps the task's own default reachable. - modes = _build_modes("Isaac-X", (), (), ("rgb", "depth"), resolve=False) + modes, _ = _build_modes("Isaac-X", (), (), ("rgb", "depth"), resolve=False) assert modes == (Mode(None, None, None), Mode(None, None, "rgb"), Mode(None, None, "depth")) + + +def _resolver(runs: dict[tuple[str | None, str | None, str | None], tuple[str, str]]): + """Return a ``_mode_resolves`` stand-in driven by a combination -> run table.""" + + def fake(task_id, physics, renderer, presets=None): + return runs.get((physics, renderer, presets)) + + return fake + + +def test_spellings_that_resolve_to_the_same_run_collapse_to_one_mode(monkeypatch) -> None: + # Isaac-Open-Drawer-Franka in miniature: passing nothing lands on the same config + # as naming the preset the task already defaults to, and the ``physx`` alias lands + # on the same config as the concrete backend it resolves to. + monkeypatch.setattr( + task_discovery, + "_mode_resolves", + _resolver( + { + (None, None, None): ("fp-physx", "PhysxCfg"), + (None, None, "isaacsim_physx"): ("fp-physx", "PhysxCfg"), + (None, None, "ovphysx"): ("fp-ov", "OvPhysxCfg"), + (None, None, "physx"): ("fp-ov", "OvPhysxCfg"), + (None, None, "newton_mjwarp"): ("fp-newton", "NewtonCfg(MJWarpSolverCfg)"), + } + ), + ) + + modes, default = _build_modes( + "Isaac-X", (), (), ("isaacsim_physx", "newton_mjwarp", "ovphysx", "physx"), resolve=True + ) + + # Five spellings, three distinct runs. + assert modes == ( + Mode(None, None, "isaacsim_physx"), + Mode(None, None, "newton_mjwarp"), + Mode(None, None, "ovphysx"), + ) + assert default == DiscoveredTask.Default(backend="PhysxCfg", mode=Mode(None, None, "isaacsim_physx")) + + +def test_runs_sharing_a_backend_but_not_a_config_stay_separate(monkeypatch) -> None: + # Isaac-Reach-Franka's controller presets all run on Newton MJWarp and are four + # different runs, so collapsing on the backend instead of the config is wrong. + monkeypatch.setattr( + task_discovery, + "_mode_resolves", + _resolver( + { + (None, None, None): ("fp-joint", "NewtonCfg(MJWarpSolverCfg)"), + (None, None, "joint_pos"): ("fp-joint", "NewtonCfg(MJWarpSolverCfg)"), + (None, None, "diffik"): ("fp-diffik", "NewtonCfg(MJWarpSolverCfg)"), + } + ), + ) + + modes, default = _build_modes("Isaac-X", (), (), ("joint_pos", "diffik"), resolve=True) + + assert modes == (Mode(None, None, "joint_pos"), Mode(None, None, "diffik")) + assert default.mode == Mode(None, None, "joint_pos") + + +def test_a_default_that_matches_no_named_preset_survives_as_its_own_mode(monkeypatch) -> None: + monkeypatch.setattr( + task_discovery, + "_mode_resolves", + _resolver( + { + (None, None, None): ("fp-own", "PhysxCfg"), + (None, None, "rgb"): ("fp-rgb", "PhysxCfg"), + } + ), + ) + + modes, default = _build_modes("Isaac-X", (), (), ("rgb",), resolve=True) + + assert modes == (Mode(None, None, None), Mode(None, None, "rgb")) + assert default == DiscoveredTask.Default(backend="PhysxCfg", mode=Mode(None, None, None)) + + +def _validator_broken_on(backend: str): + """Return a ``_mode_resolves`` stand-in that fails structurally on *backend*.""" + + def fake(task_id, physics, renderer, presets=None): + if physics == backend: + raise DiscoveryError("AttributeError: no attribute 'solver_cfg'") + return ("fp", "PhysxCfg") + + return fake + + +def test_a_combination_the_validator_cannot_judge_is_dropped_and_the_walk_continues(monkeypatch) -> None: + # One task whose config breaks the validator must not cost the caller every + # other task in the registry. Unknown is not legal either, so the combination + # is dropped rather than reported. + monkeypatch.setattr(task_discovery, "_mode_resolves", _validator_broken_on("newton_mjwarp")) + + modes, _ = _build_modes("Isaac-X", ("physx", "newton_mjwarp"), (), (), resolve=True) + + assert modes == (Mode("physx", None, None),) + + +def test_strict_raises_on_a_structural_failure_instead_of_dropping_it(monkeypatch) -> None: + # Callers policing Isaac Lab API drift want the canary, not a survivable walk. + monkeypatch.setattr(task_discovery, "_mode_resolves", _validator_broken_on("newton_mjwarp")) + + with pytest.raises(DiscoveryError): + _build_modes("Isaac-X", ("physx", "newton_mjwarp"), (), (), resolve=True, strict=True) + + +def test_uncollapsed_mode_keeps_every_validated_spelling(monkeypatch) -> None: + # A preset naming the default of its own axis resolves to the default config, so + # the collapse drops it -- but it is still a token a reader can type, so the + # documentation view has to keep it. + monkeypatch.setattr( + task_discovery, + "_mode_resolves", + _resolver( + { + (None, None, None): ("fp-default", "PhysxCfg"), + (None, None, "shapes"): ("fp-default", "PhysxCfg"), + (None, None, "cube"): ("fp-cube", "PhysxCfg"), + } + ), + ) + + collapsed, _ = _build_modes("Isaac-X", (), (), ("shapes", "cube"), resolve=True) + every, default = _build_modes("Isaac-X", (), (), ("shapes", "cube"), resolve=True, collapse=False) + + assert collapsed == (Mode(None, None, "shapes"), Mode(None, None, "cube")) + assert every == (Mode(None, None, None), Mode(None, None, "shapes"), Mode(None, None, "cube")) + # The default still names an explicit spelling, not the bare no-token mode. + assert default == DiscoveredTask.Default(backend="PhysxCfg", mode=Mode(None, None, "shapes")) + + +def test_uncollapsed_mode_still_drops_combinations_the_validator_rejects(monkeypatch) -> None: + monkeypatch.setattr( + task_discovery, + "_mode_resolves", + _resolver( + { + (None, None, None): ("fp-default", "PhysxCfg"), + (None, None, "rgb"): ("fp-rgb", "PhysxCfg"), + } + ), + ) + + every, _ = _build_modes("Isaac-X", (), (), ("rgb", "raycaster_depth"), resolve=True, collapse=False) + + assert Mode(None, None, "raycaster_depth") not in every From 2b7eb9f3ec0bed4b8d96cdaa0cabfe5377943578 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 19 Aug 2026 11:33:57 +0200 Subject: [PATCH 05/10] Trim task discovery to its contract The module was 49% prose: 240 lines of docstring and comment against 201 of code, much of it explaining bugs that no longer exist. That history belongs in the commit log, not in every reader's way -- _domain_presets carried 25 lines of docstring over 2 lines of code. Keep the contract and the non-obvious constraints: the headless-launch assumption, that presets compose across config paths but are validated one at a time, and why the collapse keys on the resolved config rather than the backend. Drop the narratives. Also folds the two single-use helpers into their call sites; the explicitness score now rides in the dedup map instead of being recomputed. --- tools/task_discovery.py | 310 +++++++++++++++------------------------- 1 file changed, 115 insertions(+), 195 deletions(-) diff --git a/tools/task_discovery.py b/tools/task_discovery.py index a66fdf26e9cc..e6e5ecedf633 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -5,31 +5,22 @@ """Enumerate registered training tasks and the backend combinations they support. -Two questions get asked of the Gym registry, and they do not have the same answer: - -* What does a task **declare**? Reading :func:`~isaaclab_tasks.utils.preset_cli.enumerate_task_presets` - is fast and is what the environment documentation reports. -* What does a task actually **resolve**? Building the config and running the runtime - validator is slow, but it is the only way to know a combination can run. The cross - product is not all legal: OVRTX is kitless and cannot share a process with Kit - physics, so ``isaacsim_physx + ovrtx`` is declared yet unusable. - -:func:`discover_tasks` answers either, selected with ``resolve``. Declared mode costs -one registry walk; resolved mode additionally costs one config resolution per -combination, which is minutes for the full registry but far cheaper than finding out -on a GPU. - -Resolving buys more than a legality check. Because each combination is resolved all -the way to an env config, combinations can be compared on what they *produce* rather -than on how they were spelled, and the ones that produce the same run collapse. That -is what makes ``modes`` a list of distinct runs: ``physics=physx`` folds into whatever -concrete backend it resolves to, and passing no tokens at all folds into whichever -preset the config already defaults to. Aliases need no table of names, and a -dispatcher can run every mode without repeating work. What the task does when given -no tokens is kept separately as ``default`` — the collapse would otherwise hide it. - -The gap between declared and resolved is itself useful: a combination that is declared -but does not resolve is documentation drift. +Two questions get asked of the Gym registry, and they have different answers: + +* What does a task **declare**? One registry walk. Fast, and unverified. +* What does a task actually **resolve**? One config build and validator run per + combination -- minutes for the full registry, but the only way to know a + combination can run. The cross product is not all legal: OVRTX is kitless and + cannot share a process with Kit physics, so ``isaacsim_physx + ovrtx`` is + declared yet unusable. + +:func:`discover_tasks` answers either, selected with ``resolve``. + +Resolving also identifies each run by the config it produces rather than by how it +was spelled, so spellings that produce the same run collapse: ``physics=physx`` +folds into the concrete backend it resolves to, and passing nothing folds into +whichever preset the config already defaults to. That last run is reported +separately as ``default``, which the collapse would otherwise hide. """ from __future__ import annotations @@ -53,17 +44,15 @@ # ordering, since :data:`~environ_docs.RL_LIBRARY_OVERRIDES` supplies it. RL_LIBRARY_PRIORITY: tuple[str, ...] = ("rl_games", "rsl_rl", "skrl", "sb3", "rlinf") -# Gym IDs excluded from the training list. The ``-Eval`` suffix marks dedicated -# evaluation variants (e.g. ``IsaacContrib-Assemble-Trocar-G129-Dex3-Eval``, an alias -# registered for RLinf eval configs) that should not appear as their own training row. +# ``-Eval`` marks dedicated evaluation variants registered as aliases, which should +# not appear as their own training row. _EVAL_TASK_SUFFIXES = ("-Eval",) -# Errors that mean the validator itself could not run, rather than that the -# combination under test was rejected. They are logged and the combination is -# dropped, so one broken task costs the caller that task and not the whole walk; -# ``strict=True`` re-raises instead, for callers policing Isaac Lab API drift. -# ``TypeError`` is included because calling the validator with the wrong argument -# type is otherwise indistinguishable from a rejected combination. +# Errors meaning the validator itself could not run, rather than that the combination +# under test was rejected. Swallowing them would mark every combination illegal, so +# they are logged and the combination dropped; ``strict=True`` re-raises instead. +# ``TypeError`` is included because calling the validator with the wrong argument type +# is otherwise indistinguishable from a rejected combination. _INFRASTRUCTURE_ERRORS = (ImportError, AttributeError, NameError, SyntaxError, TypeError) @@ -78,34 +67,26 @@ class DiscoveredTask: Args: task_id: Gym task id. scope: ``core`` or ``contrib``. - rl_libraries: RL libraries the task declares, in :data:`RL_LIBRARY_PRIORITY` - order. Empty for registered environments with no RL entry point, such as - IK, teleop and mimic tasks. + rl_libraries: RL libraries the task declares an agent config for, in + :data:`RL_LIBRARY_PRIORITY` order. Empty for registered environments with + no RL entry point, such as IK, teleop and mimic tasks. declared: Preset names the task declares, keyed by axis (``physics``, - ``renderer``, ``presets``), exactly as the registry reports them, or - ``None`` when the config could not be loaded at all. ``None`` and an - all-empty mapping are different answers: the first means unknown, the - second means the task declares nothing and runs on a fixed backend. - Nothing is filtered out: a backend a task exposes on both a typed axis - and as a ``presets=`` token appears under both, and aliases such as - ``physx`` are present alongside what they resolve to. Use ``modes`` for - a deduplicated answer; ``declared`` is the unreconciled registry view. - modes: Ways to run the task. In resolved mode these passed the runtime - validator; with ``collapse`` they are further reduced so that two token - spellings producing the same resolved config appear once — no aliases, no - double-counting. Without it every validated spelling is kept, which is what - a table of "what can I pass?" needs. In declared mode they are the raw - cross product, unverified and uncollapsed. - - Validation assumes a **headless launch**: no ``--visualizer``, no - ``--livestream``, no ``--experience`` and no ``--require_kit``. Each of - those is a Kit source, so adding one narrows the legal set — a kitless - OvPhysX combination that passes here is rejected under ``--visualizer kit``. - default: What the task does when the user passes no preset tokens, or ``None`` - in declared mode. Reported separately because the collapse folds the - no-token run into whichever named mode it matches, and a table still wants - to say what you get if you change nothing. - resolved: Whether ``modes`` was resolved and collapsed, or merely declared. + ``renderer``, ``presets``) exactly as the registry reports them, or ``None`` + when the config could not be loaded. ``None`` means unknown; an all-empty + mapping means the task declares nothing and runs on a fixed backend. + Unreconciled: aliases and cross-axis duplicates are all present. Use + ``modes`` for a deduplicated answer. + modes: Ways to run the task. In resolved mode these passed the validator, and + with ``collapse`` are reduced so each is a distinct run rather than a + distinct spelling; without it every validated spelling is kept. In declared + mode they are the raw cross product, unverified. + + Validation assumes a **headless launch** -- no ``--visualizer``, + ``--livestream``, ``--experience`` or ``--require_kit``. Each is a Kit + source, so adding one narrows the legal set. + default: What the task does when given no preset tokens, or ``None`` in + declared mode. + resolved: Whether ``modes`` was resolved, or merely declared. """ @dataclass(frozen=True) @@ -113,17 +94,14 @@ class Mode: """One way to run a task. Args: - physics: Physics preset token, or ``None`` for tasks that declare none - and reject any ``physics=`` selector. + physics: Physics preset token, or ``None`` for tasks declaring none. renderer: Renderer preset token, or ``None`` to run headless. presets: Domain preset token passed as ``presets=``, or ``None``. - Never more than one. ``presets=`` does accept a comma-separated list, - and names on *different* config paths compose fine — on - ``Isaac-Lift-KukaAllegro-Camera``, ``presets=duo_camera,depth128,cube`` - sets the camera count, the modality and the object independently. Only - names sharing a path conflict, e.g. ``presets=depth,rgb``. Discovery - validates each name on its own and never tries a pair, so ``modes`` - under-approximates a task with several independent preset axes. + Never more than one. ``presets=`` does take a comma-separated list and + names on different config paths compose (``duo_camera,depth128,cube``); + only names sharing a path conflict (``depth,rgb``). Discovery validates + each name alone and never tries a pair, so ``modes`` under-approximates + a task with several independent preset axes. """ physics: str | None @@ -135,11 +113,10 @@ class Default: """The run a task performs when given no preset tokens. Args: - backend: Concrete physics config the run resolves to, e.g. ``PhysxCfg`` - or ``NewtonCfg(MJWarpSolverCfg)``. Reported as the config class rather - than a preset name because a task's default need not have one. - mode: The entry in ``modes`` this run collapsed into — the explicit way to - ask for the same thing. + backend: Concrete physics config the run resolves to, e.g. ``PhysxCfg`` or + ``NewtonCfg(MJWarpSolverCfg)``. Reported as the config class because a + task's default need not have a preset name. + mode: The entry in ``modes`` this run collapsed into. """ backend: str | None @@ -155,30 +132,14 @@ class Default: def _domain_presets(names: list[str], typed_names: tuple[str, ...]) -> tuple[str, ...]: - """Return domain presets, dropping the ones that mirror a typed selector. - - Whether a backend lands under ``PresetTarget.DOMAIN`` or under ``PHYSICS`` / - ``RENDERER`` depends on whether its cfg class subclasses ``PhysicsCfg`` / - ``RendererCfg``, so the same name means different things on different tasks: - - * Also declared on a typed axis — reachable as ``physics=NAME``, so reporting it - again as ``presets=NAME`` double-counts one run. Dropped. - * Not declared on a typed axis — ``presets=NAME`` is the *only* way to select - that backend, and ``physics=NAME`` is rejected outright. Kept. - - Deciding by name instead of per task gets the second case backwards and silently - hides every backend such a task has (``Isaac-Open-Drawer-Franka`` has five). - - A backend name surviving here can also mean the task is inconsistent. Shared - configs pair a backend with *companion* overrides under the same name -- - ``velocity_env_cfg`` sets ``events.base_com=None`` under ``newton_mjwarp``, - because Newton does not support that randomization. The companion is normally - invisible: it rides along with the ``newton_mjwarp`` already on the physics axis - and is dropped as a mirror. It only shows up as a standalone ``presets=`` token - on a task that inherited the companion without offering the backend, where - selecting it applies a Newton workaround to a PhysX run. Reporting it is correct - -- the token is real and does change the config -- and it is worth reading as a - signal to fix the task. + """Return domain presets, dropping those the task also declares on a typed axis. + + A backend buckets under ``DOMAIN`` or under ``PHYSICS`` / ``RENDERER`` depending on + whether its cfg class subclasses ``PhysicsCfg`` / ``RendererCfg``, so the same name + means different things on different tasks. Declared on a typed axis too, it is + reachable as ``physics=NAME`` and reporting it again double-counts one run; declared + only here, ``presets=NAME`` is the sole way to select it and ``physics=NAME`` is + rejected. Deciding by name rather than per task hides every backend of the latter. """ typed = set(typed_names) return tuple(sorted(name for name in names if name not in typed)) @@ -196,9 +157,8 @@ def is_training_task(task_id: str) -> bool: def _rl_libraries_from_kwargs(kwargs: dict[str, Any]) -> tuple[str, ...]: """Return the RL libraries a registration declares an agent config for. - Entry points are matched on the stem before ``_cfg_entry_point`` so that - variants such as ``rsl_rl_recurrent_cfg_entry_point`` count towards their - library rather than being dropped. + Matched on the stem before ``_cfg_entry_point`` so variants such as + ``rsl_rl_recurrent_cfg_entry_point`` count towards their library. """ declared = set() for key in kwargs: @@ -215,18 +175,15 @@ def _rl_libraries_from_kwargs(kwargs: dict[str, Any]) -> tuple[str, ...]: def _mode_resolves( task_id: str, physics: str | None, renderer: str | None, presets: str | None = None ) -> tuple[str, str | None] | None: - """Resolve one physics/renderer/preset combination and identify the run it produces. - - An unknown preset, an unloadable config, or a rejected backend combination all - mean the same thing — the combination cannot run — so they return ``None`` alike. + """Resolve one combination and identify the run it produces. Returns: - ``None`` when the combination cannot run, else ``(fingerprint, backend)``. - *fingerprint* digests the fully resolved env config, so two token spellings - that produce the same run share it — ``presets=physx`` and ``presets=ovphysx`` - on the cabinet tasks, or passing nothing at all and naming the preset the - config already defaults to. *backend* names the concrete physics config the - run ends up with, e.g. ``PhysxCfg`` or ``NewtonCfg(MJWarpSolverCfg)``. + ``None`` when the combination cannot run -- an unknown preset, an unloadable + config and a rejected backend pairing are the same answer. Otherwise + ``(fingerprint, backend)``, where *fingerprint* digests the resolved env config + so two spellings of one run share it, and *backend* names the concrete physics + config, e.g. ``NewtonCfg(MJWarpSolverCfg)``. The solver is included because it + is what separates ``newton_mjwarp`` from ``newton_kamino``. Raises: DiscoveryError: If validation could not run at all, e.g. because an Isaac Lab @@ -256,17 +213,18 @@ def _mode_resolves( args, remaining = setup_preset_cli(parser, argv) sys.argv = [sys.argv[0]] + remaining env_cfg, _ = resolve_task_config(args.task, args.agent) - # ``_validate_runtime`` takes the resolved Kit sources, not the parsed args. - # Passing args makes every scan look Kit-backed, which fires the OvPhysX - # guard for every OvPhysX combination and marks them all unusable. config_scan = scan(env_cfg, args) _validate_runtime(config_scan, _get_kit_runtime_sources(config_scan, args)) fingerprint = hashlib.sha256(repr(env_cfg.to_dict()).encode()).hexdigest() - return fingerprint, _backend_name(config_scan.resolved_physics_cfg) + physics_cfg = config_scan.resolved_physics_cfg + solver = getattr(physics_cfg, "solver_cfg", None) + backend = None if physics_cfg is None else type(physics_cfg).__name__ + if solver is not None: + backend = f"{backend}({type(solver).__name__})" + return fingerprint, backend except ImportError: - # The task's config needs an extra that is not installed, so this - # combination cannot run in this environment. That is the same answer as a - # rejected combination, and it keeps discovery usable from a partial install. + # The config needs an extra that is not installed, so the combination cannot + # run here. Same answer as a rejection, and it keeps a partial install usable. return None except _INFRASTRUCTURE_ERRORS as exc: raise DiscoveryError( @@ -279,19 +237,6 @@ def _mode_resolves( sys.argv = original_argv -def _backend_name(physics_cfg: Any) -> str | None: - """Name the concrete physics config a run resolved to, solver included. - - Newton's solver lives on ``solver_cfg`` and is what separates ``newton_mjwarp`` - from ``newton_kamino``; the class name alone reports both as ``NewtonCfg``. - """ - if physics_cfg is None: - return None - solver = getattr(physics_cfg, "solver_cfg", None) - name = type(physics_cfg).__name__ - return f"{name}({type(solver).__name__})" if solver is not None else name - - def _build_modes( task_id: str, physics: tuple[str, ...], @@ -304,38 +249,24 @@ def _build_modes( ) -> tuple[tuple[DiscoveredTask.Mode, ...], DiscoveredTask.Default | None]: """Return the runs for one task, and what it does when given no tokens. - A task declaring renderers is expanded across them: reporting a camera task as - headless-only omits the thing under test. Domain presets are expanded one at a - time and never combined, so every preset name is validated on its own but no pair - is ever tried (see :class:`DiscoveredTask.Mode`). - - With ``collapse``, the cross product is deduplicated on the resolved config, so - each returned mode is a distinct run rather than a distinct spelling. That is what - removes selector double-counting without a table of alias names: ``physics=physx`` - and ``physics=ovphysx`` collapse wherever they resolve alike and stay separate - wherever they do not. Collapsing on the *backend* instead would be wrong — the - Reach controller presets share a backend and are four different runs. - - Without ``collapse``, every combination that validated is returned, duplicate - spellings included. That is what documentation wants: ``presets=shapes`` is a real - token a reader can type even on a task where it happens to name the default of its - own axis, and collapsing would delete it from the table. + Renderers are expanded across, since reporting a camera task as headless-only omits + the thing under test. Domain presets are expanded one at a time, never combined. - Declared mode neither validates nor collapses, because nothing has been resolved: - it returns the raw cross product and no default. + ``collapse`` deduplicates on the resolved config, so each mode is a distinct run. + Collapsing on the *backend* instead would be wrong -- the Reach controller presets + share a backend and are four different runs. Without it every validated spelling is + kept, which is what documentation needs: a preset naming the default of its own axis + is still a token a reader can type. Returns: - ``(modes, default)``. *default* is the run the task performs when the user - passes nothing, or ``None`` in declared mode / when that run cannot resolve. - It names an explicit spelling even when ``collapse`` is off. + ``(modes, default)``. *default* names an explicit spelling even when ``collapse`` + is off, and is ``None`` in declared mode or when the no-token run cannot resolve. Raises: DiscoveryError: If ``strict`` and the validator could not judge a combination. """ physics_options: tuple[str | None, ...] = physics or (None,) renderer_options: tuple[str | None, ...] = renderers or (None,) - # ``None`` is the task's own default. It survives the collapse only when it is a - # run of its own; usually it folds into whichever preset the config defaults to. domain_options: tuple[str | None, ...] = (None, *domains) if domains else (None,) if not resolve: @@ -350,15 +281,15 @@ def _build_modes( ) combinations = [(p, r, d) for p in physics_options for r in renderer_options for d in domain_options] - # The no-token run is what a user gets by changing nothing, so it always has to be - # probed. It is not always in the cross product: a task declaring physics presets - # has no ``physics=None`` column, only its declared backends. + # The no-token run always has to be probed, and is not always in the cross product: + # a task declaring physics presets has no ``physics=None`` column. if (None, None, None) not in combinations: combinations.insert(0, (None, None, None)) - # ``unique`` is built either way: even when every validated mode is returned, it is - # what identifies the explicit spelling of the no-token run. - unique: dict[str, DiscoveredTask.Mode] = {} + # Keyed by fingerprint, holding the most explicit spelling seen of each run -- naming + # the presets reproduces it even if the config's own defaults move later. Built even + # when uncollapsed, since it identifies the spelling of the no-token run. + unique: dict[str, tuple[int, DiscoveredTask.Mode]] = {} validated: list[DiscoveredTask.Mode] = [] default_key: str | None = None default_backend: str | None = None @@ -368,8 +299,8 @@ def _build_modes( except DiscoveryError as exc: if strict: raise - # Unknown is not the same answer as legal, so the combination is dropped - # -- but loudly, because it is a gap in the matrix. + # Unknown is not the same answer as legal, so drop it -- but loudly, + # because it is a gap in the matrix. logger.warning( "%s: dropping physics=%s renderer=%s presets=%s: %s", task_id, physics_name, renderer, domain, exc ) @@ -381,21 +312,16 @@ def _build_modes( validated.append(mode) if (physics_name, renderer, domain) == (None, None, None): default_key, default_backend = key, backend - # Keep the most explicit spelling of each run: naming the presets reproduces - # it even if the config's own defaults move later. + explicitness = sum(token is not None for token in (physics_name, renderer, domain)) incumbent = unique.get(key) - if incumbent is None or _explicitness(mode) > _explicitness(incumbent): - unique[key] = mode + if incumbent is None or explicitness > incumbent[0]: + unique[key] = (explicitness, mode) default = None if default_key is not None: - default = DiscoveredTask.Default(backend=default_backend, mode=unique[default_key]) - return tuple(unique.values()) if collapse else tuple(validated), default - - -def _explicitness(mode: DiscoveredTask.Mode) -> int: - """Return how many selector tokens *mode* spells out.""" - return sum(token is not None for token in (mode.physics, mode.renderer, mode.presets)) + default = DiscoveredTask.Default(backend=default_backend, mode=unique[default_key][1]) + modes = tuple(mode for _, mode in unique.values()) if collapse else tuple(validated) + return modes, default def discover_tasks( @@ -408,26 +334,23 @@ def discover_tasks( Args: specs: Gym specs to walk. When ``None``, the whole registry is scanned. - resolve: When ``True``, every backend combination is built and checked against - the runtime validator, and only combinations that can actually run are - returned. When ``False``, combinations are reported as declared, which is - fast but unverified. - strict: When ``True``, a task the validator cannot run against at all raises - instead of being logged and skipped. Off by default so that one broken - task costs the caller that task rather than the whole registry; turn it - on to police Isaac Lab API drift. - collapse: When ``True`` (the default), spellings that resolve to the same - config are reduced to one, so ``modes`` is the list of distinct runs a - dispatcher should schedule. Turn it off to keep every validated spelling — - what documentation needs, since a preset that names the default of its own - axis is still a token a reader can type. Ignored when ``resolve`` is off. + resolve: When ``True``, every combination is built and checked against the + runtime validator and only usable ones are returned. When ``False``, + combinations are reported as declared: fast but unverified. + strict: When ``True``, a task the validator cannot run against raises instead of + being logged and skipped. Off by default so one broken task costs the caller + that task rather than the whole registry; turn it on to police API drift. + collapse: When ``True`` (the default), spellings resolving to the same config are + reduced to one, so ``modes`` is the distinct runs a dispatcher should + schedule. Turn it off to keep every validated spelling, which is what + documentation needs. Ignored when ``resolve`` is off. Returns: Discovered tasks sorted by ``task_id``. Raises: - DiscoveryError: If the task packages cannot be imported, or, when ``strict``, - if any task could not be inspected. + DiscoveryError: If the task packages cannot be imported, or, when ``strict``, if + any task could not be inspected. """ import contextlib @@ -450,11 +373,8 @@ def discover_tasks( for spec in specs: if not is_training_task(spec.id) or spec.kwargs.get("deprecated"): continue - # Tasks without an RL entry point (IK, teleop, mimic) are still registered - # environments and are reported with an empty ``rl_libraries``. Callers that - # need a trainable task — a dispatcher, say — filter on it themselves rather - # than having that policy baked in here. - libraries = _rl_libraries_from_kwargs(spec.kwargs) + # Tasks with no RL entry point (IK, teleop, mimic) are still registered + # environments; callers needing a trainable task filter on ``rl_libraries``. preset_map = enumerate_task_presets(spec.id) declared_physics = tuple(sorted(preset_map.get(PresetTarget.PHYSICS, []))) if preset_map else () renderers = tuple(sorted(preset_map.get(PresetTarget.RENDERER, []))) if preset_map else () @@ -468,7 +388,7 @@ def discover_tasks( DiscoveredTask( task_id=spec.id, scope="contrib" if spec.id.startswith("IsaacContrib-") else "core", - rl_libraries=libraries, + rl_libraries=_rl_libraries_from_kwargs(spec.kwargs), declared=( None if preset_map is None From 537200531df727fe1d788890c49a28018a91cf79 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 19 Aug 2026 12:12:39 +0200 Subject: [PATCH 06/10] Close the review findings on task discovery Sort resolution failures by what they mean instead of by which builtin they happened to raise. Discovery's own imports -- two of them private sim_launcher symbols -- now sit in their own try and raise DiscoveryError, so losing one is reported as the API drift it is rather than escaping as a bare ImportError past the per-combination handler and aborting the whole walk. That was the exact failure strict= was added to make survivable, and it was not covered. A missing extra is now caught as ModuleNotFoundError rather than ImportError, which makes the ImportError entry in _INFRASTRUCTURE_ERRORS reachable and gives it a meaning: a module that imports but no longer exports what a config asks for is drift, not a rejection. Previously that entry was dead and its comment described the opposite of the behaviour. A task whose config will not load now reports no modes. It reported declared=None ("nothing is known") beside one all-None mode ("runs on its defaults"), which contradict each other; a cross product of nothing known is not a cross product of nothing declared. No task in a complete install hits this, but a partial one does. Drop the field defaults on DiscoveredTask. They allowed a default-constructed instance whose declared={} raises KeyError in the docs generator, and the one construction site passes every field anyway. Documentation corrections, all of them claims that did not survive checking: Mode.physics/renderer None means "no token passed", not "declares none"/"headless"; only --visualizer kit is a Kit source, and the headless assumption is not hermetic because LIVESTREAM is read from the environment and validation stops before the Isaac Sim availability check; contrib tasks come from isaaclab_tasks, not from the experimental package; -Eval tasks are separate registrations, not aliases; strict= drops a combination, not a task; only the RL library ordering is shared with the docs generator, so rlinf can differ between the two. Add resolution tests against real configs, kept in their own module so the unit tests stay runnable on pytest alone, and wire that unit module into tools-tests -- it was never running in CI, having been excluded as needing a full Isaac Lab install, which is not true of it. --- .github/workflows/tools-tests.yml | 8 +- tools/task_discovery.py | 83 ++++++++----- tools/test/test_task_discovery.py | 3 +- tools/test/test_task_discovery_resolve.py | 141 ++++++++++++++++++++++ 4 files changed, 205 insertions(+), 30 deletions(-) create mode 100644 tools/test/test_task_discovery_resolve.py diff --git a/.github/workflows/tools-tests.yml b/.github/workflows/tools-tests.yml index 1c217268b708..05eea14d0bc8 100644 --- a/.github/workflows/tools-tests.yml +++ b/.github/workflows/tools-tests.yml @@ -51,9 +51,13 @@ jobs: # The tests import the modules under test (crash_journal, _device_split) directly. PYTHONPATH: tools # Files are listed explicitly rather than collected from tools/: test_settings.py is a - # configuration module, not a test, and tools/test/ needs the full Isaac Lab install. + # configuration module, not a test, and most of tools/test/ needs the full Isaac Lab + # install. test_task_discovery.py is the exception - task_discovery.py defers every + # isaaclab import into a function body, so its unit tests run on pytest alone. # # --noconftest keeps tools/conftest.py out of the session. That file is the CI test # orchestrator - its pytest_sessionstart scans source/ and scripts/ and runs the whole # suite, so loading it here would ignore the files named below. - run: python3 -m pytest tools/test_crash_journal.py tools/test_device_split.py -v --noconftest + run: >- + python3 -m pytest tools/test_crash_journal.py tools/test_device_split.py + tools/test/test_task_discovery.py -v --noconftest diff --git a/tools/task_discovery.py b/tools/task_discovery.py index e6e5ecedf633..c621a6f28613 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -26,7 +26,7 @@ from __future__ import annotations import logging -from dataclasses import dataclass, field +from dataclasses import dataclass from typing import Any logger = logging.getLogger(__name__) @@ -39,20 +39,24 @@ "discover_tasks", ] -# Stable ordering for the RL library axis, shared with the environment tables so the -# two never disagree. ``rlinf`` has no discoverable entry point but is listed for -# ordering, since :data:`~environ_docs.RL_LIBRARY_OVERRIDES` supplies it. +# Stable ordering for the RL library axis, shared with the environment tables. Only the +# ordering is shared: ``environ_docs.RL_LIBRARY_OVERRIDES`` adds libraries that declare +# no entry point, so a task can carry ``rlinf`` in the tables and not in +# :attr:`DiscoveredTask.rl_libraries`. ``rlinf`` is listed here for ordering only. RL_LIBRARY_PRIORITY: tuple[str, ...] = ("rl_games", "rsl_rl", "skrl", "sb3", "rlinf") -# ``-Eval`` marks dedicated evaluation variants registered as aliases, which should -# not appear as their own training row. +# ``-Eval`` marks dedicated evaluation variants, registered separately against an +# eval-specific env cfg, which should not appear as their own training row. +# ``-Benchmark-`` tasks are perf harnesses rather than trainable environments. _EVAL_TASK_SUFFIXES = ("-Eval",) # Errors meaning the validator itself could not run, rather than that the combination # under test was rejected. Swallowing them would mark every combination illegal, so # they are logged and the combination dropped; ``strict=True`` re-raises instead. # ``TypeError`` is included because calling the validator with the wrong argument type -# is otherwise indistinguishable from a rejected combination. +# is otherwise indistinguishable from a rejected combination, and ``ImportError`` +# because a module that imports but no longer exports what a config asks for is drift +# -- a missing extra raises ``ModuleNotFoundError`` and is handled before these. _INFRASTRUCTURE_ERRORS = (ImportError, AttributeError, NameError, SyntaxError, TypeError) @@ -81,11 +85,17 @@ class DiscoveredTask: distinct spelling; without it every validated spelling is kept. In declared mode they are the raw cross product, unverified. - Validation assumes a **headless launch** -- no ``--visualizer``, + Validation assumes a **headless launch** -- no ``--visualizer kit``, ``--livestream``, ``--experience`` or ``--require_kit``. Each is a Kit - source, so adding one narrows the legal set. + source, so adding one narrows the legal set; the kitless visualizers + (``newton``, ``rerun``, ``viser``) are not. The assumption is not hermetic: + Kit sources are also read from the config's own visualizer intent, and from + the ``LIVESTREAM`` environment variable, which silently rejects every + kitless combination when set. It also stops at the validator, so a + Kit-requiring combination is reported runnable even where Isaac Sim is not + installed and the launcher would refuse it. default: What the task does when given no preset tokens, or ``None`` in - declared mode. + declared mode or when that run does not resolve. resolved: Whether ``modes`` was resolved, or merely declared. """ @@ -94,8 +104,12 @@ class Mode: """One way to run a task. Args: - physics: Physics preset token, or ``None`` for tasks declaring none. - renderer: Renderer preset token, or ``None`` to run headless. + physics: Physics preset token, or ``None`` when the run passes no + ``physics=`` token and the config's own default applies. That happens + both for tasks declaring no physics presets and for the probe of a + declaring task's default. + renderer: Renderer preset token, or ``None`` when the run passes no + ``renderer=`` token and the config's own default renderer applies. presets: Domain preset token passed as ``presets=``, or ``None``. Never more than one. ``presets=`` does take a comma-separated list and names on different config paths compose (``duo_camera,depth128,cube``); @@ -125,10 +139,11 @@ class Default: task_id: str scope: str rl_libraries: tuple[str, ...] - declared: dict[str, tuple[str, ...]] | None = field(default_factory=dict) - modes: tuple[DiscoveredTask.Mode, ...] = () - default: DiscoveredTask.Default | None = None - resolved: bool = False + # Mutable, so instances are not hashable despite ``frozen``. Treat as read-only. + declared: dict[str, tuple[str, ...]] | None + modes: tuple[DiscoveredTask.Mode, ...] + default: DiscoveredTask.Default | None + resolved: bool def _domain_presets(names: list[str], typed_names: tuple[str, ...]) -> tuple[str, ...]: @@ -193,9 +208,14 @@ def _mode_resolves( import hashlib import sys - from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, scan + # Discovery's own dependencies, two of them private. Losing one is API drift, never a + # rejected combination, so it must not fall through to the handlers below. + try: + from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, scan - from isaaclab_tasks.utils import resolve_task_config, setup_preset_cli + from isaaclab_tasks.utils import resolve_task_config, setup_preset_cli + except ImportError as exc: + raise DiscoveryError(f"discovery could not import the Isaac Lab APIs it depends on: {exc}") from exc parser = argparse.ArgumentParser() parser.add_argument("--task") @@ -222,9 +242,11 @@ def _mode_resolves( if solver is not None: backend = f"{backend}({type(solver).__name__})" return fingerprint, backend - except ImportError: + except ModuleNotFoundError: # The config needs an extra that is not installed, so the combination cannot # run here. Same answer as a rejection, and it keeps a partial install usable. + # Narrower than ``ImportError`` on purpose: a module that is absent is a missing + # extra, whereas a module that is present but missing a symbol is drift. return None except _INFRASTRUCTURE_ERRORS as exc: raise DiscoveryError( @@ -329,17 +351,19 @@ def discover_tasks( ) -> list[DiscoveredTask]: """Walk the Gym registry and return every registered training task. - Imports Isaac Lab, so it needs the project environment. Contrib tasks are included - when ``isaaclab_tasks_experimental`` is importable. + Imports Isaac Lab, so it needs the project environment. ``isaaclab_tasks`` registers + the core and contrib tasks; ``isaaclab_tasks_experimental`` is imported too when + present, for whatever it registers. Args: specs: Gym specs to walk. When ``None``, the whole registry is scanned. resolve: When ``True``, every combination is built and checked against the runtime validator and only usable ones are returned. When ``False``, combinations are reported as declared: fast but unverified. - strict: When ``True``, a task the validator cannot run against raises instead of - being logged and skipped. Off by default so one broken task costs the caller - that task rather than the whole registry; turn it on to police API drift. + strict: When ``True``, a combination the validator cannot judge raises instead + of being logged and dropped. Off by default so one unjudgeable combination + costs the caller that combination rather than the whole registry -- note the + task is still returned, with that combination missing from ``modes``. collapse: When ``True`` (the default), spellings resolving to the same config are reduced to one, so ``modes`` is the distinct runs a dispatcher should schedule. Turn it off to keep every validated spelling, which is what @@ -381,9 +405,14 @@ def discover_tasks( domains = ( _domain_presets(preset_map.get(PresetTarget.DOMAIN, []), declared_physics + renderers) if preset_map else () ) - modes, default = _build_modes( - spec.id, declared_physics, renderers, domains, resolve=resolve, strict=strict, collapse=collapse - ) + if preset_map is None: + # Nothing is known about this task, and a cross product of nothing known is + # not a cross product of nothing declared -- do not invent a runnable mode. + modes, default = (), None + else: + modes, default = _build_modes( + spec.id, declared_physics, renderers, domains, resolve=resolve, strict=strict, collapse=collapse + ) tasks.append( DiscoveredTask( task_id=spec.id, diff --git a/tools/test/test_task_discovery.py b/tools/test/test_task_discovery.py index a87d0c2e58c8..d1c1ad99965a 100644 --- a/tools/test/test_task_discovery.py +++ b/tools/test/test_task_discovery.py @@ -258,4 +258,5 @@ def test_uncollapsed_mode_still_drops_combinations_the_validator_rejects(monkeyp every, _ = _build_modes("Isaac-X", (), (), ("rgb", "raycaster_depth"), resolve=True, collapse=False) - assert Mode(None, None, "raycaster_depth") not in every + # Asserting the whole tuple, not just absence -- ``not in`` also passes on empty. + assert every == (Mode(None, None, None), Mode(None, None, "rgb")) diff --git a/tools/test/test_task_discovery_resolve.py b/tools/test/test_task_discovery_resolve.py new file mode 100644 index 000000000000..997f5b6bb134 --- /dev/null +++ b/tools/test/test_task_discovery_resolve.py @@ -0,0 +1,141 @@ +# 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 + +"""Tests for task discovery against real configs. + +Kept apart from ``test_task_discovery.py`` so that file stays importable with nothing +but pytest. Everything here needs Isaac Lab and resolves real task configs, which costs +a few seconds of warm-up and roughly 0.1s per combination. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + + +def _bootstrap_paths() -> None: + """Prepend ``tools/`` and the editable ``source/*`` packages.""" + repo_root = Path(__file__).resolve().parents[2] + prepend = [str(repo_root / "tools")] + for package_dir in sorted((repo_root / "source").iterdir()): + if (package_dir / package_dir.name).is_dir(): + prepend.append(str(package_dir)) + for path in reversed(prepend): + if path not in sys.path: + sys.path.insert(0, path) + + +_bootstrap_paths() + +pytest.importorskip("isaaclab_tasks", reason="task discovery resolution needs Isaac Lab") + +import task_discovery # noqa: E402 +from task_discovery import DiscoveryError, _mode_resolves # noqa: E402 + + +def _raise(exc: BaseException): + """Return a ``resolve_task_config`` stand-in that raises *exc*.""" + + def fake(*args, **kwargs): + raise exc + + return fake + + +def test_a_kit_backed_physics_and_a_kitless_renderer_are_rejected() -> None: + """The pairing that justifies resolving at all must actually come back rejected. + + OVRTX is kitless and cannot share a process with Isaac Sim PhysX. If the validator + ever starts accepting this, every other test here still passes. + """ + assert _mode_resolves("Isaac-Cartpole-Camera", "isaacsim_physx", "ovrtx", None) is None + assert _mode_resolves("Isaac-Cartpole-Camera", "ovphysx", "isaacsim_rtx", None) is None + assert _mode_resolves("Isaac-Cartpole-Camera", "isaacsim_physx", "isaacsim_rtx", None) is not None + assert _mode_resolves("Isaac-Cartpole-Camera", "ovphysx", "ovrtx", None) is not None + + +def test_distinct_backends_get_distinct_fingerprints() -> None: + """Guards the collapse against a config serialization that stops discriminating. + + ``to_dict`` erases class identity, so backends differ only by the values it keeps. + Should that discriminator ever be dropped upstream, two backends would silently + merge into one mode and a dispatcher would stop scheduling one of them. + """ + resolutions = [_mode_resolves("Isaac-Cartpole", name, None, None) for name in ("newton_mjwarp", "newton_kamino")] + assert all(r is not None for r in resolutions) + fingerprints = {r[0] for r in resolutions} + backends = {r[1] for r in resolutions} + assert len(backends) == 2, backends + assert len(fingerprints) == len(backends) + + +def test_an_alias_collapses_onto_the_backend_it_resolves_to() -> None: + """``physics=physx`` is an alias, so it must share a fingerprint with its target.""" + alias = _mode_resolves("Isaac-Cartpole", "physx", None, None) + concrete = _mode_resolves("Isaac-Cartpole", "ovphysx", None, None) + + assert alias is not None and concrete is not None + assert alias == concrete + + +def test_the_same_combination_fingerprints_the_same_way_twice() -> None: + """An unstable fingerprint would split one run across several modes, silently.""" + assert _mode_resolves("Isaac-Cartpole", None, None, None) == _mode_resolves("Isaac-Cartpole", None, None, None) + + +@pytest.mark.parametrize( + ("raised", "expectation"), + [ + # A config needing an uninstalled extra cannot run here -- same answer as a + # rejection, which is what keeps discovery usable from a partial install. + (ModuleNotFoundError("No module named 'isaaclab_absent_extra'"), "rejected"), + # A module that imports but no longer exports what it should is API drift. + (ImportError("cannot import name 'gone'"), "raises"), + (AttributeError("'NoneType' object has no attribute 'solver_cfg'"), "raises"), + # Anything else means the combination cannot run. + (ValueError("Invalid backend combination"), "rejected"), + ], +) +def test_failures_are_sorted_into_rejection_or_api_drift(monkeypatch, raised, expectation) -> None: + import isaaclab_tasks.utils as utils + + monkeypatch.setattr(utils, "resolve_task_config", _raise(raised)) + + if expectation == "raises": + with pytest.raises(DiscoveryError): + _mode_resolves("Isaac-Cartpole", None, None, None) + else: + assert _mode_resolves("Isaac-Cartpole", None, None, None) is None + + +def test_sys_argv_is_restored_even_when_resolution_fails(monkeypatch) -> None: + """``discover_tasks`` runs in-process from a CLI tool, so a leak would corrupt it.""" + import isaaclab_tasks.utils as utils + + monkeypatch.setattr(utils, "resolve_task_config", _raise(ValueError("nope"))) + before = list(sys.argv) + + assert _mode_resolves("Isaac-Cartpole", "ovphysx", None, "rgb") is None + + assert sys.argv == before + + +def test_an_unloadable_config_reports_unknown_and_no_modes(monkeypatch) -> None: + """``declared is None`` must not sit beside a mode claiming the task runs.""" + import gymnasium as gym + + import isaaclab_tasks.utils.preset_cli as preset_cli + + monkeypatch.setattr(preset_cli, "enumerate_task_presets", lambda task_name: None) + spec = next(spec for spec in gym.registry.values() if spec.id == "Isaac-Cartpole") + + task = task_discovery.discover_tasks([spec], resolve=False)[0] + + assert task.declared is None + assert task.modes == () + assert task.default is None From 7693317f0efd5027cc9009094b2197eed9828e3d Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 19 Aug 2026 13:58:22 +0200 Subject: [PATCH 07/10] Consolidate the discovery tests and tighten its prose Two fixtures replace the monkeypatch boilerplate that five tests each repeated, and three merges remove cases that were re-asserting a neighbour: the declared cross product becomes one parametrized test over its three shapes, the two uncollapsed cases become one (a rejected token in the same table covers both), and the fingerprint's stability and discrimination are one property, not two. The argv guarantee moves into the error-taxonomy test, where it should hold on every branch rather than on one. Prose is cut where it restated itself rather than where it carried a fact -- the launch-baseline caveat keeps both leaks, LIVESTREAM and the missing Isaac Sim check, in half the words. Also corrects this file's own docstring, which still claimed selector detection was exercised by running a tool. Selector detection is gone and there is no tool; the resolver is covered by the companion module. --- tools/task_discovery.py | 78 +++--- tools/test/test_task_discovery.py | 280 ++++++++++------------ tools/test/test_task_discovery_resolve.py | 52 ++-- 3 files changed, 183 insertions(+), 227 deletions(-) diff --git a/tools/task_discovery.py b/tools/task_discovery.py index c621a6f28613..c148938cd524 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -74,26 +74,23 @@ class DiscoveredTask: rl_libraries: RL libraries the task declares an agent config for, in :data:`RL_LIBRARY_PRIORITY` order. Empty for registered environments with no RL entry point, such as IK, teleop and mimic tasks. - declared: Preset names the task declares, keyed by axis (``physics``, - ``renderer``, ``presets``) exactly as the registry reports them, or ``None`` - when the config could not be loaded. ``None`` means unknown; an all-empty - mapping means the task declares nothing and runs on a fixed backend. - Unreconciled: aliases and cross-axis duplicates are all present. Use + declared: Preset names by axis (``physics``, ``renderer``, ``presets``) as + ``enumerate_task_presets`` reports them, or ``None`` when the config would + not load. ``None`` is unknown; an all-empty mapping is a task that declares + nothing. Unreconciled -- same-axis duplicate spellings (``physx`` beside + ``isaacsim_physx``) and cross-axis duplicates are all present, so use ``modes`` for a deduplicated answer. - modes: Ways to run the task. In resolved mode these passed the validator, and - with ``collapse`` are reduced so each is a distinct run rather than a - distinct spelling; without it every validated spelling is kept. In declared - mode they are the raw cross product, unverified. - - Validation assumes a **headless launch** -- no ``--visualizer kit``, - ``--livestream``, ``--experience`` or ``--require_kit``. Each is a Kit - source, so adding one narrows the legal set; the kitless visualizers - (``newton``, ``rerun``, ``viser``) are not. The assumption is not hermetic: - Kit sources are also read from the config's own visualizer intent, and from - the ``LIVESTREAM`` environment variable, which silently rejects every - kitless combination when set. It also stops at the validator, so a - Kit-requiring combination is reported runnable even where Isaac Sim is not - installed and the launcher would refuse it. + modes: Ways to run the task. Resolved modes passed the validator, and with + ``collapse`` are reduced to distinct runs rather than distinct spellings. + Declared modes are the raw cross product, unverified. + + Resolved against a **headless launch**. Kit sources narrow the legal set -- + ``--visualizer kit`` (not the kitless ``newton``/``rerun``/``viser``), + ``--livestream``, ``--experience``, ``--require_kit``, and the config's own + visualizer intent. Two leaks: ``LIVESTREAM`` is read from the environment + even with no flag, and validation stops before the Isaac Sim availability + check, so Kit-requiring combinations are reported runnable on machines that + cannot launch them. default: What the task does when given no preset tokens, or ``None`` in declared mode or when that run does not resolve. resolved: Whether ``modes`` was resolved, or merely declared. @@ -104,18 +101,16 @@ class Mode: """One way to run a task. Args: - physics: Physics preset token, or ``None`` when the run passes no - ``physics=`` token and the config's own default applies. That happens - both for tasks declaring no physics presets and for the probe of a - declaring task's default. - renderer: Renderer preset token, or ``None`` when the run passes no - ``renderer=`` token and the config's own default renderer applies. - presets: Domain preset token passed as ``presets=``, or ``None``. - Never more than one. ``presets=`` does take a comma-separated list and - names on different config paths compose (``duo_camera,depth128,cube``); - only names sharing a path conflict (``depth,rgb``). Discovery validates - each name alone and never tries a pair, so ``modes`` under-approximates - a task with several independent preset axes. + physics: Physics preset token, or ``None`` for a run passing no + ``physics=`` token -- the task declares none, or this is the probe of a + declaring task's own default. + renderer: Renderer preset token, or ``None`` for a run passing no + ``renderer=`` token. + presets: One ``presets=`` token, or ``None``. The token takes a comma list -- + names on different config paths compose (``duo_camera,depth128,cube``), + only same-path names conflict (``depth,rgb``) -- but discovery validates + each name alone, so ``modes`` under-approximates a task with several + independent preset axes. """ physics: str | None @@ -357,17 +352,16 @@ def discover_tasks( Args: specs: Gym specs to walk. When ``None``, the whole registry is scanned. - resolve: When ``True``, every combination is built and checked against the - runtime validator and only usable ones are returned. When ``False``, - combinations are reported as declared: fast but unverified. - strict: When ``True``, a combination the validator cannot judge raises instead - of being logged and dropped. Off by default so one unjudgeable combination - costs the caller that combination rather than the whole registry -- note the - task is still returned, with that combination missing from ``modes``. - collapse: When ``True`` (the default), spellings resolving to the same config are - reduced to one, so ``modes`` is the distinct runs a dispatcher should - schedule. Turn it off to keep every validated spelling, which is what - documentation needs. Ignored when ``resolve`` is off. + resolve: Build every combination and keep only what the validator accepts. + When ``False``, report what is declared: fast, unverified. + strict: Raise on a combination the validator cannot judge, instead of logging + and dropping it. Off by default, so one unjudgeable combination costs the + caller that combination and not the registry -- the task is still returned, + with that combination missing from ``modes``. + collapse: Reduce spellings that resolve to the same config to one, leaving the + distinct runs a dispatcher should schedule. Turn it off to keep every + validated spelling, which is what documentation needs. Ignored when + ``resolve`` is off. Returns: Discovered tasks sorted by ``task_id``. diff --git a/tools/test/test_task_discovery.py b/tools/test/test_task_discovery.py index d1c1ad99965a..f4aa6f402c52 100644 --- a/tools/test/test_task_discovery.py +++ b/tools/test/test_task_discovery.py @@ -3,10 +3,10 @@ # # SPDX-License-Identifier: BSD-3-Clause -"""Tests for the registry-independent parts of task discovery. +"""Tests for the parts of task discovery that need no Isaac Lab. -The registry walk and selector detection need Isaac Lab importable and are -exercised by running the tool; everything here is pure and runs offline. +Resolution is stubbed here so the module stays importable on pytest alone; the real +resolver is covered by ``test_task_discovery_resolve.py``. """ from __future__ import annotations @@ -37,21 +37,53 @@ def _bootstrap_paths() -> None: ) Mode = DiscoveredTask.Mode +Default = DiscoveredTask.Default + + +@pytest.fixture +def resolver(monkeypatch): + """Install a ``_mode_resolves`` driven by a ``combination -> (fingerprint, backend)`` table. + + A combination absent from the table resolves to ``None``, i.e. cannot run. + """ + + def install(runs): + monkeypatch.setattr( + task_discovery, + "_mode_resolves", + lambda task, physics, renderer, presets=None: runs.get((physics, renderer, presets)), + ) + + return install + + +@pytest.fixture +def broken_validator(monkeypatch): + """Install a ``_mode_resolves`` that fails structurally on one physics token.""" + + def install(backend): + def fake(task, physics, renderer, presets=None): + if physics == backend: + raise DiscoveryError("AttributeError: no attribute 'solver_cfg'") + return ("fp", "PhysxCfg") + + monkeypatch.setattr(task_discovery, "_mode_resolves", fake) + + return install @pytest.mark.parametrize( ("kwargs", "expected"), [ ({"rsl_rl_cfg_entry_point": "x"}, ("rsl_rl",)), - # Variant entry points belong to their library; matching the exact name - # would drop every recurrent, distillation and per-terrain config. + # Variants belong to their library; exact-name matching would drop every + # recurrent, distillation and per-terrain config. ({"rsl_rl_recurrent_cfg_entry_point": "x"}, ("rsl_rl",)), ({"skrl_flat_ppo_cfg_entry_point": "x"}, ("skrl",)), # Ordering follows RL_LIBRARY_PRIORITY, not registration order. ({"skrl_cfg_entry_point": "x", "rsl_rl_cfg_entry_point": "x"}, ("rsl_rl", "skrl")), # The env config is not an agent config. ({"env_cfg_entry_point": "x"}, ()), - ({}, ()), ], ) def test_rl_libraries_are_read_from_entry_point_stems(kwargs: dict, expected: tuple[str, ...]) -> None: @@ -72,69 +104,57 @@ def test_only_trainable_isaac_tasks_are_walked(task_id: str, expected: bool) -> assert is_training_task(task_id) is expected -def test_domain_presets_drop_names_the_task_also_declares_as_a_typed_selector() -> None: - # Reachable as ``physics=ovphysx`` / ``renderer=ovrtx``, so reporting them again - # as ``presets=`` tokens would double-count the same run. - names = ["rgb", "ovphysx", "depth", "ovrtx"] - - assert _domain_presets(names, ("ovphysx", "ovrtx")) == ("depth", "rgb") - - -def test_a_backend_exposed_only_as_a_domain_preset_is_kept() -> None: - # Isaac-Open-Drawer-Franka buckets its backends under DOMAIN because their cfg - # classes do not subclass PhysicsCfg, so ``presets=newton_mjwarp`` is the only - # way to select them and ``physics=newton_mjwarp`` is rejected. Dropping them by - # name left the task reporting one mode carrying no tokens, hiding four runs. - names = ["isaacsim_physx", "newton_kamino", "newton_mjwarp", "ovphysx", "physx"] - - assert _domain_presets(names, ()) == tuple(sorted(names)) - - -def test_modes_are_the_raw_cross_product_when_resolution_is_skipped() -> None: - # Nothing has been resolved, so nothing can be collapsed and there is no default. - modes, default = _build_modes("Isaac-X", ("physx", "newton_mjwarp"), ("ovrtx",), (), resolve=False) - - assert modes == (Mode("physx", "ovrtx", None), Mode("newton_mjwarp", "ovrtx", None)) - assert default is None - - -def test_a_task_without_presets_gets_one_mode_carrying_no_tokens() -> None: - assert _build_modes("Isaac-X", (), (), (), resolve=False)[0] == (Mode(None, None, None),) - - -def test_domain_presets_are_expanded_one_at_a_time_beside_the_default() -> None: - # Presets targeting the same field conflict, so they are never combined; the - # ``None`` entry keeps the task's own default reachable. - modes, _ = _build_modes("Isaac-X", (), (), ("rgb", "depth"), resolve=False) - - assert modes == (Mode(None, None, None), Mode(None, None, "rgb"), Mode(None, None, "depth")) - - -def _resolver(runs: dict[tuple[str | None, str | None, str | None], tuple[str, str]]): - """Return a ``_mode_resolves`` stand-in driven by a combination -> run table.""" - - def fake(task_id, physics, renderer, presets=None): - return runs.get((physics, renderer, presets)) - - return fake +@pytest.mark.parametrize( + ("names", "typed", "expected"), + [ + # Declared on a typed axis too, so reachable as ``physics=``/``renderer=`` and + # reporting them again as ``presets=`` would double-count the same run. + (["rgb", "ovphysx", "depth", "ovrtx"], ("ovphysx", "ovrtx"), ("depth", "rgb")), + # Isaac-Open-Drawer-Franka: backends bucket under DOMAIN because their cfg + # classes do not subclass PhysicsCfg, so ``presets=`` is the only way to select + # them. Dropping them by name left the task reporting one empty mode. + (["newton_mjwarp", "ovphysx", "physx"], (), ("newton_mjwarp", "ovphysx", "physx")), + ], +) +def test_domain_presets_drop_only_what_a_typed_axis_already_offers(names, typed, expected) -> None: + assert _domain_presets(names, typed) == expected -def test_spellings_that_resolve_to_the_same_run_collapse_to_one_mode(monkeypatch) -> None: - # Isaac-Open-Drawer-Franka in miniature: passing nothing lands on the same config - # as naming the preset the task already defaults to, and the ``physx`` alias lands - # on the same config as the concrete backend it resolves to. - monkeypatch.setattr( - task_discovery, - "_mode_resolves", - _resolver( - { - (None, None, None): ("fp-physx", "PhysxCfg"), - (None, None, "isaacsim_physx"): ("fp-physx", "PhysxCfg"), - (None, None, "ovphysx"): ("fp-ov", "OvPhysxCfg"), - (None, None, "physx"): ("fp-ov", "OvPhysxCfg"), - (None, None, "newton_mjwarp"): ("fp-newton", "NewtonCfg(MJWarpSolverCfg)"), - } +@pytest.mark.parametrize( + ("physics", "renderers", "domains", "expected"), + [ + # Renderers are expanded across: a camera task reported headless-only would omit + # the thing under test. + ( + ("physx", "newton_mjwarp"), + ("ovrtx",), + (), + (Mode("physx", "ovrtx", None), Mode("newton_mjwarp", "ovrtx", None)), ), + # Declaring nothing still leaves one way to run. + ((), (), (), (Mode(None, None, None),)), + # Domain presets go one at a time, never combined; ``None`` keeps the task's own + # default reachable beside them. + ((), (), ("rgb", "depth"), (Mode(None, None, None), Mode(None, None, "rgb"), Mode(None, None, "depth"))), + ], +) +def test_declared_modes_are_the_raw_cross_product(physics, renderers, domains, expected) -> None: + # Nothing has been resolved, so nothing can be collapsed and there is no default. + assert _build_modes("Isaac-X", physics, renderers, domains, resolve=False) == (expected, None) + + +def test_spellings_that_resolve_to_the_same_run_collapse_to_one_mode(resolver) -> None: + # Isaac-Open-Drawer-Franka in miniature: passing nothing lands on the same config as + # naming the preset the task already defaults to, and ``physx`` lands on the same + # config as the concrete backend it aliases. + resolver( + { + (None, None, None): ("fp-physx", "PhysxCfg"), + (None, None, "isaacsim_physx"): ("fp-physx", "PhysxCfg"), + (None, None, "ovphysx"): ("fp-ov", "OvPhysxCfg"), + (None, None, "physx"): ("fp-ov", "OvPhysxCfg"), + (None, None, "newton_mjwarp"): ("fp-newton", "NewtonCfg(MJWarpSolverCfg)"), + } ) modes, default = _build_modes( @@ -147,22 +167,18 @@ def test_spellings_that_resolve_to_the_same_run_collapse_to_one_mode(monkeypatch Mode(None, None, "newton_mjwarp"), Mode(None, None, "ovphysx"), ) - assert default == DiscoveredTask.Default(backend="PhysxCfg", mode=Mode(None, None, "isaacsim_physx")) - - -def test_runs_sharing_a_backend_but_not_a_config_stay_separate(monkeypatch) -> None: - # Isaac-Reach-Franka's controller presets all run on Newton MJWarp and are four - # different runs, so collapsing on the backend instead of the config is wrong. - monkeypatch.setattr( - task_discovery, - "_mode_resolves", - _resolver( - { - (None, None, None): ("fp-joint", "NewtonCfg(MJWarpSolverCfg)"), - (None, None, "joint_pos"): ("fp-joint", "NewtonCfg(MJWarpSolverCfg)"), - (None, None, "diffik"): ("fp-diffik", "NewtonCfg(MJWarpSolverCfg)"), - } - ), + assert default == Default(backend="PhysxCfg", mode=Mode(None, None, "isaacsim_physx")) + + +def test_runs_sharing_a_backend_but_not_a_config_stay_separate(resolver) -> None: + # Isaac-Reach-Franka's controller presets all run on Newton MJWarp and are different + # runs, so collapsing on the backend instead of the config would merge them. + resolver( + { + (None, None, None): ("fp-joint", "NewtonCfg(MJWarpSolverCfg)"), + (None, None, "joint_pos"): ("fp-joint", "NewtonCfg(MJWarpSolverCfg)"), + (None, None, "diffik"): ("fp-diffik", "NewtonCfg(MJWarpSolverCfg)"), + } ) modes, default = _build_modes("Isaac-X", (), (), ("joint_pos", "diffik"), resolve=True) @@ -171,92 +187,50 @@ def test_runs_sharing_a_backend_but_not_a_config_stay_separate(monkeypatch) -> N assert default.mode == Mode(None, None, "joint_pos") -def test_a_default_that_matches_no_named_preset_survives_as_its_own_mode(monkeypatch) -> None: - monkeypatch.setattr( - task_discovery, - "_mode_resolves", - _resolver( - { - (None, None, None): ("fp-own", "PhysxCfg"), - (None, None, "rgb"): ("fp-rgb", "PhysxCfg"), - } - ), - ) +def test_a_default_matching_no_named_preset_survives_as_its_own_mode(resolver) -> None: + resolver({(None, None, None): ("fp-own", "PhysxCfg"), (None, None, "rgb"): ("fp-rgb", "PhysxCfg")}) modes, default = _build_modes("Isaac-X", (), (), ("rgb",), resolve=True) assert modes == (Mode(None, None, None), Mode(None, None, "rgb")) - assert default == DiscoveredTask.Default(backend="PhysxCfg", mode=Mode(None, None, None)) - - -def _validator_broken_on(backend: str): - """Return a ``_mode_resolves`` stand-in that fails structurally on *backend*.""" + assert default == Default(backend="PhysxCfg", mode=Mode(None, None, None)) + + +def test_uncollapsed_keeps_every_spelling_but_still_drops_rejections(resolver) -> None: + # ``shapes`` names the default of its own axis, so it resolves to the default config + # and the collapse drops it -- but it is still a token a reader can type, which is + # what the documentation view needs. ``raycaster`` is absent from the table, so it + # does not resolve and must be dropped either way. + resolver( + { + (None, None, None): ("fp-default", "PhysxCfg"), + (None, None, "shapes"): ("fp-default", "PhysxCfg"), + (None, None, "cube"): ("fp-cube", "PhysxCfg"), + } + ) + domains = ("shapes", "cube", "raycaster") - def fake(task_id, physics, renderer, presets=None): - if physics == backend: - raise DiscoveryError("AttributeError: no attribute 'solver_cfg'") - return ("fp", "PhysxCfg") + collapsed, _ = _build_modes("Isaac-X", (), (), domains, resolve=True) + every, default = _build_modes("Isaac-X", (), (), domains, resolve=True, collapse=False) - return fake + assert collapsed == (Mode(None, None, "shapes"), Mode(None, None, "cube")) + assert every == (Mode(None, None, None), Mode(None, None, "shapes"), Mode(None, None, "cube")) + assert default == Default(backend="PhysxCfg", mode=Mode(None, None, "shapes")) -def test_a_combination_the_validator_cannot_judge_is_dropped_and_the_walk_continues(monkeypatch) -> None: - # One task whose config breaks the validator must not cost the caller every - # other task in the registry. Unknown is not legal either, so the combination - # is dropped rather than reported. - monkeypatch.setattr(task_discovery, "_mode_resolves", _validator_broken_on("newton_mjwarp")) +def test_a_combination_the_validator_cannot_judge_is_dropped_and_the_walk_continues(broken_validator) -> None: + # One task breaking the validator must not cost the caller the rest of the registry. + # Unknown is not legal either, so the combination is dropped rather than reported. + broken_validator("newton_mjwarp") modes, _ = _build_modes("Isaac-X", ("physx", "newton_mjwarp"), (), (), resolve=True) assert modes == (Mode("physx", None, None),) -def test_strict_raises_on_a_structural_failure_instead_of_dropping_it(monkeypatch) -> None: +def test_strict_raises_on_a_structural_failure_instead_of_dropping_it(broken_validator) -> None: # Callers policing Isaac Lab API drift want the canary, not a survivable walk. - monkeypatch.setattr(task_discovery, "_mode_resolves", _validator_broken_on("newton_mjwarp")) + broken_validator("newton_mjwarp") with pytest.raises(DiscoveryError): _build_modes("Isaac-X", ("physx", "newton_mjwarp"), (), (), resolve=True, strict=True) - - -def test_uncollapsed_mode_keeps_every_validated_spelling(monkeypatch) -> None: - # A preset naming the default of its own axis resolves to the default config, so - # the collapse drops it -- but it is still a token a reader can type, so the - # documentation view has to keep it. - monkeypatch.setattr( - task_discovery, - "_mode_resolves", - _resolver( - { - (None, None, None): ("fp-default", "PhysxCfg"), - (None, None, "shapes"): ("fp-default", "PhysxCfg"), - (None, None, "cube"): ("fp-cube", "PhysxCfg"), - } - ), - ) - - collapsed, _ = _build_modes("Isaac-X", (), (), ("shapes", "cube"), resolve=True) - every, default = _build_modes("Isaac-X", (), (), ("shapes", "cube"), resolve=True, collapse=False) - - assert collapsed == (Mode(None, None, "shapes"), Mode(None, None, "cube")) - assert every == (Mode(None, None, None), Mode(None, None, "shapes"), Mode(None, None, "cube")) - # The default still names an explicit spelling, not the bare no-token mode. - assert default == DiscoveredTask.Default(backend="PhysxCfg", mode=Mode(None, None, "shapes")) - - -def test_uncollapsed_mode_still_drops_combinations_the_validator_rejects(monkeypatch) -> None: - monkeypatch.setattr( - task_discovery, - "_mode_resolves", - _resolver( - { - (None, None, None): ("fp-default", "PhysxCfg"), - (None, None, "rgb"): ("fp-rgb", "PhysxCfg"), - } - ), - ) - - every, _ = _build_modes("Isaac-X", (), (), ("rgb", "raycaster_depth"), resolve=True, collapse=False) - - # Asserting the whole tuple, not just absence -- ``not in`` also passes on empty. - assert every == (Mode(None, None, None), Mode(None, None, "rgb")) diff --git a/tools/test/test_task_discovery_resolve.py b/tools/test/test_task_discovery_resolve.py index 997f5b6bb134..3b0ae72468d1 100644 --- a/tools/test/test_task_discovery_resolve.py +++ b/tools/test/test_task_discovery_resolve.py @@ -59,33 +59,28 @@ def test_a_kit_backed_physics_and_a_kitless_renderer_are_rejected() -> None: assert _mode_resolves("Isaac-Cartpole-Camera", "ovphysx", "ovrtx", None) is not None -def test_distinct_backends_get_distinct_fingerprints() -> None: - """Guards the collapse against a config serialization that stops discriminating. +def test_the_fingerprint_identifies_a_run() -> None: + """The collapse is only as good as this: stable per run, distinct across runs. ``to_dict`` erases class identity, so backends differ only by the values it keeps. - Should that discriminator ever be dropped upstream, two backends would silently - merge into one mode and a dispatcher would stop scheduling one of them. + Were that discriminator dropped upstream, two backends would merge into one mode + and a dispatcher would silently stop scheduling one of them. """ - resolutions = [_mode_resolves("Isaac-Cartpole", name, None, None) for name in ("newton_mjwarp", "newton_kamino")] - assert all(r is not None for r in resolutions) - fingerprints = {r[0] for r in resolutions} - backends = {r[1] for r in resolutions} - assert len(backends) == 2, backends - assert len(fingerprints) == len(backends) + once = _mode_resolves("Isaac-Cartpole", "newton_mjwarp", None, None) + again = _mode_resolves("Isaac-Cartpole", "newton_mjwarp", None, None) + other = _mode_resolves("Isaac-Cartpole", "newton_kamino", None, None) + + assert once is not None and other is not None + assert once == again, "an unstable fingerprint splits one run across several modes" + assert once[1] != other[1], "different backends" + assert once[0] != other[0], "...so they must not share a fingerprint" def test_an_alias_collapses_onto_the_backend_it_resolves_to() -> None: """``physics=physx`` is an alias, so it must share a fingerprint with its target.""" - alias = _mode_resolves("Isaac-Cartpole", "physx", None, None) - concrete = _mode_resolves("Isaac-Cartpole", "ovphysx", None, None) - - assert alias is not None and concrete is not None - assert alias == concrete - - -def test_the_same_combination_fingerprints_the_same_way_twice() -> None: - """An unstable fingerprint would split one run across several modes, silently.""" - assert _mode_resolves("Isaac-Cartpole", None, None, None) == _mode_resolves("Isaac-Cartpole", None, None, None) + assert _mode_resolves("Isaac-Cartpole", "physx", None, None) == _mode_resolves( + "Isaac-Cartpole", "ovphysx", None, None + ) @pytest.mark.parametrize( @@ -105,22 +100,15 @@ def test_failures_are_sorted_into_rejection_or_api_drift(monkeypatch, raised, ex import isaaclab_tasks.utils as utils monkeypatch.setattr(utils, "resolve_task_config", _raise(raised)) + # ``discover_tasks`` runs in-process from a CLI tool, so an argv leak on any of + # these paths would corrupt the caller. + before = list(sys.argv) if expectation == "raises": with pytest.raises(DiscoveryError): - _mode_resolves("Isaac-Cartpole", None, None, None) + _mode_resolves("Isaac-Cartpole", "ovphysx", None, "rgb") else: - assert _mode_resolves("Isaac-Cartpole", None, None, None) is None - - -def test_sys_argv_is_restored_even_when_resolution_fails(monkeypatch) -> None: - """``discover_tasks`` runs in-process from a CLI tool, so a leak would corrupt it.""" - import isaaclab_tasks.utils as utils - - monkeypatch.setattr(utils, "resolve_task_config", _raise(ValueError("nope"))) - before = list(sys.argv) - - assert _mode_resolves("Isaac-Cartpole", "ovphysx", None, "rgb") is None + assert _mode_resolves("Isaac-Cartpole", "ovphysx", None, "rgb") is None assert sys.argv == before From 45ed7f3e9751b77f9ccd9dc1b18eaab0a8cf0a2d Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 19 Aug 2026 14:01:58 +0200 Subject: [PATCH 08/10] Cut task discovery's prose back to its contract The module was 42% prose against 207 lines of code, because I had been putting reasoning in docstrings that belongs in the commit log and the PR. Every fact a caller needs to use the API correctly is kept -- the declared None-vs-empty distinction, the LIVESTREAM and Isaac Sim leaks in the headless assumption, the one-preset-at-a-time under-approximation, why ModuleNotFoundError is narrower than ImportError -- and the justifications around them are gone. 143 docstring lines to 107, 39 comment lines to 31. --- tools/task_discovery.py | 139 ++++++++++++++-------------------------- 1 file changed, 48 insertions(+), 91 deletions(-) diff --git a/tools/task_discovery.py b/tools/task_discovery.py index c148938cd524..d4d097029225 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -5,22 +5,10 @@ """Enumerate registered training tasks and the backend combinations they support. -Two questions get asked of the Gym registry, and they have different answers: - -* What does a task **declare**? One registry walk. Fast, and unverified. -* What does a task actually **resolve**? One config build and validator run per - combination -- minutes for the full registry, but the only way to know a - combination can run. The cross product is not all legal: OVRTX is kitless and - cannot share a process with Kit physics, so ``isaacsim_physx + ovrtx`` is - declared yet unusable. - -:func:`discover_tasks` answers either, selected with ``resolve``. - -Resolving also identifies each run by the config it produces rather than by how it -was spelled, so spellings that produce the same run collapse: ``physics=physx`` -folds into the concrete backend it resolves to, and passing nothing folds into -whichever preset the config already defaults to. That last run is reported -separately as ``default``, which the collapse would otherwise hide. +What a task **declares** is one registry walk: fast, unverified. What it **resolves** +costs a config build and a validator run per combination, and is the only way to know +a combination can run -- the cross product is not all legal, since OVRTX is kitless and +cannot share a process with Kit physics. :func:`discover_tasks` answers either. """ from __future__ import annotations @@ -39,24 +27,18 @@ "discover_tasks", ] -# Stable ordering for the RL library axis, shared with the environment tables. Only the -# ordering is shared: ``environ_docs.RL_LIBRARY_OVERRIDES`` adds libraries that declare -# no entry point, so a task can carry ``rlinf`` in the tables and not in -# :attr:`DiscoveredTask.rl_libraries`. ``rlinf`` is listed here for ordering only. +# Shared with the environment tables, but only the ordering: ``RL_LIBRARY_OVERRIDES`` +# there adds libraries declaring no entry point, so ``rlinf`` can appear in the tables +# and not in :attr:`DiscoveredTask.rl_libraries`. RL_LIBRARY_PRIORITY: tuple[str, ...] = ("rl_games", "rsl_rl", "skrl", "sb3", "rlinf") -# ``-Eval`` marks dedicated evaluation variants, registered separately against an -# eval-specific env cfg, which should not appear as their own training row. -# ``-Benchmark-`` tasks are perf harnesses rather than trainable environments. +# ``-Eval`` variants are separate registrations against an eval-specific env cfg, and +# ``-Benchmark-`` tasks are perf harnesses; neither is a training row. _EVAL_TASK_SUFFIXES = ("-Eval",) -# Errors meaning the validator itself could not run, rather than that the combination -# under test was rejected. Swallowing them would mark every combination illegal, so -# they are logged and the combination dropped; ``strict=True`` re-raises instead. -# ``TypeError`` is included because calling the validator with the wrong argument type -# is otherwise indistinguishable from a rejected combination, and ``ImportError`` -# because a module that imports but no longer exports what a config asks for is drift -# -- a missing extra raises ``ModuleNotFoundError`` and is handled before these. +# The validator could not run, as opposed to rejecting the combination. Swallowing +# these would mark every combination illegal. ``ImportError`` counts as drift because a +# missing *extra* raises ``ModuleNotFoundError`` and is handled before them. _INFRASTRUCTURE_ERRORS = (ImportError, AttributeError, NameError, SyntaxError, TypeError) @@ -71,28 +53,17 @@ class DiscoveredTask: Args: task_id: Gym task id. scope: ``core`` or ``contrib``. - rl_libraries: RL libraries the task declares an agent config for, in - :data:`RL_LIBRARY_PRIORITY` order. Empty for registered environments with - no RL entry point, such as IK, teleop and mimic tasks. - declared: Preset names by axis (``physics``, ``renderer``, ``presets``) as - ``enumerate_task_presets`` reports them, or ``None`` when the config would - not load. ``None`` is unknown; an all-empty mapping is a task that declares - nothing. Unreconciled -- same-axis duplicate spellings (``physx`` beside - ``isaacsim_physx``) and cross-axis duplicates are all present, so use - ``modes`` for a deduplicated answer. - modes: Ways to run the task. Resolved modes passed the validator, and with - ``collapse`` are reduced to distinct runs rather than distinct spellings. - Declared modes are the raw cross product, unverified. - - Resolved against a **headless launch**. Kit sources narrow the legal set -- - ``--visualizer kit`` (not the kitless ``newton``/``rerun``/``viser``), - ``--livestream``, ``--experience``, ``--require_kit``, and the config's own - visualizer intent. Two leaks: ``LIVESTREAM`` is read from the environment - even with no flag, and validation stops before the Isaac Sim availability - check, so Kit-requiring combinations are reported runnable on machines that - cannot launch them. - default: What the task does when given no preset tokens, or ``None`` in - declared mode or when that run does not resolve. + rl_libraries: Libraries the task declares an agent config for, in + :data:`RL_LIBRARY_PRIORITY` order. Empty for IK, teleop and mimic tasks. + declared: Preset names by axis (``physics``, ``renderer``, ``presets``), or + ``None`` when the config would not load. ``None`` is unknown, an all-empty + mapping is a task declaring nothing. Duplicate spellings are all present, so + use ``modes`` for a deduplicated answer. + modes: Ways to run the task; see :func:`_build_modes`. Resolved against a + headless launch, and two things escape that: ``LIVESTREAM`` is read from the + environment, and validation stops before the Isaac Sim availability check, + so Kit-requiring combinations are reported runnable where they cannot launch. + default: The no-token run, ``None`` in declared mode or when it does not resolve. resolved: Whether ``modes`` was resolved, or merely declared. """ @@ -101,16 +72,12 @@ class Mode: """One way to run a task. Args: - physics: Physics preset token, or ``None`` for a run passing no - ``physics=`` token -- the task declares none, or this is the probe of a - declaring task's own default. - renderer: Renderer preset token, or ``None`` for a run passing no - ``renderer=`` token. - presets: One ``presets=`` token, or ``None``. The token takes a comma list -- - names on different config paths compose (``duo_camera,depth128,cube``), - only same-path names conflict (``depth,rgb``) -- but discovery validates - each name alone, so ``modes`` under-approximates a task with several - independent preset axes. + physics: ``physics=`` token, or ``None`` for a run passing none -- the task + declares none, or this is the probe of its own default. + renderer: ``renderer=`` token, or ``None`` for a run passing none. + presets: One ``presets=`` token, or ``None``. The token takes a comma list + and names on different config paths compose, but discovery validates + each alone, so ``modes`` under-approximates a multi-axis task. """ physics: str | None @@ -122,9 +89,9 @@ class Default: """The run a task performs when given no preset tokens. Args: - backend: Concrete physics config the run resolves to, e.g. ``PhysxCfg`` or - ``NewtonCfg(MJWarpSolverCfg)``. Reported as the config class because a - task's default need not have a preset name. + backend: Physics config the run resolves to, e.g. + ``NewtonCfg(MJWarpSolverCfg)`` -- a class, since a default need not have + a preset name. mode: The entry in ``modes`` this run collapsed into. """ @@ -144,12 +111,9 @@ class Default: def _domain_presets(names: list[str], typed_names: tuple[str, ...]) -> tuple[str, ...]: """Return domain presets, dropping those the task also declares on a typed axis. - A backend buckets under ``DOMAIN`` or under ``PHYSICS`` / ``RENDERER`` depending on - whether its cfg class subclasses ``PhysicsCfg`` / ``RendererCfg``, so the same name - means different things on different tasks. Declared on a typed axis too, it is - reachable as ``physics=NAME`` and reporting it again double-counts one run; declared - only here, ``presets=NAME`` is the sole way to select it and ``physics=NAME`` is - rejected. Deciding by name rather than per task hides every backend of the latter. + A backend buckets under ``DOMAIN`` or a typed target by cfg class, so the same name + means different things per task: on both, it is reachable as ``physics=NAME`` and + reporting it again double-counts; here only, ``presets=NAME`` is the sole way in. """ typed = set(typed_names) return tuple(sorted(name for name in names if name not in typed)) @@ -189,22 +153,20 @@ def _mode_resolves( Returns: ``None`` when the combination cannot run -- an unknown preset, an unloadable - config and a rejected backend pairing are the same answer. Otherwise - ``(fingerprint, backend)``, where *fingerprint* digests the resolved env config - so two spellings of one run share it, and *backend* names the concrete physics - config, e.g. ``NewtonCfg(MJWarpSolverCfg)``. The solver is included because it - is what separates ``newton_mjwarp`` from ``newton_kamino``. + config and a rejected pairing are one answer. Otherwise ``(fingerprint, + backend)``: *fingerprint* digests the resolved config, so two spellings of one + run share it, and *backend* carries the solver, which is what separates + ``newton_mjwarp`` from ``newton_kamino``. Raises: - DiscoveryError: If validation could not run at all, e.g. because an Isaac Lab - import or API it depends on has changed. + DiscoveryError: If validation could not run at all. """ import argparse import hashlib import sys - # Discovery's own dependencies, two of them private. Losing one is API drift, never a - # rejected combination, so it must not fall through to the handlers below. + # Two of these are private. Losing one is drift, never a rejected combination, so + # it must not fall through to the handlers below. try: from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, scan @@ -238,10 +200,8 @@ def _mode_resolves( backend = f"{backend}({type(solver).__name__})" return fingerprint, backend except ModuleNotFoundError: - # The config needs an extra that is not installed, so the combination cannot - # run here. Same answer as a rejection, and it keeps a partial install usable. - # Narrower than ``ImportError`` on purpose: a module that is absent is a missing - # extra, whereas a module that is present but missing a symbol is drift. + # An uninstalled extra: same answer as a rejection, which keeps a partial + # install usable. Narrower than ``ImportError``, which would also swallow drift. return None except _INFRASTRUCTURE_ERRORS as exc: raise DiscoveryError( @@ -266,14 +226,11 @@ def _build_modes( ) -> tuple[tuple[DiscoveredTask.Mode, ...], DiscoveredTask.Default | None]: """Return the runs for one task, and what it does when given no tokens. - Renderers are expanded across, since reporting a camera task as headless-only omits - the thing under test. Domain presets are expanded one at a time, never combined. + Renderers are expanded across; domain presets one at a time, never combined. - ``collapse`` deduplicates on the resolved config, so each mode is a distinct run. - Collapsing on the *backend* instead would be wrong -- the Reach controller presets - share a backend and are four different runs. Without it every validated spelling is - kept, which is what documentation needs: a preset naming the default of its own axis - is still a token a reader can type. + ``collapse`` deduplicates on the resolved config -- not on the backend, which would + merge the Reach controller presets. Without it every validated spelling is kept, + which is what documentation needs. Returns: ``(modes, default)``. *default* names an explicit spelling even when ``collapse`` From a458368592fc34fbd1f4044df96d60dadb2c6674 Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Wed, 19 Aug 2026 15:16:54 +0200 Subject: [PATCH 09/10] Tighten task discovery comments Docstrings state the contract, not the reasoning behind it. Also fixes a docstring indent left mangled by an earlier edit, which ruff does not reflow. --- tools/task_discovery.py | 87 +++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 56 deletions(-) diff --git a/tools/task_discovery.py b/tools/task_discovery.py index d4d097029225..ddbd8736b56e 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -5,10 +5,9 @@ """Enumerate registered training tasks and the backend combinations they support. -What a task **declares** is one registry walk: fast, unverified. What it **resolves** -costs a config build and a validator run per combination, and is the only way to know -a combination can run -- the cross product is not all legal, since OVRTX is kitless and -cannot share a process with Kit physics. :func:`discover_tasks` answers either. +Declared combinations come from one registry walk. Resolved ones cost a config build +and a validator run each, and are the only reliable answer: the cross product is not +all legal, since OVRTX cannot share a process with Kit physics. """ from __future__ import annotations @@ -55,29 +54,21 @@ class DiscoveredTask: scope: ``core`` or ``contrib``. rl_libraries: Libraries the task declares an agent config for, in :data:`RL_LIBRARY_PRIORITY` order. Empty for IK, teleop and mimic tasks. - declared: Preset names by axis (``physics``, ``renderer``, ``presets``), or - ``None`` when the config would not load. ``None`` is unknown, an all-empty - mapping is a task declaring nothing. Duplicate spellings are all present, so - use ``modes`` for a deduplicated answer. - modes: Ways to run the task; see :func:`_build_modes`. Resolved against a - headless launch, and two things escape that: ``LIVESTREAM`` is read from the - environment, and validation stops before the Isaac Sim availability check, - so Kit-requiring combinations are reported runnable where they cannot launch. - default: The no-token run, ``None`` in declared mode or when it does not resolve. - resolved: Whether ``modes`` was resolved, or merely declared. + declared: Preset names by axis, or ``None`` if the config would not load. An + all-empty mapping means the task declares none. Keeps duplicate spellings. + modes: Ways to run the task; see :func:`_build_modes`. Resolution assumes a + headless launch, reads ``LIVESTREAM`` from the environment, and stops before + the Isaac Sim availability check. + default: The no-token run. ``None`` in declared mode, or if it does not resolve. + resolved: Whether ``modes`` was resolved or merely declared. """ @dataclass(frozen=True) class Mode: - """One way to run a task. - - Args: - physics: ``physics=`` token, or ``None`` for a run passing none -- the task - declares none, or this is the probe of its own default. - renderer: ``renderer=`` token, or ``None`` for a run passing none. - presets: One ``presets=`` token, or ``None``. The token takes a comma list - and names on different config paths compose, but discovery validates - each alone, so ``modes`` under-approximates a multi-axis task. + """The tokens one run passes, ``None`` on an axis it leaves to the config. + + ``presets`` holds at most one name even though the token takes a comma list, so + a task with several independent preset axes is under-approximated. """ physics: str | None @@ -86,13 +77,8 @@ class Mode: @dataclass(frozen=True) class Default: - """The run a task performs when given no preset tokens. - - Args: - backend: Physics config the run resolves to, e.g. - ``NewtonCfg(MJWarpSolverCfg)`` -- a class, since a default need not have - a preset name. - mode: The entry in ``modes`` this run collapsed into. + """The no-token run: the ``modes`` entry it collapsed into, and the physics + cfg class it resolves to, e.g. ``NewtonCfg(MJWarpSolverCfg)``. """ backend: str | None @@ -111,9 +97,7 @@ class Default: def _domain_presets(names: list[str], typed_names: tuple[str, ...]) -> tuple[str, ...]: """Return domain presets, dropping those the task also declares on a typed axis. - A backend buckets under ``DOMAIN`` or a typed target by cfg class, so the same name - means different things per task: on both, it is reachable as ``physics=NAME`` and - reporting it again double-counts; here only, ``presets=NAME`` is the sole way in. + Filtering by name instead would hide backends only reachable as ``presets=NAME``. """ typed = set(typed_names) return tuple(sorted(name for name in names if name not in typed)) @@ -149,14 +133,13 @@ def _rl_libraries_from_kwargs(kwargs: dict[str, Any]) -> tuple[str, ...]: def _mode_resolves( task_id: str, physics: str | None, renderer: str | None, presets: str | None = None ) -> tuple[str, str | None] | None: - """Resolve one combination and identify the run it produces. + """Resolve one combination. Returns: - ``None`` when the combination cannot run -- an unknown preset, an unloadable - config and a rejected pairing are one answer. Otherwise ``(fingerprint, - backend)``: *fingerprint* digests the resolved config, so two spellings of one - run share it, and *backend* carries the solver, which is what separates - ``newton_mjwarp`` from ``newton_kamino``. + ``None`` if it cannot run. Otherwise ``(fingerprint, backend)``: the fingerprint + digests the resolved config, so two spellings of one run share it, and the + backend carries the solver that separates ``newton_mjwarp`` from + ``newton_kamino``. Raises: DiscoveryError: If validation could not run at all. @@ -165,8 +148,7 @@ def _mode_resolves( import hashlib import sys - # Two of these are private. Losing one is drift, never a rejected combination, so - # it must not fall through to the handlers below. + # Two are private. Losing one is drift, not a rejected combination. try: from isaaclab.app.sim_launcher import _get_kit_runtime_sources, _validate_runtime, scan @@ -200,8 +182,7 @@ def _mode_resolves( backend = f"{backend}({type(solver).__name__})" return fingerprint, backend except ModuleNotFoundError: - # An uninstalled extra: same answer as a rejection, which keeps a partial - # install usable. Narrower than ``ImportError``, which would also swallow drift. + # An uninstalled extra. Narrower than ``ImportError``, which would hide drift. return None except _INFRASTRUCTURE_ERRORS as exc: raise DiscoveryError( @@ -226,11 +207,8 @@ def _build_modes( ) -> tuple[tuple[DiscoveredTask.Mode, ...], DiscoveredTask.Default | None]: """Return the runs for one task, and what it does when given no tokens. - Renderers are expanded across; domain presets one at a time, never combined. - - ``collapse`` deduplicates on the resolved config -- not on the backend, which would - merge the Reach controller presets. Without it every validated spelling is kept, - which is what documentation needs. + Renderers expand across, domain presets one at a time. ``collapse`` deduplicates on + the resolved config, not the backend, which would merge same-backend runs. Returns: ``(modes, default)``. *default* names an explicit spelling even when ``collapse`` @@ -311,14 +289,11 @@ def discover_tasks( specs: Gym specs to walk. When ``None``, the whole registry is scanned. resolve: Build every combination and keep only what the validator accepts. When ``False``, report what is declared: fast, unverified. - strict: Raise on a combination the validator cannot judge, instead of logging - and dropping it. Off by default, so one unjudgeable combination costs the - caller that combination and not the registry -- the task is still returned, - with that combination missing from ``modes``. - collapse: Reduce spellings that resolve to the same config to one, leaving the - distinct runs a dispatcher should schedule. Turn it off to keep every - validated spelling, which is what documentation needs. Ignored when - ``resolve`` is off. + strict: Raise on a combination the validator cannot judge instead of logging + and dropping it. Off by default, so the task is still returned with that + combination missing from ``modes``. + collapse: Reduce spellings resolving to the same config to one. Off keeps every + validated spelling. Ignored when ``resolve`` is off. Returns: Discovered tasks sorted by ``task_id``. From 4c742269b5600f7235fcca94aa7d2dff8d2a453f Mon Sep 17 00:00:00 2001 From: Antoine Richard Date: Thu, 20 Aug 2026 14:18:47 +0200 Subject: [PATCH 10/10] Validate the config when resolving a combination The env constructor calls cfg.validate() before it builds anything, and tasks put their cross-axis rules there: Reach rejects the newton_ik action preset unless physics is Newton, and Lift rejects camera data types the Warp renderer cannot produce. Resolution stopped short of that call, so discovery reported those pairings as runnable and they failed on the first reset instead -- six OSMO jobs for Reach alone. Registry-wide this drops 1180 resolved combinations to 1027. Spot-checked: the Warp renderer keeps rgb and depth and loses albedo and simple_shading, which is exactly what lift_env_cfg allows, and albedo still resolves on isaacsim_rtx. No task loses every mode. --- tools/task_discovery.py | 4 ++++ tools/test/test_task_discovery_resolve.py | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/tools/task_discovery.py b/tools/task_discovery.py index ddbd8736b56e..9c38d3871299 100644 --- a/tools/task_discovery.py +++ b/tools/task_discovery.py @@ -172,6 +172,10 @@ def _mode_resolves( args, remaining = setup_preset_cli(parser, argv) sys.argv = [sys.argv[0]] + remaining env_cfg, _ = resolve_task_config(args.task, args.agent) + # The env constructor validates before it builds anything, and tasks put their + # cross-axis rules there -- Reach rejects ``newton_ik`` without Newton physics. + # Skipping it reports those combinations legal and fails them at launch instead. + env_cfg.validate() config_scan = scan(env_cfg, args) _validate_runtime(config_scan, _get_kit_runtime_sources(config_scan, args)) fingerprint = hashlib.sha256(repr(env_cfg.to_dict()).encode()).hexdigest() diff --git a/tools/test/test_task_discovery_resolve.py b/tools/test/test_task_discovery_resolve.py index 3b0ae72468d1..eda584bd6d26 100644 --- a/tools/test/test_task_discovery_resolve.py +++ b/tools/test/test_task_discovery_resolve.py @@ -59,6 +59,17 @@ def test_a_kit_backed_physics_and_a_kitless_renderer_are_rejected() -> None: assert _mode_resolves("Isaac-Cartpole-Camera", "ovphysx", "ovrtx", None) is not None +def test_a_cross_axis_rule_the_task_declares_is_enforced() -> None: + """Tasks put cross-axis rules in ``validate_config``, which the env constructor runs. + + Reach rejects ``newton_ik`` without Newton physics. Resolving without validating + reported those pairings legal, and they died on the first reset instead. + """ + assert _mode_resolves("Isaac-Reach-Franka", "isaacsim_physx", None, "newton_ik") is None + assert _mode_resolves("Isaac-Reach-Franka", "ovphysx", None, "newton_ik") is None + assert _mode_resolves("Isaac-Reach-Franka", "newton_mjwarp", None, "newton_ik") is not None + + def test_the_fingerprint_identifies_a_run() -> None: """The collapse is only as good as this: stable per run, distinct across runs.