-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Enhanced rliable eval #1183
Enhanced rliable eval #1183
Conversation
1. Support for evaluating training runs 2. Improved handling of figures and axes 3. Allow passing max_env_step 4. Use min len of all experiments (bugfix, previously it would crash if experiments had different lengths)
Minor improvements in typing
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1183 +/- ##
==========================================
- Coverage 85.51% 84.75% -0.77%
==========================================
Files 104 104
Lines 8874 8976 +102
==========================================
+ Hits 7589 7608 +19
- Misses 1285 1368 +83
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Changes lgtm, I've just added some comments in case we wanted to expand. Otherwise I'll approve
@@ -11,6 +12,8 @@ | |||
with contextlib.suppress(ImportError): | |||
import wandb | |||
|
|||
log = logging.getLogger(__name__) | |||
|
|||
|
|||
class WandbLogger(BaseLogger): |
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.
Will need extra arguments for run grouping
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.
See above
|
||
test_data_found = True | ||
train_data_found = True | ||
if not test_episode_returns or env_step_at_test is None: |
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.
We probably need another mechanism here as well. In the unfortunate event of all but the last run having recorded data, env_step_at_test
may be None and the RuntimeError below is raised.
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.
I feel like we should rather improve the logging and stats-saving mechanism to ensure this never happens instead of doing more gymnastics here. Would that be possible? We can do that in a separate issue/PR.
…eval # Conflicts: # docs/spelling_wordlist.txt # tianshou/highlevel/experiment.py
11b83ab
to
21a7a3c
Compare
@maxhuettenrauch Thanks for the logger extension and review! Merging this now |
Rliable eval: multiple extensions
Also some fixes and extensions in loggers