Support constrained optimization in best_trial
#5426
Conversation
|
@HideakiImamura @nabenabe0928 Could you review this PR? |
| # violation value in the best-valued trial. | ||
| constraints = best_trial.system_attrs.get(_CONSTRAINTS_KEY) | ||
| if constraints is not None and any([x > 0.0 for x in constraints]): | ||
| complete_trials = self.get_trials(deepcopy=False, states=[TrialState.COMPLETE]) |
There was a problem hiding this comment.
Is it possible to use cache here or would it be buggy because some trials might finish between the last query and this query of self.get_trials?
There was a problem hiding this comment.
In my understanding, the cache in _get_trials is taken at the beginning of the trial, so it is not appropriate to use that cache in best_trial. It can be a strange value since best_trial is expected to be called after the trial.
optuna/study/study.py
Outdated
| feasible_trials = [] | ||
| for trial in trials: | ||
| constraints = trial.system_attrs.get(_CONSTRAINTS_KEY) | ||
| if constraints is None or all([x <= 0.0 for x in constraints]): |
There was a problem hiding this comment.
TPE and NSGA-II consider constraints is None worse than trials with constraints is not None, implying that constraints is None is infeasible.
- https://github.com/optuna/optuna/blob/master/optuna/samplers/_tpe/sampler.py#L758-L768
- https://github.com/optuna/optuna/blob/master/optuna/samplers/nsgaii/_constraints_evaluation.py#L47-L53
| if constraints is None or all([x <= 0.0 for x in constraints]): | |
| if constraints is not None and all(x <= 0.0 for x in constraints): |
There was a problem hiding this comment.
Thank you for your suggestion. I've updated it and _multi_objective.py's one. There is some duplicate code, but I will do the refactoring in another PR.
Co-authored-by: Shuhei Watanabe <47781922+nabenabe0928@users.noreply.github.com>
optuna/study/study.py
Outdated
| @@ -58,6 +59,15 @@ class _ThreadLocalStudyAttribute(threading.local): | |||
| cached_all_trials: list["FrozenTrial"] | None = None | |||
|
|
|||
|
|
|||
| def _get_feasible_trials(trials: Sequence[FrozenTrial]) -> list[FrozenTrial]: | |||
There was a problem hiding this comment.
It seems that this is identical to that of study/_multi_objective.py. Can we remove either one?
|
This pull request has not seen any recent activity. |
|
I added |
Motivation
Resolve #5184.
This change breaks backward compatibility but results in intuitive behavior for constrained optimization.
Description of the changes
best_trial: We need to check all feasible trials to support constrained optimization inbest_trialwhile keeping the current storage schema. However, there is a concern about speed degradation if all trials are checked, so thebest_trialis first determined without regard to constraints, and all trials are checked only if the best-valued trial violates constraints.best_trials: Sincebest_trialsneeds to calculate the Pareto front, the impact of filtering on speed should be small.