Revert to default inf max_trials and pool-size of 1 #659
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
Why: The new default behavior is confusing for users. It is also difficult to determine a good default max_trials, so having not enough or to many trials sampled by default at the start of HPO can be annoying for many users. Using inf by default and iterating with pool-size may be the best alternative. Now that we have a support for n-workers, the argument pool-size we previously deprecated actually make sense. By default, pool-size should be equal to number of workers. We have n-workers set to 1 by default, so by default we are back to previous behavior; sampling 1 trial at a time, until max_trials. How: The producer now takes a pool size as argument when producing. The same applies to ExperimentClient.suggest() and ExperimentClient.workon(). The pool size is used to sample multiple trials at a time and increase I/O efficiency. The producer now keeps track of number of new trials so that if multiple workers are producing new trials with a non-seed algorithm (hence they produce different trials and there are no conflicts leading to backoff) they will stop if they generated together up to `pool_size` trials. Note: Pool-size is moved to to worker configuration instead. Since pool-size relates to n-workers, which is part of worker configuration, having pool-size in worker configuration makes more sense.
Why: There was a bug in the tests. The functions to generate trials would generate more than requested because of the new behavior of producer attempting to produce all trials at once, once the value of `max_trials` was conflicting with the number of trials requested to the trial generating function for tests (`orion.testing.evc.generate_trials`). Fortunately the bug in the tests did not seem to miss any bugs in the code they were testing. How: Adjust the expected numbers based on the corrected behiavor. The numbers make indeed more sense now.
bouthilx
added
the
enhancement
donglinjy
reviewed
Sep 14, 2021
donglinjy
reviewed
Sep 14, 2021
donglinjy
reviewed
Sep 14, 2021
|
The new behavior does make more sense to me. |
donglinjy
approved these changes
Sep 14, 2021
Co-authored-by: Lin Dong <michaeltunglin@gmail.com>
|
Thanks @donglinjy for the review! |
… into feature/back_to_pool_size
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.
[Fixes #644]
Why:
The new default behavior is confusing for users. It is also difficult to
determine a good default max_trials, so having not enough or to many
trials sampled by default at the start of HPO can be annoying for many
users. Using inf by default and iterating with pool-size may be the best
alternative.
Now that we have a support for n-workers, the argument pool-size we
previously deprecated actually make sense. By default, pool-size
should be equal to number of workers. We have n-workers set to 1 by
default, so by default we are back to previous behavior; sampling 1
trial at a time, until max_trials.
How:
The producer now takes a pool size as argument when producing. The same
applies to ExperimentClient.suggest() and ExperimentClient.workon(). The
pool size is used to sample multiple trials at a time and increase I/O
efficiency.
The producer now keeps track of number of new trials so that
if multiple workers are producing new trials with a non-seed algorithm
(hence they produce different trials and there are no conflicts leading
to backoff) they will stop if they generated together up to
pool_sizetrials.
Note:
Pool-size is moved to to worker configuration instead. Since pool-size
relates to n-workers, which is part of worker configuration, having
pool-size in worker configuration makes more sense.