From 2ae0dbc846469aeb6ac0d3890f1453e6df8f79f6 Mon Sep 17 00:00:00 2001 From: Greg Brockman Date: Sun, 15 May 2016 17:22:38 -0700 Subject: [PATCH] Discard viewer object after render with close=True Fixes #95 --- gym/envs/atari/atari_env.py | 1 + gym/envs/box2d/bipedal_walker.py | 1 + gym/envs/box2d/lunar_lander.py | 1 + gym/envs/classic_control/acrobot.py | 1 + gym/envs/classic_control/cartpole.py | 1 + gym/envs/classic_control/mountain_car.py | 1 + gym/envs/classic_control/pendulum.py | 1 + gym/envs/mujoco/mujoco_env.py | 1 + gym/envs/tests/test_envs.py | 13 ++++++------ gym/monitoring/tests/__init__.py | 0 gym/monitoring/tests/helpers.py | 9 ++++++++ gym/monitoring/tests/test_monitor.py | 18 +++++----------- gym/monitoring/tests/test_monitor_envs.py | 26 +++++++++++++++++++++++ 13 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 gym/monitoring/tests/__init__.py create mode 100644 gym/monitoring/tests/helpers.py create mode 100644 gym/monitoring/tests/test_monitor_envs.py diff --git a/gym/envs/atari/atari_env.py b/gym/envs/atari/atari_env.py index a10dbc81a..7755b2d47 100644 --- a/gym/envs/atari/atari_env.py +++ b/gym/envs/atari/atari_env.py @@ -84,6 +84,7 @@ class AtariEnv(gym.Env, utils.EzPickle): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return img = self._get_image() if mode == 'rgb_array': diff --git a/gym/envs/box2d/bipedal_walker.py b/gym/envs/box2d/bipedal_walker.py index 4287197eb..a08c162fb 100644 --- a/gym/envs/box2d/bipedal_walker.py +++ b/gym/envs/box2d/bipedal_walker.py @@ -384,6 +384,7 @@ class BipedalWalker(gym.Env): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return from gym.envs.classic_control import rendering diff --git a/gym/envs/box2d/lunar_lander.py b/gym/envs/box2d/lunar_lander.py index 92a1b7040..b34bc7aab 100644 --- a/gym/envs/box2d/lunar_lander.py +++ b/gym/envs/box2d/lunar_lander.py @@ -241,6 +241,7 @@ class LunarLander(gym.Env): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return from gym.envs.classic_control import rendering diff --git a/gym/envs/classic_control/acrobot.py b/gym/envs/classic_control/acrobot.py index 5c11bc007..a4fa4a1b2 100644 --- a/gym/envs/classic_control/acrobot.py +++ b/gym/envs/classic_control/acrobot.py @@ -162,6 +162,7 @@ class AcrobotEnv(core.Env): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return s = self.state diff --git a/gym/envs/classic_control/cartpole.py b/gym/envs/classic_control/cartpole.py index 3c3e5f413..ef02933ab 100644 --- a/gym/envs/classic_control/cartpole.py +++ b/gym/envs/classic_control/cartpole.py @@ -85,6 +85,7 @@ class CartPoleEnv(gym.Env): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return screen_width = 600 diff --git a/gym/envs/classic_control/mountain_car.py b/gym/envs/classic_control/mountain_car.py index a9c10c600..3509dba40 100644 --- a/gym/envs/classic_control/mountain_car.py +++ b/gym/envs/classic_control/mountain_car.py @@ -58,6 +58,7 @@ class MountainCarEnv(gym.Env): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return screen_width = 600 diff --git a/gym/envs/classic_control/pendulum.py b/gym/envs/classic_control/pendulum.py index 8ca1c1606..7d309a12e 100644 --- a/gym/envs/classic_control/pendulum.py +++ b/gym/envs/classic_control/pendulum.py @@ -52,6 +52,7 @@ class PendulumEnv(gym.Env): if close: if self.viewer is not None: self.viewer.close() + self.viewer = None return if self.viewer is None: diff --git a/gym/envs/mujoco/mujoco_env.py b/gym/envs/mujoco/mujoco_env.py index 7461abaf2..4be91c29b 100644 --- a/gym/envs/mujoco/mujoco_env.py +++ b/gym/envs/mujoco/mujoco_env.py @@ -99,6 +99,7 @@ class MujocoEnv(gym.Env): if close: if self.viewer is not None: self._get_viewer().finish() + self.viewer = None return if mode == 'rgb_array': diff --git a/gym/envs/tests/test_envs.py b/gym/envs/tests/test_envs.py index c451f24ff..d9daa0071 100644 --- a/gym/envs/tests/test_envs.py +++ b/gym/envs/tests/test_envs.py @@ -10,13 +10,9 @@ from gym import envs # This runs a smoketest on each official registered env. We may want # to try also running environments which are not officially registered # envs. -specs = [spec for spec in envs.registry.all()] +specs = [spec for spec in envs.registry.all() if spec._entry_point is not None] @tools.params(*specs) def test_env(spec): - # Skip for deprecated envs - if spec._entry_point is None: - return - # Skip mujoco tests for pull request CI skip_mujoco = os.environ.get('TRAVIS_PULL_REQUEST', 'false') != 'false' if skip_mujoco and spec._entry_point.startswith('gym.envs.mujoco:'): @@ -38,7 +34,12 @@ def test_env(spec): assert np.isscalar(reward), "{} is not a scalar for {}".format(reward, env) assert isinstance(done, bool), "Expected {} to be a boolean".format(done) - for mode in env.metadata.get('render.modes'): + for mode in env.metadata.get('render.modes', []): + env.render(mode=mode) + env.render(close=True) + + # Make sure we can render the environment after close. + for mode in env.metadata.get('render.modes', []): env.render(mode=mode) env.render(close=True) diff --git a/gym/monitoring/tests/__init__.py b/gym/monitoring/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/gym/monitoring/tests/helpers.py b/gym/monitoring/tests/helpers.py new file mode 100644 index 000000000..4c5738556 --- /dev/null +++ b/gym/monitoring/tests/helpers.py @@ -0,0 +1,9 @@ +import contextlib +import shutil +import tempfile + +@contextlib.contextmanager +def tempdir(): + temp = tempfile.mkdtemp() + yield temp + shutil.rmtree(temp) diff --git a/gym/monitoring/tests/test_monitor.py b/gym/monitoring/tests/test_monitor.py index bcf26c597..de79da01f 100644 --- a/gym/monitoring/tests/test_monitor.py +++ b/gym/monitoring/tests/test_monitor.py @@ -1,25 +1,17 @@ -import contextlib import glob import os -import shutil -import tempfile import gym from gym import error from gym.monitoring import monitor +from gym.monitoring.tests import helpers class FakeEnv(gym.Env): def _render(self, close=True): raise RuntimeError('Raising') -@contextlib.contextmanager -def tempdir(): - temp = tempfile.mkdtemp() - yield temp - shutil.rmtree(temp) - def test_monitor_filename(): - with tempdir() as temp: + with helpers.tempdir() as temp: env = gym.make('Acrobot-v0') env.monitor.start(temp) env.monitor.close() @@ -28,7 +20,7 @@ def test_monitor_filename(): assert len(manifests) == 1 def test_close_monitor(): - with tempdir() as temp: + with helpers.tempdir() as temp: env = FakeEnv() env.monitor.start(temp) env.monitor.close() @@ -37,7 +29,7 @@ def test_close_monitor(): assert len(manifests) == 1 def test_video_callable(): - with tempdir() as temp: + with helpers.tempdir() as temp: env = gym.make('Acrobot-v0') try: env.monitor.start(temp, video_callable=False) @@ -47,7 +39,7 @@ def test_video_callable(): assert False def test_env_reuse(): - with tempdir() as temp: + with helpers.tempdir() as temp: env = gym.make('CartPole-v0') env.monitor.start(temp) env.monitor.close() diff --git a/gym/monitoring/tests/test_monitor_envs.py b/gym/monitoring/tests/test_monitor_envs.py new file mode 100644 index 000000000..211355e98 --- /dev/null +++ b/gym/monitoring/tests/test_monitor_envs.py @@ -0,0 +1,26 @@ +import numpy as np +from nose2 import tools +import os + +import logging +logger = logging.getLogger(__name__) + +from gym import envs +from gym.monitoring.tests import helpers + +specs = [spec for spec in envs.registry.all() if spec._entry_point is not None] +@tools.params(*specs) +def test_renderable_after_monitor_close(spec): + with helpers.tempdir() as temp: + env = spec.make() + # Skip un-renderable envs + if 'human' not in env.metadata.get('render.modes', []): + return + + env.monitor.start(temp) + env.reset() + env.monitor.close() + + env.reset() + env.render() + env.render(close=True)