-
Notifications
You must be signed in to change notification settings - Fork 41
Feature/ensemble map saas single task gp #669
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
base: main
Are you sure you want to change the base?
Feature/ensemble map saas single task gp #669
Conversation
…com/Bizbalt/bofire into feature/EnsembleMapSaasSingleTaskGP
…ingleTaskGP' into feature/EnsembleMapSaasSingleTaskGP
…com/Bizbalt/bofire into feature/EnsembleMapSaasSingleTaskGP_v2
…com/Bizbalt/bofire into feature/EnsembleMapSaasSingleTaskGP
|
There are a few things that seems not working yet, will you investigate this or should I have a look? |
Yes, please have a look, I do not understand the failed checks and no tests failed on our machine. Maybe the new method does not accept the input parameters excactly as the comparable AdditiveMapSaasSingleTaskGP method. |
|
I will have a look at the end of the week! Best, Johannes |
jduerholt
left a 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.
Hi @Bizbalt,
sorry that it took so long. Too much to do ;)
This looks overall very good! The failing tests are in my point of view not related to this PR.
Only two things needs to be adapted:
- Docstring
- And the model needs to be also registered here: This is a bit annoying, I know, but it is needed to avoid circular imports. Needs some refactoring ;)
AnyBotorchSurrogate = Union[
Best,
Johannes
| return isinstance(my_type, type(ContinuousOutput)) | ||
|
|
||
| class EnsembleMapSaasSingleTaskGPSurrogate(TrainableBotorchSurrogate): | ||
| """Instantiates an ``EnsembleMapSaasSingleTaskGP``, which is a batched |
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.
Hmm, this docstring does not directly apply to BoFire, more to botorch, keep it concise (https://github.com/experimental-design/bofire/blob/main/bofire/data_models/surrogates/map_saas.py), state the differences to the version that we alread have (see the link) and just list the variables that a user in BoFire can actually choose.
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.
One can also link to the docstring from botorch.
Motivation
This Feature was requested in Issue #650 and fullfilled at the BoFire hackathon on the 17th. of November 2025.
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
The tests run analogous to the AdditiveMapSaasSingleTaskGP method, as both should return the same result.