Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -966,8 +966,7 @@ jobs:
# (dexterous, many contacts). Deformable/MPM kernels are not covered;
# widen if a test job reports large cache growth.
include-files: >-
test_environments.py,
test_environments_newton.py
test_environments.py
# No Soft/Cloth: the deformable envs depend on optional extras the CI
# image does not install (Soft needs pytetwild), so they only ever fail.
test-k-expr: "Cartpole or Drawer or AnymalD or Handover"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Changed
^^^^^^^

* **Breaking:** Changed the Cabinet and Lift task families to use Newton MJWarp

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.

This entry explicitly classifies the default-backend switch as breaking, but I could not find a prior deprecation for the Cabinet or Lift default behavior. Please reconcile this with the repository policy that breaking changes require a deprecation first, or clarify why this workflow-default change is exempt.

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.

The default switch is intentional for the 3.0 release. Per the release direction, we are documenting the direct breaking migration in the changelog rather than adding a deprecation alias or transitional behavior.

by default. Pass an explicit physics preset to retain Isaac Sim PhysX.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ class CabinetSimCfg(PresetCfg):
physx: SimulationCfg = isaacsim_physx.replace(
physics=PhysxAutoCfg(isaacsim_physx=isaacsim_physx.physics, ovphysx=ovphysx.physics)
)
default: SimulationCfg = isaacsim_physx
newton_mjwarp: SimulationCfg = SimulationCfg(
dt=1 / 600,
render_interval=1,
Expand All @@ -125,6 +124,7 @@ class CabinetSimCfg(PresetCfg):
default_visualizer_cfg=VisualizerCfg(eye=(-2.0, 2.0, 2.0), lookat=(0.8, 0.0, 0.5)),
physics=NewtonCfg(solver_cfg=KaminoPADMMSolverCfg(max_contacts_per_world=64)),
)
default: SimulationCfg = newton_mjwarp


@configclass
Expand All @@ -138,9 +138,9 @@ class CabinetDecimationCfg(PresetCfg):
isaacsim_physx: int = 1
ovphysx: int = isaacsim_physx
physx: int = isaacsim_physx
default: int = isaacsim_physx
newton_mjwarp: int = 10
newton_kamino: int = 10
default: int = newton_mjwarp


##
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ class KukaAllegroPhysicsCfg(lift.PhysicsCfg):
gpu_found_lost_pairs_capacity=2**26,
)
physx = PhysxAutoCfg(isaacsim_physx=isaacsim_physx, ovphysx=ovphysx)
default = isaacsim_physx
newton_mjwarp = lift.PhysicsCfg().newton_mjwarp
default = newton_mjwarp


@configclass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ class PhysicsCfg(PresetCfg):
debug_mode=False,
)
physx = PhysxAutoCfg(isaacsim_physx=isaacsim_physx)
default = isaacsim_physx
default = newton_mjwarp


@configclass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@
@pytest.mark.parametrize("num_envs, device", [(2, "cuda"), (1, "cuda")])
@pytest.mark.parametrize(
"task_name",
setup_environment(
include_play=False, factory_envs=False, multi_agent=False, teleop_envs=False, cartpole_showcase_envs=True
),
setup_environment(factory_envs=False, multi_agent=False, teleop_envs=False, cartpole_showcase_envs=True),
)
@pytest.mark.isaacsim_ci
def test_cartpole_showcase_environments(task_name, num_envs, device):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@
@pytest.mark.parametrize("num_envs, device", [(2, "cuda")])
@pytest.mark.parametrize(
"task_name",
setup_environment(
include_play=False, factory_envs=False, multi_agent=False, teleop_envs=False, cartpole_showcase_envs=True
),
setup_environment(factory_envs=False, multi_agent=False, teleop_envs=False, cartpole_showcase_envs=True),
)
def test_cartpole_showcase_environments_with_stage_in_memory_and_clone_in_fabric_disabled(task_name, num_envs, device):
# skip test if stage in memory is not supported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
@pytest.mark.parametrize(
"task_name",
setup_environment(
include_play=False,
multi_agent=False,
factory_envs=False,
cartpole_showcase_envs=False,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
@pytest.mark.parametrize(
"task_name",
setup_environment(
include_play=False,
factory_envs=False,
multi_agent=False,
teleop_envs=False,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@


@pytest.mark.parametrize("num_envs, device", [(2, "cuda"), (1, "cuda")])
@pytest.mark.parametrize(
"task_name", setup_environment(include_play=False, factory_envs=False, multi_agent=False, teleop_envs=True)
)
@pytest.mark.parametrize("task_name", setup_environment(factory_envs=False, multi_agent=False, teleop_envs=True))
@pytest.mark.isaacsim_ci
def test_teleop_environments(task_name, num_envs, device):
# run teleop environments without stage in memory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@


@pytest.mark.parametrize("num_envs, device", [(2, "cuda")])
@pytest.mark.parametrize(
"task_name", setup_environment(include_play=False, factory_envs=False, multi_agent=False, teleop_envs=True)
)
@pytest.mark.parametrize("task_name", setup_environment(factory_envs=False, multi_agent=False, teleop_envs=True))
def test_teleop_environments_with_stage_in_memory_and_clone_in_fabric_disabled(task_name, num_envs, device):
# skip test if stage in memory is not supported
if get_isaac_sim_version().major < 5:
Expand Down
8 changes: 5 additions & 3 deletions source/isaaclab_tasks/test/core/test_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,18 @@
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
Collaborator

Choose a reason for hiding this comment

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

This parametrizes every core task with all three global preset names, but preset resolution falls back to default when a requested preset is unavailable. Consequently, a case labeled physx can actually run the newly-default Newton backend (or another task default), so the matrix does not guarantee the backend it claims to cover. Please generate only valid (task, physics preset) pairs, or use an explicit strict physics selector that errors when the backend is unavailable.

This also removes include_play=False, substantially expanding collection on top of the three-way parameterization. The third task shard took about 1h46m. Please restore the Play filter unless testing every Play variant here is intentional.

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 237d2f3: the matrix now emits only task/backend pairs that explicitly declare the requested PhysicsCfg preset. The Play filter remains removed because no registered core task IDs contain Play variants.

@pytest.mark.parametrize("num_envs, device", [(2, "cuda"), (1, "cuda")])
@pytest.mark.parametrize(
"task_name",
setup_environment(
include_play=False,
multi_agent=False,
tier="core",
),
)
@pytest.mark.isaacsim_ci
def test_environments(task_name, num_envs, device):
def test_environments(task_name, physics_preset_name, num_envs, device):
# run environments without stage in memory
_run_environments(task_name, device, num_envs, create_stage_in_memory=False)
_run_environments(
task_name, device, num_envs, create_stage_in_memory=False, physics_preset_name=physics_preset_name
)
38 changes: 0 additions & 38 deletions source/isaaclab_tasks/test/core/test_environments_newton.py

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

# TODO(mtrepte): re-enable with fabric cloning fix
# @pytest.mark.parametrize("num_envs, device", [(2, "cuda")])
# @pytest.mark.parametrize("task_name", setup_environment(include_play=False,factory_envs=False, multi_agent=False))
# @pytest.mark.parametrize("task_name", setup_environment(factory_envs=False, multi_agent=False))
# def test_environments_with_stage_in_memory_and_clone_in_fabric_disabled(task_name, num_envs, device):
# # skip test if stage in memory is not supported
# if get_isaac_sim_version().major < 5:
Expand All @@ -43,7 +43,6 @@
@pytest.mark.parametrize(
"task_name",
setup_environment(
include_play=False,
multi_agent=False,
tier="core",
),
Expand Down
5 changes: 3 additions & 2 deletions source/isaaclab_tasks/test/core/test_preset_kit_decision.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,11 @@ def test_isaacsim_physx_is_physics_selector():


def test_registered_task_physx_presets_keep_auto_selection_explicit():
"""PhysX defaults are concrete while ``physx`` remains the automatic selector."""
"""Core tasks retain explicit PhysX variants alongside automatic ``physx``."""

for task_id, task_spec in gym.registry.items():
if not task_id.startswith(("Isaac-", "IsaacContrib-")) or "env_cfg_entry_point" not in task_spec.kwargs:
entry_point = task_spec.kwargs.get("env_cfg_entry_point", "")
if not task_id.startswith("Isaac-") or "isaaclab_tasks.core" not in str(entry_point):
continue
env_cfg = load_cfg_from_registry(task_id, "env_cfg_entry_point")
presets = collect_presets(env_cfg)
Expand Down
2 changes: 1 addition & 1 deletion source/isaaclab_tasks/test/core/test_record_video.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def setup_video_params():
return num_envs, device, video_length


@pytest.mark.parametrize("task_name", setup_environment(include_play=True, tier="core"))
@pytest.mark.parametrize("task_name", setup_environment(tier="core"))
def test_record_video(task_name, setup_video_params):
"""Run random actions agent with internal VideoRecorder capturing from the active visualizer."""
num_envs, device, video_length = setup_video_params
Expand Down
57 changes: 6 additions & 51 deletions source/isaaclab_tasks/test/env_test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

"""Shared test utilities for Isaac Lab environments."""

import gc
import importlib
import os
import sys
Expand Down Expand Up @@ -76,49 +77,18 @@ def _task_tier(task_spec) -> str | None:
return None


def _has_physics_preset(raw_cfg, preset_name: str) -> bool:
"""Check if a raw (unresolved) env config has a named physics preset.

Must be called with the result of :func:`load_cfg_from_registry`, not
:func:`parse_env_cfg`, because the latter resolves all PresetCfg wrappers
to their default before returning.

Args:
raw_cfg: Raw env config from :func:`load_cfg_from_registry`.
preset_name: Name of the preset to check for (e.g., 'newton_mjwarp').

Returns:
True if ``raw_cfg.sim.physics`` is a PresetCfg with the given preset field.
"""
if isinstance(raw_cfg, dict):
return False
# If the top-level cfg is itself a PresetCfg wrapper, unwrap to its default.
env_cfg = raw_cfg
if (
hasattr(env_cfg, "__dataclass_fields__")
and hasattr(env_cfg, "default")
and not hasattr(type(env_cfg), "class_type")
):
env_cfg = env_cfg.default
physics = getattr(getattr(env_cfg, "sim", None), "physics", None)
return physics is not None and hasattr(physics, preset_name)


def setup_environment(
include_play: bool = False,
factory_envs: bool | None = None,
multi_agent: bool | None = None,
teleop_envs: bool | None = None,
cartpole_showcase_envs: bool | None = None,
pickplace_stack_envs: bool | None = None,
newton_mjwarp_envs: bool | None = None,
tier: str | None = None,
) -> list[str]:
"""
Acquire all registered Isaac environment task IDs with optional filters.

Args:
include_play: If True, include environments ending in 'Play-v0'.
factory_envs:
- True: include only Factory environments
- False: exclude Factory environments
Expand All @@ -139,10 +109,6 @@ def setup_environment(
- True: include only PickPlace/Stack environments
- False: exclude PickPlace/Stack environments
- None: include all environments regardless of pick-place/stack type
newton_mjwarp_envs:
- True: include only environments that have an MJWarp physics preset.
- False: exclude environments that have an MJWarp physics preset.
- None: include all environments regardless of MJWarp preset availability.
tier:
- "core": include only core environments (registered under ``isaaclab_tasks.core``).
- "contrib": include only contributed environments (registered under ``isaaclab_tasks.contrib``).
Expand All @@ -161,10 +127,6 @@ def setup_environment(
if "Isaac" not in task_spec.id:
continue

# filter Play environments, if needed
if not include_play and task_spec.id.endswith("Play-v0"):
continue

# apply core/contrib tier filter
if tier is not None and _task_tier(task_spec) != tier:
continue
Expand Down Expand Up @@ -211,18 +173,6 @@ def setup_environment(
continue
# if None: no filter

# apply MJWarp preset filter
if newton_mjwarp_envs is not None:
# Use load_cfg_from_registry (not parse_env_cfg) so that the PresetCfg
# wrapper on sim.physics is not yet resolved to its default.
raw_cfg = load_cfg_from_registry(task_spec.id, "env_cfg_entry_point")
has_newton_mjwarp = _has_physics_preset(raw_cfg, "newton_mjwarp")
if (newton_mjwarp_envs is True and not has_newton_mjwarp) or (
newton_mjwarp_envs is False and has_newton_mjwarp
):
continue
# if None: no filter

registered_tasks.append(task_spec.id)

# sort environments alphabetically
Expand Down Expand Up @@ -521,6 +471,11 @@ def _check_random_actions(
if env is not None:
env.close()

# Drop unreachable environment objects while the device is still alive. Warp arrays
# free device memory from a finalizer, so collect them before the simulation teardown
# destroys their streams.
gc.collect()

# Clear the simulation context singleton (also closes the USD context stage)
SimulationContext.clear_instance()

Expand Down
Loading