[PresetCLI] Add typed preset selection with task-aware help#5587
Merged
Conversation
A thin argparse adapter on top of the existing Hydra preset flow. Adds ``--physics=NAME``, ``--renderer=NAME``, and ``--presets=NAME[,NAME,...]`` flags that fold their values into the same ``presets=<csv>`` token that :func:`isaaclab_tasks.utils.hydra.resolve_presets` already consumes. The resolver, name validation, alias rewriting, error formatting, and per- ``PresetCfg`` resolution are unchanged -- the new module is purely a discoverability layer: * ``--help`` now lists the canonical preset names registered with ``@register`` plus the :class:`PresetCfg` field names declared on the task selected by ``--task=<X>``. Users no longer need to type and fail to learn what's available. * Unknown names emit a stderr hint and pass through; the existing ``hydra._format_unknown_presets_error`` produces the rich path-grouped error at resolve time as before. New module ``isaaclab.utils.preset_registry`` exposes ``PresetTarget`` (closed enum of CLI categories) and ``PresetRegistry`` with the ``register`` decorator. Backend cfg classes declare themselves once, e.g. ``@register(PresetTarget.PHYSICS, "physx")``. Legacy alias data moves onto the enum members so per-target tables are the single source hydra's resolver consults. ``hydra.py`` is not modified by this PR; resolution semantics are unchanged.
setup_cli now peeks sys.argv for --task before argparse runs, loads that task's env_cfg (safe pre-AppLauncher, enforced by test_env_cfg_no_forbidden_imports), walks PresetCfg variants via the existing collect_presets, and lists them in --help -- bucketed by target via PresetRegistry so typed flags (--physics, --renderer) list only their kind, while --presets lists every variant. The registry is no longer a help-text source; it now serves as a naming-convention hint that informs target bucketing. Without --task, help tells the user to pass one. The env_cfg load is gated on --help being in argv so normal training runs skip the work.
hydra.py had a literal _LEGACY_PRESET_ALIASES dict identical to what PresetTarget.<target>.legacy_aliases now exposes via PresetTarget.all_legacy_aliases(). Two sources of truth invite drift and silence the registry's role as canonical home for these aliases. Removes the local dict and its Newton-rationale comment (moved to the PresetTarget.PHYSICS docstring where the data lives). Six call sites in hydra now call PresetTarget.all_legacy_aliases() directly. Existing tests across test_hydra.py, test_preset_cli.py, and test_newton_solver_preset_names.py all pass; FutureWarning behavior for legacy 'newton' and 'kamino' names is unchanged.
github-actions
Bot
added
documentation
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR introduces a well-designed typed CLI layer (--physics=, --renderer=, --presets=) for task variant selection, replacing loose Hydra overrides with discoverable, typed selectors. The implementation is clean, well-documented, and provides excellent user experience with task-aware help messages.
📝 Incremental Review Update (f531b98 → f02999d)
Merge from develop + conflict resolution
✅ Previous Issues Resolved
P1 Missing import sys after refactor — All affected files now have import sys restored. The merge from develop fixed this.
⚠️ Remaining Minor Issues
P2 PresetTarget.all_legacy_aliases() double-call — Still present at lines 581-582 of source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py. The function builds a fresh dict on every call; calling it twice for lookup (membership test + value retrieval) is wasteful. Minor optimization opportunity, non-blocking.
📋 Changes in This Push
-
CI fix (
a5eb9add): UnsetsHUB__ARGS__DETECT_ONLYin Docker test containers to fix OmniHub startup and prevent slow Nucleus asset retries. -
Preset CLI integration (
f02999d0): Conflict resolution adapts preset CLI to the restructuredexport.py:setup_preset_cligains optionalargvparameter for in-process test pathsparse_export_argsuses the new signaturerun_export_with_hydrafolds typed selectors before Hydra
The changes look clean and the conflict resolution correctly integrates the preset CLI with the refactored export flow.
Automated review by Isaac Lab Review Bot
setup_cli previously called AppLauncher.add_app_launcher_args internally. That forced every adopting script to remove its existing add_launcher_args call to avoid a duplicate-argparse-option error -- a silent footgun for the 16 existing Hydra-using scripts that all call add_launcher_args today. Callers now register AppLauncher flags themselves (via the existing add_launcher_args wrapper in isaaclab_tasks.utils.sim_launcher) before calling setup_cli. setup_cli becomes single-purpose (preset CLI only), and migration into setup_cli is a strict addition next to the existing add_launcher_args call -- no removal needed. Side effect: the stub_app_launcher pytest fixture that previously monkey-patched isaaclab.app to dodge Kit init is no longer needed. Removed it and the 'import types' that only fed it; 11 test signatures lose the dummy parameter. 18 tests still pass.
The function is specifically about preset CLI setup (typed --physics / --renderer / --presets flags + parse + sys.argv fold); the bare 'setup_cli' name suggested broader CLI orchestration. The new name makes the scope explicit and parallels add_launcher_args's narrow, descriptive style. Also exposes setup_preset_cli at the package level: importable as 'from isaaclab_tasks.utils import setup_preset_cli' alongside the existing add_launcher_args / hydra_task_config / resolve_task_config.
Replaces the parse + sys.argv-clearing pattern in all 16 scripts that drive the Hydra-decorator preset flow with a single setup_preset_cli(parser) call alongside the existing add_launcher_args(parser). Now python <script>.py --physics newton_mjwarp --renderer newton_renderer --presets albedo64 works end-to-end through the Hydra resolver. Pattern breakdown: - 10 scripts: pure 3->1 collapse (random_agent, zero_agent, rl_games train/play, sb3 train/play, skrl train/play, leapp/export, sim2sim rsl_rl_transfer). Each had add_launcher_args + parse_known_args + sys.argv mutation; replaced with add_launcher_args + setup_preset_cli. - 4 benchmark scripts: capture sys.argv[1:] into hydra_args after setup_preset_cli so get_preset_string() telemetry keeps working. - 2 scripts (rsl_rl train/play): preserve external_callback intersection by applying it after setup_preset_cli, with the "presets=..." token at sys.argv[1] held aside so typed flag selections always reach Hydra regardless of what the callback chose to drop. (Slight behavior improvement over the pre-migration state, where a callback that didn't echo back the presets token would silently nullify typed selections.) Verified: 18 preset_cli tests + 79 hydra tests pass. Pre-commit clean.
…argv
setup_preset_cli used to mutate sys.argv as a side effect. Moving the
mutation out and to the caller lets scripts run argv-aware logic
(e.g. the --external_callback hook in rsl_rl/{train,play}.py that
re-reads sys.argv) BEFORE the folded form replaces the user's typed
flags. Without this, the callback would see "presets=newton_mjwarp"
instead of "--physics newton_mjwarp" and fail to recognize the user's
intent.
setup_preset_cli now returns (args, hydra_argv). Caller assigns
sys.argv = [sys.argv[0]] + hydra_argv when ready.
Also drops a defensive try/except in _enumerate_variants -- a bad
--task value or a broken cfg now propagates its natural exception
instead of being buried as a string inside --help text. Users see
the real error and fix the real cause.
_peek_task returns the LAST --task value (matching argparse's
store-action semantics for repeated flags).
Tests: existing flag-folding tests rewritten to assert the tuple
return + sys.argv non-mutation. New regression tests lock in:
sys.argv stays unmutated, hydra_argv keeps the presets token for
telemetry consumers, _peek_task is last-wins.
setup_preset_cli no longer mutates sys.argv. Each script now unpacks
(args, hydra_args) from the call and assigns sys.argv itself. For the
14 simple scripts (incl. benchmarks) this is a mechanical change: one
extra "sys.argv = [sys.argv[0]] + hydra_args" line. The benchmarks
keep the same hydra_args local name they already used for the
get_preset_string telemetry call.
For rsl_rl/{train,play}.py the change has real effect: the
--external_callback hook now runs while sys.argv is still the user's
original command line, so callbacks that re-parse sys.argv see the
typed --physics / --renderer / --presets flags rather than the folded
form. The intersection filter then runs on hydra_args (the post-fold
list) before assigning sys.argv, preserving the "presets=..." token.
A canonical preset name registered under two different PresetTargets would silently drop one binding: the help-time bucketing in preset_cli flattens the registry to a single name -> target dict, so whichever target's import landed last would win. Extend the duplicate-binding check so it scans every target, not just the one being assigned. Same-(target, name, class) re-imports remain idempotent; any other reuse raises RuntimeError at decoration time so the conflict surfaces at the point of definition, not at silently wrong help output downstream.
New public module; per the repo's "keep documentation up-to-date" rule in AGENTS.md, list it in the autosummary and add an automodule section so the API docs page picks up PresetTarget, PresetRegistry, and the register decorator.
hujc7
changed the title
sys.argv is module-level state; these private helpers were only ever called with sys.argv internally, so the parameter was boilerplate. Reading sys.argv directly removes the noise at the two call sites in setup_preset_cli and shortens the function signatures. Tests that previously passed a synthetic argv list now use monkeypatch on sys.argv -- consistent with the rest of the test file, which already does this for the setup_preset_cli tests.
ooctipus
reviewed
May 12, 2026
| from isaaclab.utils.preset_registry import PresetTarget, register | ||
|
|
||
|
|
||
| @register(PresetTarget.PHYSICS, "ovphysx") |
Collaborator
There was a problem hiding this comment.
I wonder if hydra/preset should ever be a dependency in non-isaaclab-task, when I was designing the hydra.py. I wanted to be only task-level dependency and not a cross package dependency.
Collaborator
Author
There was a problem hiding this comment.
The main problem is that without this thin register, we would need to have a hard coded list in hydra for those predefined names or types, which could potentially drift. IMO, It's more natural to put things where it's first appearing, so it's easier to maintain or scale.
Collaborator
There was a problem hiding this comment.
my main concern is that preset is really just build around idea of user configuration customization, and the issue we are having is that its a bit too flexible that is confusing. Preset has 0 value beyond task-time variation, and becomes very concerning when it leaks to core package. If you want to add a registry like this, I would think it around Idea beyond just isaaclab-task useage. Maybe a general tagging system is what you want, and preset is just leveraging the tagging system?
Collaborator
Author
There was a problem hiding this comment.
Let me check if there's a way to work without the register. To some extent yes I think, adding meta info to types so it's easier to identify them.
_bucket_variants_by_target used a flat name -> target lookup built
from PresetRegistry.names_for. That's a weak check: a task-local
preset accidentally reusing a backend's canonical name would route
to the wrong typed flag.
Replace with isinstance against PresetRegistry.classes_for(target).
Each (name, cfg) pair routes to whichever typed target's registered
class tree the cfg's type matches; cfgs that match no registered
target's classes (primitives, task-local non-backend types) fall
into DOMAIN. Subclassing works naturally via isinstance.
Return shape is unchanged -- still {PresetTarget: set[name]}. Names
remain the values, class type is purely the routing mechanism.
PresetRegistry gains a classes_for(target) classmethod that returns
a tuple suitable for isinstance() checks.
Test updates:
- test_bucket_variants now uses inline-registered mock classes and
asserts cfg-instance type drives the bucket assignment.
- test_help_with_task_shows_actual_variants uses registered-class
instances in its fake env_cfg so the typed-flag help renders
PHYSICS/RENDERER variants correctly under type-based routing.
Contributor
Greptile SummaryThis PR introduces typed
Confidence Score: 3/5Not safe to merge — seven scripts will crash on every invocation with a NameError before any training logic runs. The core library additions ( scripts/environments/random_agent.py, scripts/environments/zero_agent.py, scripts/reinforcement_learning/rl_games/play.py, scripts/reinforcement_learning/rl_games/train.py, scripts/reinforcement_learning/sb3/play.py, scripts/reinforcement_learning/skrl/play.py, scripts/reinforcement_learning/skrl/train.py — all missing Important Files Changed
Sequence DiagramsequenceDiagram
participant Script
participant setup_preset_cli
participant argparse
participant _peek_task
participant _enumerate_variants
participant hydra_resolver
Script->>setup_preset_cli: setup_preset_cli(parser)
setup_preset_cli->>_peek_task: peek sys.argv for --task
_peek_task-->>setup_preset_cli: task_name (or None)
alt --help in sys.argv AND task found
setup_preset_cli->>_enumerate_variants: load env_cfg, collect_presets
_enumerate_variants-->>setup_preset_cli: "{PresetTarget: set[name]}"
end
setup_preset_cli->>argparse: add --physics/--renderer/--presets flags
setup_preset_cli->>argparse: parse_known_args()
argparse-->>setup_preset_cli: (args, remaining)
setup_preset_cli->>setup_preset_cli: "fold typed flags + presets= tokens into presets=csv"
setup_preset_cli-->>Script: (args, hydra_argv)
Script->>Script: "sys.argv = [sys.argv[0]] + hydra_argv"
Script->>hydra_resolver: resolve_presets via Hydra decorator
hydra_resolver-->>Script: validated cfg
Reviews (1): Last reviewed commit: "Bucket help-time preset variants by cfg ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| # pass remaining args to Hydra | ||
| args_cli, hydra_args = setup_preset_cli(parser) | ||
| sys.argv = [sys.argv[0]] + hydra_args |
Contributor
There was a problem hiding this comment.
import sys after refactor
import sys was removed in this PR, but sys.argv is still used on this line at module level. Running this script will raise NameError: name 'sys' is not defined immediately on import. The same issue affects scripts/environments/zero_agent.py, scripts/reinforcement_learning/rl_games/play.py, scripts/reinforcement_learning/rl_games/train.py, scripts/reinforcement_learning/sb3/play.py, scripts/reinforcement_learning/skrl/play.py, and scripts/reinforcement_learning/skrl/train.py — all seven had import sys stripped but still call sys.argv.
Comment on lines
+580
to
+581
| if name in PresetTarget.all_legacy_aliases(): | ||
| replacement = PresetTarget.all_legacy_aliases()[name] |
Contributor
There was a problem hiding this comment.
PresetTarget.all_legacy_aliases() builds a fresh dict on every call. In apply_overrides it is called twice in a row for a single name lookup — once for the membership test and again for the value retrieval. Assign the result once.
Suggested change
| if name in PresetTarget.all_legacy_aliases(): | |
| replacement = PresetTarget.all_legacy_aliases()[name] | |
| legacy_aliases = PresetTarget.all_legacy_aliases() | |
| if name in legacy_aliases: | |
| replacement = legacy_aliases[name] |
In the earlier setup_preset_cli migration commit, sys.argv assignments were removed from these scripts (setup_preset_cli used to mutate sys.argv internally). Ruff's autofix then removed the now-unused 'import sys'. When the later API change required the scripts to assign sys.argv themselves again, the import wasn't restored -- making 14 F821 'Undefined name sys' errors that only surfaced when pre-commit was run on the full diff. Also adds four section dividers in preset_cli.py grouping the public entry point, help-text rendering, argv inspection, and help-time variant enumeration so the module is easier to navigate.
Collaborator
|
Thanks for taking this work from me@hujc7 ! If I am understanding correctly, preset_registry is just assign naming convention to each subclass? if that's the case can we build something that more along idea of generic tagging system, rather than blending with idea of preset? preset is basically just leveraging existing name and avoiding flexibile naming we prototyped some naming idea in https://github.com/isaac-sim/IsaacLab/blob/develop/source/isaaclab_tasks/isaaclab_tasks/utils/presets.py and if user import as their renderer, the environment is ensured to have consistent naming. My second questions is: how would adding this preset_registry help the isaaclab_tasks? as far as I am understanding we still need to list each option with its implementation like code snippet above, I was hoping for if we have this tagging system some of these burden can be lifted, but it doesn't seem to help in that direction? or correct me if I am wrong |
Three review-driven cleanups, no behavioral change: * PresetTarget.all_legacy_aliases() now @classmethod @functools.cache -- the merged dict is built once on first call and reused, replacing the per-call rebuild that hydra hit six times for membership tests and indexed lookups. * _peek_task() and _help_requested() collapse into a single _ArgvHelper class that scans sys.argv once and exposes task_name and help_requested as attributes. setup_preset_cli now uses one argv pass instead of two. * PresetTarget gains a "Bucketing contract" paragraph explaining that isinstance(cfg, target.base_classes) is the typed-flag routing rule: a non-PhysicsCfg/RendererCfg variant still resolves correctly at runtime (hydra matches by name) but bucket to --presets DOMAIN in --help. Documented so the design decision is visible. rsl_rl/{train,play}.py migration tightened to the minimum diff vs pre-PR: only the import line gains setup_preset_cli and parser.parse_known_args() becomes setup_preset_cli(parser); the external_callback flow, list_intersection call, variable name, and comment text are restored to their original form.
The "Pass `--task=X` along with `--help`" sentence was repeated three times in --help output, once per typed flag. Hoist it to the preset selection group's description so it prints once; per-flag help is now just the label. Add a parametrized test that locks the exact phrase produced by each _help_text branch (zero-variants, typed-only, DOMAIN-only, three-bucket mix) so wording changes are deliberate. Tighten the existing no-task test to assert the hint appears exactly once.
4 tasks
setup_preset_cli previously registered --physics / --renderer with
default argparse dests (args.physics, args.renderer), which collide
with AppLauncher's SimulationApp config keys. Specifically, AppLauncher
materializes SimulationApp config via
{k: launcher_args[k]
for k in _SIM_APP_CFG_TYPES.keys() & launcher_args.keys()}
so any args.renderer value -- including the argparse default of None --
is forwarded to SimulationApp.config["renderer"], which then crashes
with ``AttributeError: 'NoneType' object has no attribute 'lower'``
inside ``_set_render_settings``. This breaks every Kit-launching script
that uses both add_app_launcher_args and setup_preset_cli (the LEAPP
export tests in test_rsl_rl_export_flow.py surfaced it on PR CI).
Use dest=f"{label}_preset" on the typed flag registrations so the
namespace key (args.renderer_preset) doesn't shadow AppLauncher's
field. User-facing flag names stay --physics / --renderer; only the
internal namespace dest changes. Update the fold loop and the existing
test that introspected args.physics. Add a focused regression test
that asserts both ``getattr(args, 'renderer_preset', None)`` carries
the value and ``hasattr(args, 'renderer')`` is False, so the contract
fails the moment the dest mapping regresses.
ooctipus
approved these changes
May 13, 2026
setup_preset_cli now strips physics/renderer/presets off the parsed Namespace via vars(args).pop(...) right after parse_known_args, instead of registering them under <label>_preset dests. Reverts the previous dest-rename workaround, which split the contract into two surfaces: the user-facing flag (--renderer NAME) and a hidden attribute name (args.renderer_preset). Callers and tests had to know about that translation, and any future script that read args.renderer by accident would silently get a preset name string. Pop-after-parse keeps a single naming convention: --renderer at the CLI, and nothing at all on the namespace afterward. The argparse registration is unchanged, so --help still lists the typed flags; the values are consumed during the fold into hydra_argv and the namespace AppLauncher inspects on _SIM_APP_CFG_TYPES & vars(args) carries no preset keys. Add test_setup_does_not_leak_into_app_launcher_sim_app_intersection, which asserts exactly that intersection rather than checking specific attribute names. Lifts the regression coverage from the symptom (three named attrs) to the bug class (anything in PresetTarget leaking into SimulationApp.config forwarding).
Help output for the preset section now formats each variant on its own line under a bulleted ``Available:`` block, and the ``Pass --task=X`` discoverability hint sits on its own paragraph between the section description and the flag list. Long variant lists were previously collapsed into a single comma-separated wrap that buried individual names; the new layout makes each variant visually distinct. Add ``_PresetHelpFormatter`` -- an argparse formatter that wraps each blank-line-separated paragraph in the section description while honoring explicit newlines inside argument help strings. The default ``HelpFormatter`` strips newlines from both surfaces; ``RawDescriptionHelpFormatter`` preserves them in descriptions but drops wrapping; ``RawTextHelpFormatter`` preserves them everywhere but drops wrapping. None of the built-ins gives "preserve newlines AND wrap" on both surfaces, which is what the new layout needs. Arguments without embedded newlines (every AppLauncher flag) wrap exactly as before. Update the parametrized help-text tests to match the new bullet format and the more consistent broadcast-suffix phrasing for the empty DOMAIN case.
setup_preset_cli no longer registers --physics / --renderer / --presets as argparse flags. Instead it scans the parse_known_args remainder for Hydra-style key=value tokens (physics=NAME, renderer=NAME, presets=NAME[,NAME,...]) and folds them into the same presets=<csv> token the existing Hydra-decorator resolver already consumes. Eliminates the AppLauncher namespace collision at the source: with no argparse registration, the parsed namespace gains no preset attributes, so the set(_SIM_APP_CFG_TYPES) & set(vars(args)) intersection at app_launcher.py:681 cannot pick up a preset value and forward it into SimulationApp.config. The pop-after-parse workaround is therefore removed. Help-text discovery moves entirely onto the section description: when --task=X is in argv alongside --help, the description lists the PresetCfg variants present in the task's env_cfg, bucketed by typed target. Variants render one per line as bulleted lists; the ``presets:`` bucket shows only DOMAIN-bucket variants (typed names already appear under their own typed bucket). Update tests for the hydra-only grammar. The new test_namespace_carries_no_preset_attributes locks the contract that makes the AppLauncher collision impossible. Shorten the changelog fragment to a brief summary; the prose lives in the docstring and PR description.
Replace ``--physics`` / ``--renderer`` / ``--presets`` references with the Hydra-style ``physics=NAME`` / ``renderer=NAME`` / ``presets=NAME[,...]`` form. No behavior change.
Restore the prior design: each typed selector entry lists its available variants directly below it (the same layout the -- flag version used), not in a separate "Available in this task:" block.
Bullets now sit at column 33, the same column where each selector's inline ``(typed)`` / ``broadcast:`` description starts. Reads more like a standard argparse argument-description block.
…rings
Replace the three hardcoded header strings ("physics=NAME ... PhysicsCfg
variant", etc.) with a loop over ``PresetTarget``. Each row's syntax and
description are derived from the enum:
* selector key -> ``target.value``
* typed vs DOMAIN distinction -> ``target.base_classes``
* cfg class name in the description -> ``target.base_classes[0].__name__``
Column widths are now named constants (``_SELECTOR_COL``, ``_DESC_GAP``,
``_ROW_PREFIX``); the bullet indent is computed from them so adjusting the
table width keeps the variant bullets aligned.
Also rename ``PresetTarget.DOMAIN.value`` from ``"domain"`` to ``"presets"``
so the enum value matches the CLI selector key, and dispatch in
``setup_preset_cli`` via ``PresetTarget.DOMAIN.value`` instead of a
hardcoded ``"presets"`` literal.
The selector-table constants and the helpers that drive each row are now grouped under _DescriptionBuilder. Placement signals scope: they're private to the description rendering, used nowhere else, and the column widths only make sense alongside the row formatter that reads them. Also drop a one-shot local variable that didn't earn a name.
Collaborator
Author
|
/greptile review |
setup_preset_cli no longer folds. It registers the help description and
calls parse_known_args, returning the raw remainder with preset tokens
in their original physics= / renderer= / presets= form.
A new fold_preset_tokens helper carries the fold logic. Scripts call it
explicitly at the assignment line:
args_cli, remaining = setup_preset_cli(parser)
sys.argv = [sys.argv[0]] + fold_preset_tokens(remaining)
Fixes the rsl_rl --external_callback bug: list_intersection used to compare
a post-fold remainder (presets=NAME) against a pre-fold callback return
(physics=NAME) and silently dropped the preset by string mismatch. With
parse and fold separated, the intersection runs in pre-fold space where
vocabularies match; the fold runs once at the end on the surviving tokens.
Benchmark scripts fold once into hydra_args and reuse that name for both
sys.argv and get_preset_string -- the telemetry helper continues to read
the canonical presets= form unchanged.
Tests reorganized into setup_preset_cli (parse-only) and fold_preset_tokens
sections, plus two bug-fix regressions that pin both the correct
intersection-then-fold order and the wrong fold-then-intersection shape.
Collaborator
Author
|
/greptile review |
YizeWang
reviewed
May 15, 2026
| future release. | ||
| """ | ||
|
|
||
| RENDERER = ("renderer", (RendererCfg,)) |
Collaborator
Author
There was a problem hiding this comment.
Could you elaborate the question? Not too sure what it means
There was a problem hiding this comment.
Never mind. I was mistaken. Sorry for the inconvenience.
Conflict resolution touched two files: * scripts/reinforcement_learning/leapp/rsl_rl/export.py Develop (isaac-sim#5633) restructured export.py into create_arg_parser / parse_export_args / run_export_with_hydra / main_cli functions so the test suite can drive the export flow in-process. Plug the preset CLI into the new shape: - parse_export_args uses setup_preset_cli(parser, argv) to attach the preset-selection help group and parse argv (now forwarded to parse_known_args for the in-process test path) - run_export_with_hydra folds typed selectors via fold_preset_tokens before assigning sys.argv for the Hydra decorator * source/isaaclab_tasks/isaaclab_tasks/utils/preset_cli.py setup_preset_cli grows an optional argv: list[str] | None = None parameter forwarded to parse_known_args. Defaults to None so existing module-level callers keep working unchanged. Help-time variant introspection still reads sys.argv since --help only fires from the interactive command line.
hujc7
force-pushed
the
jichuanh/preset-cli-basic
branch
from
f531b98 to
f02999d
Compare
hujc7
added a commit
to hujc7/IsaacLab
that referenced
this pull request
May 15, 2026
Resolved 21 preset-CLI conflicts by taking develop's version: the preset-cli-basic work landed via isaac-sim#5587 (squash-merged into develop), which superseded the work-in-progress state on this branch with the final canonical signatures (``setup_preset_cli(parser, argv=None)``, ``fold_preset_tokens``, etc.). Cartpole task-config commits on top of preset-cli-basic are unaffected.
hujc7
added a commit
to hujc7/IsaacLab
that referenced
this pull request
May 16, 2026
Collapse 35 per-variant Cartpole gym task IDs into 4 consolidated tasks that select the variant via the typed preset CLI from isaac-sim#5587: - Isaac-Cartpole-Camera-Direct-v0 (subsumes 7 Direct-backend perception IDs) - Isaac-Cartpole-Camera-v0 (subsumes 4 manager-based perception IDs) - Isaac-Cartpole-Showcase-Direct-v0 (subsumes 15 proprioceptive showcase IDs) - Isaac-Cartpole-Camera-Showcase-Direct-v0 (subsumes 9 camera showcase IDs) Variant selection moves from the gym ID to ``presets=<name>`` (plus ``physics=`` / ``renderer=`` on the Direct-backend camera task, and ``--agent=`` where the per-shape skrl / rl_games yaml differs from the default). Each retired ID stays registered for one release with an env_cfg_entry_point that emits a DeprecationWarning naming the new task and the equivalent CLI tokens, then returns the corresponding cfg. The shim machinery is factored into a new helper: isaaclab_tasks.utils.deprecated_task_alias( old_task_id, new_command, consolidated_cfg_path, cfg_factory=None ) which wraps the warning emission plus lazy cfg resolution behind the ``"module:Name"`` entry-point string convention gym uses elsewhere. The consolidated PresetCfg classes live alongside their per-variant siblings in the existing *_env_cfg.py files; no new wrapper modules. PresetCfg's docstring now documents the underscore-prefix convention for class-local helpers (the resolver already skips them). Full per-ID migration table is in the PR description.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds typed preset selection via Hydra-style tokens —
physics=NAME/renderer=NAME/presets=NAME[,...]— that fold into the existingpresets=<csv>Hydra-decorator flow. Makes--task=X --helplist the actualPresetCfgvariants present in that task's env_cfg, bucketed by typed target. Adopted in all 16 Hydra-using scripts (rl_games/sb3/skrl/rsl_rltrain+play,environments/*,benchmarks/*,sim2sim_transfer,leapp/rsl_rl/export).API shape
setup_preset_clireturns(args, hydra_argv)without mutatingsys.argv. It registers no argparse flags for preset selection — the typed selectors are recognized as Hydra-style tokens in theparse_known_argsremainder and folded intohydra_argv[0]as a singlepresets=<csv>token.Grammar
physics=NAME— typed selector forPhysicsCfgvariantsrenderer=NAME— typed selector forRendererCfgvariantspresets=NAME[,NAME,...]— broadcast: applied to every matchingPresetCfgAll three fold into one
presets=<csv>token:presets=newton_mjwarp,newton_renderer,albedo,depth.The grammar matches Hydra's, so the same line can carry path-targeted overrides (
env.sim.dt=0.001) that flow through untouched.Namespace contract
No preset selector is registered with argparse, so the parsed
argsnamespace gains nophysics/renderer/presetsattribute. AppLauncher's name-based forwarding (set(_SIM_APP_CFG_TYPES) & set(vars(args)),app_launcher.py:681) therefore cannot pick up a preset value and push it intoSimulationApp.config— the historical--renderer→config["renderer"]→None.lower()crash class is structurally impossible. Two regression tests lock the contract.Help text layout
--task=Isaac-Cartpole-v0 --helprenders each selector with its available variants inline directly below it (bullets aligned with the description column):Typed variants appear only under their own typed selector. The
presets=listing shows only DOMAIN-bucket variants (cfgs whose type doesn't subclass any typed target's base class).Without
--task, each row shows just the selector + description and the section adds aPass --task=Xhint on its own paragraph.Test plan
pytest source/isaaclab_tasks/test/test_preset_cli.py— 24 tests pass. Coverage: enum wiring, token folding/dedupe/passthrough,_ArgvHelpersemantics, type-based bucketing, all four help-text branches (parametrized), no-sys.argv-mutation contract, namespace-clean contract, AppLauncher intersection contract,hydra_args[0]preserves thepresets=token for benchmark telemetry.pytest source/isaaclab_tasks/test/test_hydra.py— 76 tests pass; legacy-aliasFutureWarningbehavior unchanged.pre-commitclean.--task=Isaac-Cartpole-v0 --helpand--task=Isaac-Cartpole-RGB-Camera-Direct-v0 --helprender correctly.physics=newton_mjwarp renderer=newton_renderer presets=albedofolds into onepresets=<csv>token athydra_argv[0].newton→FutureWarningand resolves tonewton_mjwarp.