Skip to content

Remove _get_ν #452

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

Merged
merged 14 commits into from
May 17, 2022
Merged

Remove _get_ν #452

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c9015fa
Fix derivative of `_get_ν`
devmotion Apr 13, 2022
c4d23e8
Update Project.toml
devmotion Apr 13, 2022
08437cc
Update src/basekernels/matern.jl
devmotion Apr 13, 2022
8e4ad8f
Fix `rrule`
devmotion Apr 13, 2022
9bb7beb
Add test
devmotion Apr 13, 2022
6a467f9
Load ChainRulesTestUtils
devmotion Apr 13, 2022
b95a84a
Try to specify cotangent
devmotion Apr 13, 2022
c747f70
Update test/basekernels/matern.jl
devmotion Apr 13, 2022
aadcfa8
Update matern.jl
devmotion Apr 13, 2022
d87cc84
Update test/basekernels/matern.jl
devmotion Apr 13, 2022
e7f658b
Remove workaround
devmotion Apr 13, 2022
8473386
Update Project.toml
devmotion Apr 19, 2022
5b4e9d3
Merge branch 'master' into dw/fix_matern
devmotion Apr 19, 2022
4c45fe8
Fix test_interface
devmotion Apr 19, 2022
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
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
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.37"
version = "0.10.38"

[deps]
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
Expand Down
12 changes: 11 additions & 1 deletion src/basekernels/matern.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,18 @@ MaternKernel(; nu::Real=1.5, ν::Real=nu, metric=Euclidean()) = MaternKernel(ν,

@functor MaternKernel

# workaround for Zygote
# unclear why it's needed but it is fine since it's stated officially that we don't support differentiation with respect to ν
Copy link
Member

@st-- st-- Apr 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Without this workaround, the Zygote AD test fails. You can see this in 30385ac and the corresponding build

@inline _get_ν(k::MaternKernel) = only(k.ν)
ChainRulesCore.@non_differentiable _get_ν(k) # work-around; should be "NotImplemented" rather than NoTangent
function ChainRulesCore.rrule(::typeof(_get_ν), k::T) where {T<:MaternKernel}
function _get_ν_pullback(Δ)
dν = ChainRulesCore.@not_implemented(
"derivatives of `MaternKernel` w.r.t. order `ν` are not implemented."
)
return NoTangent(), Tangent{T}(; ν=dν, metric=NoTangent())
end
return _get_ν(k), _get_ν_pullback
end

@inline function kappa(k::MaternKernel, d::Real)
result = _matern(_get_ν(k), d)
Expand Down
2 changes: 2 additions & 0 deletions test/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[deps]
AxisArrays = "39de3d68-74b9-583c-8d2d-e117c070f3a9"
ChainRulesTestUtils = "cdddcdb0-9152-4a09-a978-84456f9df70a"
Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"
Distances = "b4f34e82-e78d-54a5-968a-f98e89d6e8f7"
Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
Expand All @@ -19,6 +20,7 @@ Zygote = "e88e6eb3-aa80-5325-afca-941959d7151f"

[compat]
AxisArrays = "0.4.3"
ChainRulesTestUtils = "1.7"
Compat = "3"
Distances = "0.10"
Documenter = "0.25, 0.26, 0.27"
Expand Down
4 changes: 4 additions & 0 deletions test/basekernels/matern.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
@test metric(k2) isa WeightedEuclidean
@test k2(v1, v2) ≈ k(v1, v2)

# Test custom `rrule` (Zygote workaround).
k = MaternKernel(; ν=rand())
test_rrule(KernelFunctions._get_ν, k ⊢ Tangent{typeof(k)}(; ν=randn(), metric=NoTangent()))

# Standardised tests.
TestUtils.test_interface(k, Float64)
test_ADs(() -> MaternKernel(; nu=ν))
Expand Down
1 change: 1 addition & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using KernelFunctions
using AxisArrays
using ChainRulesTestUtils
using Distances
using Documenter
using Functors: functor
Expand Down