first go at extending the GibbsKernel to work with other base kernels

[Cyberface]

Summary @st-- in issue https://github.com/JuliaGaussianProcesses/KernelFunctions.jl/issues/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

• adding a new function argument base_kernel which is by default the SqExponentialKernel.

What alternatives have you considered?

Breaking changes

[codecov[bot]]

Codecov Report

Merging #406 (7d8607c) into master (33d64d1) will decrease coverage by 22.34%. The diff coverage is 0.00%.

@@             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     

Impacted Files	Coverage Δ
src/kernels/gibbskernel.jl	0.00% <0.00%> (-88.89%)	:arrow_down:
src/matrix/kernelkroneckermat.jl	0.00% <0.00%> (-100.00%)	:arrow_down:
src/approximations/nystrom.jl	0.00% <0.00%> (-91.90%)	:arrow_down:
src/kernels/normalizedkernel.jl	0.00% <0.00%> (-82.36%)	:arrow_down:
src/kernels/neuralkernelnetwork.jl	0.00% <0.00%> (-77.56%)	:arrow_down:
src/kernels/overloads.jl	25.00% <0.00%> (-75.00%)	:arrow_down:
src/matrix/kernelpdmat.jl	0.00% <0.00%> (-72.73%)	:arrow_down:
src/kernels/kernelsum.jl	29.16% <0.00%> (-62.50%)	:arrow_down:
src/kernels/kernelproduct.jl	29.16% <0.00%> (-58.34%)	:arrow_down:
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 33d64d1...7d8607c. Read the comment docs.

[devmotion]

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)?

[st--]

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.

[st--]

We can't pass additional arguments to kernel(x, y), so implementation-wise the base kernel would have to be a field of kernel.

:+1: