Fix PBT issues with working dir and promotion of max fidelity trials #903
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
bouthilx
added
bug
lebrice
reviewed
May 10, 2022
There was a problem hiding this comment.
To summarize the discussion we had on Slack:
The PBT algorithm shouldn deal with copying / creating directories for trials, since it doesn't have any way of knowing which trials are actually new.
This responsibility should be moved to the Experiment, which would use the lineage information and the experiment id to create the working directory of the trial before it is passed to the User, and copy the files as necessary.
Delaunay
approved these changes
May 10, 2022
|
Actually it should be the Runner's responsibility considering coming changes with #911. The trials will be generated on a remote server and thus the directory creation/copy should not occur during the trial generation on the server but rather in the Runner which is assumed to share the same file system as workers. |
If the list of choices contains multiple types, numpy's RNG will cast all values to the type of the first item.
Why: When the trial has a parent, it means it should start from the same working_dir state. We need to fetch the parent trial and copy its working dir to the current trial's working dir. How: The runner now has a new argument, a callable that will be executed before a trial is submitted to the executor. By default this callable will take care of copying parent trial working_dir to child trial working dir, or simply create the working dir if the trial has no parent. If the callable fails, the exception is caught and delayed to be caught again during the executor's execution of the trial. This makes it possible to handle these trials the same way as if they crashed during trial execution rather than during trial preparation.
Why: The parent id is in transformed space. When converting a trial from transformed space to original space, the id of the parent must be converted too. If the parent has a parent too, then this grand-parent must be converted too, and so on. How: Recursively get to the root trial, and then compute the trial ids down to last parent trial. Tests are added to verify this.
Why: The number of trials executed `self.trials` is only incremented when trials are completed successfully, not when they crash. Thus, computing `self.trials - self.worker_broken_trials` would not count properly the number of completed trials by this runner.
bouthilx
force-pushed
the
hotfix/pbt_cp_dir
branch
from
806b722
to
98b08a1
Compare
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
It should not assume that algorithms are able to sample 2 trials at the same time. Therefore we won't test that it returns different trials one after the other. Only that seeding gives the same trial. The convergence test will figure it out if the algorithm always return the same trial.
lebrice
approved these changes
Jul 25, 2022
src/orion/algo/hebo/hebo_algo.py
Outdated
| fidelity_dim: Fidelity = self.space[self.fidelity_index] | ||
| orion_params[self.fidelity_index] = fidelity_dim.high | ||
| fidelity_dim = self.space[self.fidelity_index] | ||
| assert isinstance(fidelity_dim, TransformedDimension) |
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
lebrice
approved these changes
Jul 27, 2022
It was previously removed in PR Epistimio#903 because the Consumer is not supposed to create the directory anymore. Merging latest develop branch reintroduced it.
Pass algo object instead so that random state can be updated at end of with-clause.
lebrice
approved these changes
Jul 29, 2022
lebrice
reviewed
Jul 29, 2022
lebrice
reviewed
Jul 29, 2022
Co-authored-by: Fabrice Normandin <fabrice.normandin@gmail.com>
Since release v0.6.0 of pymoo (https://github.com/anyoptimization/pymoo/releases/tag/0.6.0), HEBO's acq_optimizers fail to import because of an ImportError during the module's own imports.
Merged
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.
@Delaunay @lebrice This includes a fugly fix to working dir paths of trials that depends on the trial id which is different in transformed space and original space. To makes things worse, the ID of the trial depends on the experiment ID which is only set outsite of the algorithm by the experiment object after the trial was suggested. Because of this we need to rename directories or create sym-links at the different levels (In algo, in algo-wrapper, in experiment) to make sure that the working dir of the trial is corresponding to the one communicated to the user through
trial.working_diror env varORION_WORKING_DIR.We could get rid of 1 level by making the trial id not depend on experiment. In the database the index of trials should depend on trial.id and trial.experiment however to make sure trials are not duplicated within a given experiment. That would require updates of database from the side of users after next release however.
I don't see a simple and clean fix however for the different working_dir inside the algorithm and inside the algo-wrapper. The parameters are different and thus the ID and working_dir is different.
Would you have any ideas?