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

Pass method #19

Merged
merged 9 commits into from
Nov 9, 2021
Merged

Pass method #19

merged 9 commits into from
Nov 9, 2021

Conversation

haesleinhuepf
Copy link
Member

Description

Hi Johannes @jo-mueller ,

I'm suggesting instead of calling if method == to call a different method now which can take care of the thresholding internall and that results in a binary matrix that allows us to directly match labels, In that way we are more flexible.

This is a resurrection of #14, this time as PR draft. Thanks for noting that there is an issue with this PR, the test shouldn't pass. We need to discuss when labels should be merged and how we can express that in the code. And furthermore, we should merge this after #10.

I'm also copying over your comment from #10 (comment) :

Regarding the new test function:

def test_match_labels2():
    
    labels_x = np.asarray([1, 1, 1, 0, 0, 2, 2, 4, 4, 4, 0], dtype=np.uint8)
    labels_y = np.asarray([1, 1, 2, 0, 0, 3, 3, 4, 4, 4, 5], dtype=np.uint8)

    reference_y_matched = np.asarray([1, 1, 1, 0, 0, 2, 2, 4, 4, 4, 5], dtype=np.uint8)

    labels_y_matched = biau.label.match_labels(labels_x, labels_y)

    assert np.array_equal(reference_y_matched, labels_y_matched)

I think we should discuss our preferred format. Right now, thresholded_intersection_over_union_matrix returns a boolean array in this branch. This leads to a behaviour in match_labels() which allows multiple labels from slice z+1 to be merged with labels from slice z whenever the overlap is larger than 0.3 (default threshold). I could imagine that this may lead to problems downstream, because a label can have an overlap of 0.3 with up to three labels in the adjacent slice.

The original behavior from the cellpose repository is a little different: The IoU matrix is not binarized, it is only thresholded in a sense that entries in the IoU matrix smaller than x are set to zero:

        IoU[IoU< threshold] = 0.0  # suppress small overlaps
        IoU[IoU< IoU.max(axis=0)] = 0.0  # suppress non-maximum overlaps

Other entries are unaffected from this so that only labels with the largest overlap will be merged.

The difference is very subtle but can lead to very different results, as the above test shows. In the case of a binary IoU matrix the correct result is this:

reference_y_matched = np.asarray([1, 1, 1, 0, 0, 2, 2, 4, 4, 4, 5], dtype=np.uint8)

In the case of a maximal IoU approach with suppressed entries, the correct result is this:

reference_y_matched = np.asarray([1, 1, 6, 0, 0, 2, 2, 4, 4, 4, 5], dtype=np.uint8)

Type of change

  • Bug-fix
  • New feature
  • Breaking change
  • Documentation update

References

Tests

  • I adapted existing tests, because
  • I added new tests to cover the code change
  • All tests pass with my change

Final checks

  • My change is the minimal possible work for the desired feature/fix
  • I updated the documentation where necessary to cover the change

@jo-mueller jo-mueller merged commit 5549200 into main Nov 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants