-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add ParabolaEnv. #29
Add ParabolaEnv. #29
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.
Bounds need to be modified
src/seals/diagnostics/parabola.py
Outdated
self._bounds = bounds | ||
|
||
super().__init__( | ||
state_space=spaces.Box(low=-bounds, high=bounds, shape=(5,)), |
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.
Bounds seems wrong:
- The last three components (a, b, and c) are always in
[-1,1]
whateverbounds
is set to. - The first two components (x, y) we say are
$\mathbb{R}^2$ in the DERAIL paper. We should either change the paper or the code.
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.
The motivation for bounding is more of a technicality than a environment design point: I just wanted to make sure that returns were bounded, (without the bounds, arbitrarily bad policies can get arbitrarily negative returns); I'd rather add that to the paper than change the code.
Codecov Report
@@ Coverage Diff @@
## master #29 +/- ##
============================================
+ Coverage 88.87% 100.00% +11.12%
============================================
Files 20 21 +1
Lines 593 619 +26
============================================
+ Hits 527 619 +92
+ Misses 66 0 -66
Continue to review full report at Codecov.
|
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 (but please update the paper to document this behaviour)
No description provided.