Skip to content
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

reactivate AD tests: mean functions #313

Merged
merged 30 commits into from
Apr 10, 2022
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1a99d74
reactivate mean function AD tests
st-- Apr 6, 2022
caeffdc
format
st-- Apr 6, 2022
9f6227f
fix test
st-- Apr 6, 2022
f13c902
revert FillArray
st-- Apr 6, 2022
6ee4c76
extend mean function tests to ColVecs/RowVecs
st-- Apr 6, 2022
9c85f6d
extend ZeroMean tests too
st-- Apr 6, 2022
83ccb64
bugfix
st-- Apr 6, 2022
2989293
add missing zero() definition for ColVecs/RowVecs
st-- Apr 6, 2022
8043454
patch bump
st-- Apr 6, 2022
940777a
revert Project.toml
st-- Apr 7, 2022
f3b736c
mean function rrules
st-- Apr 7, 2022
750ef77
revert runtests.jl
st-- Apr 7, 2022
bc0ee6a
Merge branch 'master' of github.com:JuliaGaussianProcesses/AbstractGP…
st-- Apr 7, 2022
a5365d6
clean up mean_function tests without reactivating AD
st-- Apr 7, 2022
75f95cd
Merge branch 'st/meanfunctiontest' into st/ad_tests
st-- Apr 7, 2022
1b09168
unify x...=
st-- Apr 7, 2022
80b7813
Merge branch 'st/meanfunctiontest' into st/ad_tests
st-- Apr 7, 2022
37aeb0a
rename
st-- Apr 7, 2022
a7a4d8c
remove code moved into KernelFunctions
st-- Apr 7, 2022
81841c1
Merge branch 'st/ad_tests' of github.com:JuliaGaussianProcesses/Abstr…
st-- Apr 7, 2022
77f6ebf
Merge branch 'st/meanfunctiontest' into st/ad_tests
st-- Apr 8, 2022
263a56c
remove no longer needed test
st-- Apr 8, 2022
fd69656
Merge branch 'master' into st/ad_tests
st-- Apr 8, 2022
97c5284
Update src/mean_function.jl
st-- Apr 8, 2022
9e93b93
Merge branch 'master' into st/ad_tests
st-- Apr 9, 2022
01a7ac0
Apply suggestions from code review
st-- Apr 9, 2022
4b0a683
Apply suggestions from code review
st-- Apr 9, 2022
f1df8b5
unify testcases
st-- Apr 9, 2022
145091d
remove rrules and ChainRulesCore
st-- Apr 9, 2022
41a01da
pass rng
st-- Apr 9, 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
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "AbstractGPs"
uuid = "99985d1d-32ba-4be9-9821-2ec096f28918"
authors = ["JuliaGaussianProcesses Team"]
version = "0.5.11"
version = "0.5.12"

[deps]
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4"
Expand Down
11 changes: 8 additions & 3 deletions src/mean_function.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ struct ZeroMean{T<:Real} <: MeanFunction end
"""
This is an AbstractGPs-internal workaround for AD issues; ideally we would just extend Base.map
"""
_map_meanfunction(::ZeroMean{T}, x::AbstractVector) where {T} = Zeros{T}(length(x))
_map_meanfunction(::ZeroMean{T}, x::AbstractVector) where {T} = Zeros{T}(length(x))

function ChainRulesCore.rrule(::typeof(_map_meanfunction), m::ZeroMean, x::AbstractVector)
map_ZeroMean_pullback(Δ) = (NoTangent(), NoTangent(), ZeroTangent())
map_ZeroMean_pullback(Δ) = (NoTangent(), ZeroTangent(), ZeroTangent())
return _map_meanfunction(m, x), map_ZeroMean_pullback
end

Expand All @@ -30,6 +30,11 @@ end

_map_meanfunction(m::ConstMean, x::AbstractVector) = Fill(m.c, length(x))

function ChainRulesCore.rrule(::typeof(_map_meanfunction), m::ConstMean, x::AbstractVector)
map_ConstMean_pullback(Δ) = (NoTangent(), Tangent{ConstMean}(; c=sum(Δ)), ZeroTangent())
return _map_meanfunction(m, x), map_ConstMean_pullback
end
Copy link
Member

Choose a reason for hiding this comment

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

Also, was this introduced to attempt to deal with the AD testing problems?

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, that was my attempt at working around the lack of FillArray rrules/projectors by doing it directly on the mean function call... do you think it should get removed again ?


"""
CustomMean{Tf} <: MeanFunction

Expand All @@ -40,4 +45,4 @@ struct CustomMean{Tf} <: MeanFunction
f::Tf
end

_map_meanfunction(f::CustomMean, x::AbstractVector) = map(f.f, x)
_map_meanfunction(m::CustomMean, x::AbstractVector) = map(m.f, x)
16 changes: 3 additions & 13 deletions test/mean_function.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,7 @@

for x in [x1, xD_colvecs, xD_rowvecs]
@test AbstractGPs._map_meanfunction(m, x) == zeros(N)
#differentiable_mean_function_tests(m, randn(rng, N), x)

# Manually verify the ChainRule. Really, this should employ FiniteDifferences, but
# currently ChainRulesTestUtils isn't up to handling this, so this will have to do
# for now.
y, pb = rrule(AbstractGPs._map_meanfunction, m, x)
@test y == AbstractGPs._map_meanfunction(m, x)
Δmap, Δf, Δx = pb(randn(rng, N))
@test iszero(Δmap)
@test iszero(Δf)
@test iszero(Δx)
differentiable_mean_function_tests(m, randn(rng, N), x)
end
end

Expand All @@ -30,7 +20,7 @@

for x in [x1, xD_colvecs, xD_rowvecs]
@test AbstractGPs._map_meanfunction(m, x) == fill(c, N)
#differentiable_mean_function_tests(m, randn(rng, N), x)
differentiable_mean_function_tests(m, randn(rng, N), x)
end
end

Expand All @@ -40,7 +30,7 @@

for x in [x1, xD_colvecs, xD_rowvecs]
@test AbstractGPs._map_meanfunction(m, x) == map(foo_mean, x)
#differentiable_mean_function_tests(m, randn(rng, N), x)
differentiable_mean_function_tests(m, randn(rng, N), x)
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we just call

Suggested change
differentiable_mean_function_tests(m, randn(rng, N), x)
differentiable_mean_function_tests(rng, m, x)

instead (and remove the y = ...), or otherwise remove that (currently unused) method definition of differentiable_mean_function_tests?

Copy link
Member

Choose a reason for hiding this comment

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

Seems like it would simplify things a bit, so I'm in favour.

Copy link
Member Author

Choose a reason for hiding this comment

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

which one? remove the (rng, m, x) method, or apply this suggestion?

Copy link
Member

Choose a reason for hiding this comment

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

I would be inclined to retain the method that just requires a rng, since we can now be confident that it will generate an appropriate tangent due to the call to collect.

end
end
end
28 changes: 6 additions & 22 deletions test/test_util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ end
Test _very_ basic consistency properties of the mean function `m`.
"""
function mean_function_tests(m::MeanFunction, x::AbstractVector)
@test AbstractGPs._map_meanfunction(m, x) isa AbstractVector
@test length(ew(m, x)) == length(x)
mean = AbstractGPs._map_meanfunction(m, x)
@test mean isa AbstractVector
@test length(mean) == length(x)
end

"""
Expand All @@ -87,34 +88,17 @@ end
Ensure that the gradient w.r.t. the inputs of `MeanFunction` `m` are approximately correct.
"""
function differentiable_mean_function_tests(
m::MeanFunction,
ȳ::AbstractVector{<:Real},
x::AbstractVector{<:Real};
rtol=_rtol,
atol=_atol,
m::MeanFunction, ȳ::AbstractVector, x::AbstractVector; rtol=_rtol, atol=_atol
)
# Run forward tests.
mean_function_tests(m, x)

# Check adjoint.
@assert length(ȳ) == length(x)
return adjoint_test(x -> ew(m, x), ȳ, x; rtol=rtol, atol=atol)
adjoint_test(x -> collect(AbstractGPs._map_meanfunction(m, x)), ȳ, x; rtol=rtol, atol=atol)
return nothing
end

# function differentiable_mean_function_tests(
# m::MeanFunction,
# ȳ::AbstractVector{<:Real},
# x::ColVecs{<:Real};
# rtol=_rtol,
# atol=_atol,
# )
# # Run forward tests.
# mean_function_tests(m, x)

# @assert length(ȳ) == length(x)
# adjoint_test(X->ew(m, ColVecs(X)), ȳ, x.X; rtol=rtol, atol=atol)
# end

function differentiable_mean_function_tests(
rng::AbstractRNG, m::MeanFunction, x::AbstractVector; rtol=_rtol, atol=_atol
)
Expand Down