Tutorial on Transfer Learning Bayesian Optimisation #439
Conversation
|
Thanks @jpfolch. I will provide a review tomorrow. Best, Johannes |
There was a problem hiding this comment.
Hi @jpfolch,
looks very nice. Thank you!
I let one comment inline, and here are two other ones:
- Can you also add a validator to the botorch data model (https://github.com/experimental-design/bofire/blob/main/bofire/data_models/strategies/predictives/botorch.py) that checks if a
MultiTaskGPis present, only one category/task is allowed in theTaskFeature, else we will get problems, especially as currently the fidelity/task selection is missing. You need to use a after validator ( ) - Can you add to the notebook optimization campaigns that show that using information from lower fidelity enhances the convergence in comparison to not using this information?
Best,
Johannes
| @@ -170,13 +170,21 @@ def _predict(self, transformed: pd.DataFrame) -> Tuple[np.ndarray, np.ndarray]: | |||
| # input and further transform it to a torch tensor | |||
| X = torch.from_numpy(transformed.values).to(**tkwargs) | |||
| with torch.no_grad(): | |||
| posterior = self.model.posterior(X=X, observation_noise=True) # type: ignore | |||
| # observation noise is not implemented for MultiTaskGPSurrogate, has to be treated differently | |||
| if self.surrogate_specs.surrogates[0].type == "MultiTaskGPSurrogate": | |||
There was a problem hiding this comment.
Can you add a validator to BotorchSurrogates that checks that in case one MultiTaskGP is present, all surrogates are of type MultiTaskGP. Else, we will get trouble with the different required encodings. You have to add it here: https://github.com/experimental-design/bofire/blob/main/bofire/data_models/surrogates/botorch_surrogates.py
There was a problem hiding this comment.
You were correct and the code actually crashed when using multiple models (even if all were MultiTaskGPs). I sent a fix for it and it seems to work, but I am not sure it is entirely correct; I do not really understand when we expect to have len(posterior.mean.shape) == 2 and when len(posterior.mean.shape) == 3? Any guidance would be appreciated.
There was a problem hiding this comment.
Oh, also important to mention that when I tested with a single surrogate model, or two of them, in all cases it goes through len(posterior.mean.shape) == 2 which caused my confusion...
There was a problem hiding this comment.
You have a posterior mean of shape 3 in case of using fully basesian GPs. Have a look here: https://github.com/experimental-design/bofire/blob/main/bofire/surrogates/fully_bayesian.py
There was a problem hiding this comment.
Looking, at this and how this gets really messy here, I want to propose the following solution:
If the model can handle posterior noise, we take it, else we do not take it.
try:
posterior = self.model.posterior(X=X, observation_noise=True)
except: # the error which is thorwn for MultiTask
posterior = self.model.posterior(X=X, observation_noise=False)And the rest we keep as it is. And I will in a later PR refactor this whole part. Ok for you?
|
Hi @jduerholt, I have made the requested changes, I am a little unsure about the I also added some testing, checking that the |
|
Thanks, I will have a look the latest on Monday, but I hope to get it done tomorrow! |
There was a problem hiding this comment.
Hi @jpfolch,
thank you very much. Looks very good. I think we are almost there. Have a look at my inline comments.
Regarding the notebook, only two things:
- Can you also add a "SMOKE_TEST" limitation for the iterations that you run for the notebook? So that in case of the automatest test pipleine, only two iterations per optimiuation campagin are run?
- There is a variable called
regrets_single_task_mean. Can you rename it toregrets_single_task_medianas it is actually the median? - Optional one could think of creating a benchmark class for you multitask benchmark that you created and then use the automatic benchmark excutors. But this is only optional.
Best,
Johannes
| @@ -170,13 +170,21 @@ def _predict(self, transformed: pd.DataFrame) -> Tuple[np.ndarray, np.ndarray]: | |||
| # input and further transform it to a torch tensor | |||
| X = torch.from_numpy(transformed.values).to(**tkwargs) | |||
| with torch.no_grad(): | |||
| posterior = self.model.posterior(X=X, observation_noise=True) # type: ignore | |||
| # observation noise is not implemented for MultiTaskGPSurrogate, has to be treated differently | |||
| if self.surrogate_specs.surrogates[0].type == "MultiTaskGPSurrogate": | |||
There was a problem hiding this comment.
You have a posterior mean of shape 3 in case of using fully basesian GPs. Have a look here: https://github.com/experimental-design/bofire/blob/main/bofire/surrogates/fully_bayesian.py
| @@ -170,13 +170,21 @@ def _predict(self, transformed: pd.DataFrame) -> Tuple[np.ndarray, np.ndarray]: | |||
| # input and further transform it to a torch tensor | |||
| X = torch.from_numpy(transformed.values).to(**tkwargs) | |||
| with torch.no_grad(): | |||
| posterior = self.model.posterior(X=X, observation_noise=True) # type: ignore | |||
| # observation noise is not implemented for MultiTaskGPSurrogate, has to be treated differently | |||
| if self.surrogate_specs.surrogates[0].type == "MultiTaskGPSurrogate": | |||
There was a problem hiding this comment.
Looking, at this and how this gets really messy here, I want to propose the following solution:
If the model can handle posterior noise, we take it, else we do not take it.
try:
posterior = self.model.posterior(X=X, observation_noise=True)
except: # the error which is thorwn for MultiTask
posterior = self.model.posterior(X=X, observation_noise=False)And the rest we keep as it is. And I will in a later PR refactor this whole part. Ok for you?
| @@ -227,3 +229,17 @@ def _generate_surrogate_specs( | |||
| surrogate_specs.surrogates = _surrogate_specs | |||
| surrogate_specs._check_compability(inputs=domain.inputs, outputs=domain.outputs) | |||
| return surrogate_specs | |||
|
|
|||
| @model_validator(mode="after") | |||
There was a problem hiding this comment.
This has to be moved to data_models/strategies/predictives/botorch.py, as it is just a requirement for optimization.
There was a problem hiding this comment.
The corresponding test would go then into tests/bofire/data_models/specs/strategies.py and you would add an invalid spec for SOBO for example as it inherits from the base class.
There was a problem hiding this comment.
This has to be moved to
data_models/strategies/predictives/botorch.py, as it is just a requirement for optimization.
Is it not already in the correct file?
I am adding the test now though.
| @@ -131,4 +131,10 @@ def validate_surrogates(cls, v, values): | |||
| raise ValueError( | |||
| f"Preprocessing steps for features with {key} are incompatible." | |||
| ) | |||
| # check that if any surrogate is a MultiTaskGPSurrogate, all have to be | |||
There was a problem hiding this comment.
This needs to be tested in tests/bofire/data_models/specs/surrogates.py. You can add there invalid configurations, and the error that should be thrown. This is the new default way of validating data models.
| ), | ||
| ], | ||
| ) | ||
| def test_qehvi_with_multitask(task_input, surrogate_2): |
There was a problem hiding this comment.
Can you add this to tests/bofire/strategies/test_mobo.py? qehvi is deprecated and only included vor legacy reasons.
| surrogate_data_2 = surrogate_2(inputs=inputs, outputs=outputs_2) | ||
| surrogate_data = [surrogate_data_1, surrogate_data_2] | ||
|
|
||
| # test for error if both models are not multi-task |
There was a problem hiding this comment.
This would go in the specs part, as mentioned above.
| surrogate_specs = BotorchSurrogates(surrogates=surrogate_data) | ||
|
|
||
| # test for error if task input has more than 1 allowed category | ||
| if sum(task_input.allowed) > 1: |
There was a problem hiding this comment.
This would also go in the specs, as mentioned above
| surrogate_specs=surrogate_specs, | ||
| ) | ||
|
|
||
| strategy = strategies.map(strategy_data_model) |
There was a problem hiding this comment.
This would be real functionally tested in test_mobo
| acquisition_function=qLogEI(), | ||
| ) | ||
|
|
||
| strategy = strategies.map(strategy_data_model) |
There was a problem hiding this comment.
Keep only this, the rest would be in the specs as it is part of the data model validators.
|
Hi @jduerholt, I have made the requested changes. A few points:
Thanks! |
|
Looks very good for me. Thank you very much! |
Added a tutorial on how to use BoFire for Transfer Learning Bayesian Optimisation. Also made two small modifications for the code to run:
Inverse transform for ordinal encodings transforms into
inttypeWhen predicting within the task using MultiTaskGPs we must add likelihood noise manually since the
observation_noiseflag is not supported by BoTorch for MultiTaskGPsNext step would be to incorporate a step for task selection for multi-fidelity problems.