Skip to content

Commit ba31408

Browse files
Initializes manager term classes only when sim starts (#2117)
# Description To support creation of managers before the simulation starts playing (as needed by the event manager for USD randomizations), the MR in #2040 added a callback to resolve scene entities at runtime. However, certain class-based manager terms can also not be initialized if the simulation is not playing. Those terms may often rely on parameters that are only available once simulation plays (for instance, joint position limits). This MR moves the initializations of class-based manager terms to the callback too. Fixes #2136 ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Checklist - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com>
1 parent 875dc62 commit ba31408

File tree

11 files changed

+250
-146
lines changed

11 files changed

+250
-146
lines changed

source/isaaclab/isaaclab/envs/direct_marl_env.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ def __init__(self, cfg: DirectMARLEnvCfg, render_mode: str | None = None, **kwar
135135
# that must happen before the simulation starts. Example: randomizing mesh scale
136136
if self.cfg.events:
137137
self.event_manager = EventManager(self.cfg.events, self)
138-
print("[INFO] Event Manager: ", self.event_manager)
139138

140139
# apply USD-related randomization events
141140
if "prestartup" in self.event_manager.available_modes:
@@ -198,6 +197,9 @@ def __init__(self, cfg: DirectMARLEnvCfg, render_mode: str | None = None, **kwar
198197

199198
# perform events at the start of the simulation
200199
if self.cfg.events:
200+
# we print it here to make the logging consistent
201+
print("[INFO] Event Manager: ", self.event_manager)
202+
201203
if "startup" in self.event_manager.available_modes:
202204
self.event_manager.apply(mode="startup")
203205

source/isaaclab/isaaclab/envs/direct_rl_env.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ def __init__(self, cfg: DirectRLEnvCfg, render_mode: str | None = None, **kwargs
141141
# that must happen before the simulation starts. Example: randomizing mesh scale
142142
if self.cfg.events:
143143
self.event_manager = EventManager(self.cfg.events, self)
144-
print("[INFO] Event Manager: ", self.event_manager)
145144

146145
# apply USD-related randomization events
147146
if "prestartup" in self.event_manager.available_modes:
@@ -202,6 +201,9 @@ def __init__(self, cfg: DirectRLEnvCfg, render_mode: str | None = None, **kwargs
202201

203202
# perform events at the start of the simulation
204203
if self.cfg.events:
204+
# we print it here to make the logging consistent
205+
print("[INFO] Event Manager: ", self.event_manager)
206+
205207
if "startup" in self.event_manager.available_modes:
206208
self.event_manager.apply(mode="startup")
207209

source/isaaclab/isaaclab/envs/manager_based_env.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ def __init__(self, cfg: ManagerBasedEnvCfg):
140140
# note: this is needed here (rather than after simulation play) to allow USD-related randomization events
141141
# that must happen before the simulation starts. Example: randomizing mesh scale
142142
self.event_manager = EventManager(self.cfg.events, self)
143-
print("[INFO] Event Manager: ", self.event_manager)
144143

145144
# apply USD-related randomization events
146145
if "prestartup" in self.event_manager.available_modes:
@@ -232,6 +231,8 @@ def load_managers(self):
232231
233232
"""
234233
# prepare the managers
234+
# -- event manager (we print it here to make the logging consistent)
235+
print("[INFO] Event Manager: ", self.event_manager)
235236
# -- recorder manager
236237
self.recorder_manager = RecorderManager(self.cfg.recorders, self)
237238
print("[INFO] Recorder Manager: ", self.recorder_manager)

source/isaaclab/isaaclab/envs/mdp/events.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,7 +1162,9 @@ def __init__(self, cfg: EventTermCfg, env: ManagerBasedEnv):
11621162
event_name = cfg.params.get("event_name")
11631163
texture_rotation = cfg.params.get("texture_rotation", (0.0, 0.0))
11641164

1165-
# check to make sure replicate_physics is set to False, else raise warning
1165+
# check to make sure replicate_physics is set to False, else raise error
1166+
# note: We add an explicit check here since texture randomization can happen outside of 'prestartup' mode
1167+
# and the event manager doesn't check in that case.
11661168
if env.cfg.scene.replicate_physics:
11671169
raise RuntimeError(
11681170
"Unable to randomize visual texture material with scene replication enabled."
@@ -1260,7 +1262,9 @@ def __init__(self, cfg: EventTermCfg, env: ManagerBasedEnv):
12601262
event_name = cfg.params.get("event_name")
12611263
mesh_name: str = cfg.params.get("mesh_name", "") # type: ignore
12621264

1263-
# check to make sure replicate_physics is set to False, else raise warning
1265+
# check to make sure replicate_physics is set to False, else raise error
1266+
# note: We add an explicit check here since texture randomization can happen outside of 'prestartup' mode
1267+
# and the event manager doesn't check in that case.
12641268
if env.cfg.scene.replicate_physics:
12651269
raise RuntimeError(
12661270
"Unable to randomize visual color with scene replication enabled."

source/isaaclab/isaaclab/managers/event_manager.py

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
from __future__ import annotations
99

10+
import inspect
1011
import torch
1112
from collections.abc import Sequence
1213
from prettytable import PrettyTable
@@ -186,15 +187,6 @@ def apply(
186187
if mode not in self._mode_term_names:
187188
omni.log.warn(f"Event mode '{mode}' is not defined. Skipping event.")
188189
return
189-
# check if mode is pre-startup and scene replication is enabled
190-
if mode == "prestartup" and self._env.scene.cfg.replicate_physics:
191-
omni.log.warn(
192-
"Scene replication is enabled, which may affect USD-level randomization."
193-
" When assets are replicated, their properties are shared across instances,"
194-
" potentially leading to unintended behavior."
195-
" For stable USD-level randomization, consider disabling scene replication"
196-
" by setting 'replicate_physics' to False in 'InteractiveSceneCfg'."
197-
)
198190

199191
# check if mode is interval and dt is not provided
200192
if mode == "interval" and dt is None:
@@ -363,6 +355,24 @@ def _prepare_terms(self):
363355

364356
# resolve common parameters
365357
self._resolve_common_term_cfg(term_name, term_cfg, min_argc=2)
358+
359+
# check if mode is pre-startup and scene replication is enabled
360+
if term_cfg.mode == "prestartup" and self._env.scene.cfg.replicate_physics:
361+
raise RuntimeError(
362+
"Scene replication is enabled, which may affect USD-level randomization."
363+
" When assets are replicated, their properties are shared across instances,"
364+
" potentially leading to unintended behavior."
365+
" For stable USD-level randomization, please disable scene replication"
366+
" by setting 'replicate_physics' to False in 'InteractiveSceneCfg'."
367+
)
368+
369+
# for event terms with mode "prestartup", we assume a callable class term
370+
# can be initialized before the simulation starts.
371+
# this is done to ensure that the USD-level randomization is possible before the simulation starts.
372+
if inspect.isclass(term_cfg.func) and term_cfg.mode == "prestartup":
373+
omni.log.info(f"Initializing term '{term_name}' with class '{term_cfg.func.__name__}'.")
374+
term_cfg.func = term_cfg.func(cfg=term_cfg, env=self._env)
375+
366376
# check if mode is a new mode
367377
if term_cfg.mode not in self._mode_term_names:
368378
# add new mode

source/isaaclab/isaaclab/managers/manager_base.py

Lines changed: 63 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ def __call__(self, *args) -> Any:
112112
Returns:
113113
The value of the term.
114114
"""
115-
raise NotImplementedError
115+
raise NotImplementedError("The method '__call__' should be implemented by the subclass.")
116116

117117

118118
class ManagerBase(ABC):
@@ -136,32 +136,34 @@ def __init__(self, cfg: object, env: ManagerBasedEnv):
136136
self.cfg = copy.deepcopy(cfg)
137137
self._env = env
138138

139-
# parse config to create terms information
140-
if self.cfg:
141-
self._prepare_terms()
142-
143139
# if the simulation is not playing, we use callbacks to trigger the resolution of the scene
144140
# entities configuration. this is needed for cases where the manager is created after the
145141
# simulation, but before the simulation is playing.
142+
# FIXME: Once Isaac Sim supports storing this information as USD schema, we can remove this
143+
# callback and resolve the scene entities directly inside `_prepare_terms`.
146144
if not self._env.sim.is_playing():
147145
# note: Use weakref on all callbacks to ensure that this object can be deleted when its destructor
148146
# is called
149147
# The order is set to 20 to allow asset/sensor initialization to complete before the scene entities
150148
# are resolved. Those have the order 10.
151149
timeline_event_stream = omni.timeline.get_timeline_interface().get_timeline_event_stream()
152-
self._resolve_scene_entities_handle = timeline_event_stream.create_subscription_to_pop_by_type(
150+
self._resolve_terms_handle = timeline_event_stream.create_subscription_to_pop_by_type(
153151
int(omni.timeline.TimelineEventType.PLAY),
154-
lambda event, obj=weakref.proxy(self): obj._resolve_scene_entities_callback(event),
152+
lambda event, obj=weakref.proxy(self): obj._resolve_terms_callback(event),
155153
order=20,
156154
)
157155
else:
158-
self._resolve_scene_entities_handle = None
156+
self._resolve_terms_handle = None
157+
158+
# parse config to create terms information
159+
if self.cfg:
160+
self._prepare_terms()
159161

160162
def __del__(self):
161163
"""Delete the manager."""
162-
if self._resolve_scene_entities_handle:
163-
self._resolve_scene_entities_handle.unsubscribe()
164-
self._resolve_scene_entities_handle = None
164+
if self._resolve_terms_handle:
165+
self._resolve_terms_handle.unsubscribe()
166+
self._resolve_terms_handle = None
165167

166168
"""
167169
Properties.
@@ -206,7 +208,7 @@ def find_terms(self, name_keys: str | Sequence[str]) -> list[str]:
206208
specified as regular expressions or a list of regular expressions. The search is
207209
performed on the active terms in the manager.
208210
209-
Please check the :meth:`isaaclab.utils.string_utils.resolve_matching_names` function for more
211+
Please check the :meth:`~isaaclab.utils.string_utils.resolve_matching_names` function for more
210212
information on the name matching.
211213
212214
Args:
@@ -249,11 +251,10 @@ def _prepare_terms(self):
249251
Internal callbacks.
250252
"""
251253

252-
def _resolve_scene_entities_callback(self, event):
253-
"""Resolve the scene entities configuration.
254+
def _resolve_terms_callback(self, event):
255+
"""Resolve configurations of terms once the simulation starts.
254256
255-
This callback is called when the simulation starts. It is used to resolve the
256-
scene entities configuration for the terms.
257+
Please check the :meth:`_process_term_cfg_at_play` method for more information.
257258
"""
258259
# check if config is dict already
259260
if isinstance(self.cfg, dict):
@@ -266,17 +267,26 @@ def _resolve_scene_entities_callback(self, event):
266267
# check for non config
267268
if term_cfg is None:
268269
continue
269-
# resolve the scene entity configuration
270-
self._resolve_scene_entity_cfg(term_name, term_cfg)
270+
# process attributes at runtime
271+
# these properties are only resolvable once the simulation starts playing
272+
self._process_term_cfg_at_play(term_name, term_cfg)
271273

272274
"""
273-
Helper functions.
275+
Internal functions.
274276
"""
275277

276278
def _resolve_common_term_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg, min_argc: int = 1):
277-
"""Resolve common term configuration.
279+
"""Resolve common attributes of the term configuration.
280+
281+
Usually, called by the :meth:`_prepare_terms` method to resolve common attributes of the term
282+
configuration. These include:
278283
279-
Usually, called by the :meth:`_prepare_terms` method to resolve common term configuration.
284+
* Resolving the term function and checking if it is callable.
285+
* Checking if the term function's arguments are matched by the parameters.
286+
* Resolving special attributes of the term configuration like ``asset_cfg``, ``sensor_cfg``, etc.
287+
* Initializing the term if it is a class.
288+
289+
The last two steps are only possible once the simulation starts playing.
280290
281291
By default, all term functions are expected to have at least one argument, which is the
282292
environment object. Some other managers may expect functions to take more arguments, for
@@ -303,29 +313,31 @@ def _resolve_common_term_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg,
303313
f" Received: '{type(term_cfg)}'."
304314
)
305315

306-
# iterate over all the entities and parse the joint and body names
307-
if self._env.sim.is_playing():
308-
self._resolve_scene_entity_cfg(term_name, term_cfg)
309-
310316
# get the corresponding function or functional class
311317
if isinstance(term_cfg.func, str):
312318
term_cfg.func = string_to_callable(term_cfg.func)
319+
# check if function is callable
320+
if not callable(term_cfg.func):
321+
raise AttributeError(f"The term '{term_name}' is not callable. Received: {term_cfg.func}")
313322

314-
# initialize the term if it is a class
323+
# check if the term is a class of valid type
315324
if inspect.isclass(term_cfg.func):
316325
if not issubclass(term_cfg.func, ManagerTermBase):
317326
raise TypeError(
318327
f"Configuration for the term '{term_name}' is not of type ManagerTermBase."
319328
f" Received: '{type(term_cfg.func)}'."
320329
)
321-
term_cfg.func = term_cfg.func(cfg=term_cfg, env=self._env)
330+
func_static = term_cfg.func.__call__
331+
min_argc += 1 # forward by 1 to account for 'self' argument
332+
else:
333+
func_static = term_cfg.func
322334
# check if function is callable
323-
if not callable(term_cfg.func):
335+
if not callable(func_static):
324336
raise AttributeError(f"The term '{term_name}' is not callable. Received: {term_cfg.func}")
325337

326-
# check if term's arguments are matched by params
338+
# check statically if the term's arguments are matched by params
327339
term_params = list(term_cfg.params.keys())
328-
args = inspect.signature(term_cfg.func).parameters
340+
args = inspect.signature(func_static).parameters
329341
args_with_defaults = [arg for arg in args if args[arg].default is not inspect.Parameter.empty]
330342
args_without_defaults = [arg for arg in args if args[arg].default is inspect.Parameter.empty]
331343
args = args_without_defaults + args_with_defaults
@@ -338,8 +350,22 @@ def _resolve_common_term_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg,
338350
f" and optional parameters: {args_with_defaults}, but received: {term_params}."
339351
)
340352

341-
def _resolve_scene_entity_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg):
342-
"""Resolve the scene entity configuration for the term.
353+
# process attributes at runtime
354+
# these properties are only resolvable once the simulation starts playing
355+
if self._env.sim.is_playing():
356+
self._process_term_cfg_at_play(term_name, term_cfg)
357+
358+
def _process_term_cfg_at_play(self, term_name: str, term_cfg: ManagerTermBaseCfg):
359+
"""Process the term configuration at runtime.
360+
361+
This function is called when the simulation starts playing. It is used to process the term
362+
configuration at runtime. This includes:
363+
364+
* Resolving the scene entity configuration for the term.
365+
* Initializing the term if it is a class.
366+
367+
Since the above steps rely on PhysX to parse over the simulation scene, they are deferred
368+
until the simulation starts playing.
343369
344370
Args:
345371
term_name: The name of the term.
@@ -362,3 +388,8 @@ def _resolve_scene_entity_cfg(self, term_name: str, term_cfg: ManagerTermBaseCfg
362388
omni.log.info(msg)
363389
# store the entity
364390
term_cfg.params[key] = value
391+
392+
# initialize the term if it is a class
393+
if inspect.isclass(term_cfg.func):
394+
omni.log.info(f"Initializing term '{term_name}' with class '{term_cfg.func.__name__}'.")
395+
term_cfg.func = term_cfg.func(cfg=term_cfg, env=self._env)

source/isaaclab/isaaclab/managers/observation_manager.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,14 @@ def _prepare_terms(self):
352352
# we store it as a separate list to only call reset on them and prevent unnecessary calls
353353
self._group_obs_class_modifiers: list[modifiers.ModifierBase] = list()
354354

355+
# make sure the simulation is playing since we compute obs dims which needs asset quantities
356+
if not self._env.sim.is_playing():
357+
raise RuntimeError(
358+
"Simulation is not playing. Observation manager requires the simulation to be playing"
359+
" to compute observation dimensions. Please start the simulation before using the"
360+
" observation manager."
361+
)
362+
355363
# check if config is dict already
356364
if isinstance(self.cfg, dict):
357365
group_cfg_items = self.cfg.items()
@@ -407,8 +415,10 @@ def _prepare_terms(self):
407415
# add term config to list to list
408416
self._group_obs_term_names[group_name].append(term_name)
409417
self._group_obs_term_cfgs[group_name].append(term_cfg)
418+
410419
# call function the first time to fill up dimensions
411420
obs_dims = tuple(term_cfg.func(self._env, **term_cfg.params).shape)
421+
412422
# create history buffers and calculate history term dimensions
413423
if term_cfg.history_length > 0:
414424
group_entry_history_buffer[term_name] = CircularBuffer(

0 commit comments

Comments
 (0)