Resolve nested manager callable references

Author: ooctipus

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

@@ -0,0 +1,5 @@
+Fixed
+^^^^^
+
+* Fixed observation modifiers failing class detection and signature validation after a configuration dictionary
+  round-trip by recursively resolving callable references across complete manager-term configurations.

source/isaaclab/isaaclab/managers/manager_base.py

@@ -301,7 +301,7 @@ def _resolve_common_term_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg,
         Usually, called by the :meth:`_prepare_terms` method to resolve common attributes of the term
         configuration. These include:
 
-        * Resolving the term function and checking if it is callable.
+        * Resolving callable references throughout the term configuration and checking the term function.
         * Checking if the term function's arguments are matched by the parameters.
         * Resolving special attributes of the term configuration like ``asset_cfg``, ``sensor_cfg``, etc.
         * Initializing the term if it is a class.
@@ -334,8 +334,8 @@ 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)
+
         # 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 +381,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 callable references and scene entity configurations 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,17 +391,17 @@ 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 key, value in term_cfg.__dict__.items():
+            setattr(term_cfg, key, self._resolve_param_value(term_name, key, 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) -> Any:
+        """Recursively resolve a value in a manager term configuration."""
+        if key == "func" and isinstance(value, str):
+            return string_to_callable(value)
         if isinstance(value, SceneEntityCfg):
             try:
                 value.resolve(self._env.scene)
@@ -411,7 +411,11 @@ def _resolve_param_value(self, term_name: str, key: str | int, value: Any):
             self._process_term_cfg_at_play(f"{term_name}.{key}", 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)
+                value[sub_key] = self._resolve_param_value(f"{term_name}.{key}", sub_key, sub_value)
         elif isinstance(value, (list, tuple)):
             for i, item in enumerate(value):
                 self._resolve_param_value(f"{term_name}.{key}", i, item)
+        elif not isinstance(value, type) and hasattr(value, "__dataclass_fields__") and hasattr(value, "__dict__"):
+            for sub_key, sub_value in value.__dict__.items():
+                setattr(value, sub_key, self._resolve_param_value(f"{term_name}.{key}", sub_key, sub_value))
+        return value

source/isaaclab/isaaclab/utils/modifiers/__init__.py

@@ -28,7 +28,7 @@
     cfg = modifiers.ModifierCfg(func=modifiers.clip, params={"bounds": (0.0, torch.inf)})
 
     # apply the modifier
-    my_modified_tensor = cfg.func(my_tensor, cfg)
+    my_modified_tensor = cfg.func(my_tensor, **cfg.params)
 
 
 Usage with a class modifier:

source/isaaclab/isaaclab/utils/modifiers/modifier_base.py

@@ -25,21 +25,15 @@ class ModifierBase(ABC):
     This is useful for modifiers that require stateful operations, such as rolling averages
     or delays or decaying filters.
 
-    Example pseudo-code to create and use the class:
+    Constructor settings for a class-based modifier belong to a dedicated
+    :class:`~isaaclab.utils.modifiers.ModifierCfg` subclass. For example:
 
     .. code-block:: python
 
         from isaaclab.utils import modifiers
 
-        # define custom keyword arguments to pass to ModifierCfg
-        kwarg_dict = {"arg_1": VAL_1, "arg_2": VAL_2}
-
-        # create modifier configuration object
-        # func is the class name of the modifier and params is the dictionary of arguments
-        modifier_config = modifiers.ModifierCfg(func=modifiers.ModifierBase, params=kwarg_dict)
-
-        # define modifier instance
-        my_modifier = modifiers.ModifierBase(cfg=modifier_config)
+        modifier_config = modifiers.DigitalFilterCfg(A=[0.0], B=[0.0, 1.0])
+        my_modifier = modifiers.DigitalFilter(cfg=modifier_config, data_dim=(256, 128), device="cuda")
 
     """
 

source/isaaclab/test/managers/test_manager_base.py

@@ -6,7 +6,7 @@
 # ignore private usage of variables warning
 # pyright: reportPrivateUsage=none
 
-"""Tests for recursive _process_term_cfg_at_play / _resolve_param_value.
+"""Tests for recursive manager term configuration resolution.
 
 These tests exercise ManagerBase's parameter resolution logic and do NOT
 require an Isaac Sim launch, so they can run without AppLauncher.
@@ -20,8 +20,9 @@
 import torch
 
 from isaaclab.envs import ManagerBasedEnv
-from isaaclab.managers import ManagerTermBase, ManagerTermBaseCfg
+from isaaclab.managers import ManagerTermBase, ManagerTermBaseCfg, ObservationTermCfg
 from isaaclab.managers.manager_base import ManagerBase
+from isaaclab.utils import modifiers
 
 pytestmark = pytest.mark.integration
 
@@ -217,6 +218,23 @@ def test_string_func_in_nested_term_cfg(env):
     torch.testing.assert_close(env.dummy1, 11 * torch.ones_like(env.dummy1))
 
 
+def test_callable_resolution_walks_term_fields_outside_params(env):
+    """Callable references in nested term fields are resolved by ManagerBase."""
+    cfg = {
+        "observation": ObservationTermCfg(
+            func=increment_dummy1_by_one,
+            modifiers=[modifiers.ModifierCfg(func=f"{__name__}:reset_dummy2_to_zero")],
+        )
+    }
+    manager = SimpleManager(cfg, env)
+
+    term_cfg = manager._term_cfgs[0][1]
+    assert isinstance(term_cfg, ObservationTermCfg)
+    assert term_cfg.modifiers is not None
+    modifier_cfg = term_cfg.modifiers[0]
+    assert modifier_cfg.func is reset_dummy2_to_zero
+
+
 def test_string_func_top_level_class_term(env):
     """Test that a top-level string-based func pointing to a class is properly instantiated."""
     this_module = __name__

source/isaaclab/test/managers/test_observation_manager_unit.py

@@ -15,6 +15,7 @@
 import torch
 
 from isaaclab.managers import ObservationGroupCfg, ObservationManager, ObservationTermCfg
+from isaaclab.utils import modifiers
 from isaaclab.utils.configclass import configclass
 
 pytestmark = pytest.mark.unit
@@ -46,6 +47,29 @@ def __init__(self, num_envs: int = 2) -> None:
         self.observation = torch.arange(num_envs, dtype=torch.float32).unsqueeze(-1)
 
 
+class StatefulBiasModifier(modifiers.ModifierBase):
+    """Stateful modifier used to verify lazy callable resolution."""
+
+    def __init__(self, cfg: StatefulBiasModifierCfg, data_dim: tuple[int, ...], device: str) -> None:
+        super().__init__(cfg, data_dim, device)
+        self.value = cfg.value
+        self.reset_count = 0
+
+    def reset(self, env_ids=None) -> None:
+        self.reset_count += 1
+
+    def __call__(self, data: torch.Tensor) -> torch.Tensor:
+        return data + self.value
+
+
+@configclass
+class StatefulBiasModifierCfg(modifiers.ModifierCfg):
+    """Configuration for :class:`StatefulBiasModifier`."""
+
+    func: type[StatefulBiasModifier] = StatefulBiasModifier
+    value: float = 2.0
+
+
 @configclass
 class HistoryObservationsCfg:
     """Observation configuration with group-level history."""
@@ -62,6 +86,32 @@ def __post_init__(self):
     policy: PolicyCfg = PolicyCfg()
 
 
+def test_stateful_modifier_cfg_roundtrip_preserves_func():
+    """A stateful modifier remains usable after its function becomes a lazy string."""
+    cfg = HistoryObservationsCfg()
+    cfg.policy.history_length = None
+    cfg.policy.dummy.modifiers = [StatefulBiasModifierCfg(value=2.0)]
+    cfg.from_dict(cfg.to_dict())
+    term_cfg = cfg.policy.dummy
+    assert term_cfg.modifiers is not None
+    modifier_cfg = term_cfg.modifiers[0]
+    assert isinstance(modifier_cfg, StatefulBiasModifierCfg)
+    assert modifier_cfg.params == {}
+    assert not hasattr(modifier_cfg, "class_type")
+
+    env = DummyEnv()
+    manager = ObservationManager(cfg, cast("ManagerBasedEnv", env))
+    prepared_term_cfg = manager.cfg.policy.dummy
+    assert prepared_term_cfg.modifiers is not None
+    prepared_modifier_cfg = prepared_term_cfg.modifiers[0]
+    assert isinstance(prepared_modifier_cfg.func, StatefulBiasModifier)
+    observations = manager.compute()["policy"]
+    torch.testing.assert_close(observations, env.observation + 2.0)
+
+    manager.reset()
+    assert prepared_modifier_cfg.func.reset_count == 1
+
+
 def test_compute_updates_history_only_when_requested():
     """Observation history changes only when ``update_history`` is enabled."""
     env = DummyEnv()