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

Dict Space Crash on to_jsonable #1841

Closed
rmwtoyon opened this issue Mar 10, 2020 · 5 comments
Closed

Dict Space Crash on to_jsonable #1841

rmwtoyon opened this issue Mar 10, 2020 · 5 comments

Comments

@rmwtoyon
Copy link

The to_jsonable function fails for Dict space types. Here is a simple example...

space = spaces.Dict({
  'test': spaces.Box(low=0, high=1, shape=(2,), dtype=np.float32)
})
sample = space.sample()
jsonable = space.to_jsonable(sample)

Here is the call stack...

  ...
  File "C:\Users\rwilkerson\AppData\Local\Programs\Python\Python38\lib\site-packages\gym\spaces\dict.py", line 71, in to_jsonable
    return {key: space.to_jsonable([sample[key] for sample in sample_n]) \
  File "C:\Users\rwilkerson\AppData\Local\Programs\Python\Python38\lib\site-packages\gym\spaces\dict.py", line 71, in <dictcomp>
    return {key: space.to_jsonable([sample[key] for sample in sample_n]) \
  File "C:\Users\rwilkerson\AppData\Local\Programs\Python\Python38\lib\site-packages\gym\spaces\dict.py", line 71, in <listcomp>
    return {key: space.to_jsonable([sample[key] for sample in sample_n]) \
TypeError: string indices must be integers

I can fix the issue by making the following change to `gym/spaces/dict.py'...

    def to_jsonable(self, sample_n):
        # serialize as dict-repr of vectors
        return {key: space.to_jsonable([sample[key] for sample in sample_n]) \
                for key, space in self.spaces.items()}

...to...

    def to_jsonable(self, sample_n):
        # serialize as dict-repr of vectors
        return {key: space.to_jsonable(sample_n[key]) for key, space in self.spaces.items()}
@rmwtoyon
Copy link
Author

I'm using version 0.15.6 by the way. Doesn't look like this has been fixed in newer releases.

@rmwtoyon
Copy link
Author

What is the process for fixing issues like this? Would OpenAI process a merge request to master with the proposed change above if I created one? Happy to help fix this. I'm using is_jsonable in a shared project and am trying to avoid having team members doing custom installs of gym.

Thanks in advance.

@rmwtoyon
Copy link
Author

rmwtoyon commented Mar 25, 2020

Pull request is here: #1849

@rmwtoyon
Copy link
Author

Addressing CI issues now.

@rmwtoyon
Copy link
Author

After looking at the failing test code for the pull request I'm realizing that the to_jsonable function is written assuming the caller wraps the input space object in a list. The crash occurs only when the object is not wrapped in a list.

I'm not sure I understand the logic in this design but my issue is mute if I follow that example. Closed the pull request and this issue.

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

No branches or pull requests

1 participant