Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow Box low > high and low == inf and high == -inf #495

Merged
merged 16 commits into from
May 15, 2023
92 changes: 45 additions & 47 deletions gymnasium/spaces/box.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def __init__(
if shape is not None:
assert all(
np.issubdtype(type(dim), np.integer) for dim in shape
), f"Expect all shape elements to be an integer, actual type: {tuple(type(dim) for dim in shape)}"
), f"Expected all shape elements to be an integer, actual type: {tuple(type(dim) for dim in shape)}"
shape = tuple(int(dim) for dim in shape) # This changes any np types to int
elif isinstance(low, np.ndarray):
shape = low.shape
Expand All @@ -99,7 +99,7 @@ def __init__(
shape = (1,)
else:
raise ValueError(
f"Box shape is inferred from low and high, expect their types to be np.ndarray, an integer or a float, actual type low: {type(low)}, high: {type(high)}"
f"Box shape is inferred from low and high, expected their types to be np.ndarray, an integer or a float, actual type low: {type(low)}, high: {type(high)}"
)

# Capture the boundedness information before replacing np.inf with get_inf
Expand All @@ -109,8 +109,8 @@ def __init__(
_high = np.full(shape, high, dtype=float) if is_float_integer(high) else high
self.bounded_above: NDArray[np.bool_] = np.inf > _high

low = _broadcast(low, self.dtype, shape, inf_sign="-")
high = _broadcast(high, self.dtype, shape, inf_sign="+")
low = _broadcast(low, self.dtype, shape)
high = _broadcast(high, self.dtype, shape)

assert isinstance(low, np.ndarray)
assert (
Expand All @@ -121,6 +121,18 @@ def __init__(
high.shape == shape
), f"high.shape doesn't match provided shape, high.shape: {high.shape}, shape: {shape}"

# check that we don't have invalid low or high
if not np.any(np.isnan(low) | np.isnan(high)):
assert np.all(
low <= high
), f"Some low values are greater than high, low={low}, high={high}"
assert not np.any(
np.isinf(low) & (low > 0.0)
), f"No low value can be equal to `np.inf`, low={low}"
assert not np.any(
np.isinf(high) & (high < 0.0)
), f"No high value can be equal to `-np.inf`, high={high}"

self._shape: tuple[int, ...] = shape

low_precision = get_precision(low.dtype)
Expand Down Expand Up @@ -281,38 +293,6 @@ def __setstate__(self, state: Iterable[tuple[str, Any]] | Mapping[str, Any]):
self.high_repr = _short_repr(self.high)


def get_inf(dtype: np.dtype, sign: str) -> int | float:
"""Returns an infinite that doesn't break things.

Args:
dtype: An `np.dtype`
sign (str): must be either `"+"` or `"-"`

Returns:
Gets an infinite value with the sign and dtype

Raises:
TypeError: Unknown sign, use either '+' or '-'
ValueError: Unknown dtype for infinite bounds
"""
if np.dtype(dtype).kind == "f":
if sign == "+":
return np.inf
elif sign == "-":
return -np.inf
else:
raise TypeError(f"Unknown sign {sign}, use either '+' or '-'")
elif np.dtype(dtype).kind == "i":
if sign == "+":
return np.iinfo(dtype).max - 2
elif sign == "-":
return np.iinfo(dtype).min + 2
else:
raise TypeError(f"Unknown sign {sign}, use either '+' or '-'")
else:
raise ValueError(f"Unknown dtype {dtype} for infinite bounds")


def get_precision(dtype: np.dtype) -> SupportsFloat:
"""Get precision of a data type."""
if np.issubdtype(dtype, np.floating):
Expand All @@ -325,17 +305,35 @@ def _broadcast(
value: SupportsFloat | NDArray[Any],
dtype: np.dtype,
shape: tuple[int, ...],
inf_sign: str,
) -> NDArray[Any]:
"""Handle infinite bounds and broadcast at the same time if needed."""
"""Handle infinite bounds and broadcast at the same time if needed.

This is needed primarily because:
>>> import numpy as np
>>> np.full((2,), np.inf, dtype=np.int32)
array([-2147483648, -2147483648], dtype=int32)
"""
if is_float_integer(value):
value = get_inf(dtype, inf_sign) if np.isinf(value) else value
value = np.full(shape, value, dtype=dtype)
if np.isneginf(value) and np.dtype(dtype).kind == "i":
value = np.iinfo(dtype).min + 2
elif np.isposinf(value) and np.dtype(dtype).kind == "i":
value = np.iinfo(dtype).max - 2

return np.full(shape, value, dtype=dtype)

elif isinstance(value, np.ndarray):
# this is needed because we can't stuff np.iinfo(int).min into an array of dtype float
casted_value = value.astype(dtype)

# change bounds only if values are negative or positive infinite
if np.dtype(dtype).kind == "i":
casted_value[np.isneginf(value)] = np.iinfo(dtype).min + 2
casted_value[np.isposinf(value)] = np.iinfo(dtype).max - 2

return casted_value

else:
assert isinstance(value, np.ndarray)
if np.any(np.isinf(value)):
# create new array with dtype, but maintain old one to preserve np.inf
temp = value.astype(dtype)
temp[np.isinf(value)] = get_inf(dtype, inf_sign)
value = temp
return value
# only np.ndarray allowed beyond this point
raise TypeError(
f"Unknown dtype for `value`, expected `np.ndarray` or float/integer, got {type(value)}"
)
5 changes: 0 additions & 5 deletions gymnasium/utils/passive_env_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ def _check_box_action_space(action_space: spaces.Box):
"A Box action space maximum and minimum values are equal. "
f"Actual equal coordinates: {[x for x in zip(*np.where(action_space.low == action_space.high))]}"
)
elif np.any(action_space.high < action_space.low):
logger.warn(
"A Box action space low value is greater than a high value. "
f"Actual less than coordinates: {[x for x in zip(*np.where(action_space.high < action_space.low))]}"
)


def check_space(
Expand Down
88 changes: 56 additions & 32 deletions tests/spaces/test_box.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import re
import warnings
from collections.abc import Iterable

import numpy as np
import pytest

import gymnasium as gym
from gymnasium.spaces import Box
from gymnasium.spaces.box import get_inf


@pytest.mark.parametrize(
Expand Down Expand Up @@ -61,18 +61,18 @@ def test_low_high_values(value, valid: bool):
"""Test what `low` and `high` values are valid for `Box` space."""
if valid:
with warnings.catch_warnings(record=True) as caught_warnings:
Box(low=value, high=value)
Box(low=-np.inf, high=value)
assert len(caught_warnings) == 0, tuple(
warning.message for warning in caught_warnings
)
else:
with pytest.raises(
ValueError,
match=re.escape(
"expect their types to be np.ndarray, an integer or a float"
"expected their types to be np.ndarray, an integer or a float"
),
):
Box(low=value, high=value)
Box(low=-np.inf, high=value)


@pytest.mark.parametrize(
Expand All @@ -90,7 +90,7 @@ def test_low_high_values(value, valid: bool):
1,
{"shape": (None,)},
AssertionError,
"Expect all shape elements to be an integer, actual type: (<class 'NoneType'>,)",
"Expected all shape elements to be an integer, actual type: (<class 'NoneType'>,)",
),
(
0,
Expand All @@ -102,7 +102,7 @@ def test_low_high_values(value, valid: bool):
)
},
AssertionError,
"Expect all shape elements to be an integer, actual type: (<class 'int'>, <class 'NoneType'>)",
"Expected all shape elements to be an integer, actual type: (<class 'int'>, <class 'NoneType'>)",
),
(
0,
Expand All @@ -114,21 +114,21 @@ def test_low_high_values(value, valid: bool):
)
},
AssertionError,
"Expect all shape elements to be an integer, actual type: (<class 'numpy.int64'>, <class 'NoneType'>)",
"Expected all shape elements to be an integer, actual type: (<class 'numpy.int64'>, <class 'NoneType'>)",
),
(
None,
None,
{},
ValueError,
"Box shape is inferred from low and high, expect their types to be np.ndarray, an integer or a float, actual type low: <class 'NoneType'>, high: <class 'NoneType'>",
"Box shape is inferred from low and high, expected their types to be np.ndarray, an integer or a float, actual type low: <class 'NoneType'>, high: <class 'NoneType'>",
),
(
0,
None,
{},
ValueError,
"Box shape is inferred from low and high, expect their types to be np.ndarray, an integer or a float, actual type low: <class 'int'>, high: <class 'NoneType'>",
"Box shape is inferred from low and high, expected their types to be np.ndarray, an integer or a float, actual type low: <class 'int'>, high: <class 'NoneType'>",
),
(
np.zeros(3),
Expand Down Expand Up @@ -283,29 +283,6 @@ def test_legacy_state_pickling():
assert b.high_repr == "1.0"


def test_get_inf():
"""Tests that get inf function works as expected, primarily for coverage."""
assert get_inf(np.float32, "+") == np.inf
assert get_inf(np.float16, "-") == -np.inf
with pytest.raises(
TypeError, match=re.escape("Unknown sign *, use either '+' or '-'")
):
get_inf(np.float32, "*")

assert get_inf(np.int16, "+") == 32765
assert get_inf(np.int8, "-") == -126
with pytest.raises(
TypeError, match=re.escape("Unknown sign *, use either '+' or '-'")
):
get_inf(np.int32, "*")

with pytest.raises(
ValueError,
match=re.escape("Unknown dtype <class 'numpy.complex128'> for infinite bounds"),
):
get_inf(np.complex_, "+")


def test_sample_mask():
"""Box cannot have a mask applied."""
space = Box(0, 1)
Expand All @@ -314,3 +291,50 @@ def test_sample_mask():
match=re.escape("Box.sample cannot be provided a mask, actual value: "),
):
space.sample(mask=np.array([0, 1, 0], dtype=np.int8))


@pytest.mark.parametrize(
"low, high, reason",
[
(np.inf, np.inf, "positive_inf_below"),
(np.array([0, np.inf]), np.array([np.inf, np.inf]), "positive_inf_below"),
(-np.inf, -np.inf, "negative_inf_above"),
(np.array([-np.inf, -np.inf]), np.array([0, -np.inf]), "negative_inf_above"),
(5.0, 3.0, "reverse_bounded"),
(np.array([5.0, 6.0]), np.array([1.0, 5.99]), "reverse_bounded"),
],
)
def test_invalid_low_high(low, high, reason):
"""Tests that we don't allow spaces with degenerate bounds, such as `Box(np.inf, -np.inf)`."""

if not isinstance(low, Iterable):
print_low = np.array([low])
print_high = np.array([high])
else:
print_low = low
print_high = high

if reason == "positive_inf_below":
with pytest.raises(
AssertionError,
match=re.escape(f"No low value can be equal to `np.inf`, low={print_low}"),
):
Box(low, high, dtype=np.float32)
elif reason == "negative_inf_above":
with pytest.raises(
AssertionError,
match=re.escape(
f"No high value can be equal to `-np.inf`, high={print_high}"
),
):
Box(low, high, dtype=np.float32)
elif reason == "reverse_bounded":
with pytest.raises(
AssertionError,
match=re.escape(
f"Some low values are greater than high, low={print_low}, high={print_high}"
),
):
Box(low, high, dtype=np.float32)
else:
raise AssertionError(f"Unknown reason: {reason}.")
10 changes: 0 additions & 10 deletions tests/utils/test_passive_env_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ def _modify_space(space: spaces.Space, attribute: str, value):
spaces.Box(np.zeros(5), np.zeros(5)),
"A Box observation space maximum and minimum values are equal. Actual equal coordinates: [(0,), (1,), (2,), (3,), (4,)]",
],
[
UserWarning,
spaces.Box(np.ones(5), np.zeros(5)),
"A Box observation space low value is greater than a high value. Actual less than coordinates: [(0,), (1,), (2,), (3,), (4,)]",
],
[
AssertionError,
_modify_space(spaces.Box(np.zeros(2), np.ones(2)), "low", np.zeros(3)),
Expand Down Expand Up @@ -113,11 +108,6 @@ def test_check_observation_space(test, space, message: str):
spaces.Box(np.zeros(5), np.zeros(5)),
"A Box action space maximum and minimum values are equal. Actual equal coordinates: [(0,), (1,), (2,), (3,), (4,)]",
],
[
UserWarning,
spaces.Box(np.ones(5), np.zeros(5)),
"A Box action space low value is greater than a high value. Actual less than coordinates: [(0,), (1,), (2,), (3,), (4,)]",
],
[
AssertionError,
_modify_space(spaces.Box(np.zeros(2), np.ones(2)), "low", np.zeros(3)),
Expand Down