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

Region Adjacency Graph #1031

Merged
merged 59 commits into from Jul 3, 2014

Conversation

Projects
None yet
8 participants
@vighneshbirodkar
Copy link
Contributor

vighneshbirodkar commented Jun 16, 2014

Example Images

figure_1
figure_2

@sciunto

This comment has been minimized.

Copy link
Member

sciunto commented Jun 16, 2014

it's gorgeous!

RAG Thresholding
================
This examples constructs a Region Adjacency Graph and merges region which are

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

regionS

This comment has been minimized.

@ahojnnes
This examples constructs a Region Adjacency Graph and merges region which are
similar in color. We construct a RAG and define edges as the difference in
mean color. We the join regions with similar mean color.

This comment has been minimized.

@ahojnnes
from matplotlib import pyplot as plt


def label_mask_img(img, label):

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

Would such a function be useful to include to skimage.color?

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

I think this could be implemented as an argument to color.label2rgb: I'll do that now for a separate PR.

This comment has been minimized.

@jni

jni Jun 18, 2014

Contributor

See #1033. This can now be replaced with color.label2rgb(segmentation, image, kind='avg')

difference in mean color of regions as edge weights.
Given an image and its segmentation, this method constructs the
corresponsing Region Adjacency Graph (RAG).Each node in the RAG

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

Space after .

Given an image and its segmentation, this method constructs the
corresponsing Region Adjacency Graph (RAG).Each node in the RAG
represents a contiguous pixels with in `img` the same label in

This comment has been minimized.

@ahojnnes
>>> img = data.lena()
>>> labels = segmentation.slic(img)
>>> rag = graph.rag_meancolor(img, labels)

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

Can you add a reference?

from skimage import graph


def test_threshold_cut():

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

Is there a way to test more cases?

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

Look at coveralls and make sure that all lines are covered by tests.

This comment has been minimized.

@JDWarner

JDWarner Jun 17, 2014

Contributor

@vighneshbirodkar additionally, you can generate that report locally (assuming you are running a *nix system) by running $ make coverage in the repo root. This is a lot faster than waiting for the Coveralls reports.

This examples constructs a Region Adjacency Graph and merges region which are
similar in color. We construct a RAG and define edges as the difference in
mean color. We the join regions with similar mean color.

This comment has been minimized.

@ahojnnes

ahojnnes Jun 16, 2014

Member

A reference would also be good here.

@ahojnnes

This comment has been minimized.

Copy link
Member

ahojnnes commented Jun 16, 2014

Very nice work!

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jun 16, 2014

@ahojnnes
Thanks. I am just waiting to get the tests to pass, then I'll make the suggested changes.

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jun 16, 2014

@ahojnnes
Can we have the last Travis build restarted ?

@ahojnnes

This comment has been minimized.

Copy link
Member

ahojnnes commented Jun 17, 2014

You can always reinvoke the build by closing and opening the PR.

@ahojnnes ahojnnes closed this Jun 17, 2014

@ahojnnes ahojnnes reopened this Jun 17, 2014

@jni jni closed this Jun 17, 2014

@jni jni reopened this Jun 17, 2014

@jni

This comment has been minimized.

Copy link
Contributor

jni commented Jun 17, 2014

@vighneshbirodkar I agree with @sciunto, that is a gorgeous image, who needs more sophisticated merge algorithms when thresholding produces such beauty? ;)

import numpy as np


def construct_rag_meancolor_3d(img, arr):

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

img, labels

return g


def construct_rag_meancolor_2d(img, arr):

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

There is way too much redundancy here. Do not repeat yourself. This function should be just:

def construct_rag_meancolor_2d(img, labels):
    return contruct_rag_meancolor_3d(img[np.newaxis, ...], labels[np.newaxis, ...])

Better yet, the code should be n-dimensional to begin with.

This comment has been minimized.

@vighneshbirodkar

vighneshbirodkar Jun 17, 2014

Contributor

Hey @jni
I thought that implementing a special case would be better than iterating with an array of indices for nd. I will remove the 2d method and do it using 3d though.

This comment has been minimized.

@JDWarner

JDWarner Jun 17, 2014

Contributor

@vighneshbirodkar You may well be right about speed, though in my experience I'd often happily sacrifice a few ms to have a single, unified algorithm. It really helps for maintainability. Though I'll echo @jni in wishing for full N-d in an ideal world.

g = rag.RAG()

i = 0
while i < l - 1:

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

Use for loops, not while:

for i in range l:

And while we're at it, it's best not to use variables called l, because they are difficult to read. Use depth, width, height.

current = arr[i, j, k]

try:
g.node[current]['pixel_count'] += 1

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

My preference is to avoid underscores where they are not necessary. So I would replace pixel_count with pixel count, pixel-count, or just npixels. Same below: total color or just color. @stefanv Do you have an opinion on this?

This comment has been minimized.

@JDWarner

JDWarner Jun 17, 2014

Contributor

@jni No hyphens in variable names allowed in Python, so I recommend against using them in dictionary keys as well. Otherwise agreed ;)


for x, y in g.edges_iter():
diff = g.node[x]['mean_color'] - g.node[y]['mean_color']
g[x][y]['weight'] = np.sqrt(diff.dot(diff))

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

np.linalg.norm(diff)



class RAG(nx.Graph):

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

Remove blank line before docstring

@@ -0,0 +1,166 @@
import rag
cimport numpy as cnp
import numpy as np

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

I would call this file _build_rag.pyx

Parameters
----------
label : (width, height, 3) or (width, height, depth, 3) ndarray

This comment has been minimized.

@jni

jni Jun 17, 2014

Contributor

Labels don't have a channel dimension.

@jni

This comment has been minimized.

Copy link
Contributor

jni commented Jun 17, 2014

Looks like there's a Cython build error in Python3:

https://travis-ci.org/scikit-image/scikit-image/jobs/27743297#L4578

In addition to my inline comments, I have some more, and would like to invite comments from @stefanv and @JDWarner regarding the interface.

Inheritance

I'm not sure we need to inherit from nx, rather than just use an nx.Graph directly. The interface could be more functional this way:

def merge_nodes(g, n1, n2, in_place=True):
    # code

def merge_hierarchical(g, weight_function):
    # code

def threshold(g, labels, threshold):
    # code

Performance

For some routines, including thresholding, sparse.csgraph might be faster, even including the conversion. Can you benchmark the current thresholding function compared to converting to csr (networkx has built-in conversion) and then running sparse.csgraph.connected_components(return_labels=True)?

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jun 17, 2014

@jni, @sciunto
I will work on your comments as soon as I finish addressing the Python 3 issue

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jun 17, 2014

Coverage Status

Changes Unknown when pulling 2a7d9d8 on vighneshbirodkar:rag into * on scikit-image:master*.

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jun 17, 2014

@jni
Converting to csr_matrix and then using scipy.sparse.csgraph.connected_components required about 40 times more time. A lot of it is spent in conversion

Given an image and its initial segmentation, this method constructs the
corresponsing Region Adjacency Graph (RAG). Each node in the RAG
represents a set pixels within `image` with the same

This comment has been minimized.

@stefanv

stefanv Jun 30, 2014

Member

a set of pixels

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jun 30, 2014

@stefanv @jni
So I will leave _add_edge_filter as it is, and rename the function to cut_threshold, apart from the formatting and naming conventions suggested.
Is that all ?

vighneshbirodkar added some commits Jul 1, 2014

Function to decide edge weight of edges incident on the new node.
For each neighbor `n` for `src and `dst`, `weight_func` will be
called as follows: `weight_func(src, dst, n, *extra_arguments,
**extra_keywords)`

This comment has been minimized.

@stefanv

stefanv Jul 1, 2014

Member

We should explain to the user what types src, dst, n, etc. will have. The easiest way to do that might be to refer them to the docstring of the default merge function.

This comment has been minimized.

@vighneshbirodkar

vighneshbirodkar Jul 2, 2014

Contributor

@stefanv
Is there anything required apart from what is already there in the docstring of min_weight ?

This comment has been minimized.

@stefanv

stefanv Jul 2, 2014

Member

I would add that RAG is a subclass of networkx.Graph so that users know where to look. Otherwise, no, you can just refer to that docstring here.



def max_edge(g, src, dst, n):
w1 = g[n].get(src, {'weight': -np.inf})['weight']

This comment has been minimized.

@stefanv

stefanv Jul 1, 2014

Member

I'd add an extra line here for readability:

default = {'weight': -np.inf}
w1 = g[n].get(src, default)['weight']
@stefanv

This comment has been minimized.

Copy link
Member

stefanv commented Jul 1, 2014

I left some minor comments, but I think we're basically good to go.

=======================
This example demonstrates the use of the `merge_nodes` function of a Region
Adjacency Graph (RAG).The `RAG` class represents a undirected weighted graph

This comment has been minimized.

@ahojnnes

ahojnnes Jul 2, 2014

Member

Space after "."



def display(g, title):
"""Displays a graph with the given title.

This comment has been minimized.

@jni

jni Jul 2, 2014

Contributor

"""Display a graph with the given title."""

Single line, no whitespace, present imperative.

Returns
-------
0 : int
Always returns 0. The return value is required so that `generic_fitler`

This comment has been minimized.

@jni

jni Jul 2, 2014

Contributor

filter, not fitler. =)


def max_edge(g, src, dst, n):
default = {'weight': -np.inf}
w1 = g[n].get(src, default)['weight']

This comment has been minimized.

@jni

jni Jul 2, 2014

Contributor

@stefanv this was a great suggestion, much more readable!

@jni

This comment has been minimized.

Copy link
Contributor

jni commented Jul 2, 2014

@vighneshbirodkar Okay! I picked out a couple of more typos, but they are just in the documentation, so: fix those, get Travis to pass (should just be a bunch of restarts... groan...), and it's going in, finally! =)

vighneshbirodkar added some commits Jul 3, 2014

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 3, 2014

Coverage Status

Changes Unknown when pulling df1d61e on vighneshbirodkar:rag into * on scikit-image:master*.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 3, 2014

Coverage Status

Changes Unknown when pulling df1d61e on vighneshbirodkar:rag into * on scikit-image:master*.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 3, 2014

Coverage Status

Changes Unknown when pulling df1d61e on vighneshbirodkar:rag into * on scikit-image:master*.

jni added a commit that referenced this pull request Jul 3, 2014

Merge pull request #1031 from vighneshbirodkar/rag
Add region adjacency graphs (RAGs)

This PR introduces a dependency to the NetworkX library.

@jni jni merged commit 6887f2c into scikit-image:master Jul 3, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@jni

This comment has been minimized.

Copy link
Contributor

jni commented Jul 3, 2014

BOOM. Thank you Vighnesh!

I think a blog post is in order... ;)

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jul 3, 2014

Success

@stefanv

This comment has been minimized.

Copy link
Member

stefanv commented Jul 3, 2014

Fantastic job, guys!

@vighneshbirodkar

This comment has been minimized.

Copy link
Contributor

vighneshbirodkar commented Jul 3, 2014

@jni @stefanv @sciunto @ahojnnes
Thanks a lot for all your patience.
@coveralls
Thank you too.

@ahojnnes

This comment has been minimized.

Copy link
Member

ahojnnes commented Jul 3, 2014

Excellent work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment