Skip to content

Use Botorch MultiTaskGP for transfer learning#549

Closed
AVHopp wants to merge 32 commits into
mainfrom
tl_benchmarking_investigation
Closed

Use Botorch MultiTaskGP for transfer learning#549
AVHopp wants to merge 32 commits into
mainfrom
tl_benchmarking_investigation

Conversation

@AVHopp

@AVHopp AVHopp commented May 7, 2025

Copy link
Copy Markdown
Collaborator

Replaces the custom IndexKernel construction with BoTorch's MultiTaskGP (which became possible due the added all_tasks argument).

@AVHopp AVHopp marked this pull request as draft May 7, 2025 07:40
@AVHopp AVHopp changed the title Tl benchmarking investigation Use Botorch MultiTaskGP for transfer learning May 7, 2025
@Hrovatin Hrovatin force-pushed the tl_benchmarking_investigation branch 2 times, most recently from 8fee382 to 88e1dfe Compare June 4, 2025 11:18
@Hrovatin Hrovatin marked this pull request as ready for review June 5, 2025 10:39

@AdrianSosic AdrianSosic left a comment

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.

Hi @Hrovatin, here the first batch of comments

Comment thread CHANGELOG.md Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread tests/test_transfer_learning.py Outdated
Comment thread tests/test_transfer_learning.py Outdated
@Hrovatin Hrovatin requested a review from AdrianSosic June 6, 2025 14:40
@Scienfitz

Scienfitz commented Aug 15, 2025

Copy link
Copy Markdown
Collaborator

@Hrovatin would you consider abandoning this PR? I think if this topic is picked up again its better to start afresh (and only open a PR after investigations have concluded).

@Hrovatin

Copy link
Copy Markdown
Collaborator

@Scienfitz I would keep open as the main blocker for this was randomness in benchmarks. Since that may be solved now I would suggest running benchmarks again on the new HPC (need to confirm it is also reproducible there)

@Scienfitz

Copy link
Copy Markdown
Collaborator

@Hrovatin any update?

@Hrovatin

Hrovatin commented Sep 9, 2025

Copy link
Copy Markdown
Collaborator

No, I need to first set up testing on oneHPC to reproducibly benchmark - as that seems to be the only option to make fully reproducible. I will post update here once I have the results @Scienfitz

Copilot AI review requested due to automatic review settings September 12, 2025 11:22
@Hrovatin Hrovatin force-pushed the tl_benchmarking_investigation branch from 8ce5fba to bee32aa Compare September 12, 2025 11:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Hrovatin

Hrovatin commented Sep 17, 2025

Copy link
Copy Markdown
Collaborator

@AdrianSosic @Scienfitz @AVHopp Update on the comparison of MultiTask GP from botorch and current kernel:

  • The results are not identical, but very close, except for michaelewicz (but it seems that variation is likely not significant here as well)
  • A concern: When using botorch multitask gp the hartman tl benchmark always fails due to ooo (when using at 0.05 but not 0.01 source data). I have not yet figured out why. Before investigating this we should probably make a call if we are ok with accepting some deviation from current main (named benchmarks-reproducibility-beforeBug on the plot) or not as if we decide we need 100% reproducibility anyways it also does not make sense to investigate any other issues further.
image

@AVHopp AVHopp left a comment

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.

First round of comments, but we should discuss some of the points (in particular the one regarding multiple active values) internally first.

Comment thread baybe/parameters/categorical.py
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
@Hrovatin Hrovatin force-pushed the tl_benchmarking_investigation branch from de81707 to 68a9c24 Compare September 25, 2025 07:13

@AVHopp AVHopp left a comment

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.

Would be willing to approve - however, since this is technically my PR I can't

Comment thread .github/workflows/benchmark.yml Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
Comment thread tests/test_transfer_learning.py Outdated
Comment thread tests/test_transfer_learning.py
Comment thread tests/test_transfer_learning.py Outdated
Comment thread baybe/surrogates/gaussian_process/core.py Outdated
@Hrovatin

Hrovatin commented Oct 2, 2025

Copy link
Copy Markdown
Collaborator

Results after rebase:
Note:

  • Hartmann tl did not run for the new branch due to issues in local setup (shows only the main branch). But I tested that it runs successfully in actions
  • Reproducibility is in general not 100% (also when not using the tl code that was changed)
image

Comment thread .github/workflows/benchmark.yml Outdated
@AdrianSosic AdrianSosic force-pushed the tl_benchmarking_investigation branch 4 times, most recently from 5cfb366 to 7bb49d9 Compare October 6, 2025 08:50
Hrovatin and others added 20 commits November 27, 2025 09:22
Co-authored-by: Alexander V. Hopp <alexander.hopp@merckgroup.com>
Co-authored-by: Alexander V. Hopp <alexander.hopp@merckgroup.com>
The active_dims argument can now be dropped due to #671
Unfortunately, previous botorch version have an (unnecessary?)
hard pin for gpytorch on version 1.14, causing troubles with
other tests due to the following issue, which has only be fixed
in 1.14.1:
cornellius-gp/gpytorch#2633
Does not solve the problem since there is still a failing example
@AdrianSosic AdrianSosic force-pushed the tl_benchmarking_investigation branch from 9f060b9 to ace40f7 Compare November 27, 2025 08:23
@AdrianSosic

Copy link
Copy Markdown
Collaborator

@copilot: Explain the reason for the CI failure

Copilot AI commented Nov 28, 2025

Copy link
Copy Markdown

@AdrianSosic I've opened a new pull request, #703, to work on those changes. Once the pull request is ready, I'll request review from you.

@AdrianSosic AdrianSosic force-pushed the tl_benchmarking_investigation branch from 4881367 to 4f46223 Compare November 28, 2025 10:24
@AdrianSosic

Copy link
Copy Markdown
Collaborator

Closed in favor of #743. In particular, we don't switch to MultiTaskGP because we do not want to adopt BoTorch's latest decisions on how to handle incoming task data (see validate_task_values argument). Staying with SingleTaskGP keeps full flexibility (also for implementing other transfer mechanisms that do not involve IndexKernel) and allows us to keep our GP class as a slim / general purpose skeleton that implements the core logic of a GP.

@AVHopp AVHopp deleted the tl_benchmarking_investigation branch April 23, 2026 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants