Name Newton coupled solvers in pretrained checkpoint paths - #7539
Conversation
Greptile SummaryThe PR adds deterministic checkpoint naming for coupled Newton solver trees and uses the resolved physics preset when constructing checkpoint publication jobs.
Confidence Score: 5/5The PR appears safe to merge with coupled and uncoupled checkpoint naming aligned across publication and lookup. The selected physics preset is resolved through the established task-configuration path, and both checkpoint publication and retrieval derive the backend token from the resulting solver configuration without an identified reachable regression. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Task physics preset selector] --> B[Resolve task configuration]
B --> C[Inspect physics solver tree]
C --> D{Solver type}
D -->|Uncoupled MJWarp| E[newtonmjwarp]
D -->|Coupled entries| F[Concatenate entry families]
F --> G[Append coupling scheme]
G --> H[newtonmjwarpvbdproxy]
E --> I[Checkpoint filename]
H --> I
I --> J[Publish or fetch checkpoint]
Reviews (1): Last reviewed commit: "Name Newton coupled solvers in pretraine..." | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
Reviewed the new solver-tree checkpoint naming and the publishing path that resolves physics presets before constructing checkpoint filenames. Coupled proxy configurations now produce the documented newtonmjwarpvbdproxy token while uncoupled MJWarp naming remains unchanged.
- Design and architecture: Deriving coupled checkpoint tokens from ordered solver entries plus the coupling scheme fits the existing backend-name resolution flow. Resolving the selected preset in the publishing script also aligns the producer’s filename token with the consumer’s resolved environment configuration.
- API: The filename API now intentionally accepts non-empty derived physics tokens rather than a closed backend set, while preserving workflow validation, renderer validation, and the requirement that physics and renderer arguments be supplied together. The expanded coupled-token behavior is documented in the API docstring and changelog fragment.
- Implementation: The
CouplerProxyCfgcomposition yieldsmjwarp+vbd+proxy, and_build_core_jobsuses that resolved token while retaining the preset selector for training and playback. The implementation relies on Newton solver class-name conventions and adds preset-resolution work during job construction, but the shown paths do not establish either as requiring a pre-merge fix.
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.
|
run-ci |
# Description `--checkpoint pretrained` aborted for every coupled task with `ValueError: Unsupported Newton solver for pretrained checkpoints: CouplerProxyCfg`, because the physics token was derived only from the top-level Newton solver class and a coupled solver nests its solvers in entries. A Newton coupled solver is now named by its entry solvers in order followed by its coupling scheme, so a proxy coupler over MJWarp and VBD entries resolves to `newtonmjwarpvbdproxy`. Names for uncoupled solvers are unchanged, and this matches the names the coupled checkpoints are already published under. The physics token is no longer validated against a closed set, since it is derived from the solver tree rather than chosen from a fixed list. `train_and_publish_checkpoints.py` now derives each job's physics token by resolving its preset selector, so published filenames and the names `play` looks up cannot drift apart. Fixes 6672531 Fixes 6684411 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [x] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` ## Validation `uv run isaaclab play --rl_library rsl_rl --task Isaac-Lift-Cable-Franka --checkpoint pretrained --num_envs 4 --visualizer none` now fetches `Isaac-Lift-Cable-Franka_newtonmjwarpvbdproxy_none_rsl_rl.pt` and loads the policy, where it previously aborted before launch. `Isaac-Lift-Cloth-Franka` and `Isaac-Lift-Soft-Franka` resolve to the same token and are also published under it. - `uv run --extra test python -m pytest source/isaaclab_rl/test/test_pretrained_checkpoint.py scripts/tools/test/test_train_and_publish_checkpoints.py -q` — 21 passed - `uv run isaaclab -f` ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there (cherry picked from commit c9dca35)
|
Backported to |
Description
--checkpoint pretrainedaborted for every coupled task withValueError: Unsupported Newton solver for pretrained checkpoints: CouplerProxyCfg, because the physics token was derived only from the top-level Newton solver class and a coupled solver nests its solvers in entries.A Newton coupled solver is now named by its entry solvers in order followed by its coupling scheme, so a proxy coupler over MJWarp and VBD entries resolves to
newtonmjwarpvbdproxy. Names for uncoupled solvers are unchanged, and this matches the names the coupled checkpoints are already published under. The physics token is no longer validated against a closed set, since it is derived from the solver tree rather than chosen from a fixed list.train_and_publish_checkpoints.pynow derives each job's physics token by resolving its preset selector, so published filenames and the namesplaylooks up cannot drift apart.Fixes 6672531
Fixes 6684411
Type of change
Release backport
developValidation
uv run isaaclab play --rl_library rsl_rl --task Isaac-Lift-Cable-Franka --checkpoint pretrained --num_envs 4 --visualizer nonenow fetchesIsaac-Lift-Cable-Franka_newtonmjwarpvbdproxy_none_rsl_rl.ptand loads the policy, where it previously aborted before launch.Isaac-Lift-Cloth-FrankaandIsaac-Lift-Soft-Frankaresolve to the same token and are also published under it.uv run --extra test python -m pytest source/isaaclab_rl/test/test_pretrained_checkpoint.py scripts/tools/test/test_train_and_publish_checkpoints.py -q— 21 passeduv run isaaclab -fChecklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there