Multi fidelity acquisition functions and recommender logic#756
Multi fidelity acquisition functions and recommender logic#756jpenn2023 wants to merge 70 commits into
Conversation
AdrianSosic
force-pushed
the
dev-mfbo-main-acquisition-functions
branch
from
59af949 to
dd2974f
Compare
|
Hey @jpenn2023, the branch now sits on top of the current surrogate one, so you should also have an up-to-date view now (including the commits from the surrogate) |
jpenn2023
requested review from
AVHopp,
AdrianSosic and
Scienfitz
as code owners
|
|
||
| # Jordan MHS NOTE: typing awkward here since register_buffer does not declare attr. | ||
| fidelity_columns: Tensor | ||
| fidelities_comb: Tensor |
There was a problem hiding this comment.
what does comb refer to?
There was a problem hiding this comment.
Combinations. The acqf allows for multiple fidelity columns. The multi-parameter fidelity value is then an outer product over the single-parameter fidelities, with an assumption that the costs and zetas add between fidelity parameters. Will change variable name to fidelity_combinations for now.
| r"""Second optimisation stage: choose optimal fidelity to query.""" | ||
| # Jordan MHS NOTE: casting here because botorch model likelihood is too | ||
| # broadly typed. Check best practice in case likelihood does not have noise. | ||
| likelihood = cast(GaussianLikelihood, self.model.likelihood) |
There was a problem hiding this comment.
assert instead of cast OR add workaround to set noise to 0 if not present in class (preference as discussed)
|
|
||
| self._args.current_value = current_value | ||
|
|
||
| def _set_project(self) -> None: |
There was a problem hiding this comment.
confusing name, why not set_projection?
|
|
||
| curr_val_acqf = FixedFeatureAcquisitionFunction( | ||
| acq_function=PosteriorMean(self._botorch_surrogate), | ||
| d=7, |
There was a problem hiding this comment.
hardcoded to 7? seems strange
There was a problem hiding this comment.
Good catch! This is a typo carried over from the Hartmann7 example.
| """Construct column indices and values of costs, fidelities and values for MFUCB.""" | ||
| fidelity_params = { | ||
| p | ||
| for i, p in enumerate(searchspace.parameters) |
There was a problem hiding this comment.
i is never used in this comprerehnsion and thus enumerate seems obsolete?
AVHopp
force-pushed
the
dev-mfbo-main-acquisition-functions
branch
from
bb7b334 to
dd5fc82
Compare
AVHopp
force-pushed
the
dev-mfbo-main-acquisition-functions
branch
from
dd5fc82 to
74479a9
Compare
|
Since there is another MFBO related PR that needs to be merged prior to this, this has been set to |
AVHopp
force-pushed
the
dev-mfbo-main-acquisition-functions
branch
from
74479a9 to
9565aa5
Compare
AVHopp
force-pushed
the
dev-mfbo-main-acquisition-functions
branch
3 times, most recently
from
e845f35 to
9c6c8ad
Compare
Since the ICML kernel factory is only able to handle categorical fidelities, the check needed to be tightened
Previously, search spaces that only contained task or fidelity parameters were piped through until they reached BoTorch normalization. There, they caused an onimous error about the `indices` list being empty. This is now checked earlier and returns a more informative error message.
Variant A of removing the explicit surrogate class for Multi Fidelity models. This approach now dispatches to the corresponding surrogate class based from botorch on the model-class inside the `_fit` method. That is, we do not recreate the botorch model but simply use it.
This reverts commit 6167b93.
AVHopp
force-pushed
the
dev-mfbo-main-acquisition-functions
branch
from
9c6c8ad to
7aa47eb
Compare
4 participants
No description provided.