From a4c43bad397f0b4866d61d90a0517861edb0ca53 Mon Sep 17 00:00:00 2001 From: David Hoeller Date: Mon, 1 Jul 2024 21:28:29 +0200 Subject: [PATCH 01/15] Fixed the rendering logic --- .../omni/isaac/lab/envs/direct_rl_env.py | 9 +- .../omni/isaac/lab/envs/manager_based_env.py | 9 +- .../isaac/lab/envs/manager_based_rl_env.py | 9 +- .../test/envs/test_env_rendering_logic.py | 168 ++++++++++++++++++ 4 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py index e54435bc12..3aca5e0330 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py @@ -298,11 +298,12 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: self._apply_action() # set actions into simulator self.scene.write_data_to_sim() - render = self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( - self.sim.has_gui() or self.sim.has_rtx_sensors() - ) # simulate - self.sim.step(render=render) + self.sim.step(render=False) + if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( + self.sim.has_gui() or self.sim.has_rtx_sensors() + ): + self.sim.render() # update buffers at sim dt self.scene.update(dt=self.physics_dt) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py index d5f4d1d1b1..399f659107 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py @@ -269,11 +269,12 @@ def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]: self.action_manager.apply_action() # set actions into simulator self.scene.write_data_to_sim() - render = self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( - self.sim.has_gui() or self.sim.has_rtx_sensors() - ) # simulate - self.sim.step(render=render) + self.sim.step(render=False) + if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( + self.sim.has_gui() or self.sim.has_rtx_sensors() + ): + self.sim.render() # update buffers at sim dt self.scene.update(dt=self.physics_dt) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py index 141521fdf1..fd2bf9e664 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py @@ -161,11 +161,12 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: self.action_manager.apply_action() # set actions into simulator self.scene.write_data_to_sim() - render = self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( - self.sim.has_gui() or self.sim.has_rtx_sensors() - ) # simulate - self.sim.step(render=render) + self.sim.step(render=False) + if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( + self.sim.has_gui() or self.sim.has_rtx_sensors() + ): + self.sim.render() # update buffers at sim dt self.scene.update(dt=self.physics_dt) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py new file mode 100644 index 0000000000..b69625841d --- /dev/null +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -0,0 +1,168 @@ +# Copyright (c) 2022-2024, The Isaac Lab Project Developers. +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +# ignore private usage of variables warning +# pyright: reportPrivateUsage=none + +from __future__ import annotations + +"""Launch Isaac Sim Simulator first.""" + +from omni.isaac.lab.app import AppLauncher, run_tests + +# launch omniverse app +app_launcher = AppLauncher(headless=True) +simulation_app = app_launcher.app + +"""Rest everything follows.""" + +import torch +import unittest + +import omni.usd + +from omni.isaac.lab.envs import ( + DirectRLEnv, + DirectRLEnvCfg, + ManagerBasedEnv, + ManagerBasedEnvCfg, + ManagerBasedRLEnv, + ManagerBasedRLEnvCfg, +) +from omni.isaac.lab.scene import InteractiveSceneCfg +from omni.isaac.lab.sim import SimulationCfg, SimulationContext +from omni.isaac.lab.utils import configclass + + +@configclass +class EmptyActionsCfg: + """Action specifications for the environment.""" + + pass + + +def create_manager_based_env(render_interval: int): + """Create a manager based environment.""" + + @configclass + class EnvCfg(ManagerBasedEnvCfg): + """Configuration for the test environment.""" + + decimation: int = 4 + sim: SimulationCfg = SimulationCfg(dt=0.005, render_interval=render_interval) + scene: InteractiveSceneCfg = InteractiveSceneCfg(num_envs=1, env_spacing=1.0) + actions: EmptyActionsCfg = EmptyActionsCfg() + + return ManagerBasedEnv(cfg=EnvCfg()) + + +def create_manager_based_rl_env(render_interval: int): + """Create a manager based RL environment.""" + + @configclass + class EnvCfg(ManagerBasedRLEnvCfg): + """Configuration for the test environment.""" + + decimation: int = 4 + sim: SimulationCfg = SimulationCfg(dt=0.005, render_interval=render_interval) + scene: InteractiveSceneCfg = InteractiveSceneCfg(num_envs=1, env_spacing=1.0) + actions: EmptyActionsCfg = EmptyActionsCfg() + + return ManagerBasedRLEnv(cfg=EnvCfg()) + + +def create_direct_rl_env(render_interval: int): + """Create a direct RL environment.""" + + @configclass + class EnvCfg(DirectRLEnvCfg): + """Configuration for the test environment.""" + + decimation: int = 4 + num_actions: int = 0 + num_observations: int = 0 + sim: SimulationCfg = SimulationCfg(dt=0.005, render_interval=render_interval) + scene: InteractiveSceneCfg = InteractiveSceneCfg(num_envs=1, env_spacing=1.0) + + class Env(DirectRLEnv): + """Test environment.""" + + def _pre_physics_step(self, actions): + pass + + def _apply_action(self): + pass + + def _get_observations(self): + return {} + + def _get_rewards(self): + return {} + + def _get_dones(self): + return torch.zeros(1, dtype=torch.bool), torch.zeros(1, dtype=torch.bool) + + return Env(cfg=EnvCfg()) + + +class TestEnvRenderingLogic(unittest.TestCase): + """Test the rendering logic of the different environments.""" + + def _physics_callback(self, dt): + # called at every physics step + self.physics_dt += dt + self.num_physics_steps += 1 + + def _render_callback(self, event): + # called at every render step + self.render_dt += event.payload["dt"] + self.num_render_steps += 1 + + def test_env_rendering_logic(self): + for env_type in ["manager_based_env", "manager_based_rl_env", "direct_rl_env"]: + for render_interval in [1, 4]: + with self.subTest(env_type=env_type, render_interval=render_interval): + # time tracking variables + self.physics_dt = 0.0 + self.render_dt = 0.0 + # step tracking variables + self.num_physics_steps = 0 + self.num_render_steps = 0 + # create a new stage + omni.usd.get_context().new_stage() + # create environment + if env_type == "manager_based_env": + env = create_manager_based_env(render_interval) + elif env_type == "manager_based_rl_env": + env = create_manager_based_rl_env(render_interval) + else: + env = create_direct_rl_env(render_interval) + + # we override these variables to make sure that sim.render() is called + env.sim._has_gui = True + env.sim.render_mode = SimulationContext.RenderMode.PARTIAL_RENDERING + + # add physics and render callbacks + env.sim.add_physics_callback("physics_step", self._physics_callback) + env.sim.add_render_callback("render_step", self._render_callback) + + actions = torch.zeros((env.num_envs, 0), device=env.device) + for i in range(4): + env.step(action=actions) + # check that we have completed the correct number of physics steps + self.assertEqual(self.num_physics_steps, (i + 1) * env.cfg.decimation) + # check that we have simulated physics for the correct amount of time + self.assertAlmostEqual(self.physics_dt, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) + # check that we have completed the correct number of rendering steps + self.assertEqual( + self.num_render_steps, (i + 1) * env.cfg.decimation // env.cfg.sim.render_interval + ) + # check that we have rendered for the correct amount of time + self.assertAlmostEqual(self.render_dt, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) + env.close() + + +if __name__ == "__main__": + run_tests() From 1187a75e6845da27c2a0699e0ebc089628a2d585 Mon Sep 17 00:00:00 2001 From: David Hoeller Date: Mon, 1 Jul 2024 21:41:09 +0200 Subject: [PATCH 02/15] Update changelog --- .../extensions/omni.isaac.lab/config/extension.toml | 2 +- source/extensions/omni.isaac.lab/docs/CHANGELOG.rst | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/source/extensions/omni.isaac.lab/config/extension.toml b/source/extensions/omni.isaac.lab/config/extension.toml index b86d7f1332..1b42771082 100644 --- a/source/extensions/omni.isaac.lab/config/extension.toml +++ b/source/extensions/omni.isaac.lab/config/extension.toml @@ -1,7 +1,7 @@ [package] # Note: Semantic Versioning is used: https://semver.org/ -version = "0.18.5" +version = "0.18.6" # Description title = "Isaac Lab framework for Robot Learning" diff --git a/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst b/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst index 60153cc9db..e7a1b0930b 100644 --- a/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst +++ b/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst @@ -1,6 +1,17 @@ Changelog --------- +0.18.6 (2024-07-01) +~~~~~~~~~~~~~~~~~~~ + +Fixed +^^^^^ + +* Fixed the environment stepping logic. Earlier, the environments' rendering logic was updating the kit app which + would in turn step the physics :attr:`omni.isaac.lab.sim.SimulationCfg.render_interval` times. Now, a render + call only does rendering and does not step the physics. + + 0.18.5 (2024-06-26) ~~~~~~~~~~~~~~~~~~~ From a6bd0713203fe3238983fb63b259940bbf97ab63 Mon Sep 17 00:00:00 2001 From: David Hoeller Date: Mon, 1 Jul 2024 21:41:51 +0200 Subject: [PATCH 03/15] Formatting --- source/extensions/omni.isaac.lab/docs/CHANGELOG.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst b/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst index e7a1b0930b..767ef45496 100644 --- a/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst +++ b/source/extensions/omni.isaac.lab/docs/CHANGELOG.rst @@ -7,8 +7,8 @@ Changelog Fixed ^^^^^ -* Fixed the environment stepping logic. Earlier, the environments' rendering logic was updating the kit app which - would in turn step the physics :attr:`omni.isaac.lab.sim.SimulationCfg.render_interval` times. Now, a render +* Fixed the environment stepping logic. Earlier, the environments' rendering logic was updating the kit app which + would in turn step the physics :attr:`omni.isaac.lab.sim.SimulationCfg.render_interval` times. Now, a render call only does rendering and does not step the physics. From 97de306a132f37223ec3bab3ccd7ae38fd92742a Mon Sep 17 00:00:00 2001 From: David Hoeller Date: Mon, 1 Jul 2024 22:00:23 +0200 Subject: [PATCH 04/15] Rename variables --- .../test/envs/test_env_rendering_logic.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py index b69625841d..3380d8b70f 100644 --- a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -112,12 +112,12 @@ class TestEnvRenderingLogic(unittest.TestCase): def _physics_callback(self, dt): # called at every physics step - self.physics_dt += dt + self.physics_time += dt self.num_physics_steps += 1 def _render_callback(self, event): # called at every render step - self.render_dt += event.payload["dt"] + self.render_time += event.payload["dt"] self.num_render_steps += 1 def test_env_rendering_logic(self): @@ -125,8 +125,8 @@ def test_env_rendering_logic(self): for render_interval in [1, 4]: with self.subTest(env_type=env_type, render_interval=render_interval): # time tracking variables - self.physics_dt = 0.0 - self.render_dt = 0.0 + self.physics_time = 0.0 + self.render_time = 0.0 # step tracking variables self.num_physics_steps = 0 self.num_render_steps = 0 @@ -154,13 +154,13 @@ def test_env_rendering_logic(self): # check that we have completed the correct number of physics steps self.assertEqual(self.num_physics_steps, (i + 1) * env.cfg.decimation) # check that we have simulated physics for the correct amount of time - self.assertAlmostEqual(self.physics_dt, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) + self.assertAlmostEqual(self.physics_time, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) # check that we have completed the correct number of rendering steps self.assertEqual( self.num_render_steps, (i + 1) * env.cfg.decimation // env.cfg.sim.render_interval ) # check that we have rendered for the correct amount of time - self.assertAlmostEqual(self.render_dt, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) + self.assertAlmostEqual(self.render_time, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) env.close() From 27126858ea4f48675166201b06397e70c251b4f2 Mon Sep 17 00:00:00 2001 From: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> Date: Tue, 2 Jul 2024 04:47:30 +0200 Subject: [PATCH 05/15] Apply suggestions from code review Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> --- .../omni.isaac.lab/test/envs/test_env_rendering_logic.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py index 3380d8b70f..e6b5b1ab6b 100644 --- a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -3,11 +3,6 @@ # # SPDX-License-Identifier: BSD-3-Clause -# ignore private usage of variables warning -# pyright: reportPrivateUsage=none - -from __future__ import annotations - """Launch Isaac Sim Simulator first.""" from omni.isaac.lab.app import AppLauncher, run_tests From 3112d2e5827f546d6e0c0a1b97c50b0ac4908832 Mon Sep 17 00:00:00 2001 From: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> Date: Tue, 2 Jul 2024 04:51:03 +0200 Subject: [PATCH 06/15] adds docstrings Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> --- .../omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py | 3 +++ .../omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py index 3aca5e0330..8ab8e93962 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py @@ -300,6 +300,9 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: self.scene.write_data_to_sim() # simulate self.sim.step(render=False) + # render between steps only if the GUI or an RTX sensor needs it + # note: we assume the render interval to be the shortest accepted rendering interval. + # If a camera needs rendering at a faster frequency, this will lead to errors if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( self.sim.has_gui() or self.sim.has_rtx_sensors() ): diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py index 399f659107..8b87d4db54 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py @@ -271,6 +271,9 @@ def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]: self.scene.write_data_to_sim() # simulate self.sim.step(render=False) + # render between steps only if the GUI or an RTX sensor needs it + # note: we assume the render interval to be the shortest accepted rendering interval. + # If a camera needs rendering at a faster frequency, this will lead to errors if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( self.sim.has_gui() or self.sim.has_rtx_sensors() ): From 62ed7a586618723aa0415b523a01ae31d4eb6f2c Mon Sep 17 00:00:00 2001 From: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> Date: Tue, 2 Jul 2024 04:52:51 +0200 Subject: [PATCH 07/15] shifts test to use EmptyManagerCfg Signed-off-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com> --- .../omni.isaac.lab/test/envs/test_env_rendering_logic.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py index e6b5b1ab6b..9a97e52218 100644 --- a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -32,8 +32,8 @@ @configclass -class EmptyActionsCfg: - """Action specifications for the environment.""" +class EmptyManagerCfg: + """Empty specifications for the environment.""" pass @@ -48,7 +48,8 @@ class EnvCfg(ManagerBasedEnvCfg): decimation: int = 4 sim: SimulationCfg = SimulationCfg(dt=0.005, render_interval=render_interval) scene: InteractiveSceneCfg = InteractiveSceneCfg(num_envs=1, env_spacing=1.0) - actions: EmptyActionsCfg = EmptyActionsCfg() + actions: EmptyManagerCfg = EmptyManagerCfg() + observations: EmptyManagerCfg = EmptyManagerCfg() return ManagerBasedEnv(cfg=EnvCfg()) From 866c35b812c93a992a56f1bc1d529edca37a448b Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 05:35:08 +0200 Subject: [PATCH 08/15] runs formatter --- .../omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py | 2 ++ .../omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py index 8ab8e93962..1b04373510 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py @@ -427,6 +427,8 @@ def render(self, recompute: bool = False) -> np.ndarray | None: def close(self): """Cleanup for the environment.""" if not self._is_closed: + # close entities related to the environment + # note: this is order-sensitive to avoid any dangling references if self.cfg.events: del self.event_manager del self.scene diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py index 8b87d4db54..a7a2323a91 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py @@ -273,7 +273,7 @@ def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]: self.sim.step(render=False) # render between steps only if the GUI or an RTX sensor needs it # note: we assume the render interval to be the shortest accepted rendering interval. - # If a camera needs rendering at a faster frequency, this will lead to errors + # If a camera needs rendering at a faster frequency, this will lead to errors if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( self.sim.has_gui() or self.sim.has_rtx_sensors() ): From 12b615c56f4d576a7d6a918e904b66f6741fe693 Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 05:40:20 +0200 Subject: [PATCH 09/15] hacks different variables --- .../test/envs/test_env_rendering_logic.py | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py index 9a97e52218..a176df6659 100644 --- a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -8,7 +8,8 @@ from omni.isaac.lab.app import AppLauncher, run_tests # launch omniverse app -app_launcher = AppLauncher(headless=True) +# need to set "enable_cameras" true to be able to do rendering tests +app_launcher = AppLauncher(headless=True, enable_cameras=True) simulation_app = app_launcher.app """Rest everything follows.""" @@ -64,7 +65,8 @@ class EnvCfg(ManagerBasedRLEnvCfg): decimation: int = 4 sim: SimulationCfg = SimulationCfg(dt=0.005, render_interval=render_interval) scene: InteractiveSceneCfg = InteractiveSceneCfg(num_envs=1, env_spacing=1.0) - actions: EmptyActionsCfg = EmptyActionsCfg() + actions: EmptyManagerCfg = EmptyManagerCfg() + observations: EmptyManagerCfg = EmptyManagerCfg() return ManagerBasedRLEnv(cfg=EnvCfg()) @@ -104,7 +106,7 @@ def _get_dones(self): class TestEnvRenderingLogic(unittest.TestCase): - """Test the rendering logic of the different environments.""" + """Test the rendering logic of the different environment workflows.""" def _physics_callback(self, dt): # called at every physics step @@ -126,8 +128,10 @@ def test_env_rendering_logic(self): # step tracking variables self.num_physics_steps = 0 self.num_render_steps = 0 + # create a new stage omni.usd.get_context().new_stage() + # create environment if env_type == "manager_based_env": env = create_manager_based_env(render_interval) @@ -136,9 +140,14 @@ def test_env_rendering_logic(self): else: env = create_direct_rl_env(render_interval) + # enable the flag to render the environment + # note: this is only done for the unit testing to "fake" camera rendering + env.sim.set_setting("/isaaclab/render/rtx_sensors", True) + # disable the app from shutting down when the environment is closed + env.sim._app_control_on_stop_handle = None # type: ignore + # we override these variables to make sure that sim.render() is called - env.sim._has_gui = True - env.sim.render_mode = SimulationContext.RenderMode.PARTIAL_RENDERING + self.assertEqual(env.sim.render_mode, SimulationContext.RenderMode.PARTIAL_RENDERING) # add physics and render callbacks env.sim.add_physics_callback("physics_step", self._physics_callback) @@ -157,7 +166,9 @@ def test_env_rendering_logic(self): ) # check that we have rendered for the correct amount of time self.assertAlmostEqual(self.render_time, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) - env.close() + + # close the environment + env.close() if __name__ == "__main__": From dbc98a4fd5ad23cd6ce475a5e76b9a3f77f90702 Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 05:47:17 +0200 Subject: [PATCH 10/15] cosmetic --- .../omni.isaac.lab/omni/isaac/lab/app/app_launcher.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/app/app_launcher.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/app/app_launcher.py index ed79076568..c0c57184d8 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/app/app_launcher.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/app/app_launcher.py @@ -534,7 +534,8 @@ def _create_app(self): for key, value in hacked_modules.items(): sys.modules[key] = value - def _rendering_enabled(self): + def _rendering_enabled(self) -> bool: + """Check if rendering is required by the app.""" # Indicates whether rendering is required by the app. # Extensions required for rendering bring startup and simulation costs, so we do not enable them if not required. return not self._headless or self._livestream >= 1 or self._enable_cameras From fdd8c6c54dc0f91b4dc6ebd777d1153db5eff5e3 Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 06:07:06 +0200 Subject: [PATCH 11/15] moves render check outside once --- .../omni/isaac/lab/envs/direct_rl_env.py | 12 +++++++----- .../omni/isaac/lab/envs/manager_based_env.py | 10 ++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py index 1b04373510..816ed90e1b 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/direct_rl_env.py @@ -288,9 +288,13 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: # add action noise if self.cfg.action_noise_model: action = self._action_noise_model.apply(action.clone()) - # process actions self._pre_physics_step(action) + + # check if we need to do rendering within the physics loop + # note: checked here once to avoid multiple checks within the loop + is_rendering = self.sim.has_gui() or self.sim.has_rtx_sensors() + # perform physics stepping for _ in range(self.cfg.decimation): self._sim_step_counter += 1 @@ -302,10 +306,8 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: self.sim.step(render=False) # render between steps only if the GUI or an RTX sensor needs it # note: we assume the render interval to be the shortest accepted rendering interval. - # If a camera needs rendering at a faster frequency, this will lead to errors - if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( - self.sim.has_gui() or self.sim.has_rtx_sensors() - ): + # If a camera needs rendering at a faster frequency, this will lead to unexpected behavior. + if self._sim_step_counter % self.cfg.sim.render_interval == 0 and is_rendering: self.sim.render() # update buffers at sim dt self.scene.update(dt=self.physics_dt) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py index a7a2323a91..ca48562be4 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py @@ -262,6 +262,10 @@ def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]: """ # process actions self.action_manager.process_action(action) + # check if we need to do rendering within the physics loop + # note: checked here once to avoid multiple checks within the loop + is_rendering = self.sim.has_gui() or self.sim.has_rtx_sensors() + # perform physics stepping for _ in range(self.cfg.decimation): self._sim_step_counter += 1 @@ -273,10 +277,8 @@ def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]: self.sim.step(render=False) # render between steps only if the GUI or an RTX sensor needs it # note: we assume the render interval to be the shortest accepted rendering interval. - # If a camera needs rendering at a faster frequency, this will lead to errors - if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( - self.sim.has_gui() or self.sim.has_rtx_sensors() - ): + # If a camera needs rendering at a faster frequency, this will lead to unexpected behavior. + if self._sim_step_counter % self.cfg.sim.render_interval == 0 and is_rendering: self.sim.render() # update buffers at sim dt self.scene.update(dt=self.physics_dt) From ab80f50ec381988bc53050fa31909c487139db82 Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 06:07:22 +0200 Subject: [PATCH 12/15] makes testing more robust for env render --- .../test/envs/test_env_rendering_logic.py | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py index a176df6659..2fbc376ed8 100644 --- a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -120,7 +120,7 @@ def _render_callback(self, event): def test_env_rendering_logic(self): for env_type in ["manager_based_env", "manager_based_rl_env", "direct_rl_env"]: - for render_interval in [1, 4]: + for render_interval in [1, 2, 4, 8, 10]: with self.subTest(env_type=env_type, render_interval=render_interval): # time tracking variables self.physics_time = 0.0 @@ -143,29 +143,40 @@ def test_env_rendering_logic(self): # enable the flag to render the environment # note: this is only done for the unit testing to "fake" camera rendering env.sim.set_setting("/isaaclab/render/rtx_sensors", True) + # disable the app from shutting down when the environment is closed + # FIXME: Why is this needed in this test but not in the other tests? + # Without it, the test will exit after the environment is closed env.sim._app_control_on_stop_handle = None # type: ignore - # we override these variables to make sure that sim.render() is called + # check that we are in partial rendering mode for the environment self.assertEqual(env.sim.render_mode, SimulationContext.RenderMode.PARTIAL_RENDERING) # add physics and render callbacks env.sim.add_physics_callback("physics_step", self._physics_callback) env.sim.add_render_callback("render_step", self._render_callback) + # create a zero action tensor for stepping the environment actions = torch.zeros((env.num_envs, 0), device=env.device) - for i in range(4): + + # run the environment and check the rendering logic + for i in range(50): + # apply zero actions env.step(action=actions) + # check that we have completed the correct number of physics steps self.assertEqual(self.num_physics_steps, (i + 1) * env.cfg.decimation) # check that we have simulated physics for the correct amount of time - self.assertAlmostEqual(self.physics_time, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) + self.assertAlmostEqual(self.physics_time, self.num_physics_steps * env.cfg.sim.dt) + # check that we have completed the correct number of rendering steps self.assertEqual( self.num_render_steps, (i + 1) * env.cfg.decimation // env.cfg.sim.render_interval ) # check that we have rendered for the correct amount of time - self.assertAlmostEqual(self.render_time, (i + 1) * env.cfg.decimation * env.cfg.sim.dt) + self.assertAlmostEqual( + self.render_time, self.num_render_steps * env.cfg.sim.dt * env.cfg.sim.render_interval + ) # close the environment env.close() From eb82fa70b8b0d8e038809d575f871de59e111676 Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 06:08:36 +0200 Subject: [PATCH 13/15] make enters consistent --- .../omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py index ca48562be4..f641b67b77 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_env.py @@ -262,6 +262,7 @@ def step(self, action: torch.Tensor) -> tuple[VecEnvObs, dict]: """ # process actions self.action_manager.process_action(action) + # check if we need to do rendering within the physics loop # note: checked here once to avoid multiple checks within the loop is_rendering = self.sim.has_gui() or self.sim.has_rtx_sensors() From 588b88d34e0957bdf288939ae7ce5a0572537dbd Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 06:16:44 +0200 Subject: [PATCH 14/15] adds better explainations --- .../test/envs/test_env_rendering_logic.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py index 2fbc376ed8..24af5c91d6 100644 --- a/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py +++ b/source/extensions/omni.isaac.lab/test/envs/test_env_rendering_logic.py @@ -141,7 +141,8 @@ def test_env_rendering_logic(self): env = create_direct_rl_env(render_interval) # enable the flag to render the environment - # note: this is only done for the unit testing to "fake" camera rendering + # note: this is only done for the unit testing to "fake" camera rendering. + # normally this is set to True when cameras are created. env.sim.set_setting("/isaaclab/render/rtx_sensors", True) # disable the app from shutting down when the environment is closed @@ -150,6 +151,7 @@ def test_env_rendering_logic(self): env.sim._app_control_on_stop_handle = None # type: ignore # check that we are in partial rendering mode for the environment + # this is enabled due to app launcher setting "enable_cameras=True" self.assertEqual(env.sim.render_mode, SimulationContext.RenderMode.PARTIAL_RENDERING) # add physics and render callbacks @@ -165,17 +167,25 @@ def test_env_rendering_logic(self): env.step(action=actions) # check that we have completed the correct number of physics steps - self.assertEqual(self.num_physics_steps, (i + 1) * env.cfg.decimation) + self.assertEqual( + self.num_physics_steps, (i + 1) * env.cfg.decimation, msg="Physics steps mismatch" + ) # check that we have simulated physics for the correct amount of time - self.assertAlmostEqual(self.physics_time, self.num_physics_steps * env.cfg.sim.dt) + self.assertAlmostEqual( + self.physics_time, self.num_physics_steps * env.cfg.sim.dt, msg="Physics time mismatch" + ) # check that we have completed the correct number of rendering steps self.assertEqual( - self.num_render_steps, (i + 1) * env.cfg.decimation // env.cfg.sim.render_interval + self.num_render_steps, + (i + 1) * env.cfg.decimation // env.cfg.sim.render_interval, + msg="Render steps mismatch", ) # check that we have rendered for the correct amount of time self.assertAlmostEqual( - self.render_time, self.num_render_steps * env.cfg.sim.dt * env.cfg.sim.render_interval + self.render_time, + self.num_render_steps * env.cfg.sim.dt * env.cfg.sim.render_interval, + msg="Render time mismatch", ) # close the environment From 1ea2a2aa09ebdeabb8cab9c22733f91c15c11e55 Mon Sep 17 00:00:00 2001 From: Mayank Mittal Date: Tue, 2 Jul 2024 06:19:25 +0200 Subject: [PATCH 15/15] fixes also to other env --- .../omni/isaac/lab/envs/manager_based_rl_env.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py index fd2bf9e664..96e383e024 100644 --- a/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py +++ b/source/extensions/omni.isaac.lab/omni/isaac/lab/envs/manager_based_rl_env.py @@ -154,6 +154,11 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: """ # process actions self.action_manager.process_action(action) + + # check if we need to do rendering within the physics loop + # note: checked here once to avoid multiple checks within the loop + is_rendering = self.sim.has_gui() or self.sim.has_rtx_sensors() + # perform physics stepping for _ in range(self.cfg.decimation): self._sim_step_counter += 1 @@ -163,9 +168,10 @@ def step(self, action: torch.Tensor) -> VecEnvStepReturn: self.scene.write_data_to_sim() # simulate self.sim.step(render=False) - if self._sim_step_counter % self.cfg.sim.render_interval == 0 and ( - self.sim.has_gui() or self.sim.has_rtx_sensors() - ): + # render between steps only if the GUI or an RTX sensor needs it + # note: we assume the render interval to be the shortest accepted rendering interval. + # If a camera needs rendering at a faster frequency, this will lead to unexpected behavior. + if self._sim_step_counter % self.cfg.sim.render_interval == 0 and is_rendering: self.sim.render() # update buffers at sim dt self.scene.update(dt=self.physics_dt)