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

Adding multi-discrete pretraining #407

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

MadcowD
Copy link

@MadcowD MadcowD commented Jul 16, 2019

Adds multi-discrete action space support to pretraining.

@araffin
Copy link
Collaborator

araffin commented Jul 16, 2019

Hello, thanks for the PR, please do not forget to read the contributing guide first ;)

@MadcowD
Copy link
Author

MadcowD commented Jul 16, 2019

Okay, sorry! I'll go check it out. Sorry in the middle of finishing a late NeurIPS review :S

@MadcowD
Copy link
Author

MadcowD commented Jul 16, 2019

I think I've followed the steps (ran the tests, etc) I can't seem to find a change-log. Is there anything else I am missing?

@araffin
Copy link
Collaborator

araffin commented Jul 16, 2019

The changelog is in the doc and you should add a test that check that new feature.

@araffin
Copy link
Collaborator

araffin commented Jul 16, 2019

And next time, please open an issue first, so we can discuss the feature.

@MadcowD
Copy link
Author

MadcowD commented Jul 16, 2019

Okay got it :) Sorry again to bother!

@MadcowD
Copy link
Author

MadcowD commented Jul 16, 2019

I can think of any multi-discret environments packaged with OpenAI gym. I'm currently using minerl's (https://github.com/minerllabs/minerl) environments to test, but obviously that wouldn't be a great dependency to add to the test suite unless we went through and added baselines for all of those environments (A fun task I would certainly take on, later). There is currently not a pretrain test in the test suite (it's just a part of some of the existing tests).

We could potentially add the minerl expert data to the test suite and go from there. Otherwise I'm not sure how I can provide a good test case for this PR.

@araffin
Copy link
Collaborator

araffin commented Jul 16, 2019

I think a variation of the identity env (in the common folder) should make the trick.

@DanielPBak
Copy link

This PR should also include a change to generate_expert_traj(), right?

@araffin
Copy link
Collaborator

araffin commented Oct 5, 2019

This PR should also include a change to generate_expert_traj(), right?

I would say yes.

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.

3 participants