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

AgileRL curriculum learning and self-play tutorial #1124

Merged
merged 29 commits into from
Nov 14, 2023
Merged

AgileRL curriculum learning and self-play tutorial #1124

merged 29 commits into from
Nov 14, 2023

Conversation

nicku-a
Copy link
Contributor

@nicku-a nicku-a commented Oct 27, 2023

Description

This is a tutorial on curriculum learning and self play on the connect four env. It contains pretrained model weights and curriculum lesson configs.

Fixes # (issue), Depends on # (pull request)

Type of change

  • This change requires a documentation update

Checklist:

  • [X ] I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • [ X] I have run pytest -v and no errors are present.
  • [ X] I have commented my code, particularly in hard-to-understand areas
  • [X ] I have made corresponding changes to the documentation
  • [ X] I solved any possible warnings that pytest -v has generated that are related to my code to the best of my knowledge.
  • X[ ] I have added tests that prove my fix is effective or that my feature works
  • [X ] New and existing unit tests pass locally with my changes

@elliottower
Copy link
Contributor

This looks good to me content wise and all, pre commit is an easy fix, I’ve messaged Manuel who knows about furo and the error happening in the doc tests, and sent you a message about wandb failing

@elliottower
Copy link
Contributor

Not sure why the docs tests are even running for any of the tutorials, as we have disabled pytest from running in the tutorials directory by default. Will try to figure that out and get it fixed.

@elliottower
Copy link
Contributor

Just a note I disabled the AgileRL CI because it was failing and I needed to merge some other PRs, but it can be added back in this line

tutorial: [Tianshou, CustomEnvironment, CleanRL, SB3/kaz, SB3/waterworld, SB3/connect_four, SB3/test] # TODO: add back AgileRL once issue is fixed on their end

As we previously discussed it would be a lot of training time used to test this DQN tutorial, but maybe you could at least verify it once in CI (using drastically smaller epoch lengths, etc) just to ensure things all work. Our other tutorials all have small number of steps as default just so that the CI will run, and have some comments about better training parameters.

Maybe you could do the same and comment for recommended number of steps/epochs/etc for convergence. On your own website you have the full hyperparameters and such, I think if on here we could have a working example that is tested against the new versions of PettingZoo and such, that would be useful to be able to know that it still works.

My reasoning is that by default it's a nice proof of concept to be able to train a model on your computer in <30 minutes, and run it and see that it works with no issues. Better that than running something that takes multiple hours and may end up in an error at the very end.

@elliottower
Copy link
Contributor

I think these changes should fix the docs tests but we’ll have to see

@elliottower
Copy link
Contributor

Ok the issue was there’s code in your md files, normally we embed all the code from py files but that’s fine, I’ve added it to be ignored.

@elliottower
Copy link
Contributor

I’m trying to figure out why the workflows aren’t running even though AgileRL is added back to the tutorials-test yml file, checked the previous commits and it looked exactly the same syntax wise no misspellings. It only launches 28 runs for all the other tutorials whereas it should be more and include agileRL.

Maybe you can update the branch to be up to date with master and then that might fix it? Not sure, very bizarre. It should look for all .py files in the AgileRL folder in tutorials directory which I checked does contain them.

@elliottower
Copy link
Contributor

Okay that fixed it, was a merge conflict that had to be resolved. Will have to see if they pass now.

@elliottower elliottower merged commit f27e84b into Farama-Foundation:master Nov 14, 2023
47 checks passed
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