-
Notifications
You must be signed in to change notification settings - Fork 49
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
also use the streaming max-min filter for maximum/minimum #193
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #193 +/- ##
==========================================
- Coverage 91.62% 91.49% -0.14%
==========================================
Files 9 9
Lines 1385 1387 +2
==========================================
Hits 1269 1269
- Misses 116 118 +2
Continue to review full report at Codecov.
|
Benchmark resultJudge resultBenchmark Report for /home/runner/work/ImageFiltering.jl/ImageFiltering.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/ImageFiltering.jl/ImageFiltering.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/ImageFiltering.jl/ImageFiltering.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Benchmark resultJudge resultBenchmark Report for /home/runner/work/ImageFiltering.jl/ImageFiltering.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/ImageFiltering.jl/ImageFiltering.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/ImageFiltering.jl/ImageFiltering.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
The speed improvement or worsening depends on the window size; I worry a 3x3 window might not show a speedup. The Lemire algorithm has a different order than naive The other issue is that if you want both the max & min, it's clearly better to call For these reasons I'd avoided implementing these shortcuts, in essence hoping to convince people to use |
It is still more efficient in 3*3 case, probably contributed by less memory allocation. julia> @btime mapwindow(maximum, $img2d, (3,3));
40.210 μs (54 allocations: 52.56 KiB)
julia> @btime mapwindow(f, $img2d, (3, 3));
109.429 μs (2350 allocations: 141.25 KiB) I'll need to read the paper first to see if there's a better way. I mean, to only compute the maximum in a similar way. Unsure of it now. |
Looks like the performance issue with |
Playing with https://github.com/sairus7/MaxMinFilters.jl and it turns out to be more performant than ImageFiltering's implementation. I guess this indicates that there's still room for optimization. julia> img2d = randn(30,30);
julia> @btime mapwindow(extrema, $img2d, (3,1));
21.749 μs (47 allocations: 44.91 KiB)
julia> @btime movmaxmin($img2d, 3);
5.271 μs (4 allocations: 14.63 KiB) MaxMinFilters does not apply to the multidimensional case, though; it simply treats it as a vector. I'll see what I can do to get things better here. A re-reimplementation, probably, in an independent package and get used by ImageFiltering.jl. Just took the first read on the paper and it occurs that a maximum/minimum filter is possible if we only apply the wedge update on one side. So yeah, it's doable to for Tweaking the implementation doesn't look like an easy job, so I'll revisit this when I finish my WNNM project. Probably months later, though... |
I'm unsure it should be a separate package...if anything, I'm inclined to go the other way (JuliaImages/Images.jl#898). Well, I guess it could be a separate package, it's the separate repo that I find annoying. |
The main reason they differ in performance is because this implementation supports multiple dimensions. More than half the time is due to this one comprehension: ImageFiltering.jl/src/mapwindow.jl Line 375 in 0c963dc
But that's the foundation for multidimensional support, as it allows the filter to be separable. Almost all the rest is due internal methods of Anyway, I don't think we should be so quick to abandon the current implementation. There might be a quick way to make the 1d case not need the comprehension, for example. |
I haven't read the Lemire paper yet so I don't know if there is a more efficient implementation for maximum/minimum, but now simply applying
first
/last
on theextrema
result still gives about ~4x boost.Hmmm, I plan to read that for my own interest in JuliaImages/Images.jl#918.
cc @Dsantra92