Skip to content

Commit

Permalink
Fixes behavior of shutdown app on stop in SimulationContext (#254)
Browse files Browse the repository at this point in the history
This MR fixes the behavior of
`omni.isaac.orbit.sim.SimulationCfg.shutdown_app_on_stop` to not close
the app when the flag is False. Earlier, it would throw an error
(invalid physics handles) when the stop button was pressed in the GUI
and the flag was False.

- Bug fix (non-breaking change which fixes an issue)

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] 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
  • Loading branch information
Mayankm96 committed Nov 28, 2023
1 parent 51f6292 commit 7b0db15
Show file tree
Hide file tree
Showing 14 changed files with 80 additions and 49 deletions.
2 changes: 1 addition & 1 deletion source/extensions/omni.isaac.orbit/config/extension.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]

# Note: Semantic Versioning is used: https://semver.org/
version = "0.9.49"
version = "0.9.50"

# Description
title = "ORBIT framework for Robot Learning"
Expand Down
17 changes: 17 additions & 0 deletions source/extensions/omni.isaac.orbit/docs/CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
Changelog
---------

0.9.50 (2023-11-28)
~~~~~~~~~~~~~~~~~~~

Added
^^^^^

* Hid the ``STOP`` button in the UI when running standalone Python scripts. This is to prevent
users from accidentally clicking the button and stopping the simulation. They should only be able to
play and pause the simulation from the UI.

Removed
^^^^^^^

* Removed :attr:`omni.isaac.orbit.sim.SimulationCfg.shutdown_app_on_stop`. The simulation is always rendering
if it is stopped from the UI. The user needs to close the window or press ``Ctrl+C`` to close the simulation.


0.9.49 (2023-11-27)
~~~~~~~~~~~~~~~~~~~

Expand Down
14 changes: 14 additions & 0 deletions source/extensions/omni.isaac.orbit/omni/isaac/orbit/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,8 @@ def _create_app(self):
# add orbit modules back to sys.modules
for key, value in hacked_modules.items():
sys.modules[key] = value
# hide the stop button in the toolbar
self._hide_stop_button()

def _load_extensions(self):
"""Load correct extensions based on AppLauncher's resolved config member variables."""
Expand Down Expand Up @@ -644,3 +646,15 @@ def _load_extensions(self):
"/persistent/isaac/asset_root/nvidia",
"http://omniverse-content-production.s3-us-west-2.amazonaws.com/Assets",
)

def _hide_stop_button(self):
"""Hide the stop button in the toolbar."""
import omni.kit.widget.toolbar

# grey out the stop button because we don't want to stop the simulation manually in standalone mode
toolbar = omni.kit.widget.toolbar.get_instance()
play_button_group = toolbar._builtin_tools._play_button_group # type: ignore
if play_button_group is not None:
play_button_group._stop_button.visible = False # type: ignore
play_button_group._stop_button.enabled = False # type: ignore
play_button_group._stop_button = None # type: ignore
Original file line number Diff line number Diff line change
Expand Up @@ -256,13 +256,3 @@ class SimulationCfg:
The material is created at the path: ``{physics_prim_path}/defaultMaterial``.
"""

shutdown_app_on_stop: bool = True
"""Enable/disable shutting down the application when the simulation is stopped. Default is True.
This flag is only used when running the simulation as a standalone application.
.. note::
When the simulation is stopped, the physics handles become invalidated. Thus, in the simplest case,
it is better to shutdown the application.
"""
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,17 @@ def __init__(self, cfg: SimulationCfg | None = None):
# note: we do it once here because it reads the VERSION file from disk and is not expected to change.
self._isaacsim_version = get_version()

# add callback to deal the simulation app when simulation is stopped.
# this is needed because physics views go invalid once we stop the simulation
if not builtins.ISAAC_LAUNCHED_FROM_TERMINAL:
timeline_event_stream = omni.timeline.get_timeline_interface().get_timeline_event_stream()
self._app_control_on_stop_handle = timeline_event_stream.create_subscription_to_pop_by_type(
int(omni.timeline.TimelineEventType.STOP),
lambda *args, obj=weakref.proxy(self): obj._app_control_on_stop_callback(*args),
order=10,
)
else:
self._app_control_on_stop_handle = None
# flatten out the simulation dictionary
sim_params = self.cfg.to_dict()
if sim_params is not None:
Expand Down Expand Up @@ -336,22 +347,6 @@ def get_setting(self, name: str) -> Any:
Operations - Override (standalone)
"""

def reset(self, soft: bool = False):
# need to load all "physics" information from the USD file
# FIXME: Remove this for Isaac Sim 2023.1 release if it will be fixed in the core.
if not soft:
omni.physx.acquire_physx_interface().force_load_physics_from_usd()
# play the simulation
super().reset(soft=soft)
# add callback to shutdown the simulation when it is stopped.
# we do this because physics views go invalid once we stop the simulation. So there is
# no point in keeping the simulation running.
if self.cfg.shutdown_app_on_stop and not self.timeline_callback_exists("shutdown_app_on_stop"):
self.add_timeline_callback(
"shutdown_app_on_stop",
lambda *args, obj=weakref.proxy(self): obj._shutdown_app_on_stop_callback(*args),
)

def step(self, render: bool = True):
"""Steps the physics simulation with the pre-defined time-step.
Expand Down Expand Up @@ -431,7 +426,7 @@ async def reset_async(self, soft: bool = False):
await super().reset_async(soft=soft)

"""
Initialization - Override.
Initialization/Destruction - Override.
"""

def _init_stage(self, *args, **kwargs) -> Usd.Stage:
Expand All @@ -452,6 +447,16 @@ async def _initialize_stage_async(self, *args, **kwargs) -> Usd.Stage:
# return the stage
return self.stage

@classmethod
def clear_instance(cls):
# clear the callback
if cls._instance is not None:
if cls._instance._app_control_on_stop_handle is not None:
cls._instance._app_control_on_stop_handle.unsubscribe()
cls._instance._app_control_on_stop_handle = None
# call parent to clear the instance
super().clear_instance()

"""
Helper Functions
"""
Expand Down Expand Up @@ -526,15 +531,32 @@ def _load_fabric_interface(self):
Callbacks.
"""

def _shutdown_app_on_stop_callback(self, event: carb.events.IEvent):
"""Callback to shutdown the app when the simulation is stopped.
def _app_control_on_stop_callback(self, event: carb.events.IEvent):
"""Callback to deal with the app when the simulation is stopped.
Once the simulation is stopped, the physics handles go invalid. After that, it is not possible to
resume the simulation from the last state. This leaves the app in an inconsistent state, where
two possible actions can be taken:
1. **Keep the app rendering**: In this case, the simulation is kept running and the app is not shutdown.
However, the physics is not updated and the script cannot be resumed from the last state. The
user has to manually close the app to stop the simulation.
2. **Shutdown the app**: This is the default behavior. In this case, the app is shutdown and
the simulation is stopped.
This is used only when running the simulation in a standalone python script. This is because
the physics views go invalid once we stop the simulation. So there is no point in keeping the
simulation running.
Note:
This callback is used only when running the simulation in a standalone python script. In an extension,
it is expected that the user handles the extension shutdown.
"""
# check if the simulation is stopped
if event.type == int(omni.timeline.TimelineEventType.STOP):
# keep running the simulator when configured to not shutdown the app
self.app.print_and_log(
"Simulation is stopped. The app will keep running with physics disabled."
" Press Ctrl+C or close the window to exit the app."
)
while self.app.is_running():
self.render()
# make sure that any replicator workflows finish rendering/writing
if not builtins.ISAAC_LAUNCHED_FROM_TERMINAL:
try:
Expand All @@ -559,5 +581,3 @@ def _shutdown_app_on_stop_callback(self, event: carb.events.IEvent):
self.app.shutdown()
# disabled on linux to avoid a crash
carb.get_framework().unload_all_plugins()
# exit the application
print("Exiting the application complete.")
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def setUp(self):
# Simulation time-step
self.dt = 0.005
# Load kit helper
sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0", shutdown_app_on_stop=False)
sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0")
self.sim = sim_utils.SimulationContext(sim_cfg)

def tearDown(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def setUp(self):
# Simulation time-step
self.dt = 0.01
# Load kit helper
sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0", shutdown_app_on_stop=False)
sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0")
self.sim = sim_utils.SimulationContext(sim_cfg)

def tearDown(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def setUp(self):
# Constants
self.num_envs = 128
# Load kit helper
sim_cfg = sim_utils.SimulationCfg(dt=0.01, shutdown_app_on_stop=False)
sim_cfg = sim_utils.SimulationCfg(dt=0.01)
self.sim = sim_utils.SimulationContext(sim_cfg)

# Create a ground plane
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ def test_random_actions(self):
omni.usd.get_context().new_stage()
# parse configuration
env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
# note: we don't want to shutdown the app on stop during the tests since we reload the stage
env_cfg.sim.shutdown_app_on_stop = False

# create environment
env: RLTaskEnv = gym.make(task_name, cfg=env_cfg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ def test_record_video(self):

# parse configuration
env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
# note: we don't want to shutdown the app on stop during the tests since we reload the stage
env_cfg.sim.shutdown_app_on_stop = False

# create environment
env: RLTaskEnv = gym.make(task_name, cfg=env_cfg, render_mode="rgb_array")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ def test_random_actions(self):
omni.usd.get_context().new_stage()
# parse configuration
env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
# note: we don't want to shutdown the app on stop during the tests since we reload the stage
env_cfg.sim.shutdown_app_on_stop = False

# create environment
env = gym.make(task_name, cfg=env_cfg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ def test_random_actions(self):
omni.usd.get_context().new_stage()
# parse configuration
env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
# note: we don't want to shutdown the app on stop during the tests since we reload the stage
env_cfg.sim.shutdown_app_on_stop = False

# create environment
env = gym.make(task_name, cfg=env_cfg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ def test_random_actions(self):
omni.usd.get_context().new_stage()
# parse configuration
env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
# note: we don't want to shutdown the app on stop during the tests since we reload the stage
env_cfg.sim.shutdown_app_on_stop = False

# create environment
env = gym.make(task_name, cfg=env_cfg)
Expand Down
2 changes: 1 addition & 1 deletion source/standalone/demo/play_ik_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def main():
"""Main function."""

# Load kit helper
sim_cfg = sim_utils.SimulationCfg(dt=0.01, shutdown_app_on_stop=False)
sim_cfg = sim_utils.SimulationCfg(dt=0.01)
sim = sim_utils.SimulationContext(sim_cfg)
# Set main camera
sim.set_camera_view([2.5, 2.5, 2.5], [0.0, 0.0, 0.0])
Expand Down

0 comments on commit 7b0db15

Please sign in to comment.