[Worflow] Make newton and rsl_rl default across core tasks - #6980
[Worflow] Make newton and rsl_rl default across core tasks#6980StafaH wants to merge 20 commits into
Conversation
Greptile SummaryThe PR makes Newton MJWarp and RSL-RL the defaults across core tasks while preserving explicit backend selection.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or non-blocking defects identified in the changed behavior. Default-agent lookup follows the repository's established Gym task-name convention, and the Newton physics and renderer defaults are coordinated with explicit alternatives and updated compatibility coverage. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
CLI["Unified train/play CLI"] --> Task["Read --task"]
Task --> Registry["Gym task registry"]
Registry --> Agent["default_agent = rsl_rl"]
Agent --> Backend["RSL-RL backend"]
Registry --> Config["Core task configuration"]
Config --> Physics["Newton MJWarp physics"]
Config --> Renderer["Newton Warp renderer"]
Physics --> Runtime["Kitless default runtime"]
Renderer --> Runtime
Reviews (1): Last reviewed commit: "Make newton and rsl_rl default" | Re-trigger Greptile |
There was a problem hiding this comment.
Isaac Lab Review Bot
The new task-registered RSL-RL fallback and Newton MJWarp/Newton Warp defaults are implemented consistently, but both added changelog fragments are empty despite these user-visible behavior changes.
- Design and architecture: Task-level
default_agentmetadata provides a centralized backend-selection mechanism, while the preset aliases consistently establish Newton MJWarp and the Newton Warp renderer as defaults. No architectural issue was accepted beyond the missing release documentation for these decisions. - API: Explicit
--rl_libraryand explicit physics/renderer presets remain available, but omitted selections now resolve differently. These changed CLI and default-preset behaviors require package changelog entries with appropriate migration guidance. - Implementation: The required fragment files exist for
isaaclab_rlandisaaclab_tasks, but both are empty and therefore produce no compiled changelog entry. Populate them under the appropriateChangedcategory, marking the breaking default switch and documenting how users can explicitly retain prior behavior.
Minor fixes needed. Posted 1 actionable finding inline.
Automated review; human maintainers own approval decisions.
| @configclass | ||
| class MultiBackendRendererCfg(PresetCfg): | ||
| default: IsaacRtxRendererCfg = IsaacRtxRendererCfg() | ||
| default: NewtonWarpRendererCfg = NewtonWarpRendererCfg() |
There was a problem hiding this comment.
🟡 Warning · Implementation — Empty changelog fragments for breaking default switch
This change flips resolved defaults (renderer to Newton Warp, core physics/decimation to newton_mjwarp) and makes --rl_library optional, but source/isaaclab_tasks/changelog.d/default-newton-rsl.minor.rst and source/isaaclab_rl/changelog.d/default-newton-rsl.minor.rst were added with no content, so the compiled changelog will carry no entry. Repository rules require a Changed entry per touched package, prefixed with **Breaking:**, plus migration guidance (e.g. selecting presets=isaacsim_physx,isaacsim_rtx to restore prior behavior).
…mh/new_defaults_core
Making Newton MJWarp and the Newton renderer the defaults changed what several tests resolved to, so they compared new-backend output against PhysX/Isaac-RTX baselines or exercised a backend they were not written for. Select the backend explicitly instead so the resolution no longer tracks whichever preset is currently the default: - rendering-correctness and visualizer golden tests pin isaacsim_physx and isaacsim_rtx; the baselines are renamed to state the pinned backends (contents unchanged). - the AnymalD determinism suite pins isaacsim_physx, since Newton MJWarp is not yet run-to-run deterministic on CUDA for locomotion. The recorded robot-PoV camera shared by the XR reference tasks is pinned to the Isaac RTX renderer in the config itself: the Newton renderer cannot load the UDIM textures of the GR1T2 and G1 assets, so the default flip aborted PickPlace env creation and Mimic dataset generation.
…mh/new_defaults_core
The default-flip commit renamed the golden PNGs from default_physics-default_renderer-* to isaacsim_physx-isaacsim_rtx-* but deleted the matching -stage.usda goldens instead of renaming them, so test_rendering_registered_tasks failed with "Golden stage not found" for both camera tasks.
hujc7
left a comment
There was a problem hiding this comment.
Looking great. Might still need to explicit resolve the physics for some tests if they were using the default physx?
# Conflicts: # source/isaaclab_tasks/isaaclab_tasks/core/lift/config/franka_soft/__init__.py
The develop merge bumped several editable package versions in their extension.toml files. Refresh the lockfile so it matches the merged tree.
The environment tests run every registered task sequentially in one process. Warp arrays release their device memory from a finalizer, so an array left for a later garbage collection runs wp_free_device_async against streams that SimulationContext.clear_instance() has already destroyed, segfaulting the interpreter mid-run. Collect unreachable objects while the device is still alive so those finalizers run at a safe point. Making Newton the default backend for the core tasks put many more Warp arrays in flight, which is what exposed this ordering.
# Conflicts: # source/isaaclab_tasks/test/core/test_environment_determinism.py
AntoineRichard
left a comment
There was a problem hiding this comment.
Look, good, a lot of tasks change default from PhysX to Newton. If that's what we want then it's great!
There was a problem hiding this comment.
I don't think the loc should have changed?
kellyguo11
left a comment
There was a problem hiding this comment.
should we keep all of our documentation and tutorials to use --viz newton by default with this change? and only use --viz kit when kit is required, and highlight in those docs that isaac sim is required for them
|
kwargs={ |
|
The contrib coverage split is #7068. |
Splits the default RL-library change from #6980.\n\n- Resolves `default_agent` from task registration when `--rl_library` is omitted.\n- Registers RSL-RL as the default for core tasks, including a Pendulum RSL-RL config.\n- Adds dispatch coverage and a breaking-change fragment.\n\nValidation: `uv run python -m pytest source/isaaclab_rl/test/test_entrypoints.py -q` (11 passed, 2 skipped).\n\nSupersedes the RSL-RL portion of #6980.
Splits the Ant, Humanoid, and Velocity Newton defaults from #6980.\n\n- Defaults Ant, Humanoid, and Velocity task configurations to Newton MJWarp.\n- Includes the shared three-backend core smoke-test matrix and teardown reliability cleanup.\n- Removes obsolete Play/MJWarp-discovery test utility logic.\n\nThe shared test commit is intentionally identical to the other environment shards; rebase/drop it after the first shard merges.\n\nSupersedes the locomotion portion of #6980.
Splits the Cabinet and Lift Newton defaults from #6980.\n\n- Defaults Cabinet, Lift, and Kuka Allegro task configurations to Newton MJWarp.\n- Includes the shared three-backend core smoke-test matrix and teardown reliability cleanup.\n- Removes obsolete Play/MJWarp-discovery test utility logic.\n\nThe shared test commit is intentionally identical to the other environment shards; rebase/drop it after the first shard merges.\n\nSupersedes the manipulation portion of #6980.
Splits the Cartpole and shared-renderer defaults from #6980.\n\n- Defaults Cartpole to Newton MJWarp.\n- Defaults `MultiBackendRendererCfg` to the Newton renderer while retaining explicit Isaac RTX selection.\n- Consolidates core environment smoke tests into one three-backend matrix (`newton_mjwarp`, `physx`, `isaacsim_physx`).\n- Removes obsolete Play/MJWarp-discovery test utility logic and the redundant Newton test module.\n\nValidation: `uv run python -m pytest source/isaaclab_tasks/test/core/test_preset_kit_decision.py source/isaaclab_tasks/test/core/test_runtime_compatibility.py -q` (51 passed).\n\nSupersedes the classic-controls portion of #6980. --------- Co-authored-by: ooctipus <zhengyuz@nvidia.com>
Description
Important
Confirm the pull request base before submitting. Target
developfor allcontributions. The
release/3.0.0-beta2branch is a frozen stable landingsnapshot and is not used for ongoing maintenance.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
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