RFC: strided mapwindow

[jw3126]

See #44. Here is a first implementation. The API is a bit cumbersome and needs to be improved. Currently you have to call

mapwindow(f,img,window,border,imginds)

Any suggestions for a better API? I would also like to add two convenience functions:

• strided(inds, stride) converts a stride into something that can be passed to mapwindow:

  mapwindow(f,...,strided(indices(img), (2,3))) = mapwindow(f,...)[1:2:end,1:3:end]

• mappool(f,img,window) which does image pooling. This is the special case, where the stride is the size of the window.

  mappool(f,img,window,...) = mapwindow(f,img,...,strided(indices(img), map(length,window))

[timholy]

Sorry for the delay here; I've been mulling over the API issues, and I agree it's not entirely straightforward. I keep returning to something like:

dest = PooledArray{T}(inds)
mapwindow!(f, dest, img, window, border)

and possibly a convenience method

dest = pooledarray(f, img, window, inds)

for the automatic computation of the eltype.

But to be honest I'm not convinced that's any better.

[jw3126]

None of the @assert is inside a hot loop, so I don't think they will have a performance impact. I could turn them into tests, but these are implementations details I think assertions are much more convenient then tests here.

[jw3126]

Introducing a new type of array seems a bit heavyweight for this. Is turning border into a keyword argument an option?

mapwindow(f,img,window,imginds; border=myborder)

At least border does not influence the return type AFAICT.

mapwindow!

seems to be useful also independently of this PR.

[timholy]

At least border does not influence the return type AFAICT.

It does for Fill, NoPad, and Inner. EDIT: sorry, probably doesn't affect the return type, just the dispatch. Probably not relevant, performance-wise.

Introducing a new type of array seems a bit heavyweight for this.

I agree. The only justification is whether it would be useful for "documenting" the results later. For example, if someone hands you an array that is a result of some pooling operation, does it help you to know where each "pixel" (result on a chunk) came from? Or do you not care?

And yes, mapwindow! is probably independently useful no matter what we decide here.

[jw3126]

does it help you to know where each "pixel" (result on a chunk) came from? Or do you not care?

Depends on the application. Most of the time I just want to detect, whether some property is holds in an image. For this I don't care about the original pixels. But if you search something in an image, this sounds useful. In that case you might also want to chain some mapwindow! calls? Not sure. Also if you are interested in the original pixels, would you also like to remember windowsize?

Finally one might want to use a strided mapwindow! with another array type as dest. So we would probably need

struct PooledArray{T,N,A,I} <: AbstractArray{T,N}
    data::A
    indices::I
end

This is even more heavyweight and looks a lot like an AxisArray.

Here is a list of all options I can think of:

1. Introduce new array type PooledArray that wraps the output and use mapwindow!
2. Introduce new array type that wraps the input.
3. mapwindow(f, img, windowsize, border, indices)
4. Introduce a new function getindex_mapwindow(f,img,windowsize, indices, border)
5. Make indices a keyword argument. This would break type stability if we tried to support e.g. (1:3, 2) in future.

[jw3126]

Another idea is making mapwindow lazy.

struct MapWindow{T,N,F,O,W,P} <: AbstractArray{T,N}
    f::F
    original_array::O
    windowsize::W
    pad::P
    # maybe also buffer?
end

MapWindow(f, img, windowsize)[1:4, 5:6]

mapwindow!(f,dest, img,windowsize, border) = copy!(dest, MapWindow(f,img,windowsize, border))

[timholy]

I generally love lazy, but here the problem is that it kind of violates one of the implicit "contracts" of AbstractArray: that elementwise access is cheap. I added something along these lines to ImageAxes in https://github.com/JuliaImages/ImageAxes.jl/pull/19, and ended up not making it a subtype of AbstractArray. This of course makes some things more painful, but in general I think it's better to rely on "behavior" than it is on subtypes.

Of your list of 5 options above, I'd like to strike number 2. This is really a statement about what portion of the output you want to keep, so associating it with the input seems wrong. 4 seems somehow unnecessary. I find myself roughly equally attracted by 1, 3, and 5. I can go with the API you have now, if there isn't an obviously better alternative. Since I'm on the fence, I'd be happy to have you choose.

[codecov[bot]]

Codecov Report

Merging #45 into master will decrease coverage by 1.17%. The diff coverage is 98.38%.

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
- Coverage   90.12%   88.94%   -1.18%     
==========================================
  Files           8        8              
  Lines         992     1049      +57     
==========================================
+ Hits          894      933      +39     
- Misses         98      116      +18

Impacted Files	Coverage Δ
src/mapwindow.jl	85.98% <98.38%> (+3.93%)	:arrow_up:
src/specialty.jl	47.22% <0%> (-42.26%)	:arrow_down:

---

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 941acd0...1d7c4f1. Read the comment docs.

[codecov[bot]]

Codecov Report

Merging #45 into master will increase coverage by 0.35%. The diff coverage is 94.8%.

@@            Coverage Diff             @@
##           master      #45      +/-   ##
==========================================
+ Coverage   90.12%   90.48%   +0.35%     
==========================================
  Files           8        8              
  Lines         992     1040      +48     
==========================================
+ Hits          894      941      +47     
- Misses         98       99       +1

Impacted Files	Coverage Δ
src/mapwindow.jl	86.66% <94.8%> (+4.61%)	: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 941acd0...4e1dd4e. Read the comment docs.

[jw3126]

I will go with

mapwindow(f,img,window,border,imginds)

for now. Maybe over time something better will emerge.

[jw3126]

Thanks for the feedback. There are still some corner cases, that I need to test/fix.

[jw3126]

If there are no further objections, I will merge on Monday.

[timholy]

Thanks so much, this is fantastic!