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

Tuple __eq__ fails when spaces aren't of the same type #2140

Closed
lebrice opened this issue Dec 28, 2020 · 3 comments · Fixed by #2491
Closed

Tuple __eq__ fails when spaces aren't of the same type #2140

lebrice opened this issue Dec 28, 2020 · 3 comments · Fixed by #2491

Comments

@lebrice
Copy link

lebrice commented Dec 28, 2020

When creating a gym.spaces.Tuple, if a list of spaces is passed as the spaces argument, I believe it should be converted to a tuple, as otherwise the space could be modified, which doesn't seem like an intended behaviour, and comparisons with other equivalent spaces.Tuple objects, but created with tuple arguments, will fail:

inner_spaces = [
        Box(0, 1, (2,2)),
        Discrete(2),
        Box(0, 1, (2,2)),
]
space_1 = spaces.Tuple(list(inner_spaces))
space_2 = spaces.Tuple(tuple(inner_spaces))
assert space_1 == space_2 # This fails
assert space_2 == space_1 # This also

The way I see it, this can be easily fixed in one of two ways:

  1. Convert the spaces argument to a tuple in the constructor:

    def __init__(self, spaces):
        self.spaces = tuple(spaces)
        for space in spaces:
            assert isinstance(space, Space), "Elements of the tuple must be instances of gym.Space"
        super(Tuple, self).__init__(None, None)

    However this might change the behaviour of some programs which depend on being able to modify Tuple spaces.

  2. Convert the spaces argument to a tuple just when comparing:

    def __eq__(self, other):
        return isinstance(other, Tuple) and tuple(self.spaces) == tuple(other.spaces)

    This won't copy anything if self.spaces is already a tuple, however for lists its a small copy.

Let me know what you think!

@lebrice lebrice changed the title Tuple constructur doesn't convert spaces arg to tuple, causing __eq__ to fail Tuple constructor doesn't convert spaces arg to tuple, causing __eq__ to fail Dec 28, 2020
@lebrice lebrice changed the title Tuple constructor doesn't convert spaces arg to tuple, causing __eq__ to fail Tuple __eq__ fails when spaces aren't of the same type Dec 28, 2020
lebrice added a commit to lebrice/gym that referenced this issue Dec 28, 2020
Fixes openai#2140

Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com>
@Yuchen0604
Copy link

Hi, I also use spaces.Tuple as my observation space, which algorithm do you use? I want to use dqn, and it seems that dqn requires env.ovservation_space.shape, but Tuple doesn't have the shape method. I was wondering, how do you combine Tuple with your algorithm? thanks in advance!

@jkterry1
Copy link
Collaborator

@lebrice should this issue still be open?

@lebrice
Copy link
Author

lebrice commented Aug 26, 2021

Hey @jkterry1 I'll check if it's still reproducible and I'll get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants