Adds testing to check_env that closing a closed environment doesn't raise an error (#564)

Co-authored-by: Quentin GALLOUÉDEC <gallouedec.quentin@gmail.com>
Co-authored-by: Quentin Gallouédec <45557362+qgallouedec@users.noreply.github.com>
This commit is contained in:
Mark Towers
2023-06-21 17:39:50 +01:00
committed by GitHub
parent ee2918f799
commit a40234f5ef
5 changed files with 32 additions and 5 deletions

View File

@@ -185,6 +185,7 @@ class Env(Generic[ObsType, ActType]):
"""After the user has finished using the environment, close contains the code necessary to "clean up" the environment. """After the user has finished using the environment, close contains the code necessary to "clean up" the environment.
This is critical for closing rendering windows, database or HTTP connections. This is critical for closing rendering windows, database or HTTP connections.
Calling ``close`` on an already closed environment has no effect and won't raise an error.
""" """
pass pass

View File

@@ -603,7 +603,7 @@ class MujocoRenderer:
The class has two main public methods available: The class has two main public methods available:
- :meth:`render` - Renders the environment in three possible modes: "human", "rgb_array", or "depth_array" - :meth:`render` - Renders the environment in three possible modes: "human", "rgb_array", or "depth_array"
- :meth:`close` - Closes all contexts initialized with the rendere - :meth:`close` - Closes all contexts initialized with the renderer
""" """

View File

@@ -321,3 +321,13 @@ def check_env(env: gym.Env, warn: bool = None, skip_render_check: bool = False):
logger.warn( logger.warn(
"Not able to test alternative render modes due to the environment not having a spec. Try instantialising the environment through gymnasium.make" "Not able to test alternative render modes due to the environment not having a spec. Try instantialising the environment through gymnasium.make"
) )
if env.spec is not None:
new_env = env.spec.make()
new_env.close()
try:
new_env.close()
except Exception as e:
logger.warn(
f"Calling `env.close()` on the closed environment should be allowed, but it raised an exception: {e}"
)

View File

@@ -5,6 +5,7 @@ from copy import deepcopy
from typing import TYPE_CHECKING from typing import TYPE_CHECKING
import gymnasium as gym import gymnasium as gym
from gymnasium import logger
from gymnasium.core import ActType from gymnasium.core import ActType
from gymnasium.utils.passive_env_checker import ( from gymnasium.utils.passive_env_checker import (
check_action_space, check_action_space,
@@ -39,10 +40,11 @@ class PassiveEnvChecker(gym.Wrapper, gym.utils.RecordConstructorArgs):
self.checked_reset = False self.checked_reset = False
self.checked_step = False self.checked_step = False
self.checked_render = False self.checked_render = False
self.close_called = False
def step(self, action: ActType): def step(self, action: ActType):
"""Steps through the environment that on the first call will run the `passive_env_step_check`.""" """Steps through the environment that on the first call will run the `passive_env_step_check`."""
if self.checked_step is False: if not self.checked_step:
self.checked_step = True self.checked_step = True
return env_step_passive_checker(self.env, action) return env_step_passive_checker(self.env, action)
else: else:
@@ -50,7 +52,7 @@ class PassiveEnvChecker(gym.Wrapper, gym.utils.RecordConstructorArgs):
def reset(self, **kwargs): def reset(self, **kwargs):
"""Resets the environment that on the first call will run the `passive_env_reset_check`.""" """Resets the environment that on the first call will run the `passive_env_reset_check`."""
if self.checked_reset is False: if not self.checked_reset:
self.checked_reset = True self.checked_reset = True
return env_reset_passive_checker(self.env, **kwargs) return env_reset_passive_checker(self.env, **kwargs)
else: else:
@@ -58,7 +60,7 @@ class PassiveEnvChecker(gym.Wrapper, gym.utils.RecordConstructorArgs):
def render(self, *args, **kwargs): def render(self, *args, **kwargs):
"""Renders the environment that on the first call will run the `passive_env_render_check`.""" """Renders the environment that on the first call will run the `passive_env_render_check`."""
if self.checked_render is False: if not self.checked_render:
self.checked_render = True self.checked_render = True
return env_render_passive_checker(self.env, *args, **kwargs) return env_render_passive_checker(self.env, *args, **kwargs)
else: else:
@@ -77,3 +79,17 @@ class PassiveEnvChecker(gym.Wrapper, gym.utils.RecordConstructorArgs):
self._cached_spec = env_spec self._cached_spec = env_spec
return env_spec return env_spec
def close(self):
"""Warns if calling close on a closed environment fails."""
if not self.close_called:
self.close_called = True
return self.env.close()
else:
try:
return self.env.close()
except Exception as e:
logger.warn(
"Calling `env.close()` on the closed environment should be allowed, but it raised the following exception."
)
raise e

View File

@@ -58,7 +58,7 @@ class GenericTestEnv(gym.Env):
metadata: dict[str, Any] = {"render_modes": []}, metadata: dict[str, Any] = {"render_modes": []},
render_mode: str | None = None, render_mode: str | None = None,
spec: EnvSpec = EnvSpec( spec: EnvSpec = EnvSpec(
"TestingEnv-v0", "testing-env-no-entry-point", max_episode_steps=100 "TestingEnv-v0", "tests.testing_env:GenericTestEnv", max_episode_steps=100
), ),
): ):
"""Generic testing environment constructor. """Generic testing environment constructor.