Deprecate slow binary_* functions in skimage.morphology#7665
Deprecate slow binary_* functions in skimage.morphology#7665jarrodmillman merged 16 commits intoscikit-image:mainfrom
binary_* functions in skimage.morphology#7665Conversation
Most of the time, these seem to be slower than our regular gray-type functions, e.g., `ski.morphology.binary_erosion` vs `ski.morphology.erosion`. A potential reason for this might be that the `binary_` funcs use SciPy's (regressed) slow versions under the hood (scipy issue 13991). Also if we add back a faster binary version at some point, can just do it internally for binary input. See also https://github.com/scikit-image/scikit-image/wiki/API-changes-for-skimage2#proposed-api-changes-doable-independent-of-skimage2
Creating and testing large 3D footprints with ball is very slow so that the test took 26 s in total on my machine. Removing 50-100 shaves of ~25 s without sacrificing any test coverage I think.
|
Here's a quick & dirty script (and it's output on my machine) to get an overview about the performance of the mentioned functions. Details# timeit.py
import timeit
import subprocess
import numpy as np
import skimage as ski
mask_sizes = [10, 1000]
footprint_sizes = [3, 9]
funcs = ["erosion", "dilation", "opening", "closing"]
for func in funcs:
for mask_size in mask_sizes:
for footprint_size in footprint_sizes:
print(f"{mask_size=}, {footprint_size=}")
setup = (
"import skimage as ski; import numpy as np;"
"import skimage.morphology as sm;"
f"mask = ski.data.binary_blobs(length={mask_size});"
f"footprint = sm.footprint_rectangle(({footprint_size},) * 2);"
)
test_gray = f"sm.{func}(mask, footprint)"
print(" " + test_gray, end=": ")
subprocess.run(
["python", "-m", "timeit", "-s", setup, test_gray]
)
test_binary = f"sm.binary_{func}(mask, footprint)"
print(" " + test_binary, end=": ")
subprocess.run(
["python", "-m", "timeit", "-s", setup, test_binary]
)
print()prints
|
mkcor
left a comment
There was a problem hiding this comment.
Thanks for tackling the skimage2 agenda! 🙏
There's also an instance of skimage.morphology.binary_dilation in
Do we want to update the scipy.ndimage.binary_* as well?
And should we clean up
scikit-image/skimage/morphology/misc.py
Lines 17 to 26 in 96834cf
? Maybe that's implied in the TODO for v0.28?
That whole |
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Just moves `params`, `param_names` and `setup` over to the inheriting class since the base classes are removed.
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
|
Note the release note in the PR description. This time I chose to expand a bit on the reasoning behind the deprecation since it might otherwise look like a unnecessary deprecation. |
Thanks for this thorough release note, indeed! I would just add Oxford commas (before "and I would replace "to the remaining non-deprecated functions" with "to the remaining (general, non-binary) functions for when they're used with binary inputs." |
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
Inspired by the examples of the gray morphology functions. (cherry picked from commit 3b2b1a5)
Because this function takes a `radius` to specify the neighborhood, the implied "footprint" is always circular and of uneven size. The rewording tries to make that clearer. Also use the more expressive term "neighborhood" instead of "region" in the description of the argument `radius`.
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
as suggested by Marianne. Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
|
I addressed all your comments @mkcor. Should be ready to go. :D |
|
Converting to draft so this is not accidentally merged into the pending 0.25.2 patch release. |
Description
As discussed in our last hackathon and detailed in our skimage2 agenda.
Most of the time, these functions seem to be slower than our regular gray-type functions, e.g.,
ski.morphology.binary_erosionvsski.morphology.erosion. A potential reason for this might be that thebinary_funcs use SciPy's (regressed) slow versions under the hood (scipy/scipy#13991).Also if we add back a faster binary version at some point, can just do it internally for binary input.
Checklist
./doc/examplesfor new featuresRelease note
For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.