Fix benchmarking configs + Add a minimal test suite for those configs. #653
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit patches in the changes from PR #650. It also fixes an error in the dagger configs by replacing "py/object" with "py/type". "py/object" failed the assert in sacred.config.utils.assert_is_valid_key. Finally, it adds a minimal test suite for the benchmarking configs. This commit makes progress on testing, but still does not test that all the benchmarking configs work. Running all the configs with the fast configs applied is too slow. Calling sacred's print_config command for all the configs is also too slow.
AdamGleave
requested changes
Jan 6, 2023
Member
AdamGleave
left a comment
AdamGleave
left a comment
There was a problem hiding this comment.
Thanks for adding this test! Some minor suggestions.
Avoid the overhead of subprocess by running the print_config using the experiment's run method. Also, use a path relative to the script for BENCHMARKING_DIR instead of creating a path and then checking if it exists.
…AI/imitation into fix-benchmarking-configs
Codecov Report
@@ Coverage Diff @@
## master #653 +/- ##
=======================================
Coverage 97.54% 97.54%
=======================================
Files 86 87 +1
Lines 8422 8443 +21
=======================================
+ Hits 8215 8236 +21
Misses 207 207
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This was referenced Jan 26, 2023
ernestum
added a commit
that referenced
this pull request
Jan 26, 2023
This change just made some error messages go away indicating the missing imitation.algorithms.dagger.ExponentialBetaSchedule but it did not fix the root cause.
ernestum
added a commit
that referenced
this pull request
Feb 2, 2023
This change just made some error messages go away indicating the missing imitation.algorithms.dagger.ExponentialBetaSchedule but it did not fix the root cause.
ernestum
added a commit
that referenced
this pull request
Feb 3, 2023
This change just made some error messages go away indicating the missing imitation.algorithms.dagger.ExponentialBetaSchedule but it did not fix the root cause.
ernestum
added a commit
that referenced
this pull request
Feb 13, 2023
* Undo the changes from #653 to the dagger benchmark config files. This change just made some error messages go away indicating the missing imitation.algorithms.dagger.ExponentialBetaSchedule but it did not fix the root cause. * Improve readability and interpretability of benchmarking tests. * Add exponential beta scheduler for dagger * Ignore coverage for unknown algorithms. * Cleanup and extend tests for beta schedules in dagger. --------- Co-authored-by: taufeeque9 <[email protected]>
AdamGleave
pushed a commit
that referenced
this pull request
Oct 10, 2023
* Merge py file changes from benchmark-algs * Clean parallel script * Undo the changes from #653 to the dagger benchmark config files. This change just made some error messages go away indicating the missing imitation.algorithms.dagger.ExponentialBetaSchedule but it did not fix the root cause. * Improve readability and interpretability of benchmarking tests. * Add pxponential beta scheduler for dagger * Ignore coverage for unknown algorithms. * Cleanup and extend tests for beta schedules in dagger. * Add optuna to dependencies * Fix test case * Clean up the scripts * Remove reporter(done) since mean_return is reported by the runs * Add beta_schedule parameter to dagger script * Update config policy kwargs * Changes from review * Fix errors with some configs * Updates based on review * Change metric everywhere * Separate tuning code from parallel.py * Fix docstring * Removing resume option as it is getting tricky to correctly implement * Minor fixes * Updates from review * fix lint error * Add documentation for using the tuning script * Fix lint error * Updates from the review * Fix file name test errors * Add tune_run_kwargs in parallel script * Fix test errors * Fix test * Fix lint * Updates from review * Simplify few lines of code * Updates from review * Fix test * Revert "Fix test" This reverts commit 8b55134. * Fix test * Convert Dict to Mapping in input argument * Ignore coverage in script configurations. * Pin huggingface_sb3 version. * Update to the newest seals environment versions. * Push gymnasium dependency to 0.29 to ensure mujoco envs work. * Incorporate review comments * Fix test errors * Move benchmarking/ to scripts/ and add named configs for tuned hyperparams * Bump cache version & remove unnecessary files * Include tuned hyperparam json files in package data * Update storage hash * Update search space of bc * update benchmark and hyper parameter tuning readme * Update README.md * Incorporate reviewer's comments in benchmarking readme * Update gymnasium version and render mode in eval policy * Fix error * Update commands.py hex strings --------- Co-authored-by: Maximilian Ernestus <[email protected]> Co-authored-by: ZiyueWang25 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR patches in the changes from PR #650. It also fixes an error in the dagger configs by replacing "py/object" with "py/type". "py/object" failed the assert in sacred.config.utils.assert_is_valid_key.
Testing
The PR adds a minimal test suite for the benchmarking configs. Running all the configs with the fast configs applied is too slow and requires MuJoCo. Instead, we test that the print_config command works for all the configs.