Skip to content

Deprecate implicit obsdim keyword argument #429

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

Merged
merged 15 commits into from
Jan 31, 2022
Merged
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "KernelFunctions"
uuid = "ec8451be-7e33-11e9-00cf-bbf324bd1392"
version = "0.10.28"
version = "0.10.29"

[deps]
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
Expand Down
8 changes: 3 additions & 5 deletions docs/src/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,9 @@ In short: many people like matrices, and are familiar with `obsdim`-style keywor
arguments.

All internals are implemented using `AbstractVector`s though, and the `obsdim` interface
is just a thin layer of utility functionality which sits on top of this.




is just a thin layer of utility functionality which sits on top of this. To avoid
confusion and silent errors, we do not favour a specific convention (rows or columns)
but instead it is necessary to specify the `obsdim` keyword argument explicitly.

## [Kernels for Multiple-Outputs](@id inputs_for_multiple_outputs)

Expand Down
36 changes: 29 additions & 7 deletions src/approximations/nystrom.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ function sampleindex(X::AbstractVector, r::Real)
return S
end

@deprecate sampleindex(X::AbstractMatrix, r::Real; obsdim::Integer=defaultobs) sampleindex(
vec_of_vecs(X; obsdim=obsdim), r
) false
@deprecate sampleindex(
X::AbstractMatrix, r::Real; obsdim::Union{Integer,Nothing}=defaultobs
) sampleindex(vec_of_vecs(X; obsdim=obsdim === nothing ? nothing : Int(obsdim)), r) false
Copy link
Member

Choose a reason for hiding this comment

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

What's the benefit of the ternary ? : over just

Suggested change
) sampleindex(vec_of_vecs(X; obsdim=obsdim === nothing ? nothing : Int(obsdim)), r) false
) sampleindex(vec_of_vecs(X; obsdim=obsdim), r) false

?

Copy link
Member

Choose a reason for hiding this comment

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

is it because here it was Integer whereas vec_of_vecs requires more specific Int?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, exactly. The current deprecation is not correct since it does not handle non-Int Integers. It doesn't matter much since we're allowed to remove it any time but since I had to change it anyway I fixed it.


function nystrom_sample(k::Kernel, X::AbstractVector, S::AbstractVector{<:Integer})
Xₘ = @view X[S]
Expand All @@ -21,8 +21,11 @@ function nystrom_sample(k::Kernel, X::AbstractVector, S::AbstractVector{<:Intege
end

@deprecate nystrom_sample(
k::Kernel, X::AbstractMatrix, S::Vector{<:Integer}; obsdim::Integer=defaultobs
) nystrom_sample(k, vec_of_vecs(X; obsdim=obsdim), S) false
k::Kernel,
X::AbstractMatrix,
S::Vector{<:Integer};
obsdim::Union{Integer,Nothing}=defaultobs,
) nystrom_sample(k, vec_of_vecs(X; obsdim=obsdim === nothing ? nothing : Int(obsdim)), S) false

function nystrom_pinv!(Cs::Matrix{T}, tol::T=eps(T) * size(Cs, 1)) where {T<:Real}
# Compute eigendecomposition of sampled component of K
Expand Down Expand Up @@ -97,13 +100,32 @@ function nystrom(k::Kernel, X::AbstractVector, r::Real)
return nystrom(k, X, S)
end

"""
nystrom(k::Kernel, X::AbstractMatrix, S::AbstractVector{<:Integer}; obsdim)

If `obsdim=1`, equivalent to `nystrom(k, RowVecs(X), S)`.
If `obsdim=2`, equivalent to `nystrom(k, ColVecs(X), S)`.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
function nystrom(
k::Kernel, X::AbstractMatrix, S::AbstractVector{<:Integer}; obsdim::Int=defaultobs
k::Kernel,
X::AbstractMatrix,
S::AbstractVector{<:Integer};
obsdim::Union{Int,Nothing}=defaultobs,
)
return nystrom(k, vec_of_vecs(X; obsdim=obsdim), S)
end

function nystrom(k::Kernel, X::AbstractMatrix, r::Real; obsdim::Int=defaultobs)
"""
nystrom(k::Kernel, X::AbstractMatrix, r::Real; obsdim)

If `obsdim=1`, equivalent to `nystrom(k, RowVecs(X), r)`.
If `obsdim=2`, equivalent to `nystrom(k, ColVecs(X), r)`.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
function nystrom(k::Kernel, X::AbstractMatrix, r::Real; obsdim::Union{Int,Nothing}=nothing)
return nystrom(k, vec_of_vecs(X; obsdim=obsdim), r)
end

Expand Down
61 changes: 34 additions & 27 deletions src/matrix/kernelmatrix.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@
In-place version of [`kernelmatrix`](@ref) where pre-allocated matrix `K` will be
overwritten with the kernel matrix.

kernelmatrix!(K::AbstractMatrix, κ::Kernel, X::AbstractMatrix; obsdim::Integer=2)
kernelmatrix!(K::AbstractMatrix, κ::Kernel, X::AbstractMatrix; obsdim)
kernelmatrix!(
K::AbstractMatrix,
κ::Kernel,
X::AbstractMatrix,
Y::AbstractMatrix;
obsdim::Integer=2,
obsdim,
)

Equivalent to `kernelmatrix!(K, κ, ColVecs(X))` and
`kernelmatrix(K, κ, ColVecs(X), ColVecs(Y))` respectively.
Set `obsdim=1` to get `RowVecs`.
If `obsdim=1`, equivalent to `kernelmatrix!(K, κ, RowVecs(X))` and
`kernelmatrix(K, κ, RowVecs(X), RowVecs(Y))`, respectively.
If `obsdim=2`, equivalent to `kernelmatrix!(K, κ, ColVecs(X))` and
`kernelmatrix(K, κ, ColVecs(X), ColVecs(Y))`, respectively.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
Expand All @@ -35,12 +36,13 @@ Compute the kernel `κ` for each pair of inputs in `x` and `y`.
Returns a matrix of size `(length(x), length(y))` satisfying
`kernelmatrix(κ, x, y)[p, q] == κ(x[p], y[q])`.

kernelmatrix(κ::Kernel, X::AbstractMatrix; obsdim::Int=2)
kernelmatrix(κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim::Int=2)
kernelmatrix(κ::Kernel, X::AbstractMatrix; obsdim)
kernelmatrix(κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim)

Equivalent to `kernelmatrix(κ, ColVecs(X))` and `kernelmatrix(κ, ColVecs(X), ColVecs(Y))`
respectively.
Set `obsdim=1` to get `RowVecs`.
If `obsdim=1`, equivalent to `kernelmatrix(κ, RowVecs(X))` and
`kernelmatrix(κ, RowVecs(X), RowVecs(Y))`, respectively.
If `obsdim=2`, equivalent to `kernelmatrix(κ, ColVecs(X))` and
`kernelmatrix(κ, ColVecs(X), ColVecs(Y))`, respectively.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
Expand All @@ -52,18 +54,19 @@ kernelmatrix

In place version of [`kernelmatrix_diag`](@ref).

kernelmatrix_diag!(K::AbstractVector, κ::Kernel, X::AbstractMatrix; obsdim::Int=2)
kernelmatrix_diag!(K::AbstractVector, κ::Kernel, X::AbstractMatrix; obsdim)
kernelmatrix_diag!(
K::AbstractVector,
κ::Kernel,
X::AbstractMatrix,
Y::AbstractMatrix;
obsdim::Int=2,
obsdim
)

Equivalent to `kernelmatrix_diag!(K, κ, ColVecs(X))` and
`kernelmatrix_diag!(K, κ, ColVecs(X), ColVecs(Y))` respectively.
Set `obsdim=1` to get `RowVecs`.
If `obsdim=1`, equivalent to `kernelmatrix_diag!(K, κ, RowVecs(X))` and
`kernelmatrix_diag!(K, κ, RowVecs(X), RowVecs(Y))`, respectively.
If `obsdim=2`, equivalent to `kernelmatrix_diag!(K, κ, ColVecs(X))` and
`kernelmatrix_diag!(K, κ, ColVecs(X), ColVecs(Y))`, respectively.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
Expand All @@ -79,11 +82,13 @@ Compute the diagonal of `kernelmatrix(κ, x)` efficiently.
Compute the diagonal of `kernelmatrix(κ, x, y)` efficiently.
Requires that `x` and `y` are the same length.

kernelmatrix_diag(κ::Kernel, X::AbstractMatrix; obsdim::Int=2)
kernelmatrix_diag(κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim::Int=2)
kernelmatrix_diag(κ::Kernel, X::AbstractMatrix; obsdim)
kernelmatrix_diag(κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim)

Equivalent to `kernelmatrix_diag(κ, ColVecs(X))` and
`kernelmatrix_diag(κ, ColVecs(X), ColVecs(Y))` respectively.
If `obsdim=1`, equivalent to `kernelmatrix_diag(κ, RowVecs(X))` and
`kernelmatrix_diag(κ, RowVecs(X), RowVecs(Y))`, respectively.
If `obsdim=2`, equivalent to `kernelmatrix_diag(κ, ColVecs(X))` and
`kernelmatrix_diag(κ, ColVecs(X), ColVecs(Y))`, respectively.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
Expand Down Expand Up @@ -162,10 +167,10 @@ end
# Wrapper methods for AbstractMatrix inputs to maintain obsdim interface.
#

const defaultobs = 2
const defaultobs = nothing

function kernelmatrix!(
K::AbstractMatrix, κ::Kernel, X::AbstractMatrix; obsdim::Int=defaultobs
K::AbstractMatrix, κ::Kernel, X::AbstractMatrix; obsdim::Union{Int,Nothing}=defaultobs
)
return kernelmatrix!(K, κ, vec_of_vecs(X; obsdim=obsdim))
end
Expand All @@ -175,12 +180,12 @@ function kernelmatrix!(
κ::Kernel,
X::AbstractMatrix,
Y::AbstractMatrix;
obsdim::Int=defaultobs,
obsdim::Union{Int,Nothing}=defaultobs,
)
return kernelmatrix!(K, κ, vec_of_vecs(X; obsdim=obsdim), vec_of_vecs(Y; obsdim=obsdim))
end

function kernelmatrix(κ::Kernel, X::AbstractMatrix; obsdim::Int=defaultobs)
function kernelmatrix(κ::Kernel, X::AbstractMatrix; obsdim::Union{Int,Nothing}=defaultobs)
return kernelmatrix(κ, vec_of_vecs(X; obsdim=obsdim))
end

Expand All @@ -189,7 +194,7 @@ function kernelmatrix(κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim=d
end

function kernelmatrix_diag!(
K::AbstractVector, κ::Kernel, X::AbstractMatrix; obsdim::Int=defaultobs
K::AbstractVector, κ::Kernel, X::AbstractMatrix; obsdim::Union{Int,Nothing}=defaultobs
)
return kernelmatrix_diag!(K, κ, vec_of_vecs(X; obsdim=obsdim))
end
Expand All @@ -199,19 +204,21 @@ function kernelmatrix_diag!(
κ::Kernel,
X::AbstractMatrix,
Y::AbstractMatrix;
obsdim::Int=defaultobs,
obsdim::Union{Int,Nothing}=defaultobs,
)
return kernelmatrix_diag!(
K, κ, vec_of_vecs(X; obsdim=obsdim), vec_of_vecs(Y; obsdim=obsdim)
)
end

function kernelmatrix_diag(κ::Kernel, X::AbstractMatrix; obsdim::Int=defaultobs)
function kernelmatrix_diag(
κ::Kernel, X::AbstractMatrix; obsdim::Union{Int,Nothing}=defaultobs
)
return kernelmatrix_diag(κ, vec_of_vecs(X; obsdim=obsdim))
end

function kernelmatrix_diag(
κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim::Int=defaultobs
κ::Kernel, X::AbstractMatrix, Y::AbstractMatrix; obsdim::Union{Int,Nothing}=defaultobs
)
return kernelmatrix_diag(
κ, vec_of_vecs(X; obsdim=obsdim), vec_of_vecs(Y; obsdim=obsdim)
Expand Down
17 changes: 12 additions & 5 deletions src/matrix/kernelpdmat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,13 @@ using .PDMats: PDMat
export kernelpdmat

"""
kernelpdmat(k::Kernel, X::AbstractMatrix; obsdim::Int=2)
kernelpdmat(k::Kernel, X::AbstractVector)

Compute a positive-definite matrix in the form of a `PDMat` matrix (see [PDMats.jl](https://github.com/JuliaStats/PDMats.jl)),
with the Cholesky decomposition precomputed.
The algorithm adds a diagonal "nugget" term to the kernel matrix which is increased until positive
definiteness is achieved. The algorithm gives up with an error if the nugget becomes larger than 1% of the largest value in the kernel matrix.
"""
function kernelpdmat(κ::Kernel, X::AbstractMatrix; obsdim::Int=defaultobs)
return kernelpdmat(κ, vec_of_vecs(X; obsdim=obsdim))
end

function kernelpdmat(κ::Kernel, X::AbstractVector)
K = kernelmatrix(κ, X)
Kmax = maximum(K)
Expand All @@ -31,3 +26,15 @@ function kernelpdmat(κ::Kernel, X::AbstractVector)
end
return PDMat(K + α * I)
end

"""
kernelpdmat(k::Kernel, X::AbstractMatrix; obsdim)

If `obsdim=1`, equivalent to `kernelpdmat(k, RowVecs(X))`.
If `obsdim=2`, equivalent to `kernelpdmat(k, ColVecs(X))`.

See also: [`ColVecs`](@ref), [`RowVecs`](@ref)
"""
function kernelpdmat(κ::Kernel, X::AbstractMatrix; obsdim::Union{Int,Nothing}=defaultobs)
return kernelpdmat(κ, vec_of_vecs(X; obsdim=obsdim))
end
26 changes: 21 additions & 5 deletions src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,28 @@ macro check_args(K, param, cond, desc=string(cond))
end
end

function vec_of_vecs(X::AbstractMatrix; obsdim::Int=2)
@assert obsdim ∈ (1, 2) "obsdim should be 1 or 2, see docs of kernelmatrix"
if obsdim == 1
RowVecs(X)
function deprecated_obsdim(obsdim::Union{Int,Nothing})
_obsdim = if obsdim === nothing
Base.depwarn(
"implicit `obsdim=2` argument is deprecated and now has to be passed " *
"explicitly to specify that each column corresponds to one observation",
:vec_of_vecs,
)
2
else
obsdim
end
return _obsdim
end

function vec_of_vecs(X::AbstractMatrix; obsdim::Union{Int,Nothing}=nothing)
_obsdim = deprecated_obsdim(obsdim)
if _obsdim == 1
return RowVecs(X)
elseif _obsdim == 2
return ColVecs(X)
else
ColVecs(X)
throw(ArgumentError("`obsdim` keyword argument should be 1 or 2"))
end
end

Expand Down
4 changes: 4 additions & 0 deletions test/approximations/nystrom.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@
@test kernelmatrix(k, X; obsdim=obsdim) ≈
kernelmatrix(nystrom(k, X, collect(1:dims[obsdim]); obsdim=obsdim))
end
@test kernelmatrix(@test_deprecated(nystrom(k, X, 1.0))) ≈
kernelmatrix(nystrom(k, ColVecs(X), 1.0))
@test kernelmatrix(@test_deprecated(nystrom(k, X, 1:5))) ≈
kernelmatrix(nystrom(k, ColVecs(X), 1:5))
end
2 changes: 1 addition & 1 deletion test/basekernels/fbm.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
v1 = rand(rng, 3)
v2 = rand(rng, 3)
@test k(v1, v2) ≈
(
(
sqeuclidean(v1, zero(v1))^h + sqeuclidean(v2, zero(v2))^h -
sqeuclidean(v1 - v2, zero(v1 - v2))^h
) / 2 atol = 1e-5
Expand Down
2 changes: 1 addition & 1 deletion test/basekernels/matern.jl
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
k = Matern52Kernel()
@test kappa(k, x) ≈ (1 + sqrt(5) * x + 5 / 3 * x^2)exp(-sqrt(5) * x)
@test k(v1, v2) ≈
(
(
1 + sqrt(5) * norm(v1 - v2) + 5 / 3 * norm(v1 - v2)^2
)exp(-sqrt(5) * norm(v1 - v2))
@test kappa(Matern52Kernel(), x) == kappa(k, x)
Expand Down
2 changes: 1 addition & 1 deletion test/basekernels/piecewisepolynomial.jl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
@test PiecewisePolynomialKernel(; dim=D) isa PiecewisePolynomialKernel{0}

@test repr(k) ==
"Piecewise Polynomial Kernel (degree = $(degree), ⌊dim/2⌋ = $(div(D, 2)), metric = Euclidean(0.0))"
"Piecewise Polynomial Kernel (degree = $(degree), ⌊dim/2⌋ = $(div(D, 2)), metric = Euclidean(0.0))"

k3 = PiecewisePolynomialKernel(;
degree=degree, dim=D, metric=WeightedEuclidean(ones(D))
Expand Down
4 changes: 2 additions & 2 deletions test/basekernels/wiener.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
@test k0(v1, v2) ≈ minXY
@test k1(v1, v2) ≈ 1 / 3 * minXY^3 + 1 / 2 * minXY^2 * euclidean(v1, v2)
@test k2(v1, v2) ≈
1 / 20 * minXY^5 + 1 / 12 * minXY^3 * euclidean(v1, v2) * (X + Y - 1 / 2 * minXY)
1 / 20 * minXY^5 + 1 / 12 * minXY^3 * euclidean(v1, v2) * (X + Y - 1 / 2 * minXY)
@test k3(v1, v2) ≈
1 / 252 * minXY^7 +
1 / 252 * minXY^7 +
1 / 720 *
minXY^4 *
euclidean(v1, v2) *
Expand Down
2 changes: 1 addition & 1 deletion test/kernels/gibbskernel.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@
k_gibbs = GibbsKernel(ell)

@test k_gibbs(x, y) ≈
sqrt((2 * ell(x) * ell(y)) / (ell(x)^2 + ell(y)^2)) *
sqrt((2 * ell(x) * ell(y)) / (ell(x)^2 + ell(y)^2)) *
exp(-(x - y)^2 / (ell(x)^2 + ell(y)^2))
end
4 changes: 2 additions & 2 deletions test/kernels/neuralkernelnetwork.jl
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ using KernelFunctions: NeuralKernelNetwork, LinearLayer, product, Primitive
# Vector input.
@test kernelmatrix_diag(nkn_add_kernel, x0) ≈ kernelmatrix_diag(sum_k, x0)
@test kernelmatrix_diag(nkn_add_kernel, x0, x1) ≈
kernelmatrix_diag(sum_k, x0, x1)
kernelmatrix_diag(sum_k, x0, x1)

# ColVecs input.
@test kernelmatrix_diag(nkn_add_kernel, X0) ≈ kernelmatrix_diag(sum_k, X0)
@test kernelmatrix_diag(nkn_add_kernel, X0, X1) ≈
kernelmatrix_diag(sum_k, X0, X1)
kernelmatrix_diag(sum_k, X0, X1)
end
@testset "product" begin
nkn_prod_kernel = NeuralKernelNetwork(primitives, product)
Expand Down
4 changes: 2 additions & 2 deletions test/kernels/transformedkernel.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
@test ktard(v1, v2) == (k ∘ ARDTransform(v))(v1, v2)
@test ktard(v1, v2) == k(v .* v1, v .* v2)
@test (k ∘ LinearTransform(P') ∘ ScaleTransform(s))(v1, v2) ==
((k ∘ LinearTransform(P')) ∘ ScaleTransform(s))(v1, v2) ==
(k ∘ (LinearTransform(P') ∘ ScaleTransform(s)))(v1, v2)
((k ∘ LinearTransform(P')) ∘ ScaleTransform(s))(v1, v2) ==
(k ∘ (LinearTransform(P') ∘ ScaleTransform(s)))(v1, v2)

@test repr(kt) == repr(k) * "\n\t- " * repr(ScaleTransform(s))

Expand Down
Loading