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

Custom kernel priors #219

Merged
merged 5 commits into from
Apr 29, 2024
Merged

Custom kernel priors #219

merged 5 commits into from
Apr 29, 2024

Conversation

AVHopp
Copy link
Collaborator

@AVHopp AVHopp commented Apr 25, 2024

This PR introduces custom priors for kernels. It provides an ABC Prior as well as a GammaPrior class and corresponding tests.

Note that this PR does not yet introduce custom ScaleKernels and only adjusts the priors at the point where they are used for our custom MaternKernels. At other parts of the code, we technically still use the original GPyTorch priors, which is a bit hidden by the .to_gyptorch() call. This will all be fixed in upcoming PRs.

Further note that the kernel serialization currently fails. @AdrianSosic and I already briefly investigated the error and did not manage to track it down, and agreed to investigate this issue once the code is pushed (which is the case now)

@AVHopp AVHopp self-assigned this Apr 25, 2024
@AVHopp AVHopp added the enhancement Expand / change existing functionality label Apr 25, 2024
CHANGELOG.md Show resolved Hide resolved
Copy link
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.

first round of quick comments

CHANGELOG.md Outdated Show resolved Hide resolved
baybe/kernels/priors/base.py Show resolved Hide resolved
baybe/kernels/priors/gamma.py Outdated Show resolved Hide resolved
baybe/kernels/priors/gamma.py Outdated Show resolved Hide resolved
baybe/surrogates/gaussian_process.py Show resolved Hide resolved
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

Hi @AVHopp, here my current feedback. Thanks for drafting 👍🏼

CHANGELOG.md Outdated Show resolved Hide resolved
baybe/kernels/base.py Outdated Show resolved Hide resolved
baybe/kernels/base.py Outdated Show resolved Hide resolved
baybe/kernels/priors/basic.py Outdated Show resolved Hide resolved
baybe/kernels/priors/basic.py Outdated Show resolved Hide resolved
baybe/kernels/priors/basic.py Outdated Show resolved Hide resolved
baybe/kernels/priors/basic.py Outdated Show resolved Hide resolved
baybe/surrogates/gaussian_process.py Outdated Show resolved Hide resolved
baybe/surrogates/gaussian_process.py Show resolved Hide resolved
tests/hypothesis_strategies/priors.py Show resolved Hide resolved
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

@AVHopp: I think the PR is now ready. Could you do a quick rebase to squash all the little fixup commits and then we can merge (unless @Scienfitz has more comments, of course) 🚀

baybe/kernels/priors/basic.py Outdated Show resolved Hide resolved
baybe/surrogates/gaussian_process.py Outdated Show resolved Hide resolved
tests/serialization/test_prior_serialization.py Outdated Show resolved Hide resolved
@AVHopp AVHopp force-pushed the feature/kernel_priors branch from a67d6a7 to 615b1d9 Compare April 26, 2024 13:00
CHANGELOG.md Show resolved Hide resolved
@AVHopp AVHopp force-pushed the feature/kernel_priors branch from 9bc864b to a8cf2a8 Compare April 29, 2024 12:51
@AVHopp AVHopp merged commit 6f2c432 into main Apr 29, 2024
10 checks passed
@AVHopp AVHopp deleted the feature/kernel_priors branch April 29, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Expand / change existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants