Skip to content
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

Add BoTorch kernel preset, which uses dimensions-scaled prior #483

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

Hrovatin
Copy link
Collaborator

@Hrovatin Hrovatin commented Feb 12, 2025

This can stay a draft until it is validated that the performance improves with this preset and that we even want to use it.

Changes:

  • Use base_covar_module that matches parameters from BoTorch at the time of writing.
  • As long as this is separated from using MultiTaskGP this must be pre-defined rather than using defaults to enable the use of index kernel in SingleTaskGP. -> TODO after we switch to MultiTaskGP this PR should be adapted to simply use BoTorch default rather than re-implementing custom kernel factory.

Benchmarks:

  • 4 cases as a grid of:
    • Whether using TL campaign with source data (TL) or using non-TL campaign with target data only (nonTL).
    • Optionally use Botorch preset (appended botorchPreset)
  • The nonTL line should equal some of the lines from Add benchmarks #491 branch (compare to lines named 10 (10% of source data) or TL for benchmarks where only one source data size was used). However, that is not always the case. No idea why.

@Hrovatin Hrovatin marked this pull request as draft February 12, 2025 10:32
@Hrovatin Hrovatin force-pushed the feature/botorch_kernel_preset branch 2 times, most recently from 50f75e1 to 1b46aa6 Compare February 25, 2025 07:51
Copy link
Collaborator

@AdrianSosic AdrianSosic Feb 28, 2025

Choose a reason for hiding this comment

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

Hi @Hrovatin, just two comments upfront, to save us some work later:

  • The point of the botorch preset should not be to cook up the same prior logic again in our code but to make sure that the GPs are simply called without any additional arguments. The noticable difference is that the latter automatically i) avoids any potential bugs from our end plus ii) will automatically adapt when botorch makes changes
  • The preset should not only affect the kernel priors but all priors. This is also automatically fulfilled by not passing anything to the constructor.
  • I suggest you split up the PR into two, one that brings the preset (which we want to have anyway, regardless of benchmark performance) and another that adds the benchmarks. Otherwise, this will be a ton of code to review

Cheers 🙃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah then I strongly misunderstood the aim - we should then align during the meeting next week

@Hrovatin Hrovatin force-pushed the feature/botorch_kernel_preset branch from 1b46aa6 to 22762e5 Compare March 3, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants