Skip to content

Commit 2e197b2

Browse files
devmotiondevmotion
committed
Add tests with Aqua, ExplicitImports and JET, and make them pass
1 parent 950a12c commit 2e197b2

File tree

10 files changed

+144
-50
lines changed

10 files changed

+144
-50
lines changed

‎Project.toml‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,37 @@ version = "7.11.0"
55
[deps]
66
ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"
77
DifferentiationInterface = "a0c0ee7d-e4b9-4e03-894e-1c5f64a51d63"
8-
Distributed = "8ba89e20-285c-5b6f-9357-94700520ee1b"
98
FiniteDiff = "6a86dc24-6348-571c-b903-95158fe2bd41"
109
ForwardDiff = "f6369f11-7733-5829-9624-2563aa707210"
1110
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
1211

1312
[compat]
1413
ADTypes = "1.11.0"
14+
Aqua = "0.8.14"
15+
ComponentArrays = "0.15.30"
1516
DifferentiationInterface = "0.6.43, 0.7"
17+
ExplicitImports = "1.13.2"
1618
FiniteDiff = "2.0"
1719
ForwardDiff = "0.10, 1.0"
20+
JET = "0.9, 0.10"
21+
OptimTestProblems = "2.0.3"
1822
LinearAlgebra = "<0.0.1, 1"
23+
Random = "<0.0.1, 1"
24+
RecursiveArrayTools = "3.39"
25+
SparseArrays = "<0.0.1, 1"
26+
Test = "<0.0.1, 1"
1927
julia = "1.10"
2028

2129
[extras]
22-
ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"
30+
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
2331
ComponentArrays = "b0b7db55-cfe3-40fc-9ded-d10e2dbeff66"
32+
ExplicitImports = "7d51a73a-1435-4ff3-83d9-f097790105c7"
33+
JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
2434
OptimTestProblems = "cec144fc-5a64-5bc6-99fb-dde8f63e154c"
2535
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
2636
RecursiveArrayTools = "731186ca-8d62-57ce-b412-fbd966d074cd"
2737
SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
2838
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
2939

3040
[targets]
31-
test = ["ADTypes", "ComponentArrays", "OptimTestProblems", "Random", "RecursiveArrayTools", "SparseArrays", "Test"]
41+
test = ["Aqua", "ComponentArrays", "ExplicitImports", "JET", "OptimTestProblems", "Random", "RecursiveArrayTools", "SparseArrays", "Test"]

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
NLSolversBase.jl
22
========
33

4+
[![Aqua QA](https://raw.githubusercontent.com/JuliaTesting/Aqua.jl/master/badge.svg)](https://github.com/JuliaTesting/Aqua.jl)
5+
46
Base functionality for optimization and solving systems of equations in Julia.
57

68
NLSolversBase.jl is the core, common dependency of several packages in the [JuliaNLSolvers](https://github.com/JuliaNLSolvers/) family.

‎src/NLSolversBase.jl‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import DifferentiationInterface as DI
55
using FiniteDiff: FiniteDiff
66
using ForwardDiff: ForwardDiff
77
using LinearAlgebra: LinearAlgebra
8-
import Distributed: clear!
8+
99
export AbstractObjective,
1010
NonDifferentiable,
1111
OnceDifferentiable,
@@ -47,13 +47,14 @@ export AbstractConstraints, OnceDifferentiableConstraints,
4747

4848
function finitediff_fdtype(autodiff)
4949
if autodiff == :finiteforward
50-
fdtype = Val{:forward}
50+
return Val{:forward}
5151
elseif autodiff == :finitecomplex
52-
fdtype = Val{:complex}
52+
return Val{:complex}
5353
elseif autodiff == :finite || autodiff == :central || autodiff == :finitecentral
54-
fdtype = Val{:central}
54+
return Val{:central}
55+
else
56+
throw(ArgumentError(LazyString("The autodiff value `", repr(autodiff), "` is not supported. Use `:finite` or `:forward`.")))
5557
end
56-
fdtype
5758
end
5859

5960
forwarddiff_chunksize(::Nothing) = nothing

‎src/interface.jl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ function value!!(obj::AbstractObjective, F, x)
170170
F
171171
end
172172

173-
function _clear_f!(d::NLSolversBase.AbstractObjective)
173+
function _clear_f!(d::AbstractObjective)
174174
d.f_calls = 0
175175
if d.F isa AbstractArray
176176
fill!(d.F, NaN)
@@ -181,21 +181,21 @@ function _clear_f!(d::NLSolversBase.AbstractObjective)
181181
nothing
182182
end
183183

184-
function _clear_df!(d::NLSolversBase.AbstractObjective)
184+
function _clear_df!(d::AbstractObjective)
185185
d.df_calls = 0
186186
fill!(d.DF, NaN)
187187
fill!(d.x_df, NaN)
188188
nothing
189189
end
190190

191-
function _clear_h!(d::NLSolversBase.AbstractObjective)
191+
function _clear_h!(d::AbstractObjective)
192192
d.h_calls = 0
193193
fill!(d.H, NaN)
194194
fill!(d.x_h, NaN)
195195
nothing
196196
end
197197

198-
function _clear_hv!(d::NLSolversBase.AbstractObjective)
198+
function _clear_hv!(d::AbstractObjective)
199199
d.hv_calls = 0
200200
fill!(d.Hv, NaN)
201201
fill!(d.x_hv, NaN)

‎src/objective_types/incomplete.jl‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,19 +41,63 @@ df(t::Union{InplaceObjective, NotInplaceObjective}) = t.df
4141
fdf(t::Union{InplaceObjective, NotInplaceObjective}) = t.fdf
4242

4343
# Mutating version
44-
make_f(t::InplaceObjective, x, F::Real) = x -> fdf(t)(F, nothing, x)
45-
make_f(t::InplaceObjective, x, F) = (F, x) -> fdf(t)(F, nothing, x)
46-
make_f(t::InPlaceObjectiveFGH, x, F::Real) = x -> t.fgh(F, nothing, nothing, x)
47-
make_f(t::InPlaceObjectiveFGHv, x, F::Real) = x -> t.fghv(F, nothing, nothing, x, nothing)
48-
49-
50-
make_df(t::InplaceObjective, x, F) = (DF, x) -> fdf(t)(nothing, DF, x)
51-
make_df(t::InPlaceObjectiveFGH, x, F) = (DF, x) -> t.fgh(nothing, DF, nothing, x)
52-
make_df(t::InPlaceObjectiveFGHv, x, F) = (DF, x) -> t.fghv(nothing, DF, nothing, x, nothing)
44+
function make_f(t::InplaceObjective, x, F::Real)
45+
(; fdf, fgh, fghv) = t
46+
if fdf !== nothing
47+
return let fdf = fdf, F = F
48+
x -> fdf(F, nothing, x)
49+
end
50+
elseif fgh !== nothing
51+
return let fgh = fgh, F = F
52+
x -> fgh(F, nothing, nothing, x)
53+
end
54+
elseif fghv !== nothing
55+
return let fghv = fghv, F = F
56+
x -> fghv(F, nothing, nothing, x, nothing)
57+
end
58+
else
59+
throw(ArgumentError("Cannot construct function for evaluating the objective function: No suitable function was provided."))
60+
end
61+
end
62+
make_f(t::InplaceObjective, x, F) = let fdf = t.fdf; (F, x) -> fdf(F, nothing, x); end
63+
64+
function make_df(t::InplaceObjective, x, F)
65+
(; fdf, fgh, fghv) = t
66+
if fdf !== nothing
67+
return let fdf = fdf
68+
(DF, x) -> fdf(nothing, DF, x)
69+
end
70+
elseif fgh !== nothing
71+
return let fgh = fgh
72+
(DF, x) -> fgh(nothing, DF, nothing, x)
73+
end
74+
elseif fghv !== nothing
75+
return let fghv = fghv
76+
(DF, x) -> fghv(nothing, DF, nothing, x, nothing)
77+
end
78+
else
79+
throw(ArgumentError("Cannot construct function for evaluating the gradient of the objective function: No suitable function was provided.."))
80+
end
81+
end
5382

54-
make_fdf(t::InplaceObjective, x, F::Real) = (G, x) -> fdf(t)(F, G, x)
55-
make_fdf(t::InPlaceObjectiveFGH, x, F::Real) = (G, x) -> t.fgh(F, G, nothing, x)
56-
make_fdf(t::InPlaceObjectiveFGHv, x, F::Real) = (G, x) -> t.fghv(F, G, nothing, x, nothing)
83+
function make_fdf(t::InplaceObjective, x, F::Real)
84+
(; fdf, fgh, fghv) = t
85+
if fdf !== nothing
86+
return let fdf = fdf, F = F
87+
(G, x) -> fdf(F, G, x)
88+
end
89+
elseif fgh !== nothing
90+
return let fgh = fgh, F = F
91+
(G, x) -> fgh(F, G, nothing, x)
92+
end
93+
elseif fghv !== nothing
94+
return let fghv = fghv, F = F
95+
(G, x) -> fghv(F, G, nothing, x, nothing)
96+
end
97+
else
98+
throw(ArgumentError("Cannot construct function that evaluates both the objective function and its gradient: No suitable function was provided."))
99+
end
100+
end
57101
make_fdf(t::InplaceObjective, x, F) = fdf(t)
58102

59103
# Non-mutating version
@@ -93,7 +137,7 @@ end
93137
const InPlaceFGH = InplaceObjective{<:Nothing,<:Nothing,TH,<:Nothing,<:Nothing} where {TH}
94138
const InPlaceFG_HV = InplaceObjective{<:Nothing,TFG,<:Nothing,THv,<:Nothing} where {TFG,THv}
95139
const InPlaceFGHV = InplaceObjective{<:Nothing,<:Nothing,<:Nothing,<:Nothing,TFGHv} where {TFGHv}
96-
function TwiceDifferentiable(t::InPlaceFGH, x::AbstractArray, F::Real = real(zero(eltype(x))), G::AbstractArray = alloc_DF(x, F), H = alloc_H(x, F))
140+
function TwiceDifferentiable(t::InPlaceFGH, x::AbstractArray, F::Real = real(zero(eltype(x))), G::AbstractArray = alloc_DF(x, F), H::AbstractMatrix = alloc_H(x, F))
97141
f = x -> t.fgh(F, nothing, nothing, x)
98142
df = (G, x) -> t.fgh(nothing, G, nothing, x)
99143
fdf = (G, x) -> t.fgh(F, G, nothing, x)

‎src/objective_types/nondifferentiable.jl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Used for objectives and solvers where no gradient is available/exists
2-
mutable struct NonDifferentiable{TF,TX} <: AbstractObjective
2+
mutable struct NonDifferentiable{TF<:Union{AbstractArray,Real},TX<:AbstractArray} <: AbstractObjective
33
f
44
F::TF
55
x_f::TX

‎src/objective_types/oncedifferentiable.jl‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Used for objectives and solvers where the gradient is available/exists
2-
mutable struct OnceDifferentiable{TF, TDF, TX} <: AbstractObjective
2+
mutable struct OnceDifferentiable{TF<:Union{AbstractArray,Real}, TDF<:AbstractArray, TX<:AbstractArray} <: AbstractObjective
33
f # objective
44
df # (partial) derivative of objective
55
fdf # objective and (partial) derivative of objective
@@ -15,24 +15,24 @@ end
1515
# Ambiguity
1616
OnceDifferentiable(f, x::AbstractArray,
1717
F::Real = real(zero(eltype(x))),
18-
DF::AbstractArray = alloc_DF(x, F); inplace = true, autodiff = :finite,
18+
DF::AbstractArray = alloc_DF(x, F); inplace::Bool = true, autodiff::Union{AbstractADType,Symbol,Bool} = :finite,
1919
chunk::ForwardDiff.Chunk = ForwardDiff.Chunk(x)) =
2020
OnceDifferentiable(f, x, F, DF, autodiff, chunk)
2121
#OnceDifferentiable(f, x::AbstractArray, F::AbstractArray; autodiff = :finite) =
2222
# OnceDifferentiable(f, x::AbstractArray, F::AbstractArray, alloc_DF(x, F))
2323
function OnceDifferentiable(f, x::AbstractArray,
2424
F::AbstractArray, DF::AbstractArray = alloc_DF(x, F);
25-
inplace = true, autodiff = :finite)
25+
inplace::Bool = true, autodiff::Union{AbstractADType,Symbol,Bool} = :finite)
2626
f! = f!_from_f(f, F, inplace)
2727

28-
OnceDifferentiable(f!, x::AbstractArray, F::AbstractArray, DF, autodiff)
28+
OnceDifferentiable(f!, x, F, DF, autodiff)
2929
end
3030

3131

32-
function OnceDifferentiable(f, x_seed::AbstractArray{T},
32+
function OnceDifferentiable(f, x_seed::AbstractArray,
3333
F::Real,
3434
DF::AbstractArray,
35-
autodiff, chunk) where T
35+
autodiff::Union{AbstractADType,Symbol,Bool}, chunk::ForwardDiff.Chunk)
3636
# When here, at the constructor with positional autodiff, it should already
3737
# be the case, that f is inplace.
3838
if f isa Union{InplaceObjective, NotInplaceObjective}
@@ -71,7 +71,7 @@ function OnceDifferentiable(f, x::AbstractArray, F::AbstractArray,
7171
OnceDifferentiable(f, x, F, alloc_DF(x, F), :forward, chunk)
7272
end
7373
function OnceDifferentiable(f, x_seed::AbstractArray, F::AbstractArray, DF::AbstractArray,
74-
autodiff::Symbol , chunk::ForwardDiff.Chunk = ForwardDiff.Chunk(x_seed))
74+
autodiff::Union{AbstractADType,Symbol,Bool}, chunk::ForwardDiff.Chunk = ForwardDiff.Chunk(x_seed))
7575
if f isa Union{InplaceObjective, NotInplaceObjective}
7676
fF = make_f(f, x_seed, F)
7777
dfF = make_df(f, x_seed, F)
@@ -98,7 +98,7 @@ function OnceDifferentiable(f, df,
9898
x::AbstractArray,
9999
F::Real = real(zero(eltype(x))),
100100
DF::AbstractArray = alloc_DF(x, F);
101-
inplace = true)
101+
inplace::Bool = true)
102102

103103

104104
df! = df!_from_df(df, F, inplace)
@@ -112,7 +112,7 @@ function OnceDifferentiable(f, j,
112112
x::AbstractArray,
113113
F::AbstractArray,
114114
J::AbstractArray = alloc_DF(x, F);
115-
inplace = true)
115+
inplace::Bool = true)
116116

117117
f! = f!_from_f(f, F, inplace)
118118
j! = df!_from_df(j, F, inplace)
@@ -127,7 +127,7 @@ function OnceDifferentiable(f, df, fdf,
127127
x::AbstractArray,
128128
F::Real = real(zero(eltype(x))),
129129
DF::AbstractArray = alloc_DF(x, F);
130-
inplace = true)
130+
inplace::Bool = true)
131131

132132
# f is never "inplace" since F is scalar
133133
df! = df!_from_df(df, F, inplace)
@@ -145,7 +145,7 @@ function OnceDifferentiable(f, df, fdf,
145145
x::AbstractArray,
146146
F::AbstractArray,
147147
DF::AbstractArray = alloc_DF(x, F);
148-
inplace = true)
148+
inplace::Bool = true)
149149

150150
f = f!_from_f(f, F, inplace)
151151
df! = df!_from_df(df, F, inplace)

‎src/objective_types/twicedifferentiable.jl‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Used for objectives and solvers where the gradient and Hessian is available/exists
2-
mutable struct TwiceDifferentiable{T,TDF,TH,TX} <: AbstractObjective
2+
mutable struct TwiceDifferentiable{T<:Real,TDF<:AbstractArray,TH<:AbstractMatrix,TX<:AbstractArray} <: AbstractObjective
33
f
44
df
55
fdf
@@ -17,7 +17,7 @@ mutable struct TwiceDifferentiable{T,TDF,TH,TX} <: AbstractObjective
1717
h_calls::Int
1818
end
1919
# compatibility with old constructor
20-
function TwiceDifferentiable(f, g, fg, h, x::TX, F::T = real(zero(eltype(x))), G::TG = alloc_DF(x, F), H::TH = alloc_H(x, F); inplace = true) where {T, TG, TH, TX}
20+
function TwiceDifferentiable(f, g, fg, h, x::TX, F::T = real(zero(eltype(x))), G::TG = alloc_DF(x, F), H::TH = alloc_H(x, F); inplace::Bool = true) where {T<:Real, TG<:AbstractArray, TH<:AbstractMatrix, TX<:AbstractArray}
2121
x_f, x_df, x_h = x_of_nans(x), x_of_nans(x), x_of_nans(x)
2222

2323
g! = df!_from_df(g, F, inplace)
@@ -31,10 +31,10 @@ function TwiceDifferentiable(f, g, fg, h, x::TX, F::T = real(zero(eltype(x))), G
3131
end
3232

3333
function TwiceDifferentiable(f, g, h,
34-
x::AbstractVector{TX},
34+
x::AbstractArray,
3535
F::Real = real(zero(eltype(x))),
36-
G = alloc_DF(x, F),
37-
H = alloc_H(x, F); inplace = true) where {TX}
36+
G::AbstractArray = alloc_DF(x, F),
37+
H::AbstractMatrix = alloc_H(x, F); inplace = true)
3838
g! = df!_from_df(g, F, inplace)
3939
h! = h!_from_h(h, F, inplace)
4040

@@ -47,10 +47,8 @@ end
4747

4848

4949
function TwiceDifferentiable(f, g,
50-
x_seed::AbstractVector{T},
51-
F::Real = real(zero(T)); autodiff = :finite, inplace = true) where T
52-
n_x = length(x_seed)
53-
50+
x_seed::AbstractArray,
51+
F::Real = real(zero(eltype(x_seed))); autodiff::Union{AbstractADType,Symbol,Bool} = :finite, inplace::Bool = true)
5452
g! = df!_from_df(g, F, inplace)
5553
fg! = make_fdf(x_seed, F, f, g!)
5654

@@ -63,11 +61,11 @@ function TwiceDifferentiable(f, g,
6361
TwiceDifferentiable(f, g!, fg!, h!, x_seed, F)
6462
end
6563

66-
TwiceDifferentiable(d::NonDifferentiable, x_seed::AbstractVector{T} = d.x_f, F::Real = real(zero(T)); autodiff = :finite) where {T<:Real} =
64+
TwiceDifferentiable(d::NonDifferentiable, x_seed::AbstractArray = d.x_f, F::Real = real(zero(eltype(x_seed))); autodiff::Union{AbstractADType,Symbol,Bool} = :finite) =
6765
TwiceDifferentiable(d.f, x_seed, F; autodiff = autodiff)
6866

69-
function TwiceDifferentiable(d::OnceDifferentiable, x_seed::AbstractVector{T} = d.x_f,
70-
F::Real = real(zero(T)); autodiff = :finite) where T<:Real
67+
function TwiceDifferentiable(d::OnceDifferentiable, x_seed::AbstractArray = d.x_f,
68+
F::Real = real(zero(eltype(x_seed))); autodiff::Union{AbstractADType,Symbol,Bool} = :finite)
7169
backend = get_adtype(autodiff)
7270
hess_prep = DI.prepare_hessian(d.f, backend, x_seed)
7371
function h!(_h, _x)
@@ -78,7 +76,7 @@ function TwiceDifferentiable(d::OnceDifferentiable, x_seed::AbstractVector{T} =
7876
end
7977

8078
function TwiceDifferentiable(f, x::AbstractArray, F::Real = real(zero(eltype(x)));
81-
autodiff = :finite, inplace = true)
79+
autodiff::Union{AbstractADType,Symbol,Bool} = :finite, inplace::Bool = true)
8280
backend = get_adtype(autodiff)
8381
grad_prep = DI.prepare_gradient(f, backend, x)
8482
hess_prep = DI.prepare_hessian(f, backend, x)

‎test/qa.jl‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
@testset "QA" begin
2+
@testset "Aqua" begin
3+
Aqua.test_all(NLSolversBase)
4+
end
5+
6+
@testset "ExplicitImports" begin
7+
# No implicit imports (`using XY`)
8+
@test ExplicitImports.check_no_implicit_imports(NLSolversBase) === nothing
9+
10+
# All explicit imports (`using XY: Z`) are loaded via their owners
11+
@test ExplicitImports.check_all_explicit_imports_via_owners(NLSolversBase) === nothing
12+
13+
# No explicit imports (`using XY: Z`) that are not used
14+
@test ExplicitImports.check_no_stale_explicit_imports(NLSolversBase) === nothing
15+
16+
# Nothing is accessed via modules other than its owner
17+
@test ExplicitImports.check_all_qualified_accesses_via_owners(NLSolversBase) === nothing
18+
19+
# NLSolversBase accesses almost no non-public names
20+
# The only exception is `ForwardDiff.Chunk`
21+
@test ExplicitImports.check_all_qualified_accesses_are_public(NLSolversBase; ignore = (:Chunk,)) === nothing
22+
23+
# No self-qualified accesses
24+
@test ExplicitImports.check_no_self_qualified_accesses(NLSolversBase) === nothing
25+
end
26+
27+
@testset "JET" begin
28+
# Check that there are no undefined global references and undefined field accesses
29+
JET.test_package(NLSolversBase; target_defined_modules = true, mode = :typo, toplevel_logger = nothing)
30+
31+
# Analyze methods based on their declared signature
32+
JET.test_package(NLSolversBase; target_defined_modules = true, toplevel_logger = nothing)
33+
end
34+
end

0 commit comments

Comments
 (0)