Use all trials in TPESampler even when multivariate=True
#4079
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
github-actions
bot
added
the
optuna.samplers
This was referenced Oct 17, 2022
Codecov Report
@@ Coverage Diff @@
## master #4079 +/- ##
==========================================
- Coverage 90.10% 90.10% -0.01%
==========================================
Files 161 161
Lines 12655 12652 -3
==========================================
- Hits 11403 11400 -3
Misses 1252 1252
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@not522 @HideakiImamura Could you review this PR? |
HideakiImamura
approved these changes
Oct 21, 2022
|
I resolved the conflict with #4073. PTAL 🙏 |
not522
reviewed
Oct 31, 2022
optuna/samplers/_tpe/sampler.py
Outdated
Comment on lines
396
to
399
| n_sampled = sum( | ||
| step < float("inf") and param is not None | ||
| for (step, _), param in zip(scores, list(values.values())[0]) | ||
| ) |
There was a problem hiding this comment.
Please add a comment for this counting. It is a bit complicated to understand the behavior.
|
@not522 Thanks for the comments. As discussed privately, I decided to keep the original behavior of |
not522
changed the title
TPESampler even when multivariate=True
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.
Motivation
In the current TPE, the trials without parameters that we want to sample are skipped when
multivatiate=True(link). However, whenmultivariate=False, those trials are used effectively in the following two steps.belowandabove.This PR fixes the behavior of
multivariate=Trueand makes it consistent withmultivariate=False.Description of the changes
multivariate=False.multivariateargument from_get_observation_pairs.[Note] This PR slightly changes the current behavior of
TPESamplerin the following points.multivariate=Trueandgroup=True, the trials that do not include suggesting parameters are also used. (Whenmultivariate=Trueandgroup=False, this does not change the behavior because parameters in the intersection search space are included in all trials)Comparen_startup_trialswith only the number of trials that include suggesting parameters.