update codebase to 1.0 and 1.x (continue #27) #31

johnnychen94 · 2019-12-16T00:43:37Z

This PR continues #27 and the road I choose is to:

fix those easy-to-fix broken tests in julia 1.0
fix type instability
update codes to 1.3 while keep backward compatibility to 1.0
fix the random failure in tst_scale.jl
support OffsetArray 1.0

There're some type-instability cases detected, which I've added a TODO comment for that, and probably fix them in a separate future PR.

To easily review the differences, the current base is set to update0.7, will change it to master before merging.

JuliaLang/julia#26369

src/distortedview.jl

src/operations/resize.jl

JuliaIO/FileIO.jl#245

johnnychen94 · 2019-12-16T01:09:27Z

@Evizero could you enable Auto Cancellation of the previous build in travis?

Evizero · 2019-12-16T07:43:04Z

@Evizero could you enable Auto Cancellation of the previous build in travis?

It looks enabled already

Only meta operations such as Either, Pipeline doesn't have Agumentor prefix

* maybe_copy doesn't explain its usage * replace Slice by OffsetArrays.IdentityUnitRange JuliaArrays/OffsetArrays.jl#62

detect some type instable test cases, only the outmost wrapper is instable here

src/utils.jl

johnnychen94 · 2019-12-16T23:02:09Z

test/operations/tst_either.jl

@@ -117,68 +117,68 @@ end
            @test Augmentor.supports_view(op) === false
            @test Augmentor.supports_stepview(op) === false
            @test Augmentor.supports_permute(op) === false
-            @test @inferred(Augmentor.applyeager(op, img)) == rotl90(rect)
-            @test typeof(Augmentor.applyeager(op, img)) <: OffsetArray
+            @test @inferred(Augmentor.applyeager(op, img, Augmentor.randparam(op, img))) == rotl90(rect) # TODO: update 1.2: type instable if remove Augmentor.randparam


Type instability is detected here for Julia 1.2; the lucky part is that only the out-most wrapper is affected.

Is it randparam then?

Sounds reasonable, I'll check that; not very familiar with this kind of issue at present.

It turns out that the following part doesn't work anymore

Augmentor.jl/src/operations/either.jl

Lines 175 to 181 in 6055db7

# To preserve type stability of "applyeager", always promote

# arrays to offset arrays

@inline _offsetarray(A::Array) = OffsetArray(A, 0, 0)

@inline _offsetarray(A::OffsetArray) = A

function applyeager(op::Either, img::AbstractArray, idx)

_offsetarray(applyeager(op.operations[idx], img))

end

a typical error is like

ERROR: return type OffsetArray{Gray{Normed{UInt8,8}},2,Array{Gray{Normed{UInt8,8}},2}} does not match inferred return type OffsetArray

I need time to investigate it, but for now, I'll pause it for a few days just to use this package/branch to catch up with my daily work.

Evizero

I reviewed all your code changes so far
Thanks for working on this

src/operations/resize.jl

Evizero · 2019-12-17T06:40:15Z

test/operations/tst_either.jl

@@ -117,68 +117,68 @@ end
            @test Augmentor.supports_view(op) === false
            @test Augmentor.supports_stepview(op) === false
            @test Augmentor.supports_permute(op) === false
-            @test @inferred(Augmentor.applyeager(op, img)) == rotl90(rect)
-            @test typeof(Augmentor.applyeager(op, img)) <: OffsetArray
+            @test @inferred(Augmentor.applyeager(op, img, Augmentor.randparam(op, img))) == rotl90(rect) # TODO: update 1.2: type instable if remove Augmentor.randparam


Is it randparam then?

Evizero · 2019-12-17T06:44:02Z

test/tst_distortedview.jl

    @test_reference "reference/distort_static.txt" dv

    camerao = OffsetArray(camera, (-5,-10))
    dv2 = @inferred Augmentor.DistortedView(camerao, A)
    @test size(dv2) == size(camera)
    @test eltype(dv2) == eltype(camera)
-    @test summary(dv2) == "512×512 Augmentor.DistortedView(::OffsetArray{Gray{N0f8},2}, ::Array{Float64,3} as 3×3 vector field) with element type ColorTypes.Gray{FixedPointNumbers.Normed{UInt8,8}}"
+    @test summary(dv2) == "512×512 Augmentor.DistortedView(::Array{Gray{N0f8},2}, ::Array{Float64,3} as 3×3 vector field) with eltype Gray{Normed{UInt8,8}}"


What happened here?

Looks like it's related to my previous added specification to DistortedView

It's a WIP commit, I'll recheck that later when I fix julia 1.3 deprecations.

Rechecked in 733e32b and it should be a safe change here (as far as I understand the codebase)

To successfully compare two DistortedViews A and B, their axes should be the same, i.e., axes(A)==axes(B), so I add the plain_axes in the construction method to make sure this. This is still lazy-mode since plain_axes doesn't copy data. The test in tst_distortion.jl checks that as well.

I tried to use git blame to find out how in the previous codes two arrays of different axes can compare successfully but I failed, so I find another path here.

Although I think this change meets the goal of DistortedView, it does change the behavior, hence you might need to review this commit carefully. 😂

rand is thread-safe in julia 1.3

johnnychen94 · 2019-12-17T15:33:06Z

Good news is that except a random failure due to accuracy in tst_scale.jl, the whole codebase is now 1.3-compatible. Later commits are to continue WIP commits and address issues in review comments.

Evizero · 2019-12-19T07:26:00Z

amazing news!

johnnychen94 · 2019-12-21T02:25:36Z

src/utils.jl

-const rand_mutex = Ref{Threads.Mutex}()
-
-# constant overhead of about 80 ns compared to unsafe rand
-function safe_rand(args...)


I'm a little worried here that I might misunderstand what "threadsafe" means here. Is that mean the following two scripts should output the same results?

Random.seed!(0) Threads.@threads for i in 1:5 println(rand()) end

Random.seed!(0) for i in 1:5 println(rand()) end

no. This has no influence on the ordering that threads invoke save_rand. It really just is about the function being threadsafe so that consecutive return values (regardless of calling thread) can be considered random

johnnychen94 · 2020-01-28T11:18:44Z

src/compat.jl

+if Pkg.installed()["ImageTransformations"] <= v"0.8.3"
+    Interpolations.tweight(A::AbstractArray{C}) where C<:FixedPoint = C
+end


This seems not a good idea. 👎 and it breaks the nightly test

johnnychen94 · 2020-01-28T11:21:15Z

src/operations/either.jl

+end
+@inline function applyview(op::Either, img::AbstractArray)
+    applyview(op, img, randparam(op, img))
+end


The general definition is in operation.jl

for FUN in (:applyeager, :applylazy, :applypermute, :applyaffine, :applyaffineview, :applyaffine_common, :applyaffineview_common, :applyview, :applystepview) @eval begin function ($FUN)(op::Operation, imgs::Tuple) param = randparam(op, imgs) map(img -> ($FUN)(op, img, param), imgs) end @inline function ($FUN)(op::Operation, img::AbstractArray) ($FUN)(op, img, randparam(op, img)) end end end

But the inferred type for Either will be Any(without using _offsetarray) or OffsetArray. Add a specialization here tells the compiler more type information.

This reverts commit a698382.

johnnychen94 · 2020-01-29T07:26:15Z

This PR is growing longer and older with all these commits and it becomes hard to review. Hence I'm merging this PR regardless of two bugs not fixed( #38, #39)

* add Project.toml * update test for cache and minor fix * update test for rotation and minor fix * update flipdm to reverse JuliaLang/julia#26369 * update test for crop and minor fix * update test for resize * update test for scale and minor fix * update test for zoom and minor fix * WIP: update test for distortion and minor fix * update test for either and minor fix * fix pipeline test * update summary reference for distortedview * WIP: fix tests for augment and operations * drop julia 0.7 and update CI * use FileIO v1.1.0 JuliaIO/FileIO.jl#245 * add 1.x tests to CI * revert back unnecessary changes * test windows in travis * fix randomly failed test cases for inacuraccy reasons * update to julia 1.1 * maybe_copy doesn't explain its usage * replace Slice by OffsetArrays.IdentityUnitRange JuliaArrays/OffsetArrays.jl#62 * update to julia 1.2 -- part I * WIP: update to julia 1.2 -- part II detect some type instable test cases, only the outmost wrapper is instable here * unify how types are displayed Only meta operations such as Either, Pipeline doesn't have Agumentor prefix * add more test versions * move safe_rand to compat.jl rand is thread-safe in julia 1.3 * don't allow failures for julia 1.3 * fix commit "WIP: update 1.0" * use explicit and intuitive CartesianIndex(1, 1) * add method specialization for tweight * allow FileIO 1.2 * fix type instability for Either * update reference for FixedPointNumbers v0.7 * try: relax equality check for scale * restore test cases in tst_scale.jl * Revert "add method specialization for tweight" This reverts commit a698382. * Augmentor v0.6.0-pre

johnnychen94 added 13 commits December 16, 2019 04:02

add Project.toml

86fbdb5

update test for cache and minor fix

42aa2f0

update test for rotation and minor fix

061ddc9

update flipdm to reverse

e3b61db

JuliaLang/julia#26369

update test for crop and minor fix

766744f

update test for resize

c471003

update test for scale and minor fix

1cbe9e4

update test for zoom and minor fix

75b433a

WIP: update test for distortion and minor fix

3612f17

update test for either and minor fix

970a691

fix pipeline test

c666766

update summary reference for distortedview

e33c6ae

WIP: fix tests for augment and operations

f096c05

johnnychen94 changed the base branch from master to update0.7 December 16, 2019 00:44

drop julia 0.7 and update CI

8099666

johnnychen94 commented Dec 16, 2019

View reviewed changes

src/distortedview.jl Outdated Show resolved Hide resolved

src/distortedview.jl Outdated Show resolved Hide resolved

src/operations/resize.jl Outdated Show resolved Hide resolved

johnnychen94 added 3 commits December 16, 2019 09:01

use FileIO v1.1.0

9d6a9b3

JuliaIO/FileIO.jl#245

add 1.x tests to CI

d3e60f2

revert back unnecessary changes

2e97f13

test windows in travis

6c95157

fix randomly failed test cases for inacuraccy reasons

4891a58

This was referenced Dec 16, 2019

WIP: Update1.x #29

Closed

upgrade to Julia 1.x #32

Merged

johnnychen94 added 4 commits December 17, 2019 06:38

unify how types are displayed

23ddbd1

Only meta operations such as Either, Pipeline doesn't have Agumentor prefix

update to julia 1.1

dc3682e

* maybe_copy doesn't explain its usage * replace Slice by OffsetArrays.IdentityUnitRange JuliaArrays/OffsetArrays.jl#62

update to julia 1.2 -- part I

4758e77

WIP: update to julia 1.2 -- part II

fdbb15c

detect some type instable test cases, only the outmost wrapper is instable here

johnnychen94 changed the base branch from update0.7 to jc/1.x December 16, 2019 22:44

johnnychen94 commented Dec 16, 2019

View reviewed changes

src/utils.jl Show resolved Hide resolved

johnnychen94 commented Dec 16, 2019

View reviewed changes

Evizero reviewed Dec 17, 2019

View reviewed changes

johnnychen94 added 2 commits December 17, 2019 22:36

move safe_rand to compat.jl

1587955

rand is thread-safe in julia 1.3

don't allow failures for julia 1.3

19aeb9e

johnnychen94 added 2 commits December 18, 2019 06:37

fix commit "WIP: update 1.0"

733e32b

use explicit and intuitive CartesianIndex(1, 1)

67e9784

johnnychen94 added this to the v0.6 milestone Dec 19, 2019

johnnychen94 commented Dec 21, 2019

View reviewed changes

This was referenced Jan 26, 2020

Update .travis.yml #37

Closed

specialize tweight on FixedPoint JuliaImages/ImageTransformations.jl#89

Closed

johnnychen94 added 4 commits January 27, 2020 20:32

add method specialization for tweight

a698382

allow FileIO 1.2

5f96ab5

fix type instability for Either

61d8cee

update reference for FixedPointNumbers v0.7

2bf0975

johnnychen94 commented Jan 28, 2020

View reviewed changes

johnnychen94 added 2 commits January 28, 2020 22:06

try: relax equality check for scale

f483d2c

restore test cases in tst_scale.jl

2a0c7c9

johnnychen94 mentioned this pull request Jan 29, 2020

random failure OffsetArray tests for Scale and Zoom #38

Closed

johnnychen94 added 2 commits January 29, 2020 15:04

Revert "add method specialization for tweight"

8443246

This reverts commit a698382.

Augmentor v0.6.0-pre

e915c94

johnnychen94 changed the title ~~[WIP] update code to 1.0 and 1.x~~ update codebase to 1.0 and 1.x (continue #27) Jan 29, 2020

johnnychen94 merged commit 7a30087 into Evizero:jc/1.x Jan 29, 2020

johnnychen94 deleted the jc/update1.0 branch January 29, 2020 07:30

johnnychen94 added the no changelog tell TagBot to not add this PR into release changelog label Feb 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update codebase to 1.0 and 1.x (continue #27) #31

update codebase to 1.0 and 1.x (continue #27) #31

johnnychen94 commented Dec 16, 2019 •

edited

Loading

johnnychen94 commented Dec 16, 2019

Evizero commented Dec 16, 2019

johnnychen94 Dec 16, 2019

Evizero Dec 17, 2019

johnnychen94 Dec 17, 2019 •

edited

Loading

johnnychen94 Dec 19, 2019 •

edited

Loading

Evizero left a comment

Evizero Dec 17, 2019

Evizero Dec 17, 2019

johnnychen94 Dec 17, 2019

johnnychen94 Dec 17, 2019 •

edited

Loading

johnnychen94 commented Dec 17, 2019 •

edited

Loading

Evizero commented Dec 19, 2019

johnnychen94 Dec 21, 2019

Evizero Dec 21, 2019

johnnychen94 Jan 28, 2020 •

edited

Loading

johnnychen94 Jan 28, 2020

johnnychen94 commented Jan 29, 2020

	# To preserve type stability of "applyeager", always promote
	# arrays to offset arrays
	@inline _offsetarray(A::Array) = OffsetArray(A, 0, 0)
	@inline _offsetarray(A::OffsetArray) = A
	function applyeager(op::Either, img::AbstractArray, idx)
	_offsetarray(applyeager(op.operations[idx], img))
	end

update codebase to 1.0 and 1.x (continue #27) #31

update codebase to 1.0 and 1.x (continue #27) #31

Conversation

johnnychen94 commented Dec 16, 2019 • edited Loading

johnnychen94 commented Dec 16, 2019

Evizero commented Dec 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnychen94 Dec 17, 2019 • edited Loading

Choose a reason for hiding this comment

johnnychen94 Dec 19, 2019 • edited Loading

Choose a reason for hiding this comment

Evizero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnychen94 Dec 17, 2019 • edited Loading

Choose a reason for hiding this comment

johnnychen94 commented Dec 17, 2019 • edited Loading

Evizero commented Dec 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnychen94 Jan 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnychen94 commented Jan 29, 2020

johnnychen94 commented Dec 16, 2019 •

edited

Loading

johnnychen94 Dec 17, 2019 •

edited

Loading

johnnychen94 Dec 19, 2019 •

edited

Loading

johnnychen94 Dec 17, 2019 •

edited

Loading

johnnychen94 commented Dec 17, 2019 •

edited

Loading

johnnychen94 Jan 28, 2020 •

edited

Loading