-
-
Notifications
You must be signed in to change notification settings - Fork 827
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
Fix VectorizeActionTransform
for changing spaces
#1170
Merged
pseudo-rnd-thoughts
merged 7 commits into
Farama-Foundation:main
from
pseudo-rnd-thoughts:issue-1169
Sep 20, 2024
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
06fcd42
Fix `VectorizeActionTransform` for changing spaces
pseudo-rnd-thoughts cdaa861
Remove `assert action in self.action_space`
pseudo-rnd-thoughts 31ad6e0
Update docstrings
pseudo-rnd-thoughts 35201ee
Skip the jax functional environments
pseudo-rnd-thoughts ceada90
Add `allow_module_level` to `pytest.skip`
pseudo-rnd-thoughts 4bc80cf
pre-commit
pseudo-rnd-thoughts 148c212
Update `iterate` type hint
pseudo-rnd-thoughts File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
from functools import partial | ||
|
||
import numpy as np | ||
|
||
import gymnasium as gym | ||
from gymnasium.vector import SyncVectorEnv | ||
from tests.testing_env import GenericTestEnv | ||
|
||
|
||
def test_vectorize_box_to_dict_action(): | ||
def func(x): | ||
return x["key"] | ||
|
||
envs = SyncVectorEnv([lambda: GenericTestEnv() for _ in range(2)]) | ||
envs = gym.wrappers.vector.VectorizeTransformAction( | ||
env=envs, | ||
wrapper=gym.wrappers.TransformAction, | ||
func=func, | ||
action_space=gym.spaces.Dict( | ||
{"key": gym.spaces.Box(low=0.0, high=1.0, shape=(1,), dtype=np.float32)} | ||
), | ||
) | ||
|
||
obs, _ = envs.reset() | ||
obs, _, _, _, _ = envs.step(envs.action_space.sample()) | ||
envs.close() | ||
|
||
|
||
def test_vectorize_dict_to_box_obs(): | ||
wrappers = [ | ||
partial( | ||
gym.wrappers.TransformObservation, | ||
func=lambda x: {"key1": x[0:1], "key2": x[1:]}, | ||
observation_space=gym.spaces.Dict( | ||
{ | ||
"key1": gym.spaces.Box(low=-np.inf, high=np.inf, shape=(1,)), | ||
"key2": gym.spaces.Box(low=-np.inf, high=np.inf, shape=(3,)), | ||
} | ||
), | ||
) | ||
] | ||
envs = gym.make_vec( | ||
"CartPole-v1", | ||
num_envs=2, | ||
vectorization_mode=gym.VectorizeMode.ASYNC, | ||
wrappers=wrappers, | ||
) | ||
obs, _ = envs.reset() | ||
assert obs in envs.observation_space | ||
obs, _, _, _, _ = envs.step(envs.action_space.sample()) | ||
assert obs in envs.observation_space | ||
envs.close() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: why not
self.single_action_space
?Docstring of
iterate
: "space: Observation space of a single environment in the vectorized environment."Same docstring in
concatenate
which also takes as input asingle_action_space
. (also both docstrings are completely identical, the one for iterate is wrong).This seems to suggest that
iterate
takes as argument the action space of a single action, not a batched one. Does it even matter since there is no out arg?asking because I use it in my code, just want to make sure :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had just looked at the example docstring which shows a batched space rather than a single space so I'm using the batched space version.
I think the docstring is just incorrect, will update and check the rest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like someone, possibly me, copied the concatenate docstring for some reason, fixing now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick answers and fixes :)
I think in the end it does not matter whether the first arg is the batched space or not right? Both batched and unbatched versions of a same structured space should be identical up until the base spaces, which are iterated over just with
iter
regardless of which space is given as argument. The only difference it makes is with respect to typing, and that is only if someone uses literal shape annotations in the base spaces.One more little comment before I stop bothering you:
The signature of
iterate
is:def iterate(space: Space[T_cov], items: Iterable[T_cov])
but
items
is supposed to be a single batched sample fromspace
->def iterate(space: Space[T_cov], items: T_cov)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the second point, your correct, I'll update