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

Update rllib_pistonball.py 1000 timesteps total #1141

Closed
wants to merge 6 commits into from

Conversation

elliottower
Copy link
Contributor

@elliottower elliottower commented Nov 28, 2023

Description

Tests are taking a while to finish.

Edit: looks like things aren't working due to some weird thing with the number of CPUs, I'm going to just disable this as I've run it locally and it works.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

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

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.
To upload images to a PR -- simply drag and drop or copy paste.

Checklist:

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

@elliottower
Copy link
Contributor Author

So I've spent a while trying to get the ppo pistonball tutorial to work but there seems to be some very bizarre issues (it says the env doesnt conform to the gymnasium standards, which is definitely not true). I'm going to just remove it as this is old and impossible to maintain because the original author isn't here anymore, and is directly worse than the official pistonball tutorial that uses ray AIR and allows it to use any default model in the ray catalog.

This isn't an ideal choice, but I don't have time to fix it and I tried even copy pasting all of the preprocessing and training configs onto the same pistonball tutorial and it still didn't work properly, at that point it's better to just point people towards what does work.

@elliottower
Copy link
Contributor Author

Looks like the leduc holdem tutorial at the very least passes the tests in CI now, which is definitely good (guaranteeing they stay working instead of breaking without our knowledge, with CI disabled)

@elliottower
Copy link
Contributor Author

Okay for some reason this passes with python 3.10, but no other version... I honestly have no idea what's up with this, probably going to just remove it from CI as well

@elliottower
Copy link
Contributor Author

Going to close this and just update the docs instead

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.

1 participant