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

Input dimension checking when evaluating surrogates #376

Merged
merged 5 commits into from
Jul 14, 2022

Conversation

archermarx
Copy link
Contributor

Fixes #337. Adds a new utility function _check_input_dimension which is called for all surrogates before evaluating a point. Provides a helpful error message if there's a dimension mismatch.

Example

using Surrogates

lb = 0.0
ub = 4.0
x = [1.0,2.0,3.0]
y = [4.0,5.0,6.0]
my_rad = RadialBasis(x, y, lb, ub, rad = linearRadial())
julia> my_rad(Float64[1.5])
4.5

julia> my_rad([1.0, 2.0])
ERROR: ArgumentError: This surrogate expects 1-dimensional inputs, but the input had dimension 2.
Stacktrace:
 [1] _check_dimension
   @ C:\Users\thoma\Surrogates.jl\src\utils.jl:11 [inlined]
 [2] (::RadialBasis{Surrogates.var"#1#2", Int64, Vector{Float64}, Vector{Float64}, Float64, Float64, Transpose{Float64, Vector{Float64}}, Float64, Bool})(val::Vector{Float64})
   @ Surrogates C:\Users\thoma\Surrogates.jl\src\Radials.jl:159
 [3] top-level scope
   @ REPL[39]:1

julia> my_rad(Float64[])
ERROR: ArgumentError: This surrogate expects 1-dimensional inputs, but the input had dimension 0.
Stacktrace:
 [1] _check_dimension
   @ C:\Users\thoma\Surrogates.jl\src\utils.jl:11 [inlined]
 [2] (::RadialBasis{Surrogates.var"#1#2", Int64, Vector{Float64}, Vector{Float64}, Float64, Float64, Transpose{Float64, Vector{Float64}}, Float64, Bool})(val::Vector{Float64})
   @ Surrogates C:\Users\thoma\Surrogates.jl\src\Radials.jl:159
 [3] top-level scope
   @ REPL[40]:1

Benchmarking reveals this to come at a very small cost:

Before
julia> @benchmark my_rad([1.5])
BenchmarkTools.Trial: 10000 samples with 855 evaluations.
Range (min … max): 143.392 ns … 10.876 μs ┊ GC (min … max): 0.00% … 97.94%
Time (median): 146.784 ns ┊ GC (median): 0.00%
Time (mean ± σ): 175.408 ns ± 332.221 ns ┊ GC (mean ± σ): 7.69% ± 4.01%

▆█▆▅▄▂▁ ▁ ▂▄▄▂ ▁ ▁
████████▆▆▇▅▅▅▆▅▅▅▄▅▆▅▄▄▃▅▅▅▅▅▅▅▅▄▄▅▅▅▅▄▇█████████▇▆██▇▆▅▄▅▄▄ █
143 ns Histogram: log(frequency) by time 247 ns <

Memory estimate: 144 bytes, allocs estimate: 3.

After
julia> @benchmark my_rad([1.5])
BenchmarkTools.Trial: 10000 samples with 825 evaluations.
Range (min … max): 151.394 ns … 10.147 μs ┊ GC (min … max): 0.00% … 97.38%
Time (median): 155.394 ns ┊ GC (median): 0.00%
Time (mean ± σ): 196.150 ns ± 340.000 ns ┊ GC (mean ± σ): 6.83% ± 3.89%

▇█▄▃▂▂ ▁▁▁ ▂▄▅▅▂▁▁▂ ▁
████████▇▆▅▆▇▇▇▆▆▇▆▇▆▆▆▆▆▅▅▅▇██████████████▆▄▅▅▄▅▆▅▅▆▆▅▆▄▃▅▄▄ █
151 ns Histogram: log(frequency) by time 302 ns <

Memory estimate: 144 bytes, allocs estimate: 3.

src/GEKPLS.jl Outdated Show resolved Hide resolved
@archermarx
Copy link
Contributor Author

I have removed the tests for GEKPLS for now because it expects matrix inputs instead of vectors of tuples like the rest of Surrogates.jl. When #355 is being worked on, I will revisit it

@codecov
Copy link

codecov bot commented Jul 13, 2022

Codecov Report

Merging #376 (bef6e07) into master (5bd2810) will increase coverage by 0.06%.
The diff coverage is 93.54%.

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
+ Coverage   78.90%   78.96%   +0.06%     
==========================================
  Files          16       16              
  Lines        2280     2306      +26     
==========================================
+ Hits         1799     1821      +22     
- Misses        481      485       +4     
Impacted Files Coverage Δ
src/GEKPLS.jl 91.20% <ø> (-0.93%) ⬇️
src/PolynomialChaos.jl 0.00% <0.00%> (ø)
src/Earth.jl 87.93% <100.00%> (+0.10%) ⬆️
src/GEK.jl 93.66% <100.00%> (+0.18%) ⬆️
src/InverseDistanceSurrogate.jl 100.00% <100.00%> (ø)
src/Kriging.jl 94.64% <100.00%> (+0.19%) ⬆️
src/LinearSurrogate.jl 100.00% <100.00%> (ø)
src/Lobachevsky.jl 97.24% <100.00%> (+0.05%) ⬆️
src/Radials.jl 88.28% <100.00%> (+0.10%) ⬆️
src/SecondOrderPolynomialSurrogate.jl 100.00% <100.00%> (ø)
... and 3 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@archermarx archermarx marked this pull request as ready for review July 13, 2022 15:55
@ChrisRackauckas ChrisRackauckas merged commit ba53181 into SciML:master Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Radial basis doesn't error with incorrect inputs.
2 participants