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

Fix CarRacing termination #2890

Merged
merged 1 commit into from Jun 15, 2022
Merged

Conversation

araffin
Copy link
Contributor

@araffin araffin commented Jun 14, 2022

Description

Fix CarRacing termination (both reaching end of lap and treating it not like a failure).

See #2525 (comment)

EDIT: need to be merged after #2888

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.

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • 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

@pseudo-rnd-thoughts
Copy link
Contributor

How long has this bug been in the environment? Does this need a version bump just in case?

@araffin
Copy link
Contributor Author

araffin commented Jun 14, 2022

How long has this bug been in the environment? Does this need a version bump just in case?

The timeout not treated properly? forever...
The other one, since #2888 (actually #2888 was supposed to fix two things, but only fixed one).

Does this need a version bump just in case?

Probably yes. (the models trained on previous versions will still work on that one, and the other way around though)

@pseudo-rnd-thoughts
Copy link
Contributor

@araffin Would you be able to make the version bump and add a comment to the version info in the docstring

@araffin
Copy link
Contributor Author

araffin commented Jun 15, 2022

Would you be able to make the version bump and add a comment to the version info in the docstring

Where exactly should I do that?
(it will probably be faster if you do it directly and push the commit to my PR ;))

@pseudo-rnd-thoughts
Copy link
Contributor

As Im not the owner, I can't push to PRs
On

### Version History
you need to add a new version with the changes you have made
and
id="CarRacing-v1",
this to v2

@jkterry1 jkterry1 merged commit 71f11a0 into openai:master Jun 15, 2022
@araffin
Copy link
Contributor Author

araffin commented Jun 16, 2022

This has been merged before the version bump...

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.

None yet

3 participants