diff --git a/supersuit/generic_wrappers/basic_wrappers.py b/supersuit/generic_wrappers/basic_wrappers.py index d92edb5c..faaffec6 100644 --- a/supersuit/generic_wrappers/basic_wrappers.py +++ b/supersuit/generic_wrappers/basic_wrappers.py @@ -49,9 +49,11 @@ def normalize_obs_v0(env, env_min=0.0, env_max=1.0): def clip_actions_v0(env): return action_lambda_v1( env, - lambda action, act_space: np.clip(action, act_space.low, act_space.high) - if action is not None - else None, + lambda action, act_space: ( + np.clip(action, act_space.low, act_space.high) + if action is not None + else None + ), lambda act_space: act_space, ) @@ -61,13 +63,22 @@ def change_act_space(act_space): assert isinstance( act_space, Box ), "scale_actions_v0 only works with a Box action space" - return Box(low=act_space.low * scale, high=act_space.high * scale) + # A negative scale maps [low, high] onto [high * scale, low * scale], so the + # two bounds swap places. Multiplying them in place left a Box whose low + # exceeded its high, and Gymnasium rejected it from inside its own + # constructor without naming this wrapper or its `scale` argument. + scaled_low = act_space.low * scale + scaled_high = act_space.high * scale + return Box( + low=np.minimum(scaled_low, scaled_high), + high=np.maximum(scaled_low, scaled_high), + ) return action_lambda_v1( env, - lambda action, act_space: np.asarray(action) * scale - if action is not None - else None, + lambda action, act_space: ( + np.asarray(action) * scale if action is not None else None + ), lambda act_space: change_act_space(act_space), ) diff --git a/test/aec_mock_test.py b/test/aec_mock_test.py index a425267a..ce1dcb64 100644 --- a/test/aec_mock_test.py +++ b/test/aec_mock_test.py @@ -444,3 +444,33 @@ def test_scale_action_wrapper(): base_act_spaces = {f"a{idx}": Discrete(5) for idx in range(2)} base_env = DummyScaleEnv(base_obs, base_obs_space, base_act_spaces) wrapped_env = scale_actions_v0(base_env, 2) + + +def test_scale_action_wrapper_negative_scale(): + """A negative scale mirrors the interval, so the bounds have to swap. + + `Box(low=low * scale, high=high * scale)` left low above high for a negative + scale, and Gymnasium rejected the space from inside its own constructor + without naming this wrapper or its `scale` argument. + """ + base_obs = {f"a{idx}": np.zeros([3], dtype=np.float32) for idx in range(2)} + base_obs_space = { + f"a{idx}": Box(low=np.float32(0.0), high=np.float32(10.0), shape=[3]) + for idx in range(2) + } + base_act_spaces = { + f"a{idx}": Box(low=np.float32(1.0), high=np.float32(5.0), shape=[3]) + for idx in range(2) + } + + base_env = DummyScaleEnv(base_obs, base_obs_space, base_act_spaces) + wrapped_env = scale_actions_v0(base_env, -2.0) + + space = wrapped_env.action_space(wrapped_env.possible_agents[0]) + assert np.allclose(space.low, -10.0) + assert np.allclose(space.high, -2.0) + + wrapped_env.reset() + wrapped_env.step(np.array([2, 1, 3], dtype=np.float32)) + scaled_action = wrapped_env.observe(wrapped_env.agents[0]) + assert (scaled_action == np.array([-4, -2, -6], dtype=np.float32)).all()