Add Cosine Kernel #45

sharanry · 2020-03-15T20:22:18Z

Add Cosine kernel as described in GPML.

GPML also provides derivatives of the kernel w.r.t its hyper-parameters. Holding off on that based on the suggestion of @willtebbutt.

Related Issue #44

TODO

Add tests

willtebbutt · 2020-03-15T20:40:59Z

This is great, thanks for submitting!

I don't think the scale factor is necessary, since we can handle it in a generic way with the ScaledKernel.

I wonder also whether we could handle the length of the period via the ScaleTransform, and therefore don't even need that parameter?

src/kernels/cosine.jl

Co-Authored-By: willtebbutt <[email protected]>

Co-Authored-By: David Widmann <[email protected]>

devmotion · 2020-03-16T09:22:15Z

src/kernels/cosine.jl

+
+The cosine kernel is a stationary kernel for a sinusoidal given by
+```
+    κ(x,y) = cos( 2π * (x-y) )


Since CosineKernel uses the Cityblock distance, I guess it should rather be

Suggested change

κ(x,y) = cos( 2π * (x-y) )

κ(x, y) = cos(2π * ‖x-y‖₁)

Actually is it really the city block metric? I would expect to see the euclidean one. Actually is there a reference for this kernel? That would be nice to also have it in the documentation

I haven't been able to find a proper reference for the Cosine kernel. The best I could find was GPML's official manual pg44.

Cityblock metric is something I inferred from here.

GPML's dosctring says (x-y). Please note that cosine is an even function.

Hmmm, actually, does the Cosine kernel make sense in > 1D? i.e. if you use the CityBlock metric, are the covariance matrices it produces positive definite?

GMPL only defines the cosine kernel for one-dimensional data (also stated in the manual), and hence clearly x-y and |x-y| yield the same values due to the symmetry of cosine. I agree with @theogf that probably the most natural extension to multiple dimensions would be the Euclidean distance but unfortunately I don't have any reference at hand.

Just following up on my point above, the following suggests it doesn't make sense in higher dimensions:

using Stheno using Stheno: Euclidean import Stheno: pw pw(k::Cosine, x::ColVecs) = cos.(pi .* pw(Euclidean(), x.X) ./ k.p) x = ColVecs(randn(3, 10)) eigvals(pw(Cosine(1.0), x)) 10-element Array{Float64,1}: -2.6807114471460594 -1.3962293964372774 -0.963215712851525 -0.40797823094040286 0.9649993076661302 1.6352424163325816 1.8341656854612294 2.543664324231879 3.937344284513905 4.53271876916954

Obviously you'll get different numbers than me because seeds, but looks like it doesn't make sense.

From what I looked very quickly, the cosine kernel support is only on [0,1] in 1D, which could be expanded to the unit sphere for higher dimensions

Oh, weird. Does it not extend to the entire real line by virtue of the periodicity of cos?

So, do you suggest we extend it to multi-dim sphere?

I don't know that we have the right abstractions to handle this at the minute. I would suggest sticking with the real line so that we can continue to push forwards quickly. Maybe make a note of it on the overall issue?

theogf · 2020-03-16T21:49:39Z

I found a good reference : http://www.gaussianprocess.org/gpml/code/matlab/doc/manual.pdf
p 44

sharanry · 2020-03-20T16:32:48Z

@willtebbutt @theogf @devmotion Do we stick to CityBlock or use Euclidean? The documentation isn't very clear about it. http://www.gaussianprocess.org/gpml/code/matlab/doc/manual.pdf pg44

willtebbutt · 2020-03-20T16:34:53Z

Euclidean is probably better, because I know that we have the adjoints implemented somewhere (possibly just in Stheno at the minute)

theogf · 2020-03-20T16:36:22Z

The adjoints have been added in Zygote by @devmotion now! Le ven. 20 mars 2020 à 17:35, willtebbutt <[email protected]> a écrit :

…

Euclidean is probably better, because I know that we have the adjoints implemented somewhere (possibly just in Stheno at the minute) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/theogf/KernelFunctions.jl/pull/45#issuecomment-601795160>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACXSYAYRDB2DER7XH5LZQATRIOLL5ANCNFSM4LLK7OHQ> .

-- Théo Galy-Fajou PhD Student/Research Assistant TU Berlin https://theogf.github.io/

devmotion · 2020-03-20T17:05:53Z

src/kernels/cosine.jl

+"""
+struct CosineKernel <: BaseKernel end
+
+kappa(κ::CosineKernel, d::Real) = cos(2*pi*d)


Was just browsing through the Julia documentation and noticed that there is a function cospi that computes cos(pi*x) more accurately than the "naive" implementation, particularly for large x. I guess we could use it here?

Suggested change

kappa(κ::CosineKernel, d::Real) = cos(2*pi*d)

kappa(κ::CosineKernel, d::Real) = cospi(2*d)

I'm also wondering if we should keep the scaling with 2 here - I don't know what's commonly used since the documentation of GPML (no scaling) and its implementation (scaling) are not consistent in that point and GPyTorch does not scale by a factor of 2 (according to the docs). I guess that decision could be based on the default domain that we want to support?

I am also wondering more generally about the validity of this kernel. cf this question : https://www.quora.com/Is-cosine-kernel-as-given-in-Kernel-statistics-a-radial-basis-function-kernel
I think it is indeed only valid when whatever is inside cos is determined between -pi/2 and pi/2. Is it really psd if inputs are outside of this?

Also noted here : https://en.wikipedia.org/wiki/Kernel_(statistics)#Kernel_functions_in_common_use
Apparently it should return 0 outside

Oh I just realised that these kernels are more meant for convolution stuff. Nevermind

Does it help to stay consistent with GPyTorch?

Not really, every package has its own convention... I like pi/2 because then for a distance of 1 the kernel outputs 0

sharanry · 2020-03-23T17:13:46Z

Looks like we will need to merge this before we get to Gabor kernel #52.
@willtebbutt I have made note in the original issue #44 about extending cosine to hypersphere.

@theogf @devmotion
Do you suggest any other changes or can we merge this?

willtebbutt

LGTM. Great work!

devmotion · 2020-03-23T17:39:28Z

Looks good to me, even though I'm not sure if we've actually decided on a scaling in the discussion above? Otherwise, just update the docstring to reflect whatever we end up with, and, as mentioned above, I'd suggest using cospi(...) instead of cos(pi * ...).

sharanry · 2020-03-23T17:55:41Z

Regarding scaling, I support going with κ(x,y) = cos( π * (x-y) ) to keep consistency with the documentation of GPML and GPPytorch. If this is a problem for any user, they can always rescale. Any objections?

willtebbutt · 2020-03-23T18:04:59Z

I'm happy to merge once tests have passed. Can always re-visit at a later date if we find a particularly good reason to adopt a different convention.

sharanry · 2020-03-24T10:09:56Z

@willtebbutt Looks like the tests have passed. :)

sharanry · 2020-03-24T10:32:23Z

Thanks!

Add Cosine Kernel

65ad57b

willtebbutt reviewed Mar 15, 2020

View reviewed changes

src/kernels/cosine.jl Outdated Show resolved Hide resolved

devmotion reviewed Mar 15, 2020

View reviewed changes

src/kernels/cosine.jl Outdated Show resolved Hide resolved

devmotion reviewed Mar 15, 2020

View reviewed changes

src/kernels/cosine.jl Outdated Show resolved Hide resolved

devmotion reviewed Mar 15, 2020

View reviewed changes

src/kernels/cosine.jl Outdated Show resolved Hide resolved

sharanry and others added 5 commits March 16, 2020 11:47

Update src/kernels/cosine.jl

12e7636

Co-Authored-By: willtebbutt <[email protected]>

Update src/kernels/cosine.jl

df9e6b7

Co-Authored-By: David Widmann <[email protected]>

Modify Cosine to be parameter-less

85f13a9

Add tests

2340b8b

Update Docstring

042f872

devmotion reviewed Mar 16, 2020

View reviewed changes

Change metric to Euclidean

5f6ecbc

devmotion reviewed Mar 20, 2020

View reviewed changes

sharanry mentioned this pull request Mar 20, 2020

Add Gabor Kernel #52

Merged

willtebbutt approved these changes Mar 23, 2020

View reviewed changes

Use cospi, change scaling factor to pi and update tests.

f1fdba6

Merge branch 'master' into cosine

e0c915f

willtebbutt merged commit 0654f2f into JuliaGaussianProcesses:master Mar 24, 2020

sharanry deleted the cosine branch March 24, 2020 10:32

	κ(x,y) = cos( 2π * (x-y) )
	κ(x, y) = cos(2π * ‖x-y‖₁)

	kappa(κ::CosineKernel, d::Real) = cos(2pid)
	kappa(κ::CosineKernel, d::Real) = cospi(2*d)

Add Cosine Kernel #45

Add Cosine Kernel #45

Uh oh!

Conversation

sharanry commented Mar 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

willtebbutt commented Mar 15, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willtebbutt Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willtebbutt Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

willtebbutt Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theogf commented Mar 16, 2020

Uh oh!

sharanry commented Mar 20, 2020

Uh oh!

willtebbutt commented Mar 20, 2020

Uh oh!

theogf commented Mar 20, 2020 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharanry commented Mar 23, 2020

Uh oh!

willtebbutt left a comment

Choose a reason for hiding this comment

Uh oh!

devmotion commented Mar 23, 2020

Uh oh!

sharanry commented Mar 23, 2020

Uh oh!

willtebbutt commented Mar 23, 2020

Uh oh!

sharanry commented Mar 24, 2020

Uh oh!

sharanry commented Mar 24, 2020

Uh oh!

Uh oh!

sharanry commented Mar 15, 2020 •

edited

Loading

willtebbutt Mar 16, 2020 •

edited

Loading

willtebbutt Mar 16, 2020 •

edited

Loading

willtebbutt Mar 17, 2020 •

edited

Loading