Skip to content

Conversation

batist73
Copy link
Contributor

@batist73 batist73 commented May 18, 2019

Fix for issue #892. PR #875 changed behavior of __getitem__ method of LazyFrames class. So, numpy wasn't able to convert list of LazyFrames instances in an util method.
I offer to reverse changes from #875, and introduce interface for getting count of frames and specific frames for LazyFrames instance.
Example of usage:

import numpy as np
from baselines.common.atari_wrappers import make_atari, wrap_deepmind

env = wrap_deepmind(make_atari('PongNoFrameskip-v4'), frame_stack=True)

state = env.reset()

print(type(state))
print(state.count())
print(state.frame(0).shape)

print(np.array([state]).shape)

The result:

<class 'baselines.common.atari_wrappers.LazyFrames'>
4
(84, 84)
(1, 84, 84, 4)

@pzhokhov
Copy link
Collaborator

pzhokhov commented May 31, 2019

I am a bit sceptical about reversing #875 - it was a bug, and I think the PR was a good solution; also the snippet

import numpy as np
from baselines.common.atari_wrappers import make_atari, wrap_deepmind

env = wrap_deepmind(make_atari('PongNoFrameskip-v4'), frame_stack=True)

state = env.reset()
print(np.array([state]).shape)

works for me (prints (1, 84, 84, 4)) on master branch (if I understand correctly, it does not work for you with the error similar to #892) with numpy 1.16.4. Could you specify the numpy version you are using so that I could replicate the issue? Thanks!

@pzhokhov
Copy link
Collaborator

nvm, I can reproduce after all, looking into it

@pzhokhov
Copy link
Collaborator

@batist73 okay, on a second thought, I agree - if there are aspects of the behavior different from numpy array, (such as indexing with one number that slices the last dimension instead of the first), it is better to have separate methods for that. Merging.

@pzhokhov pzhokhov merged commit 7c52085 into openai:master May 31, 2019
@pzhokhov pzhokhov mentioned this pull request May 31, 2019
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.

2 participants