[Backport] PR #7611 to release/3.0.0 - #7677
Open
isaaclab-bot[bot] wants to merge 1 commit into
Open
Conversation
#7611) The symptoms addressed by #7045 and the now-merged #7532 are real, but the selection mechanism grew into a parallel configuration system: task registrations described preset-to-agent relationships, the preset CLI rediscovered those relationships and rewrote `args.agent`, every backend had to opt into that mutation, and the documentation browser copied the same mapping again. Because #7532 has since merged, this branch also fully removes its live sentinel, second parse, mutation guard, benchmark wiring, and associated compatibility tests. Historical compiled changelog entries remain as release history. This PR makes the existing preset resolver the only owner of environment-coupled selection: ```text presets=resnet18 | v resolve_task_config(task, <library>_cfg_entry_point) | +-- environment PresetCfg root -> ResNet18 observations | `-- agent PresetCfg root -> feature-policy configuration ``` The Gym registry now selects a library or a genuinely independent training recipe. A library's canonical entry point owns any agent variants coupled to environment presets, so the same `presets=` broadcast selects both roots in one resolution pass. The final diff is **687 additions / 787 deletions: net -100 lines**. The camera task registered separate raw and feature agent keys, then repeated their relationship to environment presets in another dictionary: ```python _RAW_CAMERA_PRESETS = ("albedo", "depth", "rgb", ...) gym.register( ..., kwargs={ "env_cfg_entry_point": "...:CartpoleCameraEnvCfg", "rsl_rl_cfg_entry_point": "...:CartpoleCameraPPORunnerCfg", "rsl_rl_feature_cfg_entry_point": "...:CartpoleCameraFeaturePPORunnerCfg", "agent_preset_compatibility": { "rsl_rl_cfg_entry_point": _RAW_CAMERA_PRESETS, "rsl_rl_feature_cfg_entry_point": ("resnet18", "theia_tiny"), # The same pairings were repeated for RL Games. }, }, ) ``` The showcase tasks amplified this into one registry key per observation/action-space combination—24 noncanonical SKRL keys across the two tasks—plus another compatibility map: ```python "skrl_cfg_entry_point": "...:skrl_box_box_ppo_cfg.yaml", "skrl_box_box_cfg_entry_point": "...:skrl_box_box_ppo_cfg.yaml", "skrl_box_discrete_cfg_entry_point": "...:skrl_box_discrete_ppo_cfg.yaml", "skrl_box_multidiscrete_cfg_entry_point": "...:skrl_box_multidiscrete_ppo_cfg.yaml", "agent_preset_compatibility": { "skrl_cfg_entry_point": ("box_box",), "skrl_box_discrete_cfg_entry_point": ("box_discrete",), # ... repeat every pairing again ... }, ``` The agent package owns the variants directly: ```python @configclass class CartpoleCameraPPORunnerPresetsCfg(PresetCfg): default = CartpoleCameraPPORunnerCfg() resnet18 = CartpoleCameraFeaturePPORunnerCfg() theia_tiny = CartpoleCameraFeaturePPORunnerCfg() gym.register( ..., kwargs={ "env_cfg_entry_point": "...:CartpoleCameraEnvCfg", "rsl_rl_cfg_entry_point": "...:CartpoleCameraPPORunnerPresetsCfg", }, ) ``` RL Games uses the same canonical-root shape with `preset(default=..., resnet18=..., theia_tiny=...)`. The showcase YAML family is likewise exposed behind its sole canonical key: ```python def skrl_cfg(): configs = { path.stem.removeprefix("skrl_").removesuffix("_ppo_cfg"): load_yaml(path) for path in Path(__file__).parent.glob("skrl_*_ppo_cfg.yaml") } return preset(default=configs.pop("box_box"), **configs) "skrl_cfg_entry_point": f"{agents.__name__}:skrl_cfg" ``` The configurations remain, but the 26 preset-specific agent keys, three compatibility maps, and duplicate preset-name lists do not. subsystem's argument ```python def setup_preset_cli(parser, argv=None, *, agent_library=None): ... args, remaining = parser.parse_known_args(args_to_parse) if agent_library and task_name: _auto_select_agent(args, task_name, agent_library, args_to_parse) return args, remaining args, remaining = setup_preset_cli(parser, agent_library="rsl_rl") ``` `_auto_select_agent` scanned raw `presets=` tokens, intersected them with the compatibility maps, selected a different registry key, and rewrote `args.agent`. #7532 later added a sentinel and a second argparse pass just to decide whether that mutation was allowed to override a parser default. ```python def setup_preset_cli(parser, argv=None): ... return parser.parse_known_args(args_to_parse) args, remaining = setup_preset_cli(parser) ``` The preset CLI is again bounded to preset help and parsing. Agent configuration selection happens once, inside config resolution. ```python "skrl_amp_cfg_entry_point": "...:skrl_walk_amp_cfg.yaml" parser.add_argument("--algorithm", default="PPO", ...) agent_cfg_entry_point = ( "skrl_cfg_entry_point" if args_cli.algorithm.lower() == "ppo" else f"skrl_{args_cli.algorithm.lower()}_cfg_entry_point" ) algorithm = agent_cfg_entry_point.split("_cfg")[0].split("skrl_")[-1].lower() ``` A plain Humanoid AMP command therefore looked for the missing `skrl_cfg_entry_point`. #7045 compensated with a second CLI rule: if the canonical key was absent and exactly one other key existed, mutate `args.agent` to that key. The same key-parsing pattern also mislabeled showcase keys such as `skrl_box_discrete_cfg_entry_point` as algorithms. algorithm ```python "default_agent": "skrl", "skrl_cfg_entry_point": "...:skrl_walk_amp_cfg.yaml", "skrl_amp_cfg_entry_point": "...:skrl_walk_amp_cfg.yaml", # explicit --algorithm AMP agent_cfg_entry_point = resolve_skrl_agent_cfg_entry_point(args_cli.agent, args_cli.algorithm) env_cfg, agent_cfg = resolve_task_config(args_cli.task, agent_cfg_entry_point) algorithm = resolve_skrl_algorithm(agent_cfg, args_cli.algorithm) # reads agent.class ``` Omitting `--algorithm` now means “use the task's canonical SKRL config,” not “assume PPO.” An explicit `--algorithm` still selects an independent algorithm recipe and is checked against the resolved `agent.class`. Train, play, benchmarks, and LEAPP all use the same rule. | Surface | Before | After | | --- | --- | --- | | CLI help | `_AgentDescriptionBuilder` and `_enumerate_agents` loaded Gym metadata and rendered compatibility pairings. | Preset help describes selectable presets; agent selection is not duplicated in the preset CLI. | | Environment browser | Every generated row carried `agentPresetCompatibility`; JavaScript searched it and injected `--agent` into commands. | The row field and JavaScript rewrite are gone; generated commands pass `presets=` directly. | | Environment catalog | Treated arbitrary agent-key suffixes as algorithms, displaying recipe names such as `FEATURE` or `BOX_DISCRETE`. | Recognizes only explicit algorithm aliases, ignores preset/recipe suffixes, and deduplicates canonical aliases of the same config. | | Ray Cartpole vision jobs | Set both `--agent=rl_games_feature_cfg_entry_point` and `presets=resnet18/theia_tiny`. | Set only the preset. | | Missing-config diagnostics | Split every `*_cfg_entry_point` suffix into guessed library and algorithm labels. | Lists the exact registered configuration keys. | | LEAPP export | The shared parser accepted `agent_library` so the preset CLI could mutate `args.agent`; SKRL then reconstructed the algorithm from that key. | Uses the plain preset parser and derives SKRL identity from the resolved `agent.class`. | | Generated project defaults | Emitted the canonical `<library>_cfg_entry_point` only for PPO, so non-PPO and all multi-agent projects depended on CLI fallback. | Filters selected algorithms per workflow and marks PPO, MAPPO, or the sole selected non-PPO recipe canonical; the documented command resolves directly. | | Regression tests | Preserved compatibility dictionaries and asserted an intermediate rewritten `args.agent` value. | Assert the resolved environment and agent configs, exact canonical registrations/minimal variants, and absence of the rejected maps/helpers. | Before, users either had to provide both coupled selectors explicitly or rely on the preset CLI to synthesize the `--agent` half: ```bash uv run isaaclab train --rl_library rsl_rl \ --task Isaac-Cartpole-Camera \ --agent rsl_rl_feature_cfg_entry_point presets=resnet18 uv run isaaclab train --rl_library rsl_rl \ --task Isaac-Cartpole-Camera presets=resnet18 ``` The same simplification applies to RL Games and the showcase space matrix: ```bash uv run isaaclab train --rl_library skrl \ --task IsaacContrib-Cartpole-Showcase-Direct \ --agent skrl_box_discrete_cfg_entry_point presets=box_discrete uv run isaaclab train --rl_library skrl \ --task IsaacContrib-Cartpole-Showcase-Direct presets=box_discrete ``` Humanoid AMP no longer depends on an algorithm flag or the sole-entry fallback: ```bash uv run isaaclab train --rl_library skrl \ --task IsaacContrib-Humanoid-AMP-Walk-Direct --algorithm AMP uv run isaaclab train --rl_library skrl \ --task IsaacContrib-Humanoid-AMP-Walk-Direct ``` `--algorithm AMP` remains valid when an explicit algorithm selection is desired. Generated multi-agent projects likewise use MAPPO as their canonical SKRL config, so their documented command needs no fallback: ```bash uv run isaaclab train --rl_library skrl --task <GENERATED-MARL-TASK> ``` The structure removed or superseded here accumulated across several PRs: | Change | Mechanism introduced | Disposition in this PR | | --- | --- | --- | | #2713 | Made missing-config errors split every agent entry-point key into inferred library and algorithm labels. | Errors now report the exact registered keys instead of inventing semantics for their suffixes. | | #5605 | Added the 24 space-specific showcase SKRL keys and the RL-Games camera feature key while consolidating preset-driven Cartpole tasks. | Keeps those configurations under canonical agent preset roots and removes their preset-specific registry keys. | | #5879 | Made the generated environment catalog infer algorithm labels from arbitrary entry-point suffixes and, for RL-Games vision, from the config path. | Catalog generation recognizes only the known algorithm aliases and ignores preset/recipe suffixes. | | #5904 | Added Ray camera-feature jobs that specified both `--agent=rl_games_feature_cfg_entry_point` and `presets=resnet18/theia_tiny`. | Ray selects each feature pipeline with the preset alone. | | #5930 | Added the separate `rsl_rl_feature_cfg_entry_point` for pretrained camera features. | The canonical RSL-RL root owns its raw and feature-policy variants. | | #6553 and #6564 | Made SKRL train/play and benchmark entry points default `--algorithm` to PPO, select a registry key from that default, and reconstruct algorithm identity from explicit key suffixes. | An omitted algorithm resolves the task's canonical config first; the resolved `agent.class` is authoritative everywhere. | | #6677 | Added three `agent_preset_compatibility` maps, duplicate `_SPACE_PRESETS` / `_RAW_CAMERA_PRESETS` lists, `setup_preset_cli(..., agent_library=...)`, `_AgentDescriptionBuilder`, `_enumerate_agents`, and `agent_library=` plumbing in all eight unified RL train/play entry points. It also added help, docs, and tests around those pairings. | Deletes the maps, duplicate lists, agent-aware preset CLI API/helpers, call-site plumbing, and preservation tests. | | #6779 | Extended `agent_library` into the shared LEAPP export parser and added another SKRL helper that inferred the algorithm from the agent key. | Removes that parser parameter/helper and resolves algorithm identity from the loaded config. | | #7045 | Promoted the compatibility metadata from help text into runtime control with `_auto_select_agent`, raw `presets=` scanning, declared-domain filtering, preset-to-key matching, post-parse `args.agent` mutation, and a sole-noncanonical-agent fallback for Humanoid AMP. It also wired the two SKRL benchmark entry points. | Deletes both selection rules and their benchmark wiring. Presets compose the environment and agent roots directly; AMP is registered canonically. | | #7160 | Copied `agent_preset_compatibility` into the generated environment-browser row schema and used it in JavaScript to inject a preset-specific `--agent` into generated commands. | Removes the compatibility column and browser-side command rewrite. | | #7532 (merged) | Added `_AGENT_UNSET`, a second argparse pass in `_agent_passed_explicitly`, a looser mutation guard, six RSL-RL/RL-Games/SB3 benchmark train/play wirings, and tests for rewritten `args.agent` values and explicit-flag spellings. | Fully reverts its live implementation: there is no agent-key mutation to guard, no backend-specific preset-CLI wiring to propagate, and no preservation test for that removed behavior. | This is therefore more than an alternative fix for two symptoms: it removes the parallel preset-to-agent selection system end to end. No warning-only or no-op compatibility shim is retained for: - `agent_preset_compatibility` - preset-specific agent keys such as `rsl_rl_feature_cfg_entry_point` and the showcase space-specific SKRL keys - `setup_preset_cli(..., agent_library=...)` The separately named `pretrained_checkpoint_preset_compatibility` metadata from #7630 is intentionally retained: it declares which preset-specific checkpoint artifacts exist and never selects an agent entry point. Keeping those aliases would preserve the duplicate ownership this refactor removes. Commands using a preset-specific key should remove `--agent=<preset-specific-key>` and use the canonical library selection plus `presets=<name>`, as shown above. Older SKRL runs may have encoded a configuration-key suffix such as `box_discrete` as the algorithm in a run directory or manifest. The corrected identity is the resolved `agent.class` (`PPO` in that example), so automatic `latest` / `best` discovery cannot safely match those historical runs. Pass their checkpoint path explicitly. - Bug fix - Architecture refactor - Intentional removal of recently introduced compatibility APIs - Documentation update The four-case composition regression was also run against an untouched `develop` worktree. All four cases failed there because the environment preset changed while the canonical agent stayed at its default; all four pass on this branch. - Preset composition, CLI, RL entrypoint, generator, and benchmark adapter/API tests: **148 passed** - Pretrained-checkpoint metadata tests: **12 passed** - Environment documentation tests: **35 passed** - LEAPP selector/checkpoint tests: **11 passed, 1 skipped** - Environment browser generation: up to date for **137** training environments - JavaScript syntax, pre-commit hooks for all PR files, changelog validation against current `upstream/develop`, centralized structure audit, and `git diff --check`: passed No GPU simulation or full training job was run; the regression is exercised at configuration composition and entrypoint boundaries before environment construction. The generated environment-browser refresh also incorporates the current physics selectors for the two DrLegs rows. - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` Not applicable. - [x] I have read and understood the contribution guidelines - [x] I have run the pre-commit checks with `uv run --frozen isaaclab -f` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove the fix is effective - [x] I have added changelog fragments for every touched package - [x] My name already exists in `CONTRIBUTORS.md` (cherry picked from commit 673b673)
github-actions
Bot
added
documentation
AntoineRichard
marked this pull request as ready for review
September 9, 2026 14:26
AntoineRichard
requested review from
AntoineRichard,
ClemensSchwarke,
StafaH,
fatimaanes,
frlai,
hujc7,
kellyguo11,
ooctipus,
pbarejko and
r-schmitt
as code owners
September 9, 2026 14:26
Collaborator
|
run-ci |
AntoineRichard
approved these changes
Sep 9, 2026
isaaclab-bot
Bot
added
ci:run-docker
Contributor
 The backport appears safe to merge after the required release-maintainer review, with no actionable correctness, security, or repository-rule issues identified.
|
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Isaac Lab Review Bot
The backport consistently moves environment-coupled agent variants into canonical preset-composed configurations and derives SKRL algorithm identity from the resolved config. One template-generator validation gap remains: mixed workflow selections can silently generate a workflow without any requested agent configuration.
- Design and architecture: The canonical
<library>_cfg_entry_pointand sibling environment/agent preset-root design is applied coherently across training, playback, benchmarks, LEAPP export, task registration, and documentation. However, the template generator does not preserve the apparent invariant that every selected workflow has a compatible requested agent configuration. - API: The removed
setup_preset_cli(..., agent_library=...)parameter and preset-specific agent-selection model are updated consistently across the included call sites and documented withpresets=migration guidance. No additional API inconsistency was identified in the supplied patch. - Implementation: In
tools/template/generator.py, validation accepts algorithms supported by the union of selected workflow types, but_generate_taskslater filters algorithms separately for each workflow and drops libraries with no remaining algorithms. For a mixed single-/multi-agent project requesting only IPPO or MAPPO, this emits the single-agent task withoutdefault_agentor any agent config entry point. Validation should ensure each selected workflow retains a compatible algorithm for each requested library, or otherwise reject the specification explicitly.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| normalized_libraries = [] | ||
| for rl_library in specification["rl_libraries"]: | ||
| algorithms = [algorithm.lower() for algorithm in rl_library.get("algorithms", [])] | ||
| invalid_algorithms = sorted(set(algorithms) - allowed_algorithms) |
Contributor
There was a problem hiding this comment.
🟡 Warning · Implementation — Mixed workflows can yield agentless generated task
Validation rejects algorithms outside the union of selected workflow types, while _generate_tasks filters per workflow and drops a library that retains no compatible algorithm. Selecting both single- and multi-agent workflows with only ippo/mappo passes validation, yet the single-agent task is emitted with no *_cfg_entry_point and no default_agent, so agent config resolution fails at train time. Validate per selected workflow that each library keeps a compatible algorithm.
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
2 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.
Backports #7611 to
release/3.0.0.The original cherry-pick conflicted. An NVIDIA inference model proposed this resolution, and deterministic validation confirmed that it changes no paths outside the original PR. Because conflict resolution cannot be certified as an exact patch replay, this PR is intentionally a draft and requires release-maintainer review.
673b6734b4cfd6d7f435cee12856299717fdcd6f772533db073a1024fae8fa593c24288e24585efbf742233e91d275549220a9f885b50287114597ff