JuliaMath / Interpolations.jl

Fast, continuous interpolation of discrete datasets in Julia
http://juliamath.github.io/Interpolations.jl/
Other
533 stars 109 forks source link

Fix inference regression on 1.7 #471

Closed N5N3 closed 2 years ago

N5N3 commented 2 years ago

Since AbstractInterpolation <: AbstractArray, there's no need to define ndims and eltype ourself. gridtype is broken, (and unused), so I remove it. itptype is modified to avoid the recursive call of supertype.

codecov[bot] commented 2 years ago

Codecov Report

Merging #471 (d517db8) into master (827895e) will increase coverage by 8.98%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #471      +/-   ##
==========================================
+ Coverage   76.42%   85.41%   +8.98%     
==========================================
  Files          26       26              
  Lines        1421     1741     +320     
==========================================
+ Hits         1086     1487     +401     
+ Misses        335      254      -81     
Impacted Files Coverage Δ
src/Interpolations.jl 79.36% <100.00%> (+11.82%) :arrow_up:
src/filter1d.jl 91.66% <0.00%> (-2.88%) :arrow_down:
src/io.jl 95.29% <0.00%> (-0.66%) :arrow_down:
src/deprecations.jl 2.56% <0.00%> (-0.38%) :arrow_down:
src/rewrite.jl 0.00% <0.00%> (ø)
src/lanczos/lanczos.jl 100.00% <0.00%> (ø)
src/chainrules/chainrules.jl 100.00% <0.00%> (ø)
src/monotonic/monotonic.jl 94.57% <0.00%> (+1.05%) :arrow_up:
src/gridded/indexing.jl 75.21% <0.00%> (+3.30%) :arrow_up:
src/iterate.jl 100.00% <0.00%> (+3.44%) :arrow_up:
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 827895e...d517db8. Read the comment docs.

mkitti commented 2 years ago

Also tracking https://github.com/JuliaLang/julia/issues/43368 https://github.com/JuliaLang/julia/issues/43296

N5N3 commented 2 years ago

This PR should be enough as a temporary fix. Do we need test for itptype? looks like it is used only in count_interp_dims. Edit: Well, looks like the count_interp_dims api is also insufficient.

julia> itp = CubicSplineInterpolation((1:100,1:100),randn(100,100));

julia> Interpolations.count_interp_dims(typeof(itp))
ERROR: MethodError: no method matching count_interp_dims(::Type{BSpline{Cubic{Line{OnGrid}}}})
Closest candidates are:
  count_interp_dims(::Type{IT}, ::Any) where IT<:Interpolations.InterpolationType at C:\Users\MYJ\.julia\packages\Interpolations\3gTQB\src\Interpolations.jl:164  count_interp_dims(::Type{IT}, ::Any) where {N, IT<:Tuple{Vararg{Interpolations.InterpolationType, N}}} at C:\Users\MYJ\.julia\packages\Interpolations\3gTQB\src\Interpolations.jl:165
  count_interp_dims(::Type{BSI}, ::Any) where BSI<:Interpolations.BSplineInterpolation at C:\Users\MYJ\.julia\packages\Interpolations\3gTQB\src\b-splines\b-splines.jl:157
  ...
Stacktrace:
 [1] count_interp_dims(#unused#::Type{BSpline{Cubic{Line{OnGrid}}}}, n::Int64)
   @ Interpolations C:\Users\MYJ\.julia\packages\Interpolations\3gTQB\src\Interpolations.jl:164
 [2] count_interp_dims(#unused#::Type{Interpolations.Extrapolation{Float64, 2, ScaledInterpolation{Float64, 2, Interpolations.BSplineInterpolation{Float64, 2, OffsetArrays.OffsetMatrix{Float64, Matrix{Float64}}, BSpline{Cubic{Line{OnGrid}}}, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}}, BSpline{Cubic{Line{OnGrid}}}, Tuple{UnitRange{Int64}, UnitRange{Int64}}}, BSpline{Cubic{Line{OnGrid}}}, Throw{Nothing}}})
   @ Interpolations C:\Users\MYJ\.julia\packages\Interpolations\3gTQB\src\Interpolations.jl:163
 [3] top-level scope
   @ REPL[21]:1
mkitti commented 2 years ago

Can you write a test for the one line you added?

N5N3 commented 2 years ago

test added

mkitti commented 2 years ago

Merging