-
Couldn't load subscription status.
- Fork 41
first go at extending the GibbsKernel to work with other base kernels #406
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
first go at extending the GibbsKernel to work with other base kernels #406
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## master #406 +/- ## =========================================== - Coverage 89.23% 66.89% -22.35% =========================================== Files 52 52 Lines 1198 1193 -5 =========================================== - Hits 1069 798 -271 - Misses 129 395 +266 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
We can't pass additional arguments to kernel(x, y), so implementation-wise the base kernel would have to be a field of kernel.
More generally, I wonder though if this generalization is mathematically. IIRC we had a discussion in the GibbsKernel PR in whixh @theogf and/or @willtebbutt were worried that allowing eg different metrics would already be problematic. I wasn't sure and didn't spend more thought on it but if this would be the case, wouldn't it be even more problematic to allow general kernels (which also allow you to pass a Gaussian kernel wirh a different metric)?
As discussed in reference [4] of #372 and Paciorek's PhD thesis, I believe "stationary" is sufficient criterion (i.e. kernels that are implemented by a kappa of a scalar argument), assuming that the base kernel is positive definite in any dimension (I don't fully understand why the second part wouldn't just be trivial, but I'm assuming it's got something to do with the metric being valid in higher dimensions?) - in any case it seems that we'd have to somehow define "this kernel is stationary" - @willtebbutt suggested a trait, i.e. we need definitions isstationary(::Kernel) = nothing and isstationary(::SqExponentialKernel) = true etc. Then it should be possible to have this for arbitrary base kernels.
We can't pass additional arguments to
kernel(x, y), so implementation-wise the base kernel would have to be a field ofkernel.
👍
Summary
@st-- in issue #372 suggested modifying the GibbsKernel to take as an argument a base kernel function.
This is a first cut at trying to implement this to get the conversation going. I'm sure I haven't implemented it correctly and I have changed any docstrings yet.
Proposed changes
base_kernelwhich is by default theSqExponentialKernel.What alternatives have you considered?
Breaking changes