-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Remove unnecessary SDE resampling in PPO update #1933
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR =)
could you do the same for PPO variants in SB3 contrib?
I've created a report to check that it had no impact on the performance (it changes results because the state of the pseudo-random generator is not the same but should not impact performance): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks =)
* Remove unnecessary SDE resampling in PPO update * Update changelog.rst * Update version * Update PyTorch version on CI * Update ruff * Limit NumPy version * Reformat --------- Co-authored-by: Antonin RAFFIN <[email protected]>
Description
Remove policy.reset_noise() call in PPO update
Motivation and Context
Resampling the SDE noise in the PPO update is unnecessary, for more info see #1929
closes #1929
Types of changes
Checklist
make format
(required)make check-codestyle
andmake lint
(required)make pytest
andmake type
both pass. (required)make doc
(required)Note: You can run most of the checks using
make commit-checks
.Note: we are using a maximum length of 127 characters per line