Skip to content

Commit

Permalink
Reduces warnings produced by pytest from ~1500 to 13 (#2660)
Browse files Browse the repository at this point in the history
* Updated cartpole-v0 to v1 to prevent warning and added pytest.mark.filterwarnings for tests where warnings are unavoidable

* Change np.bool to bool as numpy raises a warning and bool is the suggested solution

* Seeding randint is deprecated in the future, integers is new solution

* Fixed errors thrown when the video recorder is deleted but not closed

* spaces.Box expects a floating array, updated all cases where this was not true and modified float32 to float64 as float array default to float64. Otherwise space.Box raises warning that dtype precision (float32) is lower than array precision (float64).

* Added pytest.mark.filterwarnings to preventing the raising of an intended warning

* Added comment to explain why a warning is raised that can't be prevented without version update to the environment

* Added comment to explain why warning is raised

* Changed values to float as expected by the box which default to float64

* Removed --forked from pytest as the pytest-forked project is no being maintained and was not raising warnings as expected

* When AsyncVectorEnv has shared_memory=True then a ValueError is raised before _state is initialised. Therefore, on the destruction on the env an error is thrown in .close_extra as _state does not exist

* Possible fix that was causing an error in test_call_async_vector_env by ensuring that pygame resources are released

* Pygame throws an error with ALSA when closed, using a fix from PettingZoo (https://github.com/Farama-Foundation/PettingZoo/blob/master/pettingzoo/__init__.py). We use the dsp audiodriver to prevent this issue

* Modification due to running pre-commit locally

* Updated cartpole-v0 to v1 to prevent warning and added pytest.mark.filterwarnings for tests where warnings are unavoidable

* Change np.bool to bool as numpy raises a warning and bool is the suggested solution

* Seeding randint is deprecated in the future, integers is new solution

* Fixed errors thrown when the video recorder is deleted but not closed

* spaces.Box expects a floating array, updated all cases where this was not true and modified float32 to float64 as float array default to float64. Otherwise space.Box raises warning that dtype precision (float32) is lower than array precision (float64).

* Added pytest.mark.filterwarnings to preventing the raising of an intended warning

* Added comment to explain why a warning is raised that can't be prevented without version update to the environment

* Added comment to explain why warning is raised

* Changed values to float as expected by the box which default to float64

* Removed --forked from pytest as the pytest-forked project is no being maintained and was not raising warnings as expected

* When AsyncVectorEnv has shared_memory=True then a ValueError is raised before _state is initialised. Therefore, on the destruction on the env an error is thrown in .close_extra as _state does not exist

* Possible fix that was causing an error in test_call_async_vector_env by ensuring that pygame resources are released

* Pygame throws an error with ALSA when closed, using a fix from PettingZoo (https://github.com/Farama-Foundation/PettingZoo/blob/master/pettingzoo/__init__.py). We use the dsp audiodriver to prevent this issue

* Modification due to running pre-commit locally
  • Loading branch information
pseudo-rnd-thoughts authored Mar 14, 2022
1 parent b1b0a12 commit 850247f
Show file tree
Hide file tree
Showing 18 changed files with 114 additions and 46 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ jobs:
--build-arg PYTHON_VERSION=${{ matrix.python-version }} \
--tag gym-docker .
- name: Run tests
run: docker run gym-docker pytest --forked --import-mode=append
run: docker run gym-docker pytest --import-mode=append
11 changes: 9 additions & 2 deletions gym/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,22 @@
from gym import vector
from gym import wrappers
import os

import sys

__all__ = ["Env", "Space", "Wrapper", "make", "spec", "register"]

# Initializing pygame initializes audio connections through SDL. SDL uses alsa by default on all Linux systems
# SDL connecting to alsa frequently create these giant lists of warnings every time you import an environment using
# pygame
# DSP is far more benign (and should probably be the default in SDL anyways)

if sys.platform.startswith("linux"):
os.environ["SDL_AUDIODRIVER"] = "dsp"

os.environ["PYGAME_HIDE_SUPPORT_PROMPT"] = "hide"

try:
import gym_notices.notices as notices
import sys

# print version warning if necessary
notice = notices.notices.get(__version__)
Expand Down
4 changes: 3 additions & 1 deletion gym/envs/box2d/car_racing.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ def __init__(self, verbose=1, lap_complete_percent=0.95):
shape=polygonShape(vertices=[(0, 0), (1, 0), (1, -1), (0, -1)])
)

# This will throw a warning in tests/envs/test_envs in utils/env_checker.py as the space is not symmetric
# or normalised however this is not possible here so ignore
self.action_space = spaces.Box(
np.array([-1, 0, 0]).astype(np.float32),
np.array([+1, +1, +1]).astype(np.float32),
Expand Down Expand Up @@ -604,9 +606,9 @@ def _create_image_array(self, screen, size):
)

def close(self):
pygame.quit()
if self.screen is not None:
pygame.display.quit()
pygame.quit()
self.isopen = False


Expand Down
5 changes: 4 additions & 1 deletion gym/envs/classic_control/pendulum.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ def __init__(self, g=10.0):
self.screen_dim = 500

high = np.array([1.0, 1.0, self.max_speed], dtype=np.float32)
# This will throw a warning in tests/envs/test_envs in utils/env_checker.py as the space is not symmetric
# or normalised as max_torque == 2 by default. Ignoring the issue here as the default settings are too old
# to update to follow the openai gym api
self.action_space = spaces.Box(
low=-self.max_torque, high=self.max_torque, shape=(1,), dtype=np.float32
)
Expand Down Expand Up @@ -187,7 +190,7 @@ def render(self, mode="human"):
scale_img = pygame.transform.smoothscale(
img, (scale * np.abs(self.last_u) / 2, scale * np.abs(self.last_u) / 2)
)
is_flip = self.last_u > 0
is_flip = bool(self.last_u > 0)
scale_img = pygame.transform.flip(scale_img, is_flip, True)
self.surf.blit(
scale_img,
Expand Down
2 changes: 1 addition & 1 deletion gym/envs/toy_text/cliffwalking.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def __init__(self):
self.nA = 4

# Cliff Location
self._cliff = np.zeros(self.shape, dtype=np.bool)
self._cliff = np.zeros(self.shape, dtype=bool)
self._cliff[3, 1:-1] = True

# Calculate transition probabilities and rewards
Expand Down
2 changes: 1 addition & 1 deletion gym/spaces/dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def __init__(
self,
spaces: dict[str, Space] | None = None,
seed: dict | int | None = None,
**spaces_kwargs: Space
**spaces_kwargs: Space,
):
assert (spaces is None) or (
not spaces_kwargs
Expand Down
2 changes: 1 addition & 1 deletion gym/spaces/discrete.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def __init__(self, n: int, seed: Optional[int] = None, start: int = 0):
super().__init__((), np.int64, seed)

def sample(self) -> int:
return self.start + self.np_random.randint(self.n)
return int(self.start + self.np_random.integers(self.n))

def contains(self, x) -> bool:
if isinstance(x, int):
Expand Down
2 changes: 1 addition & 1 deletion gym/vector/async_vector_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ def _raise_if_errors(self, successes):
raise exctype(value)

def __del__(self):
if not getattr(self, "closed", True):
if not getattr(self, "closed", True) and hasattr(self, "_state"):
self.close(terminate=True)


Expand Down
6 changes: 6 additions & 0 deletions gym/wrappers/record_video.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,9 @@ def close_video_recorder(self) -> None:
self.video_recorder.close()
self.recording = False
self.recorded_frames = 1

def close(self):
self.close_video_recorder()

def __del__(self):
self.close_video_recorder()
5 changes: 4 additions & 1 deletion tests/envs/test_envs.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
# This runs a smoketest on each official registered env. We may want
# to try also running environments which are not officially registered
# envs.
@pytest.mark.filterwarnings(
"ignore:.*We recommend you to use a symmetric and normalized Box action space.*"
)
@pytest.mark.parametrize("spec", spec_list)
def test_env(spec):
# Capture warnings
Expand Down Expand Up @@ -74,7 +77,7 @@ def test_reset_info(spec):

# Run a longer rollout on some environments
def test_random_rollout():
for env in [envs.make("CartPole-v0"), envs.make("FrozenLake-v1")]:
for env in [envs.make("CartPole-v1"), envs.make("FrozenLake-v1")]:
agent = lambda ob: env.action_space.sample()
ob = env.reset()
for _ in range(10):
Expand Down
12 changes: 8 additions & 4 deletions tests/envs/test_registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ def register_some_envs():


def test_make():
env = envs.make("CartPole-v0")
assert env.spec.id == "CartPole-v0"
env = envs.make("CartPole-v1")
assert env.spec.id == "CartPole-v1"
assert isinstance(env.unwrapped, cartpole.CartPoleEnv)


Expand Down Expand Up @@ -164,6 +164,10 @@ def test_make_with_kwargs():
assert env.arg3 == "override_arg3"


@pytest.mark.filterwarnings(
"ignore:.*The environment Humanoid-v0 is out of date. You should consider upgrading to "
"version `v3` with the environment ID `Humanoid-v3`.*"
)
def test_make_deprecated():
try:
envs.make("Humanoid-v0")
Expand All @@ -174,8 +178,8 @@ def test_make_deprecated():


def test_spec():
spec = envs.spec("CartPole-v0")
assert spec.id == "CartPole-v0"
spec = envs.spec("CartPole-v1")
assert spec.id == "CartPole-v1"


def test_spec_with_kwargs():
Expand Down
74 changes: 57 additions & 17 deletions tests/spaces/test_spaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Expand All @@ -30,7 +34,9 @@
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
}
),
Expand Down Expand Up @@ -61,13 +67,17 @@ def test_roundtripping(space):
[
Discrete(3),
Discrete(5, start=-2),
Box(low=np.array([-10, 0]), high=np.array([10, 10]), dtype=np.float32),
Box(low=np.array([-10.0, 0.0]), high=np.array([10.0, 10.0]), dtype=np.float64),
Box(low=-np.inf, high=np.inf, shape=(1, 3)),
Tuple([Discrete(5), Discrete(10)]),
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Expand All @@ -78,7 +88,9 @@ def test_roundtripping(space):
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
}
),
Expand All @@ -98,8 +110,14 @@ def test_equality(space):
(MultiDiscrete([2, 2, 100]), MultiDiscrete([2, 2, 8])),
(MultiBinary(8), MultiBinary(7)),
(
Box(low=np.array([-10, 0]), high=np.array([10, 10]), dtype=np.float32),
Box(low=np.array([-10, 0]), high=np.array([10, 9]), dtype=np.float32),
Box(
low=np.array([-10.0, 0.0]),
high=np.array([10.0, 10.0]),
dtype=np.float64,
),
Box(
low=np.array([-10.0, 0.0]), high=np.array([10.0, 9.0]), dtype=np.float64
),
),
(
Box(low=-np.inf, high=0.0, shape=(2, 1)),
Expand Down Expand Up @@ -156,7 +174,11 @@ def test_sample(space):
[
(Discrete(5), MultiBinary(5)),
(
Box(low=np.array([-10, 0]), high=np.array([10, 10]), dtype=np.float32),
Box(
low=np.array([-10.0, 0.0]),
high=np.array([10.0, 10.0]),
dtype=np.float64,
),
MultiDiscrete([2, 2, 8]),
),
(
Expand Down Expand Up @@ -247,7 +269,7 @@ def test_box_dtype_check():
space = Box(0, 2, tuple(), dtype=np.float32)

# casting will match the correct type
assert space.contains(0.5)
assert space.contains(np.array(0.5, dtype=np.float32))

# float64 is not in float32 space
assert not space.contains(np.array(0.5))
Expand All @@ -264,7 +286,11 @@ def test_box_dtype_check():
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Expand All @@ -274,7 +300,9 @@ def test_box_dtype_check():
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
}
),
Expand Down Expand Up @@ -317,7 +345,11 @@ def sample_equal(sample1, sample2):
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Expand All @@ -327,7 +359,9 @@ def sample_equal(sample1, sample2):
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
}
),
Expand All @@ -353,15 +387,21 @@ def test_seed_reproducibility(space):
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Dict(
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]),
high=np.array([1.0, 5.0]),
dtype=np.float64,
),
}
),
Expand Down Expand Up @@ -580,7 +620,7 @@ def test_discrete_legacy_state_pickling():
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(low=np.array([0.0, 0.0]), high=np.array([1, 5]), dtype=np.float64),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Expand All @@ -591,7 +631,7 @@ def test_discrete_legacy_state_pickling():
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]), high=np.array([1, 5]), dtype=np.float64
),
}
),
Expand Down
4 changes: 2 additions & 2 deletions tests/spaces/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
Tuple(
[
Discrete(5),
Box(low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32),
Box(low=np.array([0.0, 0.0]), high=np.array([1.0, 5.0]), dtype=np.float64),
]
),
Tuple((Discrete(5), Discrete(2), Discrete(2))),
Expand All @@ -24,7 +24,7 @@
{
"position": Discrete(5),
"velocity": Box(
low=np.array([0, 0]), high=np.array([1, 5]), dtype=np.float32
low=np.array([0.0, 0.0]), high=np.array([1.0, 5.0]), dtype=np.float64
),
}
),
Expand Down
Loading

0 comments on commit 850247f

Please sign in to comment.