Provide better support for manually-specified borders (fixes #85)

[timholy]

@zygmuntszpak, does this seem OK?

[zygmuntszpak]

Thank you for fixing this. I will try to write some further border tests for you tomorrow so that we have a record that the other padding styles also work as expected.

[timholy]

Test failure on master is addressed by https://github.com/JuliaLang/julia/pull/30164

[zygmuntszpak]

I've written the following tests which you could add to your current set:

B = zeros(similar(A))
C = zeros(similar(A))
r1 = imfilter!(B, A, Kernel.gaussian((1,1),(3,3)), Fill(0))
r2 = imfilter!(C, A,  Kernel.gaussian((1,1),(3,3)), Fill(10))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

B = zeros(similar(A))
C = zeros(similar(A))
r1 = imfilter!(B, A, Kernel.gaussian((1,1),(3,3)), Fill(0, (3,3)))
r2 = imfilter!(C, A,  Kernel.gaussian((1,1),(3,3)), Fill(10, (3,3)))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

B = zeros(similar(A))
C = zeros(similar(A))
r1 = imfilter!(B, A, Kernel.gaussian((1,1),(3,3)), Fill(0, (3,3),(3,3)))
r2 = imfilter!(C, A,  Kernel.gaussian((1,1),(3,3)), Fill(10, (3,3),(3,3)))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

B = zeros(similar(A))
C = zeros(similar(A))
r1 = imfilter!(B, A, Kernel.gaussian((1,1),(3,3)), Fill(0, [3,3],[3,3]))
r2 = imfilter!(C, A,  Kernel.gaussian((1,1),(3,3)), Fill(10, [3,3],[3,3]))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

 g = collect(KernelFactors.gaussian(1,3))
r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(0))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(10))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(0, (3,3)))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(10, (3,3)))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(0, (3,3),(3,3)))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(10, (3,3),(3,3)))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(0, [3,3],[3,3]))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(10, [3,3],[3,3]))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(0))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,0}(centered(g)),), Fill(10))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,1}(centered(g)),), Fill(0, (3,3)))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,1}(centered(g)),), Fill(10, (3,3)))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,1}(centered(g)),), Fill(0, (3,3),(3,3)))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,1}(centered(g)),), Fill(10, (3,3),(3,3)))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

r1 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,1}(centered(g)),), Fill(0, [3,3],[3,3]))
r2 = imfilter(A, ( ImageFiltering.ReshapedOneD{2,1}(centered(g)),), Fill(10, [3,3],[3,3]))
@test r1[4,4] == r2[4,4]
@test r1[1,1] != r2[1,1]

I'd like to draw your attention to the following particular tests. They are unusual in that a plain-text error message is displayed in the user, but a genuine error is not thrown. Instead the caller obtains a matrix filled with zeros. I'm not sure if this is intended or not.

# These return a message that an error was thrown, but don't actually throw an
# error. Instead, they return a matrix filled with zeros. 
r1 = imfilter(A, Kernel.gaussian((1,1),(3,3)), Fill(0, (1,0)))
r1 = imfilter(A, Kernel.gaussian((1,1),(3,3)), Fill(0, (0,1)))
r1 = imfilter(A, Kernel.gaussian((1,1),(3,3)), Fill(0, (0,0)))

[codecov[bot]]

Codecov Report

Merging #86 into master will increase coverage by 0.7%. The diff coverage is 57.14%.

@@            Coverage Diff            @@
##           master      #86     +/-   ##
=========================================
+ Coverage   61.86%   62.56%   +0.7%     
=========================================
  Files           8        8             
  Lines        1070     1074      +4     
=========================================
+ Hits          662      672     +10     
+ Misses        408      402      -6

Impacted Files	Coverage Δ
src/kernelfactors.jl	59.66% <ø> (+2.52%)	:arrow_up:
src/imfilter.jl	63.83% <100%> (-1.09%)	:arrow_down:
src/border.jl	63.75% <33.33%> (-1.22%)	:arrow_down:
src/utils.jl	53.57% <50%> (-1.99%)	:arrow_down:
src/kernel.jl	82.47% <0%> (+12.37%)	: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 73e73bc...7afa780. Read the comment docs.

[timholy]

I'd like to draw your attention to the following particular tests.

You must be using something other than Julia 1.x? zeros(::Array) isn't supported anymore, and those tests work as expected for me (and on CI).

[zygmuntszpak]

I was using 0.7 but I was also using Juno which automatically launches Julia with multiple threads. When I launch Julia from the terminal and evaluate

 @test_throws DimensionMismatch imfilter(A, Kernel.gaussian((1,1),(3,3)), Fill(0, (0,0)))

the test passes.

On the other hand, with multiple threads one gets a message:

@test_throws DimensionMismatch imfilter(A, Kernel.gaussian((1,1),(3,3)), Fill(0, (0,0)))

Error thrown in threaded loop on thread 0: Base.DimensionMismatch(msg="requested indices (1:8, 0:9) and kernel indices (Base.Slice(-1:1), Base.Slice(0:0)) do not agree with indices of padded input, (Base.Slice(1:8), Base.Slice(1:8))")Test Failed
at none:1
  Expression: imfilter(A, Kernel.gaussian((1, 1), (3, 3)), Fill(0, (0, 0)))
    Expected: DimensionMismatch
  No exception thrown
ERROR: There was an error during testing

So one receives a message that an error was thrown in a loop in a thread, but a genuine exception is not thrown. Is this perhaps currently just a limitation of Julia?

[timholy]

Oh interesting. I get the same thing if I use

$  JULIA_NUM_THREADS=2 julia runtests.jl

from the Linux prompt. Issue: #87. No time to look at it now.