Support arbitrary indices #18
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
==========================================
+ Coverage 91.25% 91.66% +0.41%
==========================================
Files 4 4
Lines 160 180 +20
==========================================
+ Hits 146 165 +19
- Misses 14 15 +1
Continue to review full report at Codecov.
|
|
0.6 failure needs fixing in Rotations, presumably. |
| @@ -36,5 +36,25 @@ tfm = recenter(RotMatrix(-pi/4), center(img_pyramid)) | |||
| imgr = warp(img_pyramid, tfm) | |||
| @test indices(imgr) == (0:6, 0:6) | |||
|
|
|||
| # I do this very complicated because turns out NaNs are hard to compare... | |||
| # Use map and === because of the NaNs | |||
src/resizing.jl
Outdated
| restrict!(out, A, dim) | ||
| out | ||
| end | ||
|
|
||
| # out should have efficient linear indexing | ||
| @generated function restrict!{T,N}(out::AbstractArray{T,N}, A::AbstractArray, dim) | ||
| function restrict!{T,N}(out::AbstractArray{T,N}, A::AbstractArray, dim) | ||
| if dim > N |
There was a problem hiding this comment.
Shouldn't we throw an error in such a case?
There was a problem hiding this comment.
I'm not opposed to doing so, but I was mimicking this:
julia> a = 1:3
1:3
julia> sum(a, 1)
1-element Array{Int64,1}:
6
julia> sum(a, 2)
3-element Array{Int64,1}:
1
2
3There was a problem hiding this comment.
Ah, I see. I think consistency with Base is the cleanest way, yes
| stride_1 = 1 | ||
| @nexprs $N d->(stride_{d+1} = stride_d*size(out,d)) | ||
| @nexprs $N d->offset_d = 0 | ||
| z = convert(T, zero(first(A))) |
There was a problem hiding this comment.
Not certain, but I think I put it this way originally to support the following:
julia> a = [rand(3) for i = 1:5]
5-element Array{Array{Float64,1},1}:
[0.465388,0.525425,0.753336]
[0.890223,0.465896,0.191895]
[0.205448,0.896783,0.0923702]
[0.538556,0.862554,0.581898]
[0.731725,0.887581,0.0116221]
julia> restrict(a, 1)
3-element Array{Array{Float64,1},1}:
[0.45525,0.379187,0.424642]
[0.459918,0.780504,0.239633]
[0.500502,0.659429,0.151285]Clearly I should add that to the tests.
test/resizing.jl
Outdated
| @@ -30,6 +30,10 @@ | |||
| img1 = colorview(RGB, fill(0.9, 3, 5, 5)) | |||
| img2 = colorview(RGB, fill(N0f8(0.9), 3, 5, 5)) | |||
| @test isapprox(channelview(restrict(img1)), channelview(restrict(img2)), rtol=0.01) | |||
| # Non-1 indices | |||
| @test parent(restrict(OffsetArray(A, (-2,1,0)), 1)) == restrict(A, 1) | |||
There was a problem hiding this comment.
did you consider:
@test parent(@inferred(restrict(OffsetArray(A, (-2,1,0)), 1))) == restrict(A, 1)There was a problem hiding this comment.
In fact, there was a type instability due to this method, since A can change its type. Thanks for the suggestion!
|
Very cool! Made some comments here and there when something caught my eye |
|
Thanks for the review, @Evizero! |
This uses generated functions for looping instead of CartesianRange because of julia issue 9080.
|
Worth a tag? I am eyeing the merge of JuliaImages/ImageInTerminal.jl#9 |
This leverages recent changes in Interpolations (JuliaMath/Interpolations.jl#146) to provide support for arbitrary indices in warping. It also rewrites
restrictto support arbitrary indices. Note that this very similar to #12, but by keeping any branches out of the innermost loop and by circumventing JuliaLang/julia#9080, I've been able to keep the performance essentially unchanged (less than 10% difference).