Unverified Commit 7379dcee authored by Mayank Mittal's avatar Mayank Mittal Committed by GitHub

Fixes mode-based checks inside the `EventManager.apply` call (#777)

# Description

Noticed that we were checking for function arguments inside a for-loop,
which isn't necessary. Moved this check outside to make it simpler to
read the code.

Also noticed a small corner case in the event manager when reset is
called and `env_ids` is None. In that case, it would bypass the check
for min steps between reset and directly apply the term to the
environment. I am not sure if that was intentional. if so, I can revert
the behavior.

## 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
- [x] 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
parent 765666d5
[package] [package]
# Note: Semantic Versioning is used: https://semver.org/ # Note: Semantic Versioning is used: https://semver.org/
version = "0.21.1" version = "0.21.2"
# Description # Description
title = "Isaac Lab framework for Robot Learning" title = "Isaac Lab framework for Robot Learning"
......
Changelog Changelog
--------- ---------
0.21.2 (2024-08-13)
~~~~~~~~~~~~~~~~~~~
Fixed
^^^^^
* Moved event mode-based checks in the :meth:`omni.isaac.lab.managers.EventManager.apply` method outside
the loop that iterates over the event terms. This prevents unnecessary checks and improves readability.
* Fixed the logic for global and per environment interval times when using the "interval" mode inside the
event manager. Earlier, the internal lists for these times were of unequal lengths which led to wrong indexing
inside the loop that iterates over the event terms.
0.21.1 (2024-08-06) 0.21.1 (2024-08-06)
~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~
......
...@@ -519,7 +519,7 @@ class DirectRLEnv(gym.Env): ...@@ -519,7 +519,7 @@ class DirectRLEnv(gym.Env):
if self.cfg.events: if self.cfg.events:
if "reset" in self.event_manager.available_modes: if "reset" in self.event_manager.available_modes:
env_step_count = self._sim_step_counter // self.cfg.decimation env_step_count = self._sim_step_counter // self.cfg.decimation
self.event_manager.apply(env_ids=env_ids, mode="reset", global_env_step_count=env_step_count) self.event_manager.apply(mode="reset", env_ids=env_ids, global_env_step_count=env_step_count)
# reset noise models # reset noise models
if self.cfg.action_noise_model: if self.cfg.action_noise_model:
......
...@@ -337,10 +337,10 @@ class ManagerBasedEnv: ...@@ -337,10 +337,10 @@ class ManagerBasedEnv:
""" """
# reset the internal buffers of the scene elements # reset the internal buffers of the scene elements
self.scene.reset(env_ids) self.scene.reset(env_ids)
# apply events such as randomizations for environments that need a reset # apply events such as randomization for environments that need a reset
if "reset" in self.event_manager.available_modes: if "reset" in self.event_manager.available_modes:
env_step_count = self._sim_step_counter // self.cfg.decimation env_step_count = self._sim_step_counter // self.cfg.decimation
self.event_manager.apply(env_ids=env_ids, mode="reset", global_env_step_count=env_step_count) self.event_manager.apply(mode="reset", env_ids=env_ids, global_env_step_count=env_step_count)
# iterate over all managers and reset them # iterate over all managers and reset them
# this returns a dictionary of information which is stored in the extras # this returns a dictionary of information which is stored in the extras
......
...@@ -319,7 +319,7 @@ class ManagerBasedRLEnv(ManagerBasedEnv, gym.Env): ...@@ -319,7 +319,7 @@ class ManagerBasedRLEnv(ManagerBasedEnv, gym.Env):
# apply events such as randomizations for environments that need a reset # apply events such as randomizations for environments that need a reset
if "reset" in self.event_manager.available_modes: if "reset" in self.event_manager.available_modes:
env_step_count = self._sim_step_counter // self.cfg.decimation env_step_count = self._sim_step_counter // self.cfg.decimation
self.event_manager.apply(env_ids=env_ids, mode="reset", global_env_step_count=env_step_count) self.event_manager.apply(mode="reset", env_ids=env_ids, global_env_step_count=env_step_count)
# iterate over all managers and reset them # iterate over all managers and reset them
# this returns a dictionary of information which is stored in the extras # this returns a dictionary of information which is stored in the extras
......
...@@ -191,7 +191,7 @@ class EventTermCfg(ManagerTermBaseCfg): ...@@ -191,7 +191,7 @@ class EventTermCfg(ManagerTermBaseCfg):
""" """
interval_range_s: tuple[float, float] | None = None interval_range_s: tuple[float, float] | None = None
"""The range of time in seconds at which the term is applied. """The range of time in seconds at which the term is applied. Defaults to None.
Based on this, the interval is sampled uniformly between the specified Based on this, the interval is sampled uniformly between the specified
range for each environment instance. The term is applied on the environment range for each environment instance. The term is applied on the environment
...@@ -202,21 +202,24 @@ class EventTermCfg(ManagerTermBaseCfg): ...@@ -202,21 +202,24 @@ class EventTermCfg(ManagerTermBaseCfg):
""" """
is_global_time: bool = False is_global_time: bool = False
""" Whether randomization should be tracked on a per-environment basis. """Whether randomization should be tracked on a per-environment basis. Defaults to False.
If True, the same time for the interval is tracked for all the environments instead of If True, the same interval time is used for all the environment instances.
tracking the time per-environment. If False, the interval time is sampled independently for each environment instance
and the term is applied when the current time hits the interval time for that instance.
Note: Note:
This is only used if the mode is ``"interval"``. This is only used if the mode is ``"interval"``.
""" """
min_step_count_between_reset: int = 0 min_step_count_between_reset: int = 0
"""The minimum number of environment steps between when term is applied. """The number of environment steps after which the term is applied since its last application. Defaults to 0.
When mode is "reset", the term will not be applied on the next reset unless When the mode is "reset", the term is only applied if the number of environment steps since
the number of steps since the last application of the term has exceeded this. its last application exceeds this quantity. This helps to avoid calling the term too often,
This is useful to avoid calling this term too often and improve performance. thereby improving performance.
If the value is zero, the term is applied on every call to the manager with the mode "reset".
Note: Note:
This is only used if the mode is ``"reset"``. This is only used if the mode is ``"reset"``.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment