-
Notifications
You must be signed in to change notification settings - Fork 0
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
Preference Comparisons with Updated AutoResetWrapper #34
Conversation
Do you want me to review this yet, or should I wait for upstream changes in seals? |
I'd say we wait for upstream in seals so I can update the |
Gotcha |
I ran code_checks and format. |
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
Add ability to do pref comp training with both AutoResetWrapper and corrected terminal observation.
Add a test to check new AutoResetWrapper behavior. This is just the test I wrote for
seals
. Not sure if this is the best way to do this. The advantage of leaving that test in here is that if something about the ARW behavior changes upstream, the test will easily notify us.Will keep open and not merge for now, until the AutoResetWrapper changes are upstreamed in
seals
(see HumanCompatibleAI/seals#69).