Fix #55

[tomasaschan]

Thanks to @mbauman's troubleshooting it seems this was a quite easy fix; the following plots look reasonable for both linear and quadratic interpolation (only linear shown):

# using Gadfly, DataFrames
reload("Grid")

f(x) = sin(2pi * (x-1) / 10)
dfs = Array(DataFrame,0)

for (i,bc) in enumerate([
        Grid.BCnan,
        Grid.BCna,
        Grid.BCperiodic,
        Grid.BCreflect,
        Grid.BCnearest,
        Grid.BCfill])
    itp = bc != Grid.BCfill ? Grid.InterpGrid(f(1:11), bc, Grid.InterpLinear) :
        Grid.InterpGrid(f(1:11), 1, Grid.InterpLinear)
    push!(dfs, DataFrame(x=-9:.1:40,y=[itp[x]+(i-1)*3 for x in -9:.1:40],t="$bc"))
end

df = vcat(dfs)
plot(df,x=:x,y=:y,color=:t,Geom.path,Guide.colorkey("Boundary condition"))

I didn't test cubic interpolation yet simply because I know that it's not rock solid anyway, so I wanted to get this up for a Travis run first. (Did any tests break from this behavior before? If not, what's the best way to test that this actually works the way we want it?)

[coveralls]

Coverage decreased (-20.61%) to 76.51% when pulling 73137f64b4588880c0f1ef01d435a1c4ee3dd03f on fix-extrap-issues into 2c7a96ce2563065748f2c5178cdb77e2182bf311 on master.

[tomasaschan]

Failed on 0.4 because of Woodbury matrices - will merging #56 fix that?

I think a 20% coverage decrease because of these changes is very unlikely - could the difference be due to the "new" metric? (Has coverage results not been collected since it was introduced?)

[mbauman]

YAY! Nice work. Is that one-sample hold in BCreflect and BCperiodic new? Or is that an existing issue? Or is it expected behavior? (I've never really used boundary conditions)

[tomasaschan]

@mbauman The one-cell shift is actually the desired behavior in some applications (notably for images, which happens to be the application Tim originally built the library for) - basically, it is to make sure that x[n+1:2n] == reverse(x). So it's not really a bug per se, although it's not the expected behavior in many other applications (for example when extrapolating sin(x), which happens to be the example I used...). Interpolations.jl has taken this into account by design, and so lets the user choose if they want this behavior or the one you were expecting.

tl;dr: it's neither new, unknown, optimal or unexpected :)

[mbauman]

:+1:

[mbauman]

I should have pushed that WIP a long time ago. Thanks for finishing it up.

[tomasaschan]

np. I had to change a total of one extra line :)

[timholy]

Thanks, folks. I am just swamped, and it's wonderful to have such awesome collaborators.

[tomasaschan]

:cocktail: :tada: :beers:

[timholy]

Re the coverage stats: it's likely that the test failure caused the decrease. When it fails a tests, it doesn't run as much code, but it still submits the results to Coveralls.

[coveralls]

Coverage decreased (-15.86%) to 81.26% when pulling 86baa24394d7241faec189ecc377589e64f3b59c on fix-extrap-issues into 2c7a96ce2563065748f2c5178cdb77e2182bf311 on master.

[tomasaschan]

The build still fails on 0.4 with the message Woodbury is not defined; it turns out that it doesn't load the package, since the ppa for nightlies is still on an earlier build, but apparently it isn't available anyway. I'll drill down and see if I can figure out why.

[timholy]

The "push" failed, but the "pr" (pull request) succeeded. I'm not certain of the difference between them, but if I had to guess I'd say the "push" adds on to the branch that you started with, whereas the "pr" is for what would happen should you merge this to master. Since your Woodbury fix occurred after you got this started, I think that explains the difference.

So I'd say, merge at will.

[tomasaschan]

Yeah, that might explain it. Might as well rebase to test the theory :)

[coveralls]

Coverage remained the same at 81.26% when pulling 9eb54a8a921b8ed5f5337c4d42b1aa32f03e8fb3 on fix-extrap-issues into e02d09ff1fa05b734a810f8c842c1e7fdb97e8f2 on master.