Small fixes #964
Merged
Small fixes #964
Conversation
This file contains 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
lebrice
reviewed
Jul 7, 2022
| producer = Producer(experiment) | ||
|
|
||
| experiment_client = ExperimentClient(experiment, producer) | ||
| experiment_client = ExperimentClient(experiment) |
There was a problem hiding this comment.
Yes exactly! I found the same bug thanks to type hints on my end yesterday:
|
The pre-commit hook complains about things that did not change in this PR. Should I just ignore it? |
4 tasks
|
It's weird that these errors do not show up in the CI for the dev branch. If you are able to fix these it would be great. Otherwise we'll do it in another PR. |
|
I did a close/reopen to re-run the tests |
bouthilx
added
the
enhancement
lebrice
added a commit
to lebrice/orion
that referenced
this pull request
Jul 29, 2022
Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Merged
lebrice
added a commit
that referenced
this pull request
Sep 30, 2022
* Import files from old PR, start fresh Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Cleaning up algo_wrapper.py Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Minor typing improvements in utils/__init__.py Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * add transform-related methods to AlgoWrapper Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Clean up multi_task_algo.py Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * [big] move wrappers to folder, simplify wrappers Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Add tests for the wrappers Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Move algo wrappers to folder, split up tests a bit Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Fix cyclical import issue with knowledbe base Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Remove accidentally added .pre-commit-config.yaml Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * "algorithms.algorithm" -> "algorithms.unwrapped" Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Have wrappers return the algo's config, not theirs Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Remove duplicated code from SpaceTransform Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Rename test files for algo wrappers Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Fix bug in test_gridsearch.py Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Remove more duplicated code from SpaceTransform Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Husk of unit tests for multi-task wrapper Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Rename multi_task_wrapper.py -> multi_task.py Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Start to write body of the test Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Remove other duplicate methods of SpaceTransform Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Add dataclasses for various objects Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Adapting previous changes from config_dataclasses Signed-off-by: Fabrice Normandin <normandf@mila.quebec> * Adapting previous changes from config_dataclasses Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix / reformat docstrings Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix typo in log.debug call Co-authored-by: Xavier Bouthillier <xavier.bouthillier@gmail.com> * Fix outdated docstrings Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Revert changes to ugly part of testing/__init__.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix isort issues Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix isort / flake8 issues Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add an intermediate TransformWrapper ABC Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add optional `max_trials` property to algorithm Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove outdated/misleading comments Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix various pylint/flake8 errors Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Touchups on the types of primary_algo.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Slight refactoring in serializable.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Rename AlgoType TypeVar to AlgoT Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Start to add tests for ExperimentInfo dataclass Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Allow passing Algo type to create_experiment Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Minor tweak in AlgoWrapper.__repr__ Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Adding (failing) tests for warm-starting. Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add Knowledge Base arg to Experiment and Producer Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add KnowledgeBase as argument to workon functions Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Move warm-start method to right wrapper class Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add a simple test for warm starting Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Improve __repr__ of Registry Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Type the `space` property of ExperimentClient Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove line that caused bug in experiment_builder Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add tests for KB + Warm-Starting Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * (big, ugly commit) Add tests, clarify, refactor Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add repr for RegistryMapping Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Type the _results and _params attributes of Trial Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Move and Rename algo for unit tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test for Multi-Task wrapper collisions Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix bug in AlgoWrapper (see desc.) Subclasses of AlgoWrapper (in particular the InsistSuggest wrapper) did not register the trials into their registry when suggesting or observing. This caused an issue where the registry mapping wasn't working properly in the MultiTaskWrapper. - Made the `suggest` and `observe` of AlgoWrapper use `self.register` so that the registry mapping and collision detection now works properly. Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test for suggest to always give task_id=0 Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix small bug in gridsearch.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Simplification: kb is only attr of Experiment Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Moved "functional" tests to different file Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix pylint errors Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add unwrap convenience method on AlgoWrapper Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Misc changes (test cleanup, copy status) Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove unused warm_start_mode context manager Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Use the new max_trials property on algo Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add tests for setting max_trials and n_observed Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix renaming of SpaceTransform algo wrapper Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Don't register trials from other tasks in algo Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix randomness of flaky-ish test Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove dict[k, v] type annotation for python < 3.9 Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Move / Simplify the Config classes -> TypedDicts Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Type out the Random algo, fix a test Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fixing some broken tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix experiment_builder tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix broken test in test_experiment.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test for ExperimentConfig fields Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Minor touchups in experiment_config.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Removed unused typeddicts in experiment_config.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove outdated todo Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * [breaking] Knowledge Base implementation Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove unused knowledge_base argument to workon Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix instantiation of KB in exp builder Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * [nit] Fix typing of ExperimentStats fields Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * [optional] Type out storage/base.py and misc types Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Adapting the MultiTask wrapper tests, add stubs Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Move test_experiment_config to reflect src Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test for KB Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add more tests for the KnowledgeBase Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix incorrect type for experiment id in docstrings Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix error in fixture, adjust docstrings Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Pass experiment config instead of experiment obj Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix docstrings, use Unpack[] to type **kwargs Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Move and update functional tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove unused code block in test Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add note about the Unpack[] annotation Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add PartialExperimentConfig typeddict Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Trying to make functional tests pass... Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix pylint error in experiment.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix bugs in test_knowledge_base Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Pass kb to instantiate_algo, minor typing stuffs Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Clarify potential issue in register, touchups Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Minor typing touchups Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Simplify functional tests for warm_starting Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add tests for how to pass the algorithm Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Minor typing improvements to experiment_builder.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix assignment of max_trials in exp client Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Use KnowledgeBase in functional test, remove todos Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix pylint error Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix import error for TypedDict Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Re-introduce fix from #964 (?) Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix bug in algo creation logic Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add missing types in BaseAlgorithm Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add some of the missing types in exp builder Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove leftover todo in BaseAlgorithm Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix type annotation on create_experiment Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove extra type annotation on create_experiment Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix bug in test_tpe (added wrapper) Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix value in DumbAlgo Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix test condition Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove fixme comment Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix most PBT tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Misc changes Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Misc typing changes Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Debugging the PBT Errors Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix broken AlgoWrapper tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add missing register method on AlgoWrapper Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove unused `original_space` property Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix docstring and warning in BaseAlgorithm.get_id Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix bug introduced previously Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Minor improvements to TransformWrapper.suggest Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Use self.reverse_transform in get_original_parent Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Ugly temporary fix to bug that affected PBT (desc) There is a weird bug that was happening in PBT: - The trials that PBT suggest have a parent id. - When the SpaceTransform wrapper calls observe, the completed trials don't have the same parent as they did when they were suggested. Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove hacky fix, fix bug source (hopefully) Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Make _get_original_parent "static" again Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add todo for later (copying attributes explicitly) Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix minor bug in DumbAlgo-related test Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Greatly reduce number of warnings Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove Registry from InsistSuggestWrapper Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add tests to increase coverage of Registry class Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add more coverage for _instantiate_knowledge_base Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add a bit more coverage for AlgoWrapper Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Fix bug in _instantiate_knowledge_base Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add tests for _instantiate_algo Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add generic test class for AlgoWrappers Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove redundant fixture in AlgoWrapper tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Clean / minor changes to test_knowledge_base.py Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Use asserts to avoid writing useless tests Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test case for no compatible trials found Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Clean up testing utility function a bit Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test for not warm-starting twice Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add test for "space already has task_id" error Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add tests for is_warmstarteable function Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Standardize the imports of ExperimentConfig Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Add more tests for _instantiate_kb Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> * Remove redundante else clause in _instantiate_algo Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> Signed-off-by: Fabrice Normandin <normandf@mila.quebec> Signed-off-by: Fabrice Normandin <fabrice.normandin@gmail.com> Co-authored-by: Fabrice Normandin <normandf@mila.quebec> Co-authored-by: Xavier Bouthillier <xavier.bouthillier@gmail.com>
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 is a collection of small fixes for bugs I found while investigating the code.
If one of the fixes is questionable, I can remove the commit in question from the PR on request.
Checklist
Documentation
Quality
$ tox -e lint)