Skip to content

Botorch preset#757

Open
AdrianSosic wants to merge 21 commits into
dev/gpfrom
feature/botorch_preset
Open

Botorch preset#757
AdrianSosic wants to merge 21 commits into
dev/gpfrom
feature/botorch_preset

Conversation

@AdrianSosic
Copy link
Copy Markdown
Collaborator

@AdrianSosic AdrianSosic commented Mar 2, 2026

DevPR, parent is #745

Adds the BOTORCH preset for GPs.

Important information

  • I think it's critical to actually assert that the preset exactly recovers the BoTorch behavior, in the form of a test, for mainly two reasons:
    1. The construction involves quite a few things to be configured, i.e. handling both singletask/multitask (the latter even requiring a new custom gpytorch module), setting all sorts of priors correctly, etc. Blindly believing that everything is correct and then just claiming this is the BoTorch behavior seems like a bad idea. The test ensures this explicitly.
    2. It also as an automatic alert mechanism for all situations when something is changed on the BoTorch side, informing us about breaking changes that yield different behavior but are not fully documented (which happened already several times).
  • I've also invested quite some effort to test the new multitask mean logic, i.e. that it not only recovers the BoTorch logic but that it also fills one of the missing gaps that will ultimately make our transfer learning model truely scale-invariant w.r.t. the different input tasks. In particular, I made sure that the only missing piece is the noise model stratification, which should be added in a follow-up PR and is the analogous to the stratification over means shipped by this PR. (This explains the changes to the streamlit script.)

@AdrianSosic AdrianSosic self-assigned this Mar 2, 2026
@AdrianSosic AdrianSosic added the new feature New functionality label Mar 2, 2026
@AdrianSosic AdrianSosic mentioned this pull request Mar 2, 2026
25 tasks
@AdrianSosic AdrianSosic changed the base branch from main to dev/gp March 2, 2026 15:15
@AdrianSosic AdrianSosic added the dev label Mar 2, 2026
@AdrianSosic AdrianSosic force-pushed the feature/botorch_preset branch from a149bb3 to c3885f6 Compare March 3, 2026 08:34
@AdrianSosic AdrianSosic force-pushed the feature/botorch_preset branch from c3885f6 to 4f4fd55 Compare April 17, 2026 08:18
@AdrianSosic AdrianSosic marked this pull request as ready for review April 17, 2026 10:46
Copilot AI review requested due to automatic review settings April 17, 2026 10:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new BOTORCH Gaussian Process preset intended to exactly mirror BoTorch GP behavior (single- and multi-task), alongside supporting kernel-factory refactors and validation improvements.

Changes:

  • Introduce GaussianProcessPreset.BOTORCH and implement BoTorch-aligned kernel / mean / likelihood factories (including custom GPyTorch components for multitask mean + likelihood).
  • Add parameter-kind classification (ParameterKind) and automatic parameter-kind validation for kernel factories via a new _KernelFactory base.
  • Extend tests (BoTorch parity + kernel-factory validation) and update the Streamlit surrogate-model demo to expose GP presets and multitask/transfer-learning toggles.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
baybe/surrogates/gaussian_process/presets/botorch.py Adds BoTorch preset factories for kernel/mean/likelihood.
baybe/surrogates/gaussian_process/components/_gpytorch.py Introduces custom GPyTorch mean + multitask likelihood helper to match BoTorch behavior.
baybe/surrogates/gaussian_process/components/kernel.py Adds _KernelFactory w/ parameter-kind validation; adjusts ICMKernelFactory.
baybe/surrogates/gaussian_process/presets/{edbo,edbo_smoothed}.py Migrates kernel factories to _KernelFactory and updates dimension logic.
baybe/surrogates/gaussian_process/presets/{core,__init__.py} Adds BOTORCH enum entry and re-exports preset factories.
baybe/parameters/{enum,base.py} Adds ParameterKind and exposes Parameter.kind.
baybe/parameters/selectors.py Removes _ParameterSelectorMixin (superseded by _KernelFactory).
tests/test_gp.py Adds parity test asserting BOTORCH preset reproduces BoTorch posterior stats.
tests/test_kernel_factories.py Adds tests for kernel-factory parameter-kind validation.
streamlit/surrogate_models.py Adds GP preset selector + multitask / transfer-learning UI and logic.
docs/conf.py Updates nitpick ignore list for refactor.
CHANGELOG.md Documents new presets and kernel-factory parameter-kind validation.
Comments suppressed due to low confidence (1)

baybe/surrogates/gaussian_process/presets/edbo.py:85

  • effective_dims is computed as train_x.shape[-1], which includes the task feature column in transfer-learning searchspaces. Since this factory explicitly excludes TaskParameter via parameter_selector, the priors should likely be based on the non-task dimensionality (i.e. exclude the task feature) to avoid skewing the dimension-dependent prior heuristics. Consider deriving effective_dims from the selected non-task dimensions instead (e.g., subtract task dims or count comp-rep columns for selected parameters).
        effective_dims = train_x.shape[-1]

        switching_condition = _contains_encoding(
            searchspace.discrete, _EDBO_ENCODINGS
        ) and (effective_dims >= 50)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread baybe/surrogates/gaussian_process/presets/edbo_smoothed.py
Comment thread baybe/surrogates/gaussian_process/components/kernel.py Outdated
Comment thread tests/test_gp.py Outdated
Comment thread streamlit/surrogate_models.py
Comment thread CHANGELOG.md Outdated
Comment thread tests/test_gp.py
Comment thread baybe/surrogates/gaussian_process/components/kernel.py Outdated
Comment thread tests/test_gp.py Outdated
Comment thread streamlit/surrogate_models.py Outdated
Comment thread baybe/surrogates/gaussian_process/components/_gpytorch.py
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 24, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The streamlit changes should be a separate PR as they have nothing to do with new presets. I do see the value in extending the streamlit but those two things should still not be combined imo.

Comment thread CHANGELOG.md Outdated
Comment thread baybe/surrogates/gaussian_process/components/_gpytorch.py
Comment thread baybe/surrogates/gaussian_process/presets/botorch.py
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread streamlit/surrogate_models.py
Comment thread baybe/surrogates/gaussian_process/presets/botorch.py Outdated
@AVHopp AVHopp added this to the 0.15.0 milestone Apr 27, 2026
@AdrianSosic AdrianSosic force-pushed the feature/botorch_preset branch from eeaa63d to 7bb590b Compare April 27, 2026 15:45
Copy link
Copy Markdown
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two bugs or uncatched errors in the streamlit

Comment thread streamlit/surrogate_models.py Outdated
Comment thread streamlit/surrogate_models.py
Comment thread streamlit/surrogate_models.py
Copy link
Copy Markdown
Collaborator

@Scienfitz Scienfitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sneaky global change to mll is not acceptable

Comment thread baybe/surrogates/gaussian_process/presets/botorch.py Outdated
Comment thread tests/test_gp.py Outdated
Comment thread streamlit/surrogate_models.py
@AdrianSosic AdrianSosic force-pushed the feature/botorch_preset branch 6 times, most recently from 7ac4c25 to 9b90df2 Compare May 8, 2026 09:12
@AdrianSosic AdrianSosic force-pushed the feature/botorch_preset branch 3 times, most recently from 71b8c2b to 5c1f0c9 Compare May 18, 2026 09:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

baybe/surrogates/gaussian_process/core.py:193

  • fit_criterion_factory changed its init alias from criterion_or_factory to fit_criterion_or_factory, which is a breaking change for any callers still using the old keyword. If this is intended, consider keeping criterion_or_factory as a deprecated alias (or otherwise providing a compatibility shim) so existing user code/configs don’t fail on upgrade.
    fit_criterion_factory: FitCriterionFactoryProtocol = field(
        alias="fit_criterion_or_factory",
        factory=BayBEFitCriterionFactory,
        converter=partial(  # type: ignore[misc]
            to_component_factory, component_type=GPComponentType.CRITERION
        ),
        validator=is_callable(),
    )

Comment thread baybe/surrogates/gaussian_process/components/_gpytorch.py
Comment thread streamlit/surrogate_models.py Outdated
Comment thread baybe/surrogates/gaussian_process/presets/botorch.py Outdated
Comment thread baybe/surrogates/gaussian_process/presets/botorch.py Outdated
Copy link
Copy Markdown
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor things

Comment thread baybe/surrogates/gaussian_process/components/kernel.py
Comment thread baybe/surrogates/gaussian_process/presets/botorch.py
Comment thread baybe/surrogates/gaussian_process/presets/botorch.py Outdated
set(d.tolist()) if (d := task_kernel.active_dims) is not None else all_idcs
)

if not base_idcs <= allowed_base_idcs:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it allowed that they are a subset? (also for task idxs)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A a user, when I explicitly pass a kernel instead of relying to the defaults, I can restrict that kernel to any active dimensions. As long as the numeric part references any subset of the numeric parameters, I'm fine. Same for the task kernel. But the factory has no direct control over this, so all we can do is validate. Makes sense?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense. I think I was thinking for some reason this is checking the "overall" / "final" kernel. There we would expect that each comp column has a kernel associated, right?

MEAN_FACTORY = BotorchMeanFactory()
LIKELIHOOD_FACTORY = BotorchLikelihoodFactory()
# Botorch dictates no specific criterion, so we fill the preset with our default
FIT_CRITERION_FACTORY = BayBEFitCriterionFactory()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arguing that the fit_gpytorch_mll offers a generic interface and hence has no default is omitting that botorch uses MLL for all its examples and code - probably also in the paper. I think its fair to say the BOTORCH fit criterion is MLL

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah sure, but what do you expect me to do? remove the comment? Can do, if you prefer

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems I need to rephrase my comment: botorch uses the MLL in its examples, also for TL. Subsequently your assignment of the baybe factory for the likelihood is not correct as this would be only correct if we had no evidence of what botorch uses for TL

Comment thread tests/test_gp.py

active_settings.random_seed = 1337
gp = GaussianProcessSurrogate.from_preset(
"BOTORCH", fit_criterion_or_factory=FitCriterion.MARGINAL_LOG_LIKELIHOOD
Copy link
Copy Markdown
Collaborator

@Scienfitz Scienfitz May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pending discussion: should MLL be set by the botorch preset?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MLL is set by the botorch preset – for the single-task case. The question is rather: how do want the multi-task case to execute and/or do we need to adjust what botorch sets for this case? From the test perspective, everything works as long as the botorch landing page code (which is executed by the test) uses the same criterion as the preset. But we can change either of the two

@AdrianSosic AdrianSosic force-pushed the feature/botorch_preset branch from baa28f7 to 7996e1e Compare May 18, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev new feature New functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants