New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add log-linear algorithm for 2d Pareto front. #2503
Changes from 2 commits
11b5187
54c51a6
0fbc94a
846b78f
ab17d55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,46 @@ | |
from optuna.trial import TrialState | ||
|
||
|
||
def _get_pareto_front_trials(study: "optuna.study.BaseStudy") -> List[FrozenTrial]: | ||
def _get_pareto_front_trials_2d(study: "optuna.study.BaseStudy") -> List[FrozenTrial]: | ||
trials = [trial for trial in study.trials if trial.state == TrialState.COMPLETE] | ||
points = sorted( | ||
( | ||
+_normalize_value(trial.values[0], study.directions[0]), | ||
-_normalize_value(trial.values[1], study.directions[1]), | ||
index, | ||
) | ||
for index, trial in enumerate(trials) | ||
) | ||
|
||
mask = [False] * len(trials) | ||
|
||
def set_mask(width: int, hi: int) -> None: | ||
for k in range(hi - width, hi): | ||
_, _, index = points[k] | ||
mask[index] = True | ||
|
||
width = 0 | ||
best_y = float("inf") | ||
curr_x = float("nan") | ||
for i, (x, y, _) in enumerate(points): | ||
y = -y | ||
if curr_x != x: | ||
set_mask(width, hi=i) | ||
width = 0 | ||
if y > best_y or (y == best_y and width == 0): | ||
continue | ||
if y < best_y: | ||
width = 0 | ||
width += 1 | ||
best_y = y | ||
curr_x = x | ||
set_mask(width, hi=len(points)) | ||
|
||
pareto_front = [trial for trial, keep in zip(trials, mask) if keep] | ||
return pareto_front | ||
|
||
|
||
def _get_pareto_front_trials_nd(study: "optuna.study.BaseStudy") -> List[FrozenTrial]: | ||
pareto_front = [] | ||
trials = [t for t in study.trials if t.state == TrialState.COMPLETE] | ||
|
||
|
@@ -26,6 +65,12 @@ def _get_pareto_front_trials(study: "optuna.study.BaseStudy") -> List[FrozenTria | |
return pareto_front | ||
|
||
|
||
def _get_pareto_front_trials(study: "optuna.study.BaseStudy") -> List[FrozenTrial]: | ||
if len(study.directions) == 2: | ||
return _get_pareto_front_trials_2d(study) # Log-linear in number of trials. | ||
return _get_pareto_front_trials_nd(study) # Quadratic in number of trials. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The non-2d scenario seems not to be covered by any test case in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a 3d test case. |
||
|
||
|
||
def _dominates( | ||
trial0: FrozenTrial, trial1: FrozenTrial, directions: Sequence[StudyDirection] | ||
) -> bool: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about sorting the trials themself, and using
_dominates
function as done in_get_pareto_front_trials_nd
?This code does not preserve the order of all trials and pareto front trials. so it will be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of using
_dominates
to simplify the code. I implemented your suggestion.The only difference between my implementation and your suggestion is that I am still using
mask
because the order of trials needs to be preserved when filtering (as you point out in your comment).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To preserve the order of trials, we only need to sort the Pareto front trials with its
trial.number
. Could you remove themask
? (I have noticed this simple fix after posting the above messages. Sorry.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch. Done.