Skip to content

Cleanup patches, Compose agent variants through canonical preset roots - #7611

Merged
ooctipus merged 2 commits into
isaac-sim:developfrom
ooctipus:refactor/agent-preset-composition
Sep 9, 2026
Merged

Cleanup patches, Compose agent variants through canonical preset roots#7611
ooctipus merged 2 commits into
isaac-sim:developfrom
ooctipus:refactor/agent-preset-composition

Conversation

@ooctipus

@ooctipus ooctipus commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Description

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:

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.

Before and after

1. Environment-coupled agent variants

Before: repeat the pairing, then teach the CLI to translate it

The camera task registered separate raw and feature agent keys, then repeated their relationship to environment presets in another dictionary:

_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:

"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",
# ... one key for every remaining space combination ...
"agent_preset_compatibility": {
    "skrl_cfg_entry_point": ("box_box",),
    "skrl_box_discrete_cfg_entry_point": ("box_discrete",),
    # ... repeat every pairing again ...
},

After: the canonical agent entry point is a preset root

The agent package owns the variants directly:

@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:

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)

# Gym registration
"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.

2. Preset CLI ownership

Before: parse, rediscover task metadata, and mutate another subsystem's argument

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

# Every caller had to opt into the second selector.
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.

After: parse preset syntax and return it unchanged

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.

3. Humanoid AMP and SKRL algorithm identity

Before: infer both the config and algorithm from CLI/key spelling

# Humanoid AMP registered no canonical SKRL key.
"skrl_amp_cfg_entry_point": "...:skrl_walk_amp_cfg.yaml"

# SKRL defaulted to PPO and reconstructed meaning from registry-key text.
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.

After: the task owns its default; the resolved config owns its algorithm

# Humanoid AMP registration
"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.

4. Downstream consumers

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.

5. Commands

Before, users either had to provide both coupled selectors explicitly or rely on the preset CLI to synthesize the --agent half:

# Before: two names for one camera-feature choice
uv run isaaclab train --rl_library rsl_rl \
    --task Isaac-Cartpole-Camera \
    --agent rsl_rl_feature_cfg_entry_point presets=resnet18

# After: one preset is composed into both roots
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:

# Before
uv run isaaclab train --rl_library skrl \
    --task IsaacContrib-Cartpole-Showcase-Direct \
    --agent skrl_box_discrete_cfg_entry_point presets=box_discrete

# After
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:

# Before, without relying on CLI auto-selection
uv run isaaclab train --rl_library skrl \
    --task IsaacContrib-Humanoid-AMP-Walk-Direct --algorithm AMP

# After: the canonical task config is 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:

uv run isaaclab train --rl_library skrl --task <GENERATED-MARL-TASK>

Exact lineage of the removed mechanism

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.

Intentional compatibility removal

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.

Type of change

  • Bug fix
  • Architecture refactor
  • Intentional removal of recently introduced compatibility APIs
  • Documentation update

Validation

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.

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Screenshots

Not applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with uv run --frozen isaaclab -f
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove the fix is effective
  • I have added changelog fragments for every touched package
  • My name already exists in CONTRIBUTORS.md

@github-actions github-actions Bot added documentation Improvements or additions to documentation isaac-lab Related to Isaac Lab team infrastructure labels Sep 7, 2026
@ooctipus
ooctipus force-pushed the refactor/agent-preset-composition branch from 1378798 to e77186b Compare September 7, 2026 10:56
@ooctipus
ooctipus marked this pull request as ready for review September 7, 2026 11:30
@ooctipus
ooctipus requested a review from a team September 7, 2026 11:30
@ooctipus ooctipus changed the title Compose agent variants through canonical preset roots Cleanup patches, Compose agent variants through canonical preset roots Sep 7, 2026
@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR consolidates environment-coupled agent selection under canonical preset roots instead of registry compatibility maps and CLI-side agent rewriting.

  • Adds canonical agent preset families for camera and showcase tasks.
  • Makes resolved SKRL agent.class authoritative for algorithm identity across train, play, benchmark, and export workflows.
  • Removes preset-specific registry aliases and agent-selection behavior from the preset CLI and environment browser.
  • Updates diagnostics, documentation, generated browser data, changelogs, and regression coverage.

Confidence Score: 5/5

The PR appears safe to merge, with the canonical preset composition, SKRL algorithm identity, generated browser schema, and affected registrations aligned by implementation and tests.

No actionable failures remain: affected preset families match their environment contracts, SKRL callers consistently consume the resolved algorithm identity, and the documentation schema migration is positionally consistent.

Important Files Changed

Filename Overview
source/isaaclab_tasks/isaaclab_tasks/utils/preset_cli.py Removes registry-driven agent discovery and mutation, leaving preset help and argument parsing as the CLI's sole responsibilities.
source/isaaclab_tasks/isaaclab_tasks/core/cartpole/init.py Replaces camera feature-specific agent keys and compatibility metadata with canonical RL-Games and RSL-RL preset roots.
source/isaaclab_tasks/isaaclab_tasks/core/cartpole/agents/rsl_rl_ppo_cfg.py Adds the canonical RSL-RL preset family for default, ResNet18, and Theia Tiny camera policies.
source/isaaclab_tasks/isaaclab_tasks/contrib/cartpole_showcase/cartpole/agents/init.py Collects the complete showcase SKRL YAML family behind one canonical preset-producing entry point.
source/isaaclab_rl/isaaclab_rl/skrl.py Centralizes canonical SKRL entry-point selection and validates algorithm identity from the resolved configuration.
tools/environ_docs.py Removes agent-preset compatibility data from generated documentation and limits displayed algorithms to recognized algorithm entry points.
docs/source/_static/css/environment-browser.js Adopts the simplified generated row schema and stops injecting preset-specific agent arguments into commands.
source/isaaclab_tasks/test/core/test_agent_preset_composition.py Adds regression coverage proving environment and agent roots compose from the same preset and registrations remain canonical-only.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    CLI["CLI: task, library, presets=name"] --> Resolver["resolve_task_config"]
    Registry["Canonical library cfg entry point"] --> Resolver
    Resolver --> EnvRoot["Environment PresetCfg root"]
    Resolver --> AgentRoot["Agent PresetCfg root"]
    EnvRoot --> EnvVariant["Matching environment variant"]
    AgentRoot --> AgentVariant["Matching agent variant or compatible default"]
    EnvVariant --> Runtime["Training / play / benchmark / export"]
    AgentVariant --> Runtime
Loading

Reviews (1): Last reviewed commit: "Refactor agent variants into canonical p..." | Re-trigger Greptile

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isaac Lab Review Bot

The canonical preset-root design consistently unifies environment and agent variant selection, but the patch immediately removes two existing user-facing compatibility surfaces: preset-specific Gym agent entry-point keys and the public setup_preset_cli(..., agent_library=...) parameter. Both require a deprecation and migration period under repository policy.

  • Design and architecture: Using canonical <library>_cfg_entry_point preset roots removes duplicated registry/CLI/browser dispatch logic, and the updated resolver, browser schema, SKRL algorithm resolution, and Humanoid AMP registration follow that model consistently. Transitional aliases can preserve compatibility without changing the new canonical ownership model.
  • API: Existing --agent values such as the camera feature and showcase space-specific entry points now fail registry lookup, while out-of-tree callers passing agent_library= to the exported setup_preset_cli API now receive TypeError. Retain deprecated compatibility aliases and accept the legacy keyword with a warning for a migration window.
  • Implementation: In-repository callers were migrated, SKRL algorithm identity is now derived from resolved agent.class, and canonical AMP lookup remains reachable. However, updating only in-tree consumers does not satisfy the compatibility contract for public callers and previously valid commands; targeted deprecated shims are needed before removing those surfaces.

Minor fixes needed. Posted 2 actionable findings inline.

Automated review; human maintainers own approval decisions.

Comment thread source/isaaclab_tasks/isaaclab_tasks/core/cartpole/__init__.py
Comment thread source/isaaclab_tasks/isaaclab_tasks/utils/preset_cli.py
@ooctipus
ooctipus force-pushed the refactor/agent-preset-composition branch from e77186b to c307886 Compare September 8, 2026 03:22
@ooctipus

ooctipus commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 8, 2026
@kellyguo11 kellyguo11 moved this to In review in Isaac Lab Sep 8, 2026

@hujc7 hujc7 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving. The canonical-preset-root model holds up: I verified that presets=resnet18 / theia_tiny compose the feature runner and the feature observation group for both RSL-RL and RL-Games, that all 15 showcase space presets pair their env spaces with the matching SKRL model config, and that SKRL algorithm identity now follows agent.class for AMP, IPPO and MAPPO. No comments follow on any of that.

Two inline items, both non-blocking. The first is the one I would still fix before merge: the project template was not migrated alongside the in-repo AMP tasks, so generated projects on a non-PPO algorithm no longer resolve their agent config.

Comment thread source/isaaclab_tasks/isaaclab_tasks/utils/preset_cli.py
Comment thread source/isaaclab_rl/isaaclab_rl/entrypoints/backends/train_skrl.py Outdated
…reset-composition

# Conflicts:
#	docs/source/_static/css/environment-browser.js
#	source/isaaclab_tasks/isaaclab_tasks/core/cartpole/__init__.py
@ooctipus

ooctipus commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

run-ci

@isaaclab-bot isaaclab-bot Bot added ci:run-docker Trigger the on-demand Docker and GPU CI workflow and removed ci:run-docker Trigger the on-demand Docker and GPU CI workflow labels Sep 9, 2026
@ooctipus
ooctipus merged commit 673b673 into isaac-sim:develop Sep 9, 2026
54 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in Isaac Lab Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation infrastructure isaac-lab Related to Isaac Lab team

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants