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

feature.peak_local_max changes labels #5047

Closed
odoublewen opened this issue Nov 2, 2020 · 4 comments · Fixed by #4760
Closed

feature.peak_local_max changes labels #5047

odoublewen opened this issue Nov 2, 2020 · 4 comments · Fixed by #4760

Comments

@odoublewen
Copy link
Contributor

Description

When non sequential labels are provided as input to peak_local_max, the values get altered.

If the function needs to update the values, it ought to copy the matrix.

I suppose this bug will be addresses by #4760 (there is a description "labels renumbering is avoided" in that PR). But I didn't find any explicit description of this bug, which seems serious due to its silent nature, so I thought I should document it.

Way to reproduce

import numpy as np
from skimage import feature
im = np.zeros([10, 10])
labels = np.zeros([10, 10]).astype(int)

label = 2
for x in range(0, 10, 2):
    for y in range(0, 10, 2):
        im[x,y] = 1
        labels[x:x+1, y:y+1] = label
        label += 2
labels
array([[ 2,  0,  4,  0,  6,  0,  8,  0, 10,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [12,  0, 14,  0, 16,  0, 18,  0, 20,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [22,  0, 24,  0, 26,  0, 28,  0, 30,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [32,  0, 34,  0, 36,  0, 38,  0, 40,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [42,  0, 44,  0, 46,  0, 48,  0, 50,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0]])
res = feature.peak_local_max(im, labels=labels, num_peaks_per_label=1)
labels
array([[ 1,  0,  2,  0,  3,  0,  4,  0,  5,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [ 6,  0,  7,  0,  8,  0,  9,  0, 10,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [11,  0, 12,  0, 13,  0, 14,  0, 15,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [16,  0, 17,  0, 18,  0, 19,  0, 20,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0],
       [21,  0, 22,  0, 23,  0, 24,  0, 25,  0],
       [ 0,  0,  0,  0,  0,  0,  0,  0,  0,  0]])
from __future__ import print_function
import sys; print(sys.version)
import platform; print(platform.platform())
import skimage; print("scikit-image version: {}".format(skimage.__version__))
import numpy; print("numpy version: {}".format(numpy.__version__))
3.8.5 (default, Jul 21 2020, 10:48:26) 
[Clang 11.0.3 (clang-1103.0.32.62)]
macOS-10.15.7-x86_64-i386-64bit
scikit-image version: 0.17.2
numpy version: 1.19.2

Version information

# Paste the output of the following python commands
from __future__ import print_function
import sys; print(sys.version)
import platform; print(platform.platform())
import skimage; print("scikit-image version: {}".format(skimage.__version__))
import numpy; print("numpy version: {}".format(numpy.__version__))
# your output here
3.8.5 (default, Jul 21 2020, 10:48:26) 
[Clang 11.0.3 (clang-1103.0.32.62)]
macOS-10.15.7-x86_64-i386-64bit
scikit-image version: 0.17.2
numpy version: 1.19.2
@rfezzani
Copy link
Member

rfezzani commented Nov 3, 2020

Thank you @odoublewen for the report! I confirm the bug and that #4760 is not affected 🎉
I hope it can be ready for 0.18 release...

@odoublewen
Copy link
Contributor Author

odoublewen commented Nov 3, 2020

@rfezzani I am puzzling over the enumerate() call in this code chuck from your branch:

for label_idx, roi in enumerate(ndi.find_objects(_labels)):
    label_mask = labels[roi] == label_idx + 1

Since enumerate() yields a tuple with a sequential index, and then the mask is created by looking for equality between the label matrix and the index... it seems like it wouldn't work right in the case of non-sequential labels.

But I must be missing something. (Apologies for making this offhand comment without spending the time to play around with the code, and delve into the unit tests.)

And thank you again for your work on this much needed refactoring!

@rfezzani
Copy link
Member

rfezzani commented Nov 3, 2020

@odoublewen, you forgot to mention the essential

if roi is None:
    continue

present in the cited code chunk! In fact, ndi.find_objects returns None for skipped label indices 😉.

@odoublewen
Copy link
Contributor Author

Hi @rfezzani - I can confirm that I am no longer seeing the bug reported in this issue, but since upgrading from 0.17.2, I am seeing a new behavior, which I think might be a new bug. When I use num_peaks_per_label=1, with 0.17.2, I used to get the expected 1 peak per label. But with 0.18.0 and 0.18.1, some of the labels are missing. I haven't been able to figure out why, and I don't have a reproducible example ready yet. If I can create a small dataset that reproduces the problem, I'll open a new issue. But just thought I'd ping you ahead of that with my concerns.

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 a pull request may close this issue.

2 participants