-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Julia 1.6's new reinterpretarray machinery #145
Conversation
Codecov Report
@@ Coverage Diff @@
## master #145 +/- ##
=========================================
Coverage ? 74.41%
=========================================
Files ? 10
Lines ? 516
Branches ? 0
=========================================
Hits ? 384
Misses ? 132
Partials ? 0
Continue to review full report at Codecov.
|
So, it seems that our benchmarks don't run anymore as part of CI. Intended? I see #123. I should also acknowledge that because I've tightened the "bounds" on what we consider acceptable, julia> include("benchmarks.jl")
[ Info: Benchmark tests are warnings for now
┌ Warning: colorview2: failed on mysum_elt_boundscheck with eltype Float32, time ratio 8.859957776213934, tol 5
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: colorview2: failed on mysum_elt_inbounds with eltype Float32, time ratio 8.861587413823582, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: channelview1: failed on mysum_index_inbounds_simd with eltype Float32, time ratio 27.78320479862896, tol 20
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:119
┌ Warning: colorview2: failed on mysum_index_inbounds_simd with eltype Float32, time ratio 6.7447698744769875, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: colorview1: failed on myfill1! with eltype Float32, time ratio 11.369456762188495, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:132
┌ Warning: colorview2: failed on myfill1! with eltype Float32, time ratio 18.31693755262115, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: channelview1: failed on myfill2! with eltype Float32, time ratio 29.777311566200062, tol 20
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:119
┌ Warning: colorview1: failed on myfill2! with eltype Float32, time ratio 13.63551190747891, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:132
┌ Warning: colorview2: failed on myfill2! with eltype Float32, time ratio 12.960752632917918, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: colorview2: failed on mysum_elt_boundscheck with eltype Float64, time ratio 7.129490249743414, tol 5
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: colorview2: failed on mysum_elt_inbounds with eltype Float64, time ratio 7.149125814192664, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: colorview1: failed on myfill1! with eltype Float64, time ratio 7.043604325936975, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:132
┌ Warning: colorview2: failed on myfill1! with eltype Float64, time ratio 9.796395232297128, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138
┌ Warning: colorview1: failed on myfill2! with eltype Float64, time ratio 6.146765267631468, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:132
┌ Warning: colorview2: failed on myfill2! with eltype Float64, time ratio 9.315949234411704, tol 3
└ @ Main ~/.julia/dev/ImageCore/test/benchmarks.jl:138 but on 1.6 I get julia> include("benchmarks.jl")
[ Info: Benchmark tests are warnings for now
julia> |
I'm a little unsure what to benchmark so #123 is probably abandoned. The noise you mentioned is also concerning that I failed to restart that... FYI, https://github.com/JuliaImages/ImageContrastAdjustment.jl has a well-running benchmark CI that you can use to get some idea. |
For now, though, should we still be running the benchmarks? I'm guessing that in Lines 32 to 37 in d9dccd9
"CI" isn't set by GitHub Actions?
EDIT: oh wait, we specifically don't run them on CI. Can you remind me why? |
One drawback of setting up benchmarkCI is that we hand almost everything to PkgBenchmark and so we can't do t_baseline = @belapsed f_baseline(...)
t = @belapsed f(...)
ratio = t/t_baseline we might need two benchmark CIs for the very different two benchmark tasks:
|
Gotcha. Seems complicated enough that we should just accept the status quo here and deal with it separately. I'll leave this open for a bit for review, in case there are any concerns. |
When available, this switches to using Julia 1.6's new
reinterpret(reshape, T, A)
for ourcolorview
/channelview
functionality. As documented in #142, this essentially puts to rest the performance problems we've had since Julia 1.0 withReinterpretArray
.The tests will still not quite pass on nightly due to
show
failures, but I plan to fix that in a separate PR.Closes #142