Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Test-only change.

* Reduced the default ``num_steps`` from 100 to 20 in ``_run_environments``.
* Tuned ``test_environments`` to pass ``force_interval_events=True`` so ``EventManager`` interval handlers
(e.g. locomotion ``push_robot``) run on step 1 instead of never firing within the legacy step budget.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Fixed
^^^^^

* Removed the stale file-level ``@pytest.mark.xfail`` decorator on
``test_environments_newton`` (the cited Hydra deep-nesting issue was already
resolved by PR #5029 and follow-ups #5130 / #5177).
47 changes: 45 additions & 2 deletions source/isaaclab_tasks/test/env_test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,49 @@ def setup_environment(
]


def _force_interval_events_to_fire_immediately(env_cfg) -> None:
"""Rewrite every interval-mode event term so it fires on the first ``step()``.

The :class:`isaaclab.managers.EventManager` samples ``time_left`` from
``interval_range_s`` at reset, then fires the term once ``time_left < 1e-6``
after subtracting ``dt`` each step. Setting both bounds of the range to
``1e-6`` guarantees the sampled ``time_left`` lands at the trigger threshold
on the first step, regardless of ``is_global_time``.

Mutates ``env_cfg.events`` in place. No-op if ``env_cfg.events`` is ``None``
or has no interval-mode terms.

Args:
env_cfg: A parsed env config.
"""
events_cfg = getattr(env_cfg, "events", None)
if events_cfg is None:
return
for term_name in dir(events_cfg):
if term_name.startswith("_"):
continue
try:
term = getattr(events_cfg, term_name, None)
except AttributeError:
continue
if (
term is not None
and getattr(term, "mode", None) == "interval"
and getattr(term, "interval_range_s", None) is not None
):
term.interval_range_s = (1e-6, 1e-6)
Comment on lines +238 to +250
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 dir() iteration may skip frozen-dataclass configs silently

dir(events_cfg) returns all public names — including callable methods and descriptors. The try/except AttributeError guards against runtime property errors, but the subsequent assignment term.interval_range_s = (1e-6, 1e-6) (line 250) is unguarded. If any EventTermCfg subclass is declared with frozen=True (or uses __setattr__ guards), the assignment raises FrozenInstanceError (a subclass of TypeError) and propagates uncaught, aborting the test with a confusing traceback rather than a helpful assertion failure. Wrapping the assignment in a try/except (AttributeError, TypeError) block would make the helper resilient to these cases.



def _run_environments(
task_name,
device,
num_envs,
num_steps=100,
num_steps=20,
multi_agent=False,
create_stage_in_memory=False,
disable_clone_in_fabric=False,
physics_preset_name: str | None = None,
force_interval_events: bool = False,
):
"""Run all environments and check environments return valid signals.

Expand All @@ -239,6 +273,8 @@ def _run_environments(
disable_clone_in_fabric: Whether to disable fabric cloning.
physics_preset_name: Name of the physics preset to apply (e.g., 'newton_mjwarp').
If None, uses the environment's default physics.
force_interval_events: If True, rewrite interval-mode event terms so they
fire on the first ``step()``.
"""

# skip test if stage in memory is not supported
Expand Down Expand Up @@ -300,6 +336,7 @@ def _run_environments(
create_stage_in_memory=create_stage_in_memory,
disable_clone_in_fabric=disable_clone_in_fabric,
physics_preset_name=physics_preset_name,
force_interval_events=force_interval_events,
)
print(f""">>> Closing environment: {task_name}""")
print("-" * 80)
Expand All @@ -309,11 +346,12 @@ def _check_random_actions(
task_name: str,
device: str,
num_envs: int,
num_steps: int = 100,
num_steps: int = 20,
multi_agent: bool = False,
create_stage_in_memory: bool = False,
disable_clone_in_fabric: bool = False,
physics_preset_name: str | None = None,
force_interval_events: bool = False,
):
"""Run random actions and check environments return valid signals.

Expand All @@ -327,6 +365,8 @@ def _check_random_actions(
disable_clone_in_fabric: Whether to disable fabric cloning.
physics_preset_name: Name of the physics preset to apply (e.g., 'newton_mjwarp').
If None, uses the environment's default physics.
force_interval_events: If True, rewrite interval-mode event terms so they
fire on the first ``step()``.
"""
# create a new context stage, if stage in memory is not enabled
if not create_stage_in_memory:
Expand Down Expand Up @@ -355,6 +395,9 @@ def _check_random_actions(
if disable_clone_in_fabric:
env_cfg.scene.clone_in_fabric = False

if force_interval_events:
_force_interval_events_to_fire_immediately(env_cfg)

# filter based off multi agents mode and create env
if multi_agent:
if not hasattr(env_cfg, "possible_agents"):
Expand Down
8 changes: 7 additions & 1 deletion source/isaaclab_tasks/test/test_environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,10 @@
@pytest.mark.isaacsim_ci
def test_environments(task_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,
force_interval_events=True,
create_stage_in_memory=False,
)
Loading