add enforce connectivity feature in analyze method #4

Cuda-Chen · 2020-10-24T17:09:53Z

Add enforce connectivity feature in analyze method.
The enforce connectivity implementation is based on scikit-image.

Because the original implementation uses 0-indexing, so I tried to translate into 1-indexing.
Though so far there are no any problems about unbounded error, it is ought to conduct test because I think
the implementation still have some problems in BFS part.

Feel free to edit and delete file changes. I am first time to Julia development, so I use some dirty ways to let
the test run.

1. Add superpixel creation test. 2. Superpixel creation test failed.

Superpixel analysis succeeded, but got stack overflow in synthesize.jl:30.

Seems unbounded problem when enforce_connectivity = true. Yet need further testing. Still stackoverflow error persists when trying synthesize image from SLIC-generated (analyze.jl) Superpixels.

Cuda-Chen · 2020-10-24T17:12:45Z

As a reminder, I think there is a problem about the synthesis method.
When I tried to synthesize Superpixels generated by analyze method, the synthesis method always gives me stackoverflow error.

johnnychen94 · 2020-10-25T11:57:05Z

Thank you so much for doing this! I'm a bit busy these days, I'll try to manage this before next weekend! (Need some time to revisit the SLIC details)

Also, the code was written long before when I wasn't so familiar with performance tweaks in Julia. So I'll need to revisit my own codes, too.

Cuda-Chen · 2020-10-25T13:47:53Z

Got it! Waiting for good news!

Cuda-Chen · 2020-12-16T01:32:07Z

Hi @johnnychen94,
is there any updates so that I can give you a hand?

johnnychen94 · 2020-12-16T06:30:49Z

Sorry I have forgotten about this, too busy with the PhD research pressure 😨 . I'll check it this weekend, I promise.

If you're sure that the implementation is correct, I can skip the check and just do some optimization/tweaks on it.

johnnychen94

Without looking at the detailed algorithm, mostly it looks good.

We can add one more method so that there's no need to manually write Lab.(input_image)

_slic(alg::SLIC, img::AbstractArray{<:Colorant}) = _slic(alg, of_eltype(Lab, img))

of_eltype is from MappedArrays, which gives a lazy version (no memory allocation) of Lab.(img).

johnnychen94 · 2020-12-16T06:41:18Z

demo.jl

@@ -0,0 +1,7 @@
+using Images, ImageCore


We'll need to remove this eventually, but yeah this can be a good start-up demo for me :P

johnnychen94 · 2020-12-16T06:43:49Z

src/SuperPixels.jl

@@ -15,6 +15,6 @@ export
    imsize,
    # algorithms
    synthesize, Raw, Average,
-    analyze, SLIC
+    analyze, SLIC, _slic


The API I have in mind is

alg = SLIC(n_segments=100, compactness=10.0, max_iter=10, enforce_connectivity=false) analyze(alg, img)

so _slic here is just an internal implementation, we don't need to export this.

FYI, according to the discussion in JuliaImages/ImageBinarization.jl#23, we may need to swap the arguments in future PR, i.e., analyze(img, alg)

johnnychen94 · 2020-12-16T06:45:52Z

src/analyze.jl

    if alg.enforce_connectivity
-        nothing # TODO
+        println("zergling")


We need to remove these prints.

johnnychen94 · 2020-12-16T06:47:59Z

src/analyze.jl

+                end
+            end
+        end
+        nearest_segments = copy(nearest_segments_final)


What's the usage of this copy?

This copy is not to change the source code.
But it will be fine to use nearest_segments_final directly in the future.

johnnychen94 · 2020-12-16T06:54:12Z

src/analyze.jl

+
+        start_label = 1
+        mask_label = start_label - 1 # indicates the label of this pixel has not been assigned
+        nearest_segments_final = fill(mask_label, height, width)


It occurs to me that we can reuse the memory allocated by nearest_segments.

johnnychen94 · 2020-12-16T06:55:17Z

src/analyze.jl

+                if nearest_segments[y, x] == mask_label continue end
+                if nearest_segments_final[y, x] > mask_label continue end


In Julia we usually use

Suggested change

if nearest_segments[y, x] == mask_label continue end

if nearest_segments_final[y, x] > mask_label continue end

nearest_segments[y, x] == mask_label && continue

nearest_segments_final[y, x] > mask_label && continue

johnnychen94 · 2020-12-16T06:57:37Z

src/analyze.jl

+
+                # Preform BFS to find the size of superpixel with 
+                # same lable number
+                while bfs_visited < current_segment_size <= max_size


It might be worth adding @inbounds to this deeply nested for loop so as to skip unnecessary checks during getindex and setindex, e.g., yy = coord_list[bfs_visited + 1, 1] + dy[i] and coord_list[current_segment_size + 1, 1] = yy

Suggested change

while bfs_visited < current_segment_size <= max_size

@inbounds while bfs_visited < current_segment_size <= max_size

Though it sounds great, BFS usually needs to check bound.
I will try to research about BFS non-checking bounds method.

johnnychen94 · 2020-12-16T06:58:23Z

src/analyze.jl

+
+                # merge the superpixel to its neighbor if it is too small
+                if current_segment_size < min_size
+                    for i = 1:current_segment_size


if there's no if statements in the for-loop, it's usually a good suggestion to add @simd macro to do the CPU level parallelization.

Suggested change

for i = 1:current_segment_size

@inbounds @simd for i = 1:current_segment_size

also remove println.

To be clear. This and the @inbounds comments are for minor performance tweaks. I'm not sure if these are useful, so benchmarks are needed. If you believe these are useless, feel free to ignore them.

These are very low-level tweaks where the for loops contains only trivial operations. As a simple example:

function arrsum(X) rst = zero(eltype(X)) for i in 1:length(X) rst += X[i] end rst end function arrsum_simd(X) rst = zero(eltype(X)) @simd for i in 1:length(X) rst += @inbounds X[i] end rst end julia> @btime arrsum($X) 833.455 μs (0 allocations: 0 bytes) 499930.1049151086 julia> @btime arrsum_inbounds($X) 207.907 μs (0 allocations: 0 bytes) 499930.1049150961

Cuda-Chen · 2020-12-16T07:49:45Z

Thanks for your review!
I will apply these suggestions then re-commit the changes.

johnnychen94

Will there be more commits in? I can do the cleanups in #2 if there aren't more.

johnnychen94 · 2020-12-19T19:44:45Z

Project.toml

 ImageCore = "a09fc81d-aa75-5fe9-8630-4744c3626534"
 ImageDraw = "4381153b-2b60-58ae-a1ba-fd683676385f"
+ImageMagick = "6218d12a-5da1-5696-b52f-db25d2ecc6d1"
+Images = "916415d5-f1e6-5110-898d-aaa5f9f070e0"
 Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"


We don't need these new deps: BenchmarkTools, DelimitedFiles, ImageMagick, and Images.
Those are only for demo purposes.

Cuda-Chen · 2020-12-20T02:28:41Z

There won't be any commits in.
You can start to cleanup.

johnnychen94 · 2020-12-21T00:03:56Z

I'm merging this into #2, and will try to merge #2 this week.

Cuda-Chen added 5 commits October 14, 2020 13:31

finish enforce connectivity part, waiting for testing

5dfede2

add testing script and update dependencies

d34c4d7

add superpixel creation test with testing failed

6ee0b31

1. Add superpixel creation test. 2. Superpixel creation test failed.

issue: stack overflow in synthesize.jl:30

9ada14a

Superpixel analysis succeeded, but got stack overflow in synthesize.jl:30.

seems unbounded problem solved yet need further testing

acd640d

Seems unbounded problem when enforce_connectivity = true. Yet need further testing. Still stackoverflow error persists when trying synthesize image from SLIC-generated (analyze.jl) Superpixels.

johnnychen94 reviewed Dec 16, 2020

View reviewed changes

Cuda-Chen added 3 commits December 16, 2020 17:22

applying changes of suggestions

e3fbe62

fix typo

923541e

benchmarking

73c853d

johnnychen94 reviewed Dec 20, 2020

View reviewed changes

johnnychen94 merged commit 9ad8154 into johnnychen94:analyze Dec 21, 2020

johnnychen94 mentioned this pull request Dec 21, 2020

[WIP] add analyze method SLIC to generate superpixels from image #2

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add enforce connectivity feature in analyze method #4

add enforce connectivity feature in analyze method #4

Cuda-Chen commented Oct 24, 2020

Cuda-Chen commented Oct 24, 2020

johnnychen94 commented Oct 25, 2020 •

edited

Loading

Cuda-Chen commented Oct 25, 2020

Cuda-Chen commented Dec 16, 2020

johnnychen94 commented Dec 16, 2020

johnnychen94 left a comment •

edited

Loading

johnnychen94 Dec 16, 2020

johnnychen94 Dec 16, 2020

johnnychen94 Dec 16, 2020

johnnychen94 Dec 16, 2020

johnnychen94 Dec 16, 2020

Cuda-Chen Dec 16, 2020

johnnychen94 Dec 16, 2020

johnnychen94 Dec 16, 2020

johnnychen94 Dec 16, 2020

Cuda-Chen Dec 16, 2020

johnnychen94 Dec 16, 2020 •

edited

Loading

johnnychen94 Dec 16, 2020 •

edited

Loading

Cuda-Chen commented Dec 16, 2020

johnnychen94 left a comment

johnnychen94 Dec 19, 2020

Cuda-Chen commented Dec 20, 2020

johnnychen94 commented Dec 21, 2020

		if nearest_segments[y, x] == mask_label continue end
		if nearest_segments_final[y, x] > mask_label continue end

	while bfs_visited < current_segment_size <= max_size
	@inbounds while bfs_visited < current_segment_size <= max_size

	for i = 1:current_segment_size
	@inbounds @simd for i = 1:current_segment_size

add enforce connectivity feature in analyze method #4

add enforce connectivity feature in analyze method #4

Conversation

Cuda-Chen commented Oct 24, 2020

Cuda-Chen commented Oct 24, 2020

johnnychen94 commented Oct 25, 2020 • edited Loading

Cuda-Chen commented Oct 25, 2020

Cuda-Chen commented Dec 16, 2020

johnnychen94 commented Dec 16, 2020

johnnychen94 left a comment • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnnychen94 Dec 16, 2020 • edited Loading

Choose a reason for hiding this comment

johnnychen94 Dec 16, 2020 • edited Loading

Choose a reason for hiding this comment

Cuda-Chen commented Dec 16, 2020

johnnychen94 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cuda-Chen commented Dec 20, 2020

johnnychen94 commented Dec 21, 2020

johnnychen94 commented Oct 25, 2020 •

edited

Loading

johnnychen94 left a comment •

edited

Loading

johnnychen94 Dec 16, 2020 •

edited

Loading

johnnychen94 Dec 16, 2020 •

edited

Loading