Skip to content

Commit 3446f28

Browse files
hobostayclaude
andauthored
fix(core): avoid KeyError when batch_size is given without num_envs (#423)
## Problem `envpool.make(...)` raises a `KeyError: 'num_envs'` whenever `batch_size` is supplied without an explicit `num_envs`. In `EnvRegistry._make_env_spec` (`envpool/registration.py`), the `batch_size` validation indexed `kwargs["num_envs"]` directly: ```python if "batch_size" in kwargs: assert 0 <= kwargs["batch_size"] <= kwargs["num_envs"] # KeyError ``` …while the two seed-normalization blocks immediately above already use the safe form `kwargs.get("num_envs", 1)`. `num_envs` is never injected into `kwargs` — it only appears there if the user passes it — so the bare subscript crashes before `gen_config` is ever reached. This is reachable through the documented public API: - `batch_size` is a first-class `make()` argument ([README](../blob/main/README.md) / `docs/content/python_interface.rst`). - `batch_size=0` is a **valid** value meaning "default to `num_envs`" (`core/env_spec.h` turns `batch_size == 0` into `num_envs`). ## Reproduction ```python import envpool envpool.make("Pong-v5", env_type="gymnasium", batch_size=0) # KeyError: 'num_envs' <- before this PR ``` ## Fix Use the same default as the neighboring validation (one-line change, matches the existing `kwargs.get("num_envs", 1)` pattern): ```python if "batch_size" in kwargs: assert 0 <= kwargs["batch_size"] <= kwargs.get("num_envs", 1) ``` Behavior after the fix: | call | before | after | | --- | --- | --- | | `batch_size=0` (no `num_envs`) | `KeyError` ✗ | **OK** (defaults to `num_envs`) | | `batch_size=1` (no `num_envs`) | `KeyError` ✗ | **OK** | | `batch_size=2` (no `num_envs`, out of range) | `KeyError` ✗ | `AssertionError` (correctly rejected) | | `num_envs=64, batch_size=16` (README usage) | OK | OK (unchanged) | The documented multi-env usage is unaffected; only the spurious `KeyError` is removed and out-of-range values are now rejected with a proper `AssertionError`. ## Test Added `test_make_batch_size_without_num_envs` to `envpool/make_test.py`, which fails (`KeyError`) before the fix and passes after. ## Validation ``` make ruff py-format # pass make mypy # changed files clean (under py311 target) ``` Co-authored-by: hobostay <hobostay@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8849255 commit 3446f28

2 files changed

Lines changed: 13 additions & 1 deletion

File tree

envpool/make_test.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,18 @@ def test_make_atari(self) -> None:
282282
env_gym.close()
283283
env_gymnasium.close()
284284

285+
def test_make_batch_size_without_num_envs(self) -> None:
286+
# Regression: passing batch_size without num_envs used to raise
287+
# KeyError on the bare kwargs["num_envs"] subscript in
288+
# EnvRegistry._make_env_spec. batch_size=0 means "default to
289+
# num_envs" (see core/env_spec.h) and must be accepted; an
290+
# out-of-range batch_size must raise AssertionError, not KeyError.
291+
env = envpool.make_gymnasium("CartPole-v1", batch_size=0)
292+
env.close()
293+
self.assertRaises(
294+
AssertionError, envpool.make_gymnasium, "CartPole-v1", batch_size=2
295+
)
296+
285297
def test_make_vizdoom(self) -> None:
286298
try:
287299
with _temporary_workdir(prefix="envpool-vizdoom-smoke-"):

envpool/registration.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def _make_env_spec(
235235
if "num_envs" in kwargs:
236236
assert kwargs["num_envs"] >= 1
237237
if "batch_size" in kwargs:
238-
assert 0 <= kwargs["batch_size"] <= kwargs["num_envs"]
238+
assert 0 <= kwargs["batch_size"] <= kwargs.get("num_envs", 1)
239239
if "max_num_players" in kwargs:
240240
assert 1 <= kwargs["max_num_players"]
241241

0 commit comments

Comments
 (0)