[Odin] Fix preset-based agent selection for rsl_rl, rl_games and sb3

[AntoineRichard]

Description

Preset-based --agent auto-selection is dead code for rsl_rl, rl_games and sb3. Two defects sit in series on the same code path:

1. The selection guard cannot see past the CLI default. _auto_select_agent in source/isaaclab_tasks/isaaclab_tasks/utils/preset_cli.py was only reached when args.agent is None. add_common_train_args registers --agent with default=agent_default, and every backend except skrl passes a non-None agent_default (rsl_rl_cfg_entry_point, rl_games_cfg_entry_point, sb3_cfg_entry_point). The parsed value is therefore never None, so the branch never runs.
2. The benchmark entrypoints never asked for it. benchmark_train_{rsl_rl,rl_games,sb3}.py and benchmark_play_{rsl_rl,rl_games,sb3}.py called setup_preset_cli(parser, argv) without agent_library, so auto-selection was not attempted at all (and the registered-agent help listing was missing too). Only the skrl benchmark entrypoints wired it.

Provenance and symptom

Benchmark sweep dispatch 20260901-153531 (image built from release/3.0.0 at f88dbc59c82, rsl_rl) produced 60 failed rows: resnet18 (30) and theia_tiny (30), failing 100% across all three renderers and all physics backends. Every one died before the first training step:

ValueError: Observation 'critic' in observation set 'critic' not found in the observations
from the environment. Available observations from the environment: ['policy']

Call chain: OnPolicyRunner.__init__ → ppo.py construct_algorithm → rsl_rl/utils/utils.py:233 resolve_obs_groups.

Why this is an agent-entrypoint bug, not a missing observation group

Isaac-Cartpole-Camera already declares the correct pairing:

"agent_preset_compatibility": {
    "rsl_rl_cfg_entry_point": _RAW_CAMERA_PRESETS,
    "rsl_rl_feature_cfg_entry_point": ("resnet18", "theia_tiny"),
    ...
}

and CartpoleCameraFeaturePPORunnerCfg sets obs_groups with critic: ["policy"]. The feature entry point exists, is registered, is correct — it was simply unreachable, so resnet18/theia_tiny ran against the raw-camera CartpoleCameraPPORunnerCfg, whose obs_groups asks for a critic group the env does not expose. Nothing needs to be added to the environment.

Why both defects are in one PR

Neither half fixes the observed failure alone; this was measured, not assumed. Reverting either half of the change and running the regression test:

state	result
defect 2 fixed only (agent_library wired, guard unchanged)	4 failed — --agent still parses to the non-None default, so the guard rejects
defect 1 fixed only (guard fixed, agent_library not wired)	4 failed — if agent_library and ... is False, auto-selection never attempted
both fixed	passes

They are two links in one chain, so splitting them yields a PR that fixes nothing observable and a PR that cannot be tested end-to-end.

Scope: train and play, benchmark and non-benchmark

To pre-empt the obvious question — this is not a play-only or a train-only fix.

path	affected by	fixed by	proven by
benchmark_train_{rsl_rl,rl_games,sb3}	defects 1 + 2	preset_cli.py + agent_library= wiring	test_training_request_selects_preset_compatible_agent — this is the observed 60-row failure
benchmark_play_{rsl_rl,rl_games,sb3}	defects 1 + 2	preset_cli.py + agent_library= wiring	test_play_request_selects_preset_compatible_agent
isaaclab_rl train_*/play_* (non-benchmark)	defect 1 only — they already pass agent_library	preset_cli.py alone; no file in this PR touches them	test_setup_preset_cli_auto_selects_agent_over_non_none_default

Each half of the benchmark wiring was reverted independently and re-tested:

• revert the three train wirings → the 4 training selection cases fail. Load-bearing for the reported failure.
• revert the three play wirings → the 2 playback selection cases fail.

There is no train/play asymmetry that would justify wiring only one of them: both register --agent with the same non-None default via the same helper, both call setup_preset_cli, and both feed args.agent into the same resolve_task_config(...) and then into the same OnPolicyRunner(...) construction. The reason the sweep only surfaced the training failure is ordering, not asymmetry — a row that dies at train_rc=1 never reaches playback. Measured on the playback path before the wiring was added:

$ BenchmarkPlayRequest(backend="rsl_rl", task="Isaac-Cartpole-Camera", presets=("resnet18",))
args.agent : rsl_rl_cfg_entry_point          # raw-camera config, wrong
# with the wiring:
args.agent : rsl_rl_feature_cfg_entry_point  # correct

Playback would therefore have loaded a feature-trained checkpoint into the raw-camera policy architecture.

Fix chosen

Detect an explicitly typed --agent by re-parsing the same argv into a namespace pre-seeded with a sentinel: argparse only applies a default for a destination the namespace does not already carry, so the sentinel survives unless the user actually typed the flag. Auto-selection runs only when it does survive; an explicit --agent still wins. The six benchmark entrypoints now pass agent_library.

The probe uses the same parser on the same argv as the real parse, so its verdict is argparse's verdict. Verified across every spelling — --agent V, --agent=V, the abbreviation --age V, repeated flags, and --agent after a -- separator (correctly not explicit: argparse does not set it in the real parse either). A literal argv scan for --agent would get the abbreviation wrong and silently override a user's explicit choice, which is why the probe is preferred; the repo's existing ExplicitAction idiom would work too but requires touching all ten --agent registration sites across three packages, and a missed site fails the same silent way.

Rejected: make --agent default to None everywhere

This is the obvious fix and it is not safe. skrl can default to None because train_skrl.py reconstructs the entry point from --algorithm when it is None. The other three pass args_cli.agent straight into resolve_task_config(...), which has no such fallback — hydra.py:619 sets agent_cfg = load_cfg_from_registry(...) if agent_entry else None, and the entrypoints then dereference agent_cfg.max_iterations. A None default would break every plain --task=X run that relies on the canonical entry point, i.e. the overwhelmingly common case. Fixing that would mean adding a fallback to each of the six benchmark entrypoints plus the four train/play entrypoints: a much wider blast radius than the bug.

Blast radius

Behavior changes only where auto-selection actually fires, and only when the user did not type --agent:

• Rule 1 (preset-based) fires only for tasks that opted in with agent_preset_compatibility — today Isaac-Cartpole-Camera and the two cartpole-showcase tasks. That is the declared contract finally being honored. Anyone who wants the previous (broken) pairing can still pass --agent rsl_rl_cfg_entry_point explicitly; covered by a test.
• Rule 2 (default-absent) fires only when <library>_cfg_entry_point is not registered and exactly one other entry point is. That path previously resolved an unregistered entry point and crashed, so this is strictly a repair.
• Everything else — no preset pairing declared, or the canonical default is registered — keeps the exact default it had; covered by a test for all three libraries.
• skrl is unaffected: it already passed agent_default=None, and the explicit/implicit distinction collapses to the old is None check for it.

rl_games and sb3 share the defect and are fixed by the same change. sb3 has no task declaring agent_preset_compatibility, so there is no positive selection test for it — its wiring is covered by the shared code path and by a default-preservation test only.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Screenshots

Not applicable.

Validation

Regression tests were confirmed to fail on develop without the fix and pass with it.

Before the fix (production changes reverted to develop, tests in place):

$ uv run --frozen --extra test python -m pytest source/isaaclab_tasks/test/core/test_preset_cli.py \
    source/isaaclab/test/benchmark/test_api.py -q -p no:warnings \
    -k "preset_compatible_agent or keeps_backend_default_agent or auto_selects_agent_over_non_none or keeps_explicit_agent"

FAILED test_preset_cli.py::test_setup_preset_cli_auto_selects_agent_over_non_none_default
FAILED test_api.py::test_training_request_selects_preset_compatible_agent[resnet18-rsl_rl]
FAILED test_api.py::test_training_request_selects_preset_compatible_agent[resnet18-rl_games]
FAILED test_api.py::test_training_request_selects_preset_compatible_agent[theia_tiny-rsl_rl]
FAILED test_api.py::test_training_request_selects_preset_compatible_agent[theia_tiny-rl_games]
5 failed, 6 passed, 53 deselected

E  AssertionError: assert 'rl_games_cfg_entry_point' == 'rl_games_feature_cfg_entry_point'

After the fix:

$ uv run --frozen --extra test python -m pytest source/isaaclab_tasks/test/core/test_preset_cli.py \
    source/isaaclab_tasks/test/core/test_hydra.py source/isaaclab/test/benchmark \
    source/isaaclab_rl/test/test_entrypoints_common.py -q -p no:warnings
492 passed, 1 skipped in 30.89s

$ uv run --frozen --extra test --extra skrl --extra sb3 --extra rl-games --extra rsl-rl \
    python -m pytest source/isaaclab_rl/test/test_typed_preset_cli_train_play.py -q -p no:warnings
8 passed in 33.27s

(The last suite fails on a bare --extra test environment on develop too — ModuleNotFoundError: No module named 'skrl' etc. — so it was rerun with the RL extras.)

$ uv run --frozen python tools/changelog/cli.py check develop
✓ All modified packages have valid changelog fragments.

$ uv run --frozen isaaclab -f
all hooks passed

Tests added:

• source/isaaclab_tasks/test/core/test_preset_cli.py — auto-selection over a non-None default; explicit --agent wins over the preset, parametrized over the three spellings argparse accepts. The two pre-existing auto-selection tests only covered skrl with agent_default=None, which is exactly why this bug went unnoticed.
• source/isaaclab/test/benchmark/test_api.py — drives the real benchmark entrypoints via BenchmarkTrainingRequest/BenchmarkPlayRequest and _parse_args, asserting the feature entry point is selected for resnet18/theia_tiny × rsl_rl/rl_games on the train path (mirroring the failing rows), the same on the play path, and that the canonical default survives otherwise (rsl_rl, rl_games, sb3).

Both assert the selected agent config, not merely absence of an exception.

Not run: a real training job on the failing rows (requires GPU sim). The failure is a construction-time config selection, fully reproduced at CLI level.

Related PRs — checked, no overlap

• Select pretrained checkpoints from resolved task configs #7491 (Select pretrained checkpoints from resolved task configs) touches isaaclab_rl/entrypoints/common.py, the four play_* entrypoints, the four benchmark_play_* entrypoints, cartpole/__init__.py and test_preset_cli.py — adjacent, but changes neither agent_default nor the selection guard. Textual conflicts are possible in test_preset_cli.py and the benchmark_play_* files (one-line setup_preset_cli(...) call); no semantic conflict.
• fix(rsl_rl): default obs_groups for new runners #6440 (fix(rsl_rl): default obs_groups for new runners) adds an obs_groups default in isaaclab_rl/rsl_rl/utils.py. It would mask this symptom for runners that omit obs_groups, but CartpoleCameraPPORunnerCfg sets obs_groups explicitly, so it does not fix these 60 rows — and it would not make the correct feature config get selected either. Complementary, not duplicate.
• gh pr list --search "agent preset" returned nothing touching this code.

[greptile-apps]

Greptile Summary

The PR repairs preset-based agent selection when --agent has a non-None parser default and enables that selection in the RSL-RL, RL-Games, and SB3 benchmark entrypoints.

• Distinguishes an explicit --agent argument from a parser-supplied default using a sentinel-backed probe parse.
• Preserves backend defaults when no unambiguous preset-compatible agent is available.
• Wires agent_library through six benchmark train/play entrypoints.
• Adds regression coverage for preset selection, explicit overrides, and default preservation.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable correctness, security, or compatibility issue identified.

The new selection path preserves explicit user choices and existing defaults while selecting a different agent only when task registration provides an unambiguous compatible entry point.

Important Files Changed

Filename	Overview
source/isaaclab_tasks/isaaclab_tasks/utils/preset_cli.py	Adds explicit-agent detection and returns an optional auto-selected entry point without overwriting valid defaults.
source/isaaclab/isaaclab/benchmark/entrypoints/backends/rsl_rl/benchmark_train_rsl_rl.py	Enables RSL-RL benchmark training to participate in preset-compatible agent selection.
source/isaaclab/isaaclab/benchmark/entrypoints/backends/rl_games/benchmark_train_rl_games.py	Enables RL-Games benchmark training to participate in preset-compatible agent selection.
source/isaaclab/isaaclab/benchmark/entrypoints/backends/sb3/benchmark_train_sb3.py	Supplies the SB3 library identifier while retaining its canonical agent default when no alternate match exists.
source/isaaclab/test/benchmark/test_api.py	Adds integration-level regression tests for compatible-agent selection and backend-default preservation.
source/isaaclab_tasks/test/core/test_preset_cli.py	Covers non-None defaults, explicit agent precedence, and no-selection fallback behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Parse benchmark or train arguments] --> B{Was --agent explicitly supplied?}
    B -->|Yes| C[Preserve explicit agent]
    B -->|No| D[Enumerate task agent entry points]
    D --> E{Exactly one preset-compatible agent?}
    E -->|Yes| F[Select compatible agent]
    E -->|No| G[Preserve parser default]
    C --> H[Resolve task and agent configuration]
    F --> H
    G --> H

Loading

_{Reviews (1): Last reviewed commit: "Fix preset-based agent selection for rsl..." | Re-trigger Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

Reviewed the preset-agent selection fix across the shared CLI helper and all six rsl_rl, rl_games, and sb3 benchmark train/play entrypoints. The patch distinguishes an explicit --agent from an argparse default, enables backend-aware selection in the benchmark paths, and preserves canonical defaults when no unambiguous preset-compatible agent exists.

• Design and architecture: Returning str | None from _auto_select_agent centralizes namespace mutation in setup_preset_cli and cleanly preserves caller defaults when selection is absent or ambiguous. The sentinel namespace probe is consistent with argparse behavior and retains explicit-user-choice precedence. Re-parsing does execute parser actions twice, but the affected entrypoints use ordinary argument actions; custom side-effecting actions remain a non-blocking future compatibility consideration.
• API: setup_preset_cli remains signature-compatible, and its agent_library behavior is now documented. The changed _auto_select_agent helper is private, its local callers were updated, and both affected source packages include changelog fragments. Explicit --agent values continue to override automatic selection.
• Implementation: The producer-to-consumer path was traced from benchmark request argv generation through setup_preset_cli to resolve_task_config. Preset compatibility selects the feature entrypoints for rsl_rl and rl_games, ambiguous or absent matches retain existing defaults, and the default-absent rule still supports tasks with a single noncanonical agent entrypoint. Tests cover feature-preset selection, explicit override precedence, and default preservation across the wired backends. The added hasattr guard means parsers without an agent destination no longer receive one implicitly, but the changed benchmark consumers all define --agent.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.