[Tasks] Fix pretrained checkpoint lookup for domain presets - #7594
[Tasks] Fix pretrained checkpoint lookup for domain presets#7594StafaH wants to merge 2 commits into
Conversation
Greptile SummaryThis PR incorporates non-default domain presets into canonical pretrained-checkpoint identity while preserving unsuffixed names for aliases of the default configuration. It also declares the Cartpole depth checkpoint and uses generated compatibility metadata to disable unsupported pretrained selections in the environment browser.
Confidence Score: 4/5The lookup and browser changes appear safe to merge, with a non-blocking gap in the supported workflow for publishing preset-specific checkpoints. Runtime lookup correctly distinguishes non-default domain presets and preserves default aliases, but the repository’s publishing job model cannot convey those presets and therefore cannot generate the new suffixed publication paths. Files Needing Attention: source/isaaclab_rl/isaaclab_rl/utils/pretrained_checkpoint.py and scripts/tools/train_and_publish_checkpoints.py Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
O["CLI preset overrides"] --> R["Resolve non-default domain presets"]
R --> N["Canonical checkpoint filename"]
B["Physics and renderer backends"] --> N
L["RL library"] --> N
N --> F["Fetch published checkpoint"]
M["Task compatibility metadata"] --> G["Generated environment-browser rows"]
G --> U["Enable or disable pretrained option"]
P["Checkpoint publishing workflow"] -. "domain preset not currently conveyed" .-> N
Reviews (1): Last reviewed commit: "Resolve pretrained checkpoints by preset" | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
The preset-aware filename and browser integration are consistent for backend-aware checkpoints, but backend-less legacy lookup now fails when process arguments include a non-default domain preset.
- Design and architecture: Central preset resolution gives backend-aware checkpoint consumers a consistent filename identity. However, resolving presets implicitly from process-wide
sys.argvbefore determining whether the caller requested the legacy layout couples that compatibility path to unrelated CLI state. - API: The new parameters are keyword-only and otherwise backward compatible, but
get_published_pretrained_checkpointstill documents omission of both backends as selecting the legacy layout. Withpresets=<non-default>insys.argv, that documented call now raisesValueErrorinstead of returning the legacy checkpoint. Preset resolution should occur only for backend-aware lookup. - Implementation: Preset suffix construction, publish/fetch paths, cache naming, generated browser metadata, and UI updates on preset or library changes were traced and are internally consistent. The remaining implementation fix is to avoid forwarding implicitly resolved preset names into backend-less legacy filename generation.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
Description
Published checkpoint lookup included the task, physics backend, renderer backend, and RL library, but ignored domain presets. As a result, a depth policy could not be found and an unsuffixed RGB policy could be selected for an incompatible preset.
This change:
No new dependencies.
Type of change
Release backport
Screenshots
Not applicable; the UI change disables the existing pretrained-checkpoint checkbox for unsupported preset selections.
Validation
Checklist