-
Notifications
You must be signed in to change notification settings - Fork 273
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 gym make support for Meta-World envs #498
base: master
Are you sure you want to change the base?
Add gym make support for Meta-World envs #498
Conversation
…rld into reginald-mclean-removeV1_refactor
Few things I noticed:
Also I think for simplicity it should be possible to just have a single definition of |
Good catch, remnant of previous attempt at creating ML envs.
Will update
I don't know if I agree with this. We can include the wrapper for completeness and show examples of enabling it, but I think we shouldn't influence users to use the wrapper by default.
Agreed
It would be possible but I think it might become a bit of a convoluted function to write/maintain. Unless there's a clean way of merging them, I think keeping them separate makes the most sense for maintenance reasons. |
From experience, having tonnes of registered environment can make life easier but it can mean many more environments. For future proofing, an alternative approach is We can keep, Similarly for Is there any reason for 1 and 10 only, could we make it I'm purely spitballing ideas, you don't need to take any of them |
fd0201e
to
50f9a21
Compare
@pseudo-rnd-thoughts
Both of the above commands gives the user control over the environments that they want to use in a multi-task or meta-RL setting, instead of the predefined ones. I agree, it is a LOT of environments to add, but there are also lots of different use cases of MW environments. Some of them are single environments (ie 'Meta-World/reach-v3'), some of them are the smaller MT/ML environments (ie 'Meta-World/ML-train-reach-v3'), and some of them are the pre-defined environment sets (MT10, MT50, ML10, ML45). |
@reginald-mclean In my opinion, I would work to keep the number of environments to a minimal. This reduces the mess you need to maintain and provides more opinions to the users on what they do. Environment parameters are your friend here, minimising what you need to maintain while adding flexibility to users I say this as I remove over 800 environment from Atari, currently there are over 1000 environments registered for only 100 games. For ALE, there are 14 environments registered for each game which in my opinion is crazy and very few people actually use the extra / special registered environments which can be accessed through parameters. |
@pseudo-rnd-thoughts ok I have thought about it and I definitely agree with your comment about having minimal environments. I've moved the environments into their base environments (ie MT1, MT10, MT50), and allowed for the various different features via arguments. I've had a few extra commits slip into this PR, just trying to figure out how to remove them |
506576f
to
d330e05
Compare
This PR is built on top of #499 to add gym.make support for the environments in Meta-World. They are organized as follows:
And there are also gym.make_vec commands that return multiple environments wrapped in a sync or async wrapper: