Skip to content
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

Add compact watershed and clean up existing watershed #2211

Merged
merged 33 commits into from
Aug 27, 2016

Conversation

jni
Copy link
Member

@jni jni commented Jul 22, 2016

Cleaned up the watershed code, which had lots of legacy code buried in there. Then on that nice frame I hung the new compact watershed algorithm, which gives much nicer results even for puny values of compactness.

Checklist

Gallery example to follow... Update: it's here! See output below.

@soupault soupault added the ⏩ type: Enhancement Improve existing features label Jul 22, 2016
@jni
Copy link
Member Author

jni commented Jul 22, 2016

figure_1

@sciunto
Copy link
Member

sciunto commented Jul 22, 2016

Nice results @jni !

A quick comment. How this example looks like with the compact watershed? http://scikit-image.org/docs/dev/auto_examples/segmentation/plot_watershed.html#sphx-glr-auto-examples-segmentation-plot-watershed-py

Also, is it necessary to make two different examples on watershed in the gallery instead of improving the existing one? I'm afraid that the multiplication of examples on same/similar functions will increase the entropy and the likelihood to find the right example.

@jni
Copy link
Member Author

jni commented Jul 22, 2016

@sciunto compact watershed will do worse in the above example, unfortunately (it will cause the smaller object to "leak" into the bigger one). So I think it's best to leave it as a separate example...

@jni
Copy link
Member Author

jni commented Jul 27, 2016

@scikit-image/core anyone up for reviewing this?

CC @LeeKamentsky @thouis @kevin-keraudren

@LeeKamentsky
Copy link

👍 from me, esp for the cleanup. A lot of that was long overdue. The compact watershed is optional which gives people the choice whether or not to use it and IMHO, it is a great idea, especially for blobby objects. Another possibility would be to sort by pixel intensity and then distance in the heap code, instead of conflating the pixel value as a combination of pixel intensity and distance, but as it stands, you have a dial to tweak to offset the weighting of distance in your segmentation rather than having a tiebreaker and pragmatically, that's probably better.

So, if you need my blessing, here it is and thanks for improving the code.

The *compact* watershed transform remedies this by favoring seeds that are
close to the pixel being considered.

Both are implemented in the :py:func:`skimage.morphology.watershed` function.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both what I am wondering while reading? Maybe change to "Both algorithms..."

@kevin-keraudren
Copy link
Contributor

Since the compact watershed is very similar to SLIC, why not do some cross-referencing with the See also sections of the docstrings?
Also, you could add the compact watershed to the SuperPixel example: http://scikit-image.org/docs/dev/auto_examples/plot_segmentations.html

@jni
Copy link
Member Author

jni commented Jul 28, 2016

@LeeKamentsky

So, if you need my blessing, here it is and thanks for improving the code.

Thanks! =D Actually the purpose of pinging you was more in case you wanted to incorporate it into CellProfiler (though I realise that's not your primary concern anymore).

@kevin-keraudren

I would also consider adding the geodesic watershed:

Capital idea! As I was writing it I had a feeling that it wouldn't be too much trouble computing the geodesic, but I didn't realise just how trivial it would be.

@ahojnnes thanks for the review, working on it! =)

@codecov-io
Copy link

codecov-io commented Jul 28, 2016

Current coverage is 90.75% (diff: 84.50%)

Merging #2211 into master will increase coverage by 0.05%

@@             master      #2211   diff @@
==========================================
  Files           303        303          
  Lines         21849      21797    -52   
  Methods           0          0          
  Messages          0          0          
  Branches       2019       2002    -17   
==========================================
- Hits          19817      19782    -35   
+ Misses         1670       1657    -13   
+ Partials        362        358     -4   

Powered by Codecov. Last update c394264...d34f7ab

@jni
Copy link
Member Author

jni commented Jul 28, 2016

@kevin-keraudren so I feel like I need an additional float accumulator for the geodesic, containing the distance travelled thus far. This can be reconstructed from the difference between the current element's value and the image value at that location — but only if euclidean compactness is turned off. (With it on, you still can do it, but it's just a mess.) I'm wondering whether I should leave this for another PR... Or maybe I'm just missing something?

@kevin-keraudren
Copy link
Contributor

@jni In the case of the normal watershed, we initialise the seeds with elem.value = image[index] and new pixels are added to the heap with:

new_elem.value = image[index]
if compactness > 0:
    new_elem.value += (compactness *
                                     _euclid_dist(index, elem.source, strides))

In the case of the geodesic watershed, there is no need for another accumulator, instead we initialise the seeds with elem.value = 0 and we add new points to the heap with:

new_elem.value = elem.value + sqrt((gradient_cost*(image[index] - image[elem.index])**2 +
                                                           euclidean_cost * _euclid_dist(index, elem.index, strides)**2))

So it requires more code changes than I first suggested, and either we are in normal watershed/compact watershed mode, or we are in geodesic watershed mode, but we should not have compact-geodesic-watershed mode!

@jni
Copy link
Member Author

jni commented Jul 29, 2016

@kevin-keraudren I think I'll leave the geodesic for another time then. I think it would actually make a lot of sense for multichannel images, too (instead of having to compute the gradient beforehand).

@ahojnnes @sciunto @soupault this is ready for another round of review, I think!

@@ -55,6 +153,9 @@ def watershed(image, markers, connectivity=None, offset=None, mask=None):
mask: ndarray of bools or 0s and 1s, optional
Array of same shape as `image`. Only points at which mask == True
will be labeled.
compactness : float, optional
Use compact watershed [3]_ with given compactness parameter.
This results in more regularly-shaped watershed basins.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A larger or smaller value? This is ambiguous to me...

@ahojnnes
Copy link
Member

LGTM, apart from Travis...

@jni
Copy link
Member Author

jni commented Aug 1, 2016

Build succeeds locally after latest (blind) fix... @stefanv @blink1073 any ideas on how to troubleshoot this Sphinx error?

Warning, treated as error:
<autosummary>:1: WARNING: citation not found: R314

This is used in computing the Euclidean distance between raveled
indices.
compactness : float
A value >0 implements the compact watershed algorithm (see .py file).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after > sign?

@jni
Copy link
Member Author

jni commented Aug 25, 2016

@sciunto, I've rebased on top of current master, which has a single-threaded sphinx build. No code has changed so I suggest merging if Travis is happy. =)

@jni
Copy link
Member Author

jni commented Aug 25, 2016

I'll point out that Travis has passed (\o/) except for the Xcode bit which hasn't started (won't start?).

@sciunto
Copy link
Member

sciunto commented Aug 27, 2016

OK, I restarted travis and now, it passes. I'm also +1, so I merge.

Thanks @jni It's a very nice piece of work.

@sciunto sciunto merged commit 74c6b8d into scikit-image:master Aug 27, 2016
@jni jni deleted the compact-watershed branch September 8, 2016 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants