Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion envpool/python/envpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,14 @@ def close(self: EnvPool) -> None:
import cv2

cv2.destroyWindow(self._render_window_name)
except Exception:
except ImportError:
pass
except Exception:
warnings.warn(
"Failed to close render window",
RuntimeWarning,
stacklevel=2,
)
self._render_window_open = False
close = getattr(super(), "close", None)
if close is not None:
Expand Down
2 changes: 1 addition & 1 deletion envpool/python/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def check_key_duplication(cls: Any, keytype: str, keys: list[str]) -> None:
ukeys, counts = np.unique(keys, return_counts=True)
if not np.all(counts == 1):
dup_keys = ukeys[counts > 1]
raise SystemError(
raise RuntimeError(
f"{cls} c++ code error. {keytype} keys {list(dup_keys)} are duplicated. "
f"Please report to the author of {cls}."
)
63 changes: 40 additions & 23 deletions envpool/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ def register(
if "base_path" not in kwargs:
kwargs["base_path"] = base_path
for alias in (task_id, *aliases):
assert alias not in self.specs
if alias in self.specs:
raise ValueError(
f"Duplicate registration: '{alias}' is already registered."
)
self.specs[alias] = (import_path, spec_cls, dict(kwargs))
self.envpools[alias] = {
"dm": (import_path, dm_cls),
Expand Down Expand Up @@ -216,10 +219,11 @@ def _make_env_spec(
# check arguments
if "seed" in kwargs: # Issue 214
if self._is_env_seed_sequence(kwargs["seed"]):
assert "env_seed" not in kwargs, (
"Pass either `seed` as an int or seed list, or "
"`env_seed`, but not both."
)
if "env_seed" in kwargs:
raise ValueError(
"Pass either `seed` as an int or seed list, or "
"`env_seed`, but not both."
)
kwargs["env_seed"] = self._normalize_env_seed(
kwargs["seed"],
kwargs.get("num_envs", 1),
Expand All @@ -233,11 +237,17 @@ def _make_env_spec(
kwargs.get("num_envs", 1),
)
if "num_envs" in kwargs:
assert kwargs["num_envs"] >= 1
if kwargs["num_envs"] < 1:
raise ValueError("num_envs must be >= 1.")
if "batch_size" in kwargs:
assert 0 <= kwargs["batch_size"] <= kwargs["num_envs"]
if not (0 <= kwargs["batch_size"] <= kwargs["num_envs"]):

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve default num_envs when validating batch_size

When a caller supplies batch_size but relies on the default num_envs=1, this replacement now always evaluates kwargs["num_envs"] and raises KeyError before gen_config can apply the common default. This is a regression for the optimized-mode path this change is meant to fix: before this commit, python -O stripped the assert and allowed valid calls such as make_spec(task_id, batch_size=1) to use the default num_envs; use kwargs.get("num_envs", 1) (and the same value in the message) for this validation.

Useful? React with 👍 / 👎.

raise ValueError(
f"batch_size must be in [0, num_envs({kwargs['num_envs']}), "
f"got {kwargs['batch_size']}."
)
if "max_num_players" in kwargs:
assert 1 <= kwargs["max_num_players"]
if kwargs["max_num_players"] < 1:
raise ValueError("max_num_players must be >= 1.")

spec_cls = getattr(importlib.import_module(import_path), spec_cls)
config = spec_cls.gen_config(**kwargs)
Expand Down Expand Up @@ -274,10 +284,14 @@ def make(
"after resets."
)

assert task_id in self.specs, (
f"{task_id} is not supported, `envpool.list_all_envs()` may help."
)
assert env_type in ["dm", "gymnasium"]
if task_id not in self.specs:
raise ValueError(
f"{task_id} is not supported, `envpool.list_all_envs()` may help."
)
if env_type not in ["dm", "gymnasium"]:
raise ValueError(
f"env_type must be 'dm' or 'gymnasium', got '{env_type}'."
)

spec = self._make_env_spec(
task_id,
Expand Down Expand Up @@ -312,9 +326,10 @@ def make_spec(self, task_id: str, **make_kwargs: Any) -> EnvSpec:
@staticmethod
def _assert_int32_seed(seed: Any) -> None:
INT_MAX = 2**31
assert -INT_MAX <= seed < INT_MAX, (
f"Seed should be in range of int32, got {seed}"
)
if not (-INT_MAX <= seed < INT_MAX):
raise ValueError(
f"Seed should be in range of int32, got {seed}"
)

@staticmethod
def _is_env_seed_sequence(seed: Any) -> bool:
Expand All @@ -324,17 +339,19 @@ def _is_env_seed_sequence(seed: Any) -> bool:

def _normalize_env_seed(self, seed: Any, num_envs: int) -> list[int]:
if isinstance(seed, np.ndarray):
assert seed.ndim == 1, (
"`seed` as an array must be 1-dimensional, "
f"got shape {seed.shape}"
)
if seed.ndim != 1:
raise ValueError(
"`seed` as an array must be 1-dimensional, "
f"got shape {seed.shape}"
)
seed = seed.tolist()
else:
seed = list(seed)
assert len(seed) == num_envs, (
"When `seed` is a sequence, its length must match `num_envs`, "
f"got len(seed) = {len(seed)} and num_envs = {num_envs}"
)
if len(seed) != num_envs:
raise ValueError(
"When `seed` is a sequence, its length must match `num_envs`, "
f"got len(seed) = {len(seed)} and num_envs = {num_envs}"
)
normalized_seed = [int(s) for s in seed]
for s in normalized_seed:
self._assert_int32_seed(s)
Expand Down
Loading