JuliaMath / Interpolations.jl

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

Add native GPU support. #504

Closed N5N3 closed 2 years ago

N5N3 commented 2 years ago

~Only 1d and 2d BSpline Interpolations are tested though.~ Test has been added for Gridded and Lanczos. ~Since this overload needs GPUArraysCore.jl, we need to drop < 1.6 compatibility if we don't want to add more @require.~ Base.getindex is overloaded with a InterpGetIndex wrapper in the latest design. Thus there's no need to depend on GPUArraysCore.jl. (We can keep < 1.6 compatibility now)

codecov[bot] commented 2 years ago

Codecov Report

Merging #504 (074110e) into master (4c29ad6) will increase coverage by 0.18%. The diff coverage is 97.05%.

@@            Coverage Diff             @@
##           master     #504      +/-   ##
==========================================
+ Coverage   86.91%   87.10%   +0.18%     
==========================================
  Files          27       28       +1     
  Lines        1819     1845      +26     
==========================================
+ Hits         1581     1607      +26     
  Misses        238      238              
Impacted Files Coverage Δ
src/b-splines/indexing.jl 78.57% <75.00%> (ø)
src/gpu_support.jl 97.67% <97.67%> (ø)
src/Interpolations.jl 75.92% <100.00%> (-3.61%) :arrow_down:
src/b-splines/b-splines.jl 90.09% <100.00%> (+0.09%) :arrow_up:
src/extrapolation/extrapolation.jl 70.78% <100.00%> (ø)
src/gridded/indexing.jl 75.21% <100.00%> (ø)
src/lanczos/lanczos.jl 100.00% <100.00%> (ø)
src/scaling/scaling.jl 90.57% <100.00%> (ø)
src/gridded/gridded.jl 98.41% <0.00%> (+1.58%) :arrow_up:

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 4c29ad6...074110e. Read the comment docs.

mkitti commented 2 years ago

I'm a bit wary of dependency creep. Is there a way to do this via a subpackage?

johnnychen94 commented 2 years ago

The additional dependencies are quite small (loading time from 0.74s to 0.79s in my machine). But they request bumping Julia compat to 1.6, which is okay as well (I think).

Either living in here or in the new interpolationsGPU would be great!

cc: @timholy @ChantalJuntao (from https://github.com/JuliaImages/ImageTransformations.jl/pull/156)

mkitti commented 2 years ago

In particular, can you please update this document: https://github.com/JuliaMath/Interpolations.jl/blob/master/docs/src/devdocs.md

N5N3 commented 2 years ago

@mkitti devdoc has been updated (including the new InterpGetindex wrapper). The Adapt based transformation itself should be familiar to every GPU end users. So I just emphasize that 'eltype' may change during adaption.

mkitti commented 2 years ago

Is this ready to merge?

N5N3 commented 2 years ago

Yes, CI passed. The test also passed locally with 1.6.7.

ChantalJuntao commented 2 years ago

~Only 1d and 2d BSpline Interpolations are tested though.~ Test has been added for Gridded and Lanczos. ~Since this overload needs GPUArraysCore.jl, we need to drop < 1.6 compatibility if we don't want to add more @require.~ Base.getindex is overloaded with a InterpGetIndex wrapper in the latest design. Thus there's no need to depend on GPUArraysCore.jl. (We can keep < 1.6 compatibility now)

This is kind of late, but was it OK to merge this without tests for 3D? And I thought that Cl's tests don't cover GPU code?

mkitti commented 2 years ago

Feel free to submit another pull request. As far as I know there are no GPUs to run CI on here.