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

In-place interpolations revisited #36

Merged
merged 3 commits into from
Jun 1, 2015
Merged

In-place interpolations revisited #36

merged 3 commits into from
Jun 1, 2015

Conversation

timholy
Copy link
Member

@timholy timholy commented May 29, 2015

By comparison with #34, this is probably a better approach: rather than defining a new boundary condition Interior and then making the indexing rules treat padding differently for this condition, this PR adds a pad parameter to BSplineInterpolation: for instance, pad=0 for anything other than Quadratic, for which it is typically 1 unless (1) using Reflect or (2) using interpolate!. The code generation uses this parameter to compute appropriate index offsets.

The first commit in this PR also improves the type-stability (and adds tests for it) of several key functions. Also, I think the size function was broken (it included the consequences of padding, which I assume is not what you wanted?); this is fixed in the 2nd commit by exploiting the new pad parameter.

The return type of interpolate() was not inferrable, which would have
led to significant performance penalties when used without a
function barrier.

This also makes `size` inferrable, which is helpful because its
output is often used for array creation, and the created array's
type (dimensionality parameter) would not be inferrable.
Formerly, the output of size included padding, which I suspect
is not how we want it to work. By including the padding in the type,
we can also generate more reliable code (see `define_indices` in
`b-splines/quadratic.jl`)

This also prepares the way for in-place interplation (which would use
no padding).
@timholy
Copy link
Member Author

timholy commented May 29, 2015

With the improvements in filtering, this is nice to see (suitable warmup has been done already):

julia> A = rand(300,300,30);

julia> Ai = copy(A);

julia> @time itp = interpolate!(Ai, BSpline(Quadratic(Flat)), OnCell);
  54.937 milliseconds (198 allocations: 49757 bytes)

julia> Ag = copy(A);

julia> @time yi = InterpGrid(Ag, BCreflect, InterpQuadratic);
 299.404 milliseconds (108 k allocations: 26187 KB, 20.47% gc time)

But this was not so nice:

julia> function interp_all!(dest, itp)
           for k = 1:size(itp,3)
               for j = 1:size(itp,2)
                   for i = 1:size(itp,1)
                       dest[i,j,k] = itp[i,j,k]
                   end
               end
           end
           dest
       end
interp_all! (generic function with 1 method)

julia> B = similar(A);

julia> @time interp_all!(B, itp);
  10.251 seconds      (2700 k allocations: 42188 KB)

julia> @time interp_all!(B, yi);
   1.069 seconds      (4 allocations: 144 bytes)

I'll see if I can fix that.

timholy added a commit that referenced this pull request Jun 1, 2015
In-place interpolations revisited
@timholy timholy merged commit 7669772 into master Jun 1, 2015
@timholy timholy deleted the teh/inplace2 branch June 1, 2015 11:12
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.

1 participant