Merge branch 'develop' into frlai/improve_leapp_export

Author: frlai

CONTRIBUTORS.md

@@ -123,6 +123,7 @@ Guidelines for modifications:
 * Jiwen Cai
 * Johnson Sun
 * Juana Du
+* Kai Pei
 * Kaixi Bao
 * Kourosh Darvish
 * Kousheek Chakraborty

source/isaaclab/changelog.d/mimic-max-num-failures.rst

@@ -0,0 +1,8 @@
+Fixed
+^^^^^
+
+* Fixed :attr:`~isaaclab.envs.mimic_env_cfg.DataGenConfig.max_num_failures` being ignored. The field
+  documented a cap on failed generation attempts but was never read, so a run with
+  :attr:`~isaaclab.envs.mimic_env_cfg.DataGenConfig.generation_guarantee` enabled retried without
+  bound on a task with a low success rate. Its default is now ``None`` (no limit) and setting it to
+  an integer stops generation once that many attempts have failed.

source/isaaclab/changelog.d/resolvable-string-modifier.rst

@@ -0,0 +1,4 @@
+Fixed
+^^^^^
+
+* Fixed function and class-based observation modifiers failing after a configuration dictionary round-trip.

source/isaaclab/changelog.d/skip-checkout-only-unit-tests.skip

@@ -0,0 +1 @@
+Test-only: added a source checkout root fixture for tests that inspect repository artifacts.

source/isaaclab/isaaclab/envs/mimic_env_cfg.py

@@ -38,8 +38,21 @@ class DataGenConfig:
     Keeping failed demonstrations is useful for visualizing and debugging low success rates.
     """
 
-    max_num_failures: int = 50
-    """Maximum number of failures allowed before stopping generation."""
+    max_num_failures: int | None = None
+    """Maximum number of failed generation attempts before stopping, or None for no limit.
+
+    Only applies together with :attr:`generation_guarantee`. With the guarantee enabled, generation
+    keeps retrying until :attr:`generation_num_trials` demos succeed, so a task whose success rate is
+    low can run for an unbounded number of attempts; this caps that. Defaults to None so the
+    guarantee keeps its usual meaning unless a limit is asked for.
+
+    With the guarantee disabled, generation already stops after :attr:`generation_num_trials`
+    attempts and this field is ignored, so setting it cannot cut a fixed-attempt run short.
+
+    The bound is read once per simulation step. Attempts that end on the step that crosses it are
+    already complete, so a run over ``num_envs`` parallel environments can record up to
+    ``num_envs - 1`` failures beyond the bound; it is exact whenever attempts end on separate steps.
+    """
 
     seed: int = 1
     """Seed for randomization to ensure reproducibility."""

source/isaaclab/isaaclab/managers/manager_base.py

@@ -10,11 +10,13 @@
 import weakref
 from abc import ABC, abstractmethod
 from collections.abc import Sequence
+from dataclasses import fields
 from typing import TYPE_CHECKING, Any
 
 import isaaclab.utils.string as string_utils
 from isaaclab.physics import PhysicsEvent, PhysicsManager
 from isaaclab.utils import class_to_dict, string_to_callable
+from isaaclab.utils.modifiers import ModifierCfg
 
 from .manager_term_cfg import ManagerTermBaseCfg
 from .scene_entity_cfg import SceneEntityCfg
@@ -334,8 +336,7 @@ def _resolve_common_term_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg,
             )
 
         # get the corresponding function or functional class
-        if isinstance(term_cfg.func, str):
-            term_cfg.func = string_to_callable(term_cfg.func)
+        term_cfg.func = self._resolve_param_value(term_name, "func", term_cfg.func, resolve_callable=True)
         # check if function is callable
         if not callable(term_cfg.func):
             raise AttributeError(f"The term '{term_name}' is not callable. Received: {term_cfg.func}")
@@ -381,7 +382,7 @@ def _process_term_cfg_at_play(self, term_name: str, term_cfg: ManagerTermBaseCfg
         This function is called when the simulation starts playing. It is used to process the term
         configuration at runtime. This includes:
 
-        * Resolving the scene entity configuration for the term.
+        * Resolving scene entity configurations and nested terms throughout the term configuration.
         * Initializing the term if it is a class.
 
         Since the above steps rely on PhysX to parse over the simulation scene, they are deferred
@@ -391,27 +392,49 @@ def _process_term_cfg_at_play(self, term_name: str, term_cfg: ManagerTermBaseCfg
             term_name: The name of the term.
             term_cfg: The term configuration.
         """
-        for key, value in term_cfg.params.items():
-            self._resolve_param_value(term_name, key, value)
+        for field in fields(term_cfg):
+            value = getattr(term_cfg, field.name)
+            resolved_value = self._resolve_param_value(
+                term_name, field.name, value, resolve_callable=field.name == "func"
+            )
+            if resolved_value is not value:
+                setattr(term_cfg, field.name, resolved_value)
 
-        # resolve string func references then initialize class-based terms
-        if isinstance(term_cfg.func, str):
-            term_cfg.func = string_to_callable(term_cfg.func)
+        # initialize class-based terms
         if inspect.isclass(term_cfg.func):
             term_cfg.func = term_cfg.func(cfg=term_cfg, env=self._env)
 
-    def _resolve_param_value(self, term_name: str, key: str | int, value: Any):
-        """Recursively resolve a single param value (SceneEntityCfg, nested term cfgs, dicts, lists)."""
+    def _resolve_param_value(
+        self, term_name: str, key: str | int, value: Any, *, resolve_callable: bool = False
+    ) -> Any:
+        """Recursively resolve manager-owned values in a term configuration."""
+        if resolve_callable and isinstance(value, str):
+            return string_to_callable(value)
         if isinstance(value, SceneEntityCfg):
             try:
                 value.resolve(self._env.scene)
             except ValueError as e:
                 raise ValueError(f"Error while parsing '{term_name}:{key}'. {e}")
         elif isinstance(value, ManagerTermBaseCfg):
             self._process_term_cfg_at_play(f"{term_name}.{key}", value)
+        elif isinstance(value, ModifierCfg):
+            for field in fields(value):
+                field_value = getattr(value, field.name)
+                resolved_value = self._resolve_param_value(
+                    f"{term_name}.{key}", field.name, field_value, resolve_callable=field.name == "func"
+                )
+                if resolved_value is not field_value:
+                    setattr(value, field.name, resolved_value)
         elif isinstance(value, dict):
             for sub_key, sub_value in value.items():
-                self._resolve_param_value(f"{term_name}.{key}", sub_key, sub_value)
-        elif isinstance(value, (list, tuple)):
+                value[sub_key] = self._resolve_param_value(f"{term_name}.{key}", sub_key, sub_value)
+        elif isinstance(value, list):
             for i, item in enumerate(value):
-                self._resolve_param_value(f"{term_name}.{key}", i, item)
+                value[i] = self._resolve_param_value(f"{term_name}.{key}", i, item)
+        elif isinstance(value, tuple):
+            resolved_items = tuple(
+                self._resolve_param_value(f"{term_name}.{key}", i, item) for i, item in enumerate(value)
+            )
+            if any(resolved is not original for resolved, original in zip(resolved_items, value, strict=True)):
+                value = resolved_items
+        return value

source/isaaclab/isaaclab/managers/observation_manager.py

@@ -396,7 +396,10 @@ def compute_group(self, group_name: str, update_history: bool = False) -> torch.
             # apply post-processing
             if term_cfg.modifiers is not None:
                 for modifier in term_cfg.modifiers:
-                    obs = modifier.func(obs, **modifier.params)
+                    if isinstance(modifier.func, modifiers.ModifierBase):
+                        obs = modifier.func(obs)
+                    else:
+                        obs = modifier.func(obs, **modifier.params)
             if isinstance(term_cfg.noise, noise.NoiseCfg):
                 obs = term_cfg.noise.func(obs, term_cfg.noise)
             elif isinstance(term_cfg.noise, noise.NoiseModelCfg) and term_cfg.noise.func is not None:
@@ -579,25 +582,22 @@ def _prepare_terms(self):
                 if term_cfg.modifiers is not None:
                     # initialize list of modifiers for term
                     for mod_cfg in term_cfg.modifiers:
-                        # check if class modifier and initialize with observation size when adding
-                        if isinstance(mod_cfg, modifiers.ModifierCfg):
-                            # to list of modifiers - instantiate class-based modifiers
-                            if inspect.isclass(mod_cfg.func):
-                                mod_cfg.func = mod_cfg.func(cfg=mod_cfg, data_dim=obs_dims, device=self._env.device)
-                                # verify the instance is the correct type
-                                if not isinstance(mod_cfg.func, modifiers.ModifierBase):
-                                    raise TypeError(
-                                        f"Modifier function '{mod_cfg.func}' for observation term '{term_name}'"
-                                        f" is not an instance of 'ModifierBase'. Received: '{type(mod_cfg.func)}'."
-                                    )
-                                # add to list of class modifiers
-                                self._group_obs_class_instances.append(mod_cfg.func)
-                        else:
+                        if not isinstance(mod_cfg, modifiers.ModifierCfg):
                             raise TypeError(
                                 f"Modifier configuration '{mod_cfg}' of observation term '{term_name}' is not of"
                                 f" required type ModifierCfg, Received: '{type(mod_cfg)}'"
                             )
 
+                        # construct stateful modifiers with the observation size
+                        if inspect.isclass(mod_cfg.func):
+                            mod_cfg.func = mod_cfg.func(cfg=mod_cfg, data_dim=obs_dims, device=self._env.device)
+                            if not isinstance(mod_cfg.func, modifiers.ModifierBase):
+                                raise TypeError(
+                                    f"Modifier function '{mod_cfg.func}' for observation term '{term_name}'"
+                                    f" is not an instance of 'ModifierBase'. Received: '{type(mod_cfg.func)}'."
+                                )
+                            self._group_obs_class_instances.append(mod_cfg.func)
+
                         # check if function is callable
                         if not callable(mod_cfg.func):
                             raise AttributeError(

source/isaaclab/isaaclab/utils/modifiers/modifier_cfg.py

@@ -3,35 +3,38 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
+from __future__ import annotations
+
 from collections.abc import Callable
 from dataclasses import MISSING
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
 import torch
 
 from isaaclab.utils.configclass import configclass
 
-from . import modifier
+if TYPE_CHECKING:
+    from .modifier import DigitalFilter, Integrator
+    from .modifier_base import ModifierBase
 
 
 @configclass
 class ModifierCfg:
-    """Configuration parameters modifiers"""
-
-    func: Callable[..., torch.Tensor] = MISSING
-    """Function or callable class used by modifier.
+    """Configuration parameters for function and class modifiers."""
 
-    The function must take a torch tensor as the first argument. The remaining arguments are specified
-    in the :attr:`params` attribute.
+    func: Callable[..., torch.Tensor] | type[ModifierBase] | str = MISSING
+    """Function or :class:`ModifierBase` class used by the modifier.
 
-    It also supports `callable classes <https://docs.python.org/3/reference/datamodel.html#object.__call__>`_,
-    i.e. classes that implement the ``__call__()`` method. In this case, the class should inherit from the
-    :class:`ModifierBase` class and implement the required methods.
+    Functions must take a tensor as their first argument. Classes must inherit from :class:`ModifierBase`; the
+    observation manager constructs them with the configuration, observation dimensions, and device.
     """
 
     params: dict[str, Any] = dict()
-    """The parameters to be passed to the function or callable class as keyword arguments. Defaults to
-    an empty dictionary."""
+    """Parameters used by the modifier. Defaults to an empty dictionary.
+
+    Function modifiers receive them as keyword arguments on each call. Class modifiers access them through this
+    configuration during construction; they are not forwarded to the constructed instance.
+    """
 
 
 @configclass
@@ -41,7 +44,7 @@ class DigitalFilterCfg(ModifierCfg):
     For more information, please check the :class:`DigitalFilter` class.
     """
 
-    func: type[modifier.DigitalFilter] = modifier.DigitalFilter
+    func: type[DigitalFilter] | str = "{DIR}.modifier:DigitalFilter"
     """The digital filter function to be called for applying the filter."""
 
     A: list[float] = MISSING
@@ -72,7 +75,7 @@ class IntegratorCfg(ModifierCfg):
     For more information, please check the :class:`Integrator` class.
     """
 
-    func: type[modifier.Integrator] = modifier.Integrator
+    func: type[Integrator] | str = "{DIR}.modifier:Integrator"
     """The integrator function to be called for applying the integrator."""
 
     dt: float = MISSING

source/isaaclab/test/app/test_experience_files.py

@@ -12,19 +12,17 @@
 
 pytestmark = pytest.mark.unit
 
-APPS_DIR = Path(__file__).resolve().parents[4] / "apps"
-
 # ``.kit`` files repeat section headers, so they cannot be parsed as TOML.
 _SECTION_RE = re.compile(r"^\s*\[+(?P<name>[^\[\]]+)\]+\s*$")
 _DEPENDENCY_RE = re.compile(r'^\s*"(?P<name>[^"]+)"\s*=')
 
 
-def _kit_dependencies(experience: str) -> set[str]:
+def _kit_dependencies(apps_dir: Path, experience: str) -> set[str]:
     """Collect the extension names declared in an experience file's dependency sections."""
     dependencies: set[str] = set()
     in_dependencies = False
 
-    for line in (APPS_DIR / experience).read_text(encoding="utf-8").splitlines():
+    for line in (apps_dir / experience).read_text(encoding="utf-8").splitlines():
         section = _SECTION_RE.match(line)
         if section is not None:
             in_dependencies = section.group("name").strip() == "dependencies"
@@ -38,9 +36,9 @@ def _kit_dependencies(experience: str) -> set[str]:
 
 
 @pytest.mark.parametrize("experience", ["isaaclab.python.kit", "isaaclab.python.headless.kit"])
-def test_base_experiences_enable_native_storage(experience: str):
+def test_base_experiences_enable_native_storage(source_checkout_root: Path, experience: str):
     """Test the base experiences load the extension that applies ``ISAACSIM_ASSET_ROOT``."""
-    assert "isaacsim.storage.native" in _kit_dependencies(experience)
+    assert "isaacsim.storage.native" in _kit_dependencies(source_checkout_root / "apps", experience)
 
 
 @pytest.mark.parametrize(
@@ -52,6 +50,6 @@ def test_base_experiences_enable_native_storage(experience: str):
         ("isaaclab.python.xr.openxr.headless.kit", "isaaclab.python.xr.openxr"),
     ],
 )
-def test_derived_experiences_inherit_native_storage(experience: str, base: str):
+def test_derived_experiences_inherit_native_storage(source_checkout_root: Path, experience: str, base: str):
     """Test the derived experiences inherit native storage from a base experience."""
-    assert base in _kit_dependencies(experience)
+    assert base in _kit_dependencies(source_checkout_root / "apps", experience)

source/isaaclab/test/cli/test_install.py

@@ -476,11 +476,12 @@ class TestPinkIkStack:
     stack from there instead of mirroring the versions.
     """
 
-    def test_stack_derived_from_root_pyproject_pins(self):
+    def test_stack_derived_from_root_pyproject_pins(self, source_checkout_root: Path):
         """The derived stack covers every stack package, exactly pinned, markers stripped."""
         from isaaclab.cli.commands import install
 
-        stack = install._pink_ik_stack()
+        with mock.patch.object(install, "ISAACLAB_ROOT", source_checkout_root):
+            stack = install._pink_ik_stack()
         assert [install._requirement_name(r) for r in stack] == list(install._PINK_IK_PACKAGES)
         assert any(r.startswith("pin-pink==") for r in stack), "pin-pink must stay exactly pinned"
         assert any(r.startswith("daqp==") for r in stack), "daqp must stay exactly pinned"