-
Notifications
You must be signed in to change notification settings - Fork 724
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 VecNormalize normalization #609
Conversation
Note that after the _update_rews() refactor, self.ret doesn't update anymore if `not self.training`.
Thanks for the PR, I'll try to do a review before next week ;) |
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 this PR, overall looks good. Would benefit from some more docs and slightly more comprehensive tests. After that would be happy to approve.
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, some minor point need to be discussed still
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 apart from @araffin's comments
Co-Authored-By: Adam Gleave <adam@gleave.me>
Thanks for the reviews! |
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 =)
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.
Looking good, two things I'm unsure on and would like feedback.
i dont have a strong opinion for your two comments |
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
* Fix errors due to SB change hill-a/stable-baselines#609 * normalize_{observation=>obs} * setup.py: Bump update note
Description
Decouples normalization from mean/stddev tracking functionality.
Exposes reward and observation normalization for use outside of
.step()
andreset()
.Add
get_original_rews()
function which returns the most recent unnormalized rewards.reset()
now updates statistics for observations in addition to normalizing.Motivation and Context
closes #602.
Types of changes
Checklist:
pytest
andpytype
both pass.