Skip to content

[Workflow] Default locomotion core tasks to Newton - #7064

Merged
StafaH merged 20 commits into
isaac-sim:developfrom
StafaH:mh/default-newton-locomotion
Aug 15, 2026
Merged

[Workflow] Default locomotion core tasks to Newton#7064
StafaH merged 20 commits into
isaac-sim:developfrom
StafaH:mh/default-newton-locomotion

Conversation

@StafaH

@StafaH StafaH commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

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.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR changes the Ant, Humanoid, and Velocity task defaults to Newton MJWarp and consolidates core environment smoke testing across explicit physics presets.

  • Updates locomotion physics and observation defaults.
  • Consolidates backend smoke tests and improves environment teardown.
  • Removes obsolete Newton-specific test discovery utilities.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
source/isaaclab_tasks/test/core/test_environments.py Builds a shared parametrized core-task smoke-test matrix from declared physics presets.
source/isaaclab_tasks/test/env_test_utils.py Applies explicit physics overrides during smoke tests and performs garbage collection before simulation teardown.
source/isaaclab_tasks/isaaclab_tasks/core/velocity/velocity_env_cfg.py Changes the shared rough-terrain Velocity physics default to Newton MJWarp.
source/isaaclab_tasks/isaaclab_tasks/core/locomotion/ant/ant_direct_env_cfg.py Changes the direct Ant environment’s default physics backend to Newton MJWarp.
source/isaaclab_tasks/isaaclab_tasks/core/locomotion/humanoid/humanoid_direct_env_cfg.py Changes the direct Humanoid environment’s default physics backend to Newton MJWarp.

Reviews (2): Last reviewed commit: "Revert "Split environment smoke tests by..." | Re-trigger Greptile

# Local imports should be imported last
from env_test_utils import _run_environments, setup_environment # isort: skip

@pytest.mark.parametrize("physics_preset_name", ["newton_mjwarp", "physx", "isaacsim_physx"])

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.

P2 Matrix silently uses default backend

The matrix applies every backend name to every core task without checking that the preset exists. For tasks such as Fourbar Pole, which only exposes newton_kamino, the newton_mjwarp, physx, and isaacsim_physx cases silently fall back to Kamino, so passing tests report backend coverage they did not exercise.

Knowledge Base Used: isaaclab_tasks: Task Registration and Organization

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 52e373f: the smoke matrix now derives only task/backend pairs whose raw config declares the requested PhysicsCfg preset, so unsupported Fourbar combinations are omitted rather than falling back.

@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 review traced the Newton MJWarp default changes across Ant, Humanoid, and velocity configurations, along with the consolidated three-backend smoke-test matrix and teardown cleanup. The proposed Play-environment finding is not supported as a defect because the helper’s filter removal is explicit and the patch does not establish that the newly collected Play variants are redundant.

  • Design and architecture: Physics defaults are consistently redirected to existing newton_mjwarp presets while retaining explicit PhysX, OvPhysX, and Newton Kamino selections. Manager-based Ant and Humanoid observation defaults are aligned with the selected backend. The shared rough-terrain default intentionally affects all configurations derived from RoughPhysicsCfg, which remains the principal breadth-of-impact consideration.
  • API: The existing physics preset names remain available for explicit selection, and the changelog identifies the default-backend change as breaking with migration guidance. The removed include_play and newton_mjwarp_envs parameters belong to the test utility surface, and the supplied patch updates the shown callers consistently.
  • Implementation: The deleted Newton-specific test module is replaced by a single core test parametrized over newton_mjwarp, physx, and isaacsim_physx; the workflow cache-warmer input is updated accordingly. Removing the Play filter broadens test collection, but the evidence does not demonstrate that this intended coverage is materially redundant or erroneous. The added gc.collect() runs before simulation-context teardown, matching its stated Warp finalizer ordering requirement.

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.

@AntoineRichard AntoineRichard 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.

LGTM. We may need to communicate that to QA. Also should update the environment docs? It may list the defaults? (I'm not sure) LGTM otherwise.

@AntoineRichard

Copy link
Copy Markdown
Collaborator

@StafaH Could you update the AnyMal-D visualizer test factory to select the explicit physx preset instead of default?

In source/isaaclab_visualizers/test/visualizer_integration_utils.py, _make_anymal_d_env() currently does:

preset_key = "newton_mjwarp" if backend_kind == "newton" else "default"

This PR changes the AnyMal-D default to newton_mjwarp, so the physx-anymal_d-* cases now initialize Newton and compare Newton output against the PhysX golden images. The CI log confirms Registered backend newton, producing the three stable golden-image failures.

Please make the PhysX branch resolve the named physx preset, ideally through _apply_env_cfg_preset() as the other visualizer factories do, while retaining newton_mjwarp for the Newton branch.

@AntoineRichard

Copy link
Copy Markdown
Collaborator

@StafaH The AnyMal-D determinism failures should also be fixed by keeping these existing tests on explicit PhysX.

test_locomotion_env_determinism currently calls _test_environment_determinism(task_name, device) for both Isaac-Velocity-Flat-AnymalD and Isaac-Velocity-Rough-AnymalD. Since this PR changes their default to Newton MJWarp, the CUDA cases now exercise nondeterministic MJWarp reductions and fail the strict equality assertions, while the CPU cases pass.

Could you pass physics_preset_name="physx" for these two tests? This preserves their previous PhysX determinism coverage. Newton determinism is already covered separately by test_newton_cartpole_env_determinism, which explicitly enables deterministic_mode="gpu_to_gpu".

@AntoineRichard

Copy link
Copy Markdown
Collaborator

@StafaH One more issue with the consolidated backend matrix in test_environments.py: it parametrizes every core task with newton_mjwarp, physx, and isaacsim_physx without checking whether each task actually exposes that preset.

Global preset resolution falls back to default when the requested name is absent. As a result, a case labeled physx can silently run Kamino or Newton and report backend coverage it did not exercise. For example, tasks that only expose newton_kamino will run their default for all three labels.

Could you generate only valid (task_name, physics_preset_name) pairs by inspecting the raw configuration presets before parametrization? Alternatively, use an explicit physics selector that raises when the requested preset is unavailable. The important part is that unsupported combinations should be omitted or reported, not silently fall back.

The removal of the Play-task filter also collects the Play variants in addition to their base tasks and substantially expands this already tripled matrix. The third task shard was still running after roughly 80 minutes. Unless separate Play coverage is intentional here, please restore the Play filter to avoid duplicate environment smoke coverage.

@StafaH
StafaH requested a review from matthewtrepte as a code owner August 13, 2026 19:14
@StafaH

StafaH commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review findings in 52e373f: the core smoke matrix now filters to PhysicsCfg presets actually declared by each task; AnyMal-D visualizer tests select explicit physx/newton_mjwarp presets; and AnyMal-D determinism is explicitly pinned to physx. The obsolete Play filter remains removed because the current core registry contains no Play task variants.

@StafaH StafaH closed this Aug 14, 2026
@StafaH StafaH reopened this Aug 14, 2026
…locomotion

# Conflicts:
#	.github/workflows/build.yaml
#	source/isaaclab_tasks/test/contrib/test_cartpole_showcase_environments_with_stage_in_memory.py
#	source/isaaclab_tasks/test/contrib/test_contrib_environments_smoke.py
#	source/isaaclab_tasks/test/contrib/test_pickplace_stack_environments.py
#	source/isaaclab_tasks/test/contrib/test_teleop_environments.py
#	source/isaaclab_tasks/test/contrib/test_teleop_environments_with_stage_in_memory.py
#	source/isaaclab_tasks/test/core/test_environments_isaacsim_physx.py
#	source/isaaclab_tasks/test/core/test_environments_newton.py
#	source/isaaclab_tasks/test/core/test_environments_ovphysx.py
#	source/isaaclab_tasks/test/core/test_environments_with_stage_in_memory.py
#	source/isaaclab_tasks/test/core/test_record_video.py
#	source/isaaclab_tasks/test/env_test_utils.py
@StafaH
StafaH merged commit 98babb6 into isaac-sim:develop Aug 15, 2026
47 of 48 checks passed
@StafaH
StafaH deleted the mh/default-newton-locomotion branch August 18, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants