8000 Trivial image check for peak_local_max · Issue #3990 · scikit-image/scikit-image · GitHub
[go: up one dir, main page]

Skip to content

Trivial image check for peak_local_max #3990

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

Closed
hmaarrfk opened this issue Jul 6, 2019 · 4 comments · Fixed by #4760
Closed

Trivial image check for peak_local_max #3990

hmaarrfk opened this issue Jul 6, 2019 · 4 comments · Fixed by #4760

Comments

@hmaarrfk
Copy link
Member
hmaarrfk commented Jul 6, 2019

Description

peak_local_max, current tests for trivial images. That is, if an image is constant, and exactly constant, then it will return that no local peaks are found. If even one pixel is off, then it will return that many local peaks were found.

I think this test is too much of an edge case, and should simply be removed possibly without deprecation.

The relevant code is:

    # no peak for a trivial image
    if np.all(image == image.flat[0]):
        if indices is True:
            return np.empty((0, 2), np.int)
        else:
            return out

(not linking to the original code since it will be obsolete soon).

I think this is such an edge case that it isn't worth the cost imposed on checking equality on every other user of this function.

  1. Real cameras cannot actually take images with a constant value. Either dark noise, or shot noise from incomming photons will cause natural fluctuations in an image.
  2. Images that are computer generated have better ways of testing if they are of a constant value.

The users that may have trivial images in their analysis pipelines should find better ways of detecting them.

We've tried to think of better ways for testing for the trivial case, but due to the amount of options that the peak_local_max function provides, it seems like it is more complicated than it seems at first glance:

#3984 (comment)

cc: @clementkng and @brownmk
xref: #3984

@sciunto
Copy link
Member
sciunto commented Jul 9, 2019

If we choose to remove this trivial check, I think it would be worth adding it back to corner_peaks, as I think this function can easily face trivial images if corner detectors fail.

@rfezzani
Copy link
Member
rfezzani commented May 18, 2020

I see one major drawback to the current implementation of the trivial image check: when labels are provided and the whole image is not uniform but the labelled regions are:

>>> from skimage.feature import peak_local_max
>>> img = np.random.rand(10, 10)
>>> img[2:-2, 2:-2] = 1
>>> labels = np.zeros((10, 10), int)
>>> labels[2:-2, 2:-2] = 1
>>> out = peak_local_max(img, labels=labels, indices=False)

bug

We can, like @hmaarrfk is proposing, remove the test, but it can also be delayed after coordinates computation by checking if the number of returned coordinates is equal to data size (please see #3984 (comment)).

An other major drawback I see in the current implementation, is the use of the indices argument that defines what the function returns. I would like to remove it and make the function always return local maxima indices, but this has to be another PR ;)

@rfezzani
Copy link
Member

As proposed in #4751, using a generator to perform the test makes it O(1) instead of O(n).
But this approach doesn't solve the issue described in my previous comment :/

@rfezzani
Copy link
Member

In #4760, the trivial check deported to _get_peak_mask function. This prevent from evaluating image == image.flat[0] like it is made in the actual implementation.

Doing so also allows to check labels area.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants
0