JuliaMath / Interpolations.jl

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

Gpu support for FilledExtrapolation #541

Closed drewrobson closed 1 year ago

drewrobson commented 1 year ago

I ran into an error when trying to use FilledExtrapolation on the GPU. Turns out src/gpu_support.jl was missing support for FilledExtrapolation. This PR fixes it. For example, the following now works:

using Interpolations, CUDA, Adapt
A = rand(Float32, 10)
eitp = extrapolate(interpolate(A, BSpline(Linear())), 0.0f0)
d_eitp = adapt(CuArray{Float32}, eitp)
@assert eitp.(-5:5) == Array(d_eitp.(-5:5))
codecov[bot] commented 1 year ago

Codecov Report

Base: 87.74% // Head: 87.78% // Increases project coverage by +0.03% :tada:

Coverage data is based on head (f21ad6b) compared to base (f432a7b). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #541 +/- ## ========================================== + Coverage 87.74% 87.78% +0.03% ========================================== Files 28 28 Lines 1861 1866 +5 ========================================== + Hits 1633 1638 +5 Misses 228 228 ``` | [Impacted Files](https://codecov.io/gh/JuliaMath/Interpolations.jl/pull/541?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath) | Coverage Δ | | |---|---|---| | [src/extrapolation/filled.jl](https://codecov.io/gh/JuliaMath/Interpolations.jl/pull/541?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL2V4dHJhcG9sYXRpb24vZmlsbGVkLmps) | `85.36% <ø> (ø)` | | | [src/gpu\_support.jl](https://codecov.io/gh/JuliaMath/Interpolations.jl/pull/541?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath#diff-c3JjL2dwdV9zdXBwb3J0Lmps) | `97.95% <100.00%> (+0.23%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaMath)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

mkitti commented 1 year ago

Thanks. Changing a mutable struct to immutable might be a breaking change though. I'm not sure why it is mutable to begin with.

This will require a minor version change at least.

hzarei4 commented 1 year ago

I ran into an error when trying to use FilledExtrapolation on the GPU. Turns out src/gpu_support.jl was missing support for FilledExtrapolation. This PR fixes it. For example, the following now works:

using Interpolations, CUDA, Adapt
A = rand(Float32, 10)
eitp = extrapolate(interpolate(A, BSpline(Linear())), 0.0f0)
d_eitp = adapt(CuArray{Float32}, eitp)
@assert eitp.(-5:5) == Array(d_eitp.(-5:5))

Hello, I tried this code on Julia 1.8.5 on Linux Ubuntu 22.04 with a Nvidia GPU but it does not work and raises this error.

ERROR: MethodError: objects of type CuArray{Float32, 1, CUDA.Mem.DeviceBuffer} are not callable Use square brackets [] for indexing an Array.

I think the Interpolations.extrapolation on the GPU is not working properly.

mkitti commented 1 year ago

Have you tried the master branch of this package? This code has not been released yet.

hzarei4 commented 1 year ago

@mkitti Thanks for your answer.

Yes, I have tried the master branch. Do I have to try this (https://github.com/drewrobson/Interpolations.jl) branch?

hzarei4 commented 1 year ago

I tried Drew Robson's branch (https://github.com/drewrobson/Interpolations.jl), and it ran successfully.

Is it possible to merge his branch with the master branch? @mkitti

Thanks.

mkitti commented 1 year ago

This pull request is merged. Can you test the master branch here?

mkitti commented 1 year ago

I see we're in a conversational loop. I'm confused why the master branch here does not work since the changes should be in https://github.com/JuliaMath/Interpolations.jl/commit/85d3ccd515c397b4f37778d8826cc028cf13133c

mkitti commented 1 year ago

How did you try the master branch here?

hzarei4 commented 1 year ago

Thanks for your answer.

I tried the master branch of the Interpolations package and it is running now. (https://github.com/JuliaMath/Interpolations.jl)

It is interesting that if I try to install Interpolations.jl from the Julia package registry (] add Interpolations), it will not work. (I do not know why)

mkitti commented 1 year ago

] add Interpolations will install the latest released version, which was six months ago. ] add Interpolations#master will grab the development branch with the commit of interest. So perhaps your actual request is for us to make a new release of Interpolations.jl.

hzarei4 commented 1 year ago

@mkitti Thank you so much for the answer. Sorry, I was at the conference last week.

Yes, the ] add Interpolations#master is working. Have you considered releasing the new version of the Interpolations in the near future?