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

[WIP] Get maximum branch #101

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions skan/csr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@
from scipy.sparse import csgraph
from scipy import spatial
import numba

from .nputil import pad, raveled_steps_to_neighbors


## NBGraph and Numba-based implementation

csr_spec = [
Expand Down Expand Up @@ -435,10 +433,40 @@ def path_means(self):
means : array of float
The average pixel value along each path in the skeleton.
"""

sums = np.add.reduceat(self.paths.data, self.paths.indptr[:-1])
lengths = np.diff(self.paths.indptr)
return sums / lengths

def path_maximum_length(self):
"""Compute the longest path.

Returns
-------
max_length : array of float
The maximum length in the skeleton.
"""

sums = np.maximum.reduceat(self.paths.data, self.paths.indptr[:-1])
Copy link
Owner

Choose a reason for hiding this comment

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

This should be called maxes, obviously. ;)

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I still don't get what you mean :( Do you mean the ufunc maximum? Or the docstring naming?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think Juan means that he would prefer the variable sums instead be called maxes (because it was arrived at using the numpy maximum function, I imagine)

Copy link
Author

Choose a reason for hiding this comment

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

Ok thanks, I missunderstood.

snowformatics marked this conversation as resolved.
Show resolved Hide resolved

# We test on _testdata.skeleton1
# self.paths.data = [1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1.]
# self.paths.indptr[:-1] = [0 11 14 17]
# If I understood correctly, np.ufunc.reduceat will slice an array according to a list and apply a function
# which means:
# 0-11 --> [1. 1. 1. 1. 1. 1. 1. 1. 1. 1. 1.] --> maximum --> 1.
# 11-14 --> [1. 1. 1. ] --> maximum --> 1.
# 14-17 --> [1. 1. 1. ] --> maximum --> 1.
# 17: --> [1. 1. 1. 1.] --> maximum --> 1.
# which will end up with sums = [1. 1. 1. 1.]
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, so the problem is that the test data is not "interesting" enough for this problem. To make it more interesting, you should change the test data to be e.g. random numbers. Or whatever, like a pyramid, [1, 2, 1], [1, 2, 2, 1], [1, 2, 3, 2, 1]...?

Then, you can just return the maxes! We are only after the maximum value, right?

The key is that when you replace the values of the skeleton by the distance transform, rather than just 1s, getting the max will tell you the maximum width of the skeleton at each branch.

On the other hand, maybe what you want is the minimum width? Or the width at the midpoint of the branch? If you think about how a skeleton image tends to look, the junctions are almost always farther away from the edge of the object than the middle of the branches.

Sorry, I'm about to hop in bed, I hope that this makes sense, if not I will try to draw a diagram tomorrow, sometimes text is not enough. 😊

Copy link
Author

Choose a reason for hiding this comment

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

I see, I will try and come back again :) Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jni perhaps you should try and clarify this (@snowformatics did say below they aren't sure how to follow your testing suggestions). This'll be the most important next step to unstick this PR



# self.paths.indptr = [ 0 11 14 17 21]
# length = [21-17, 17-14, 14-11, 11-0]
lengths = np.diff(self.paths.indptr)

# Not sure what should be returned, sums*lengths?

def path_stdev(self):
"""Compute the standard deviation of values along each path.

Expand Down
13 changes: 13 additions & 0 deletions skan/test/test_csr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from skan import csr

rundir = os.path.dirname(os.path.abspath(__file__))
print (rundir)
snowformatics marked this conversation as resolved.
Show resolved Hide resolved
sys.path.append(rundir)

from skan._testdata import (tinycycle, tinyline, skeleton0, skeleton1,
Expand Down Expand Up @@ -125,6 +126,18 @@ def test_pixel_values():
assert_almost_equal(stats.loc[0, 'mean pixel value'], expected)


def test_maximum_length():
image = np.random.random((45,))
expected = np.max(image[1:-1])
stats = csr.summarise(image)
print (csr.path_maximum_length())
print (stats)
#print (stats.loc[0, 'max-length'], expected)
#assert_almost_equal(stats.loc[0, 'max-length'], expected)
Copy link
Owner

Choose a reason for hiding this comment

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

As above: you need to make a slightly more interesting test case for this, just by varying the values inside the skeleton.





def test_tip_junction_edges():
stats1 = csr.summarise(skeleton4)
assert stats1.shape[0] == 3 # ensure all three branches are counted