Closed tlnagy closed 4 years ago
Looks like my PR introduces quite a few allocations which are causing some slowdowns. Not sure where they are creeping in just yet. Looks like I had some parameterization problems, the updated PR has the same performance as master
Master
julia> @benchmark segments = watershed(dist, markers)
BenchmarkTools.Trial:
memory estimate: 3.24 MiB
allocs estimate: 119
--------------
minimum time: 41.722 ms (0.00% GC)
median time: 42.512 ms (0.00% GC)
mean time: 43.070 ms (0.92% GC)
maximum time: 48.598 ms (5.49% GC)
--------------
samples: 117
evals/sample: 1
This PR
julia> @benchmark segments = watershed(dist, markers)
BenchmarkTools.Trial:
memory estimate: 3.37 MiB
allocs estimate: 119
--------------
minimum time: 44.658 ms (0.00% GC)
median time: 45.398 ms (0.00% GC)
mean time: 45.813 ms (0.92% GC)
maximum time: 48.927 ms (5.84% GC)
--------------
samples: 110
evals/sample: 1
Sorry for the delay here. It will take me a little while to read the paper so that I can understand this. Once you get eager to have this merged strip the WIP and ping me.
The TL;DR on the compact watershed algorithm is that, instead of assigning the final label as a pixel is pushed into the queue, it updates the label every time it encounters that position if the new label is more optimal than the previous label. More optimal in this case is a linear combination of the euclidean distance between the seed and the pixel and the typical brightness of the pixel. It doesn't lock in the final labelling until the pixels come off the queue, this lets it find better labelings later and replace initial labelings, which cannot happen with standard watershed.
I hope that helps explain what happens.
I rebased, fixed the PR based on your suggestions, fixed a representation error bug with segmenting fixed point number images, and added tests.
I think this is ready to merge once the tests pass.
@timholy Whenever you get a chance, could you merge this and tag a new release? Thanks!
Reminder: retrigger the test after https://github.com/JuliaImages/ImageSegmentation.jl/pull/44 being merged. (Done)
I updated this PR by adding back the type paramerization in PixelKey. This fixes the performance regressions, it's basically back to the original speed. The compact algorithm now always uses floats to avoid the representation error when accumulating using fixed point numbers. @timholy and @johnnychen94, I believe this is ready to merge.
This PR:
julia> @benchmark segments = watershed(dist, markers)
BenchmarkTools.Trial:
memory estimate: 3.44 MiB
allocs estimate: 121
--------------
minimum time: 35.417 ms (0.00% GC)
median time: 39.968 ms (0.00% GC)
mean time: 39.711 ms (0.11% GC)
maximum time: 42.989 ms (0.00% GC)
--------------
samples: 126
evals/sample: 1
Master:
julia> @benchmark segments = watershed(dist, markers)
BenchmarkTools.Trial:
memory estimate: 3.24 MiB
allocs estimate: 119
--------------
minimum time: 33.161 ms (0.00% GC)
median time: 36.747 ms (0.00% GC)
mean time: 36.770 ms (0.11% GC)
maximum time: 41.609 ms (0.00% GC)
--------------
samples: 136
evals/sample: 1
Not sure why Travis isn't showing up under checks, but the build passed: https://travis-ci.org/github/JuliaImages/ImageSegmentation.jl/builds/672668131
Also this should fix #34
@johnnychen94 and @timholy I finally got around to making all the suggested changes. Hope this is finally ready for a merge!
Bump @johnnychen94 and @timholy. Would be great to have this merged!
Thanks for the work and patience @tlnagy !
Could you tag a new version when you get a chance? Thanks!
Thanks @tlnagy, and @johnnychen94 for merging!
This PR adds support for the compact watershed algorithm (https://www.tu-chemnitz.de/etit/proaut/publications/cws_pSLIC_ICPR.pdf). This algorithm is nice because it results in more uniform watershed basins as you increase the
compactness
parameter.I still need to do the following before I think it should be merged:
Example segmentation following the coin example from the docs:
closes #34