JuliaImages / ImageSegmentation.jl

Partitioning images into meaningful regions
Other
47 stars 23 forks source link

Some necessary functions for RAG and removing small segments #7

Closed annimesh2809 closed 7 years ago

annimesh2809 commented 7 years ago

The following functions have been added:

All these functions have been verified to be working on N-d SegmentedImage. For 512x512 color images, region_adjaceny_graph takes ~50ms and others take ~100ms. Tests will be added soon.

There is also a performance fix for fast_scanning.

annimesh2809 commented 7 years ago

Seeing the travis build, it seems like ImageTransformations.jl is breaking with Julia 0.7.

Evizero commented 7 years ago

ImageTransformations.jl is breaking with Julia 0.7

:(

Are we at all close to 0.7? I have not spend any time investigating the required changes

annimesh2809 commented 7 years ago

Whoops sorry! I meant Julia nightly :sweat_smile:

Evizero commented 7 years ago

No no I understood. I am just wondering if we are close to a point in time where the nightly fails should be addressed since at the moment I am rather ignorant towards them

timholy commented 7 years ago

0.7 will have some nice goodies; I've made quite a few contributions myself, including a better inlining algorithm in the compiler---which turned out to yield quite a few performance improvements---and more user-friendly (i.e., useful for debugging) error messages for InexactError, DomainError, and OverflowError. That said, I'm not all that worried about targeting 0.7 yet, because there are going to be a lot of breakages still to come. The worst one currently is the migration of FFTW out of base (https://github.com/JuliaMath/FFTW.jl/issues/13), which is currently making Images not even buildable. So I've been considering adding nightly to the allow_failures matrix.

codecov[bot] commented 7 years ago

Codecov Report

Merging #7 into master will decrease coverage by 20.73%. The diff coverage is 4%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #7       +/-   ##
===========================================
- Coverage     100%   79.26%   -20.74%     
===========================================
  Files           5        5               
  Lines         281      352       +71     
===========================================
- Hits          281      279        -2     
- Misses          0       73       +73
Impacted Files Coverage Δ
src/ImageSegmentation.jl 100% <ø> (ø) :arrow_up:
src/core.jl 2.7% <0%> (-97.3%) :arrow_down:
src/fast_scanning.jl 98.48% <100%> (-1.52%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d7dff8e...f5db63a. Read the comment docs.

codecov[bot] commented 7 years ago

Codecov Report

Merging #7 into master will increase coverage by 0.12%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #7      +/-   ##
=========================================
+ Coverage   99.37%   99.5%   +0.12%     
=========================================
  Files           6       6              
  Lines         321     400      +79     
=========================================
+ Hits          319     398      +79     
  Misses          2       2
Impacted Files Coverage Δ
src/ImageSegmentation.jl 100% <ø> (ø) :arrow_up:
src/fast_scanning.jl 98.48% <100%> (-0.03%) :arrow_down:
src/core.jl 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 205a6a4...7b2f7dc. Read the comment docs.

timholy commented 7 years ago

Just checking, anything particular holding this up?

annimesh2809 commented 7 years ago

Sorry, completely forgot about this. I was working on increasing the performance of region splitting and merging. I will make the required changes ASAP.

timholy commented 7 years ago

No problem, that sounds like important work!

annimesh2809 commented 7 years ago

The labels applied to the segments in a SegmentedImage have not been restricted to be "consecutive" integers from 1 to no_of_segments. Currently, they can be any positive integer. However, the vertex labels in a SimpleWeightedGraph must be consecutive integers, hence I thought of adding the vert_map.

We have two options: 1) Restrict the segment labels to be consecutive positive integers starting from 1. This will allow us to remove vert_map. 2) Allow arbitrary positive segment labels. This will require vert_map.

Which one should we go for?

timholy commented 7 years ago

Do you have a favorite? I would be strongly influenced by your own thoughts on the matter.

My own thoughts: presumably this boils down to, "how much of an advantage would it be to allow non-consecutive segment labels?" I can imagine segmenting two corresponding images, one in which there's a ball in the picture and the other in which there is not (imagine two images from a sports game separated by half a second, where the ball flies in or out of the view). You might want to "seed" some segmentation algorithm using the exact same labels for both images, and so you might want to just omit the seed corresponding to "ball" from the one that doesn't have it. Assuming that applications like this are not unreasonable, it seems that given the constraints of SimpleWeightedGraphs that perhaps we'd better keep vert_map.

But if this isn't right, and there really aren't strong justifications for non-consecutive labels, maybe it would be better to skip them just so we can simplify the indexing here.

annimesh2809 commented 7 years ago

seeded_region_growing allows for arbitrary labels as supplied by the user. The user might want to have some a specific label for some interested regions, this can't be achieved if we force consecutive labels. Also, efficient implementation of some algorithms like fast_scanning produces SegmentedImage with non-consecutive labels. Considering these, I think we should allow non-consecutive labels.

timholy commented 7 years ago

LGTM!