Skip to content

Add kappa to stationary Gabor kernel #79

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Add kappa to stationary Gabor kernel #79

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7bf3e68
Add stationary kappa to stationary gabor kernel
sharanry Apr 10, 2020
6276381
Merge branch 'master' into gabor_fix
sharanry Apr 17, 2020
b9cb30a
Fix style issues in gabor tests
sharanry Apr 17, 2020
42ce46f
Revert Base.show for Gabor to master version
sharanry Apr 17, 2020
2560647
Fix bug with kappa
sharanry Apr 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/basekernels/gabor.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

Gabor kernel with length scale ell and period p. Given by
```math
κ(x,y) = h(x-z), h(t) = exp(-sum(t.^2./(ell.^2)))*cos(pi*sum(t./p))
κ(x,y) = h(x-z), h(t) = exp(-sum(t.^2./(ell.^2)))*cos(pi*sum(t./p))
```

"""
Expand Down Expand Up @@ -51,9 +51,13 @@ function Base.getproperty(k::GaborKernel, v::Symbol)
end
end

Base.show(io::IO, κ::GaborKernel) = print(io, "Gabor Kernel (ell = ", κ.ell, ", p = ", κ.p, ")")
function kappa(κ::GaborKernel, d::Real)
return kappa(κ.kernel.kernels[1], d^2) * kappa(κ.kernel.kernels[2], d)
end

kappa(κ::GaborKernel, x, y) = kappa(κ.kernel, x, y)
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this? Can't we just use the generic implementation in

KernelFunctions.jl/src/generic.jl

Line 7 in 89baf9c

kappa(κ::Kernel, x, y) = kappa(κ, evaluate(metric(κ), x, y))
now that a metric is defined for the Gabor kernel?

Suggested change
kappa(κ::GaborKernel, x, y) = kappa(κ.kernel, x, y)

Copy link
Contributor Author

@sharanry sharanry Apr 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I was wondering why without this the test kappa(k, x, y) == kappa(k, evaluate(KernelFunctions.metric(k), x, y)) was failing without this line. There was an error in the kappa(::GaborKernel, d::Real).

Apparently, kappa(k::TransformedKernel, d) = kappa(k.kernel, d), so it doesn't do the transformation before applying the kernel using the distance measure. I think this is a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Present fix is to modify the kappa to give the intended output of gabor kernel

function kappa(κ::GaborKernel, d::Real)
    return kappa(κ.kernel.kernels[1], (d/κ.ell)^2) * kappa(κ.kernel.kernels[2], d/κ.p)
end

Copy link
Member

Choose a reason for hiding this comment

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

That's not something we want to do since ell and p do not have to be scalar valued. It seems it just doesn't make sense to define kappa(::GaborKernel, ::Real) and metric(::GaborKernel), and we should just keep the old implementation of forwarding kappa to the underlying kernel? If we want to do any optimization based on the metric, it should be done in KernelProd (see the PR for its refactoring).


kappa(κ::GaborKernel, x, y) = kappa(κ.kernel, x ,y)
metric(::GaborKernel) = Euclidean()

function kernelmatrix(
κ::GaborKernel,
Expand All @@ -76,3 +80,6 @@ function kerneldiagmatrix(
obsdim::Int=defaultobs) #TODO Add test
kerneldiagmatrix(κ.kernel, X; obsdim=obsdim)
end

Base.show(io::IO, κ::GaborKernel) = print(io, "Gabor Kernel (ell = ", κ.ell, ", p = ", κ.p, ")")

8 changes: 6 additions & 2 deletions test/basekernels/gabor.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
k = GaborKernel(ell=ell, p=p)
@test k.ell ≈ ell atol=1e-5
@test k.p ≈ p atol=1e-5
@test kappa(k,v1,v2) ≈ exp(-sqeuclidean(v1,v2) ./(k.ell.^2))*cospi(euclidean(v1,v2)./ k.p) atol=1e-5
@test kappa(k,v1,v2) ≈ kappa(transform(SqExponentialKernel(), 1/k.ell),v1,v2)*kappa(transform(CosineKernel(), 1/k.p), v1,v2) atol=1e-5
@test kappa(k, v1, v2) ≈ exp(-sqeuclidean(v1, v2) ./ (k.ell.^2)) *
cospi(euclidean(v1, v2) ./ k.p) atol=1e-5
@test kappa(k, v1, v2) ≈ kappa(transform(SqExponentialKernel(), 1 / k.ell),v1, v2) *
kappa(transform(CosineKernel(), 1 / k.p), v1, v2) atol=1e-5

k = GaborKernel()
@test k.ell ≈ 1.0 atol=1e-5
@test k.p ≈ 1.0 atol=1e-5

@test kappa(k, v1, v2) ≈ kappa(k, evaluate(KernelFunctions.metric(k), v1, v2))
@test repr(k) == "Gabor Kernel (ell = 1.0, p = 1.0)"
end