[Enhancement] Remove / refactor code duplicated from openai gym

[lebrice]

Question / Enhancement

Are there any plans to eventually remove/refactor the portions of this repo that appear to be duplicated from gym?

I've just started using this repo, and so far I've observed a bit of overlap, for instance:

• VecEnv --> Subclass of gym.vector.VectorEnv with the added get_attr, set_attr, env_method etc. methods.
• DummyVecEnv --> gym.vector.SyncVectorEnv (same as above)
• SubprocVecEnv --> gym.vector.AsyncVectorEnv (same as above, with custom worker passed to the constructor)
• common.monitor.Monitor --> gym.wrappers.Monitor?
• common.atari_wrappers --> gym.wrappers.atari_preprocessing?

There are probably some differences I'm not aware of between your versions and those of gym, or it might even be the case that some of the gym modules/components mentioned above were introduced to gym as a consequence of this repo, I don't know.
In any case, I believe it could help keep things tidy moving forward to replace these with their equivalents or refactor them into subclasses derived from their gym equivalents.

Additionally, in my very biased opinion (see below), this would also allow users to reuse their existing tools/infrastructure code built on top of these gym components. For example, (and not to toot my own horn here), but I opened these two PR's on the gym repo, which I believe your repo could potentially benefit from:

• this PR to add a new BatchedVectorEnv to the gym.vector package, which allows for a much greater number of parallel environments compared to AsyncVectorEnv (SubprocVecEnv in this repo) by using chunking to reduce the overhead from multiprocessing
• this other PR to make it easy to add support for custom spaces to the built-in 'space' utility functions from gym.spaces.utils and gym.vector.utils (flatdims, flatten_space, create_shared_memory, read_from_shared_memory, etc etc,), which means that you'd essentially get "native" support for any new user-defined Space subclasses, even in vectorized environments, if you used these built-in functions in your Vectorized environments and in other parts of you repo. (I'm not 100% sure, but this could also potentially help address issue [Feature Request] basic dict/tuple support for observations #216 ).

[araffin]

Hello,

Thanks for the suggestions.
Regarding switching to gym implementations, my answer is already there ;)
#1 (comment)

This is mostly about VecEnv but the comment is valid for other parts of gym:
those objects are critical to SB3 and Gym is not reliable enough for us to use them (not the same standard of documentation and testing for instance).

common.atari_wrappers --> gym.wrappers.atari_preprocessing?

I did that in the beginning... and then couldn't reproduce SB2 results, so I switched back to SB wrappers here:

#28 (comment)

this PR to add a new BatchedVectorEnv

Looks interesting. Do you have performance benchmark somewhere? (as an example, DummyVecEnv vs Subproc benchmark here)
If there is a good gain of speed, we would be happy to have the PR ported to SB3 ;)

this other PR to make it easy to add support for custom spaces to the b

I have mixed feeling about that. Mostly because spaces described by Gym are not supposed to change.
Also, I find it easier to look at only one function to compare outputs especially when it is recurrent by design (e.g. the flatten() helper) rather than looking at many different functions.

could also potentially help address issue #216 ).

The main problem from issue #216 mostly comes from storage and feeding the right input to the right network.