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

Add support for tuple input on MultiBinary space #2023

Merged
merged 3 commits into from Sep 25, 2020

Conversation

lsylusiyao
Copy link
Contributor

When I'm using ray-project/ray, I've found out that when I'm using multi-dimensions space as the input n of MultiBinary, problem occurs on init part. After debugging, it appeared that the dealing way of n is not suitable for multi-dimensions input.
Here's the issue in ray-project/ray: #10024 on ray.

Copy link
Collaborator

@pzhokhov pzhokhov left a comment

Choose a reason for hiding this comment

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

I don't quite understand the desired behavior of this change. Do you want the samples to be multi-dimensional boolean tensors? That does not sound unreasonable, but why then there is a check of length != 1?
Arguably, contains should also be modified to check that the shape of the sample matches the shape of space (that's not implemented now, but I think it becomes more relevant if we allow multi-dimensional multibinary spaces)
Also, please update the docstring too to reflect the change

@@ -32,7 +37,7 @@ def contains(self, x):

def to_jsonable(self, sample_n):
return np.array(sample_n).tolist()

Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove the spaces

@lsylusiyao
Copy link
Contributor Author

@pzhokhov Thank you for all your suggestions.
I've added length != 1 because I'd like to deal with input like this: self.obs = MultiBinary([3]). However, now I believe that this is not essential since the docstrings give out the usage.
Also I've finish the shape check of contains.

@pzhokhov pzhokhov merged commit 27b6816 into openai:master Sep 25, 2020
zlig pushed a commit to zlig/gym that referenced this pull request Sep 6, 2021
* Add support for tuple input on MultiBinary space

* Change input of multibinary

* Remove check of length != 1; Add shape check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants