-
Notifications
You must be signed in to change notification settings - Fork 43
Probabiltiy of Feasibility ACQF #580
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
Changes from all commits
144b83a
0c45764
3cbf237
15dd67a
748bcd3
91e5b87
3b4bc5a
8b29993
957b71e
bdebf7f
5c069ed
3dd72ab
3729322
8b55230
57efae2
c5f2011
f97a2eb
b4f3697
a0a4dd9
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 |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| qLogEI, | ||
| qLogNEHVI, | ||
| qLogNEI, | ||
| qLogPF, | ||
| qNegIntPosVar, | ||
| qNEHVI, | ||
| qNEI, | ||
|
|
@@ -38,16 +39,11 @@ | |
| qNEHVI, | ||
| qLogNEHVI, | ||
| qNegIntPosVar, | ||
| qLogPF, | ||
| ] | ||
|
|
||
| AnySingleObjectiveAcquisitionFunction = Union[ | ||
|
Contributor
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. didn't you say above this was a single objective acquisition function? shouldn't it appear here?
Contributor
Author
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. It is kind of a special case of a single objective acqf, as it should be only used for |
||
| qNEI, | ||
| qEI, | ||
| qSR, | ||
| qUCB, | ||
| qPI, | ||
| qLogEI, | ||
| qLogNEI, | ||
| qNEI, qEI, qSR, qUCB, qPI, qLogEI, qLogNEI, qLogPF | ||
| ] | ||
|
|
||
| AnyMultiObjectiveAcquisitionFunction = Union[qEHVI, qLogEHVI, qNEHVI, qLogNEHVI] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,13 @@ | ||
| from abc import abstractmethod | ||
| from typing import Annotated, List, Literal, Optional, Union | ||
| from typing import Annotated, Any, List, Literal, Optional, Union | ||
|
|
||
| import pandas as pd | ||
| from pydantic import Field, field_validator | ||
| from pydantic import Field, PositiveInt, field_validator | ||
|
|
||
| from bofire.data_models.base import BaseModel | ||
| from bofire.data_models.constraints.api import IntrapointConstraint | ||
| from bofire.data_models.domain.api import Domain | ||
| from bofire.data_models.objectives.api import ConstrainedObjective | ||
|
|
||
|
|
||
| class EvaluateableCondition: | ||
|
|
@@ -15,11 +17,68 @@ def evaluate(self, domain: Domain, experiments: Optional[pd.DataFrame]) -> bool: | |
|
|
||
|
|
||
| class Condition(BaseModel): | ||
| type: str | ||
| type: Any | ||
|
|
||
|
|
||
| class SingleCondition(BaseModel): | ||
| type: str | ||
| type: Any | ||
|
|
||
|
|
||
| class FeasibleExperimentCondition(SingleCondition, EvaluateableCondition): | ||
| """Condition to check if a certain number of feasible experiments are available. | ||
|
|
||
| For this purpose, the condition checks if there are any kind of ConstrainedObjective's | ||
| in the domain. If, yes it checks if there is a certain number of feasible experiments. | ||
| The condition is fulfilled if the number of feasible experiments is smaller than | ||
| the number of required feasible experiments. It is not fulfilled when there are no | ||
| ConstrainedObjective's in the domain. | ||
| This condition can be used in scenarios where there is a large amount of output constraints | ||
| and one wants to make sure that they are fulfilled before optimizing the actual objective(s). | ||
| To do this, it is best to combine this condition with the SoboStrategy and qLogPF | ||
| as acquisition function. | ||
|
|
||
| Attributes: | ||
| n_required_feasible_experiments: Number of required feasible experiments. | ||
| threshold: Threshold for the feasibility calculation. Default is 0.9. | ||
| """ | ||
|
|
||
| type: Literal["FeasibleExperimentCondition"] = "FeasibleExperimentCondition" | ||
| n_required_feasible_experiments: PositiveInt = 1 | ||
| threshold: Annotated[float, Field(ge=0, le=1)] = 0.9 | ||
|
|
||
| def evaluate(self, domain: Domain, experiments: Optional[pd.DataFrame]) -> bool: | ||
| constrained_outputs = domain.outputs.get_by_objective(ConstrainedObjective) | ||
| if len(constrained_outputs) == 0: | ||
| return False | ||
|
|
||
| if experiments is None: | ||
| return True | ||
|
|
||
| valid_experiments = ( | ||
| constrained_outputs.preprocess_experiments_all_valid_outputs(experiments) | ||
| ) | ||
| relevant_constraints = domain.constraints.get(IntrapointConstraint) | ||
| if len(relevant_constraints) > 0: | ||
| valid_experiments = valid_experiments[ | ||
| relevant_constraints.is_fulfilled(valid_experiments) | ||
| ] | ||
|
|
||
| # TODO: have a is fulfilled for input features --> work for future PR | ||
|
Contributor
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. rather use issues than comments in the code.
Contributor
Author
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. done: #593 I let the comment in the code so that one knows where to add it in the code. |
||
| feasibilities = pd.concat( | ||
| [ | ||
| feat( | ||
| valid_experiments[feat.key], | ||
| valid_experiments[feat.key], # type: ignore | ||
| ) | ||
| for feat in constrained_outputs | ||
| ], | ||
| axis=1, | ||
| ).product(axis=1) | ||
|
|
||
| return bool( | ||
| feasibilities[feasibilities >= self.threshold].sum() | ||
| < self.n_required_feasible_experiments | ||
| ) | ||
|
|
||
|
|
||
| class NumberOfExperimentsCondition(SingleCondition, EvaluateableCondition): | ||
|
|
@@ -72,4 +131,9 @@ def evaluate(self, domain: Domain, experiments: Optional[pd.DataFrame]) -> bool: | |
| return False | ||
|
|
||
|
|
||
| AnyCondition = Union[NumberOfExperimentsCondition, CombiCondition, AlwaysTrueCondition] | ||
| AnyCondition = Union[ | ||
| NumberOfExperimentsCondition, | ||
| CombiCondition, | ||
| AlwaysTrueCondition, | ||
| FeasibleExperimentCondition, | ||
| ] | ||
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.
Is there a literature reference that you can add?
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.
No, this is based on the BoFire PR, in which they implemented it: meta-pytorch/botorch#2815
Uh oh!
There was an error while loading. Please reload this page.
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.
Would it make sense to add a link to this paper mentioned in the BoTorch API reference and a link to the BoTorch API reference itself?
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.
The paper does not really fit, it does not mention the new acqf and is just explaining why one should use log based acqfs ;)
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.
Now I am confused. Ok, it is not the main topic of the paper. But at least there is this section.

Or is this PR about something else and I missed it?
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.
This is the formula for constrained expected improvement, which we were using automatically when we use EI in combination with output constraints. This PR introduced the qLogPF (PF = Probabiliy of feasibility) acquisition function which is just the feasibility term without EI.
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.
Aaaah. Got it. Now your new documentation (
strategies.md) makes sense to me :D.