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.

Sign up for GitHub

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

Jump to bottom

threshold_li: fast, histogram-based code path for integer images #5050

Merged
merged 12 commits into from
Sep 2, 2021
Merged

threshold_li: fast, histogram-based code path for integer images #5050

merged 12 commits into from
Sep 2, 2021

Conversation

yacth
Copy link
Contributor

@yacth yacth commented Nov 4, 2020

Description

Enchancement of threshold_li using histograms for uint8 and uint16 images following issue #5033

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

@pep8speaks
Copy link

pep8speaks commented Nov 4, 2020 •
edited

Hello @yacth! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 745:46: W291 trailing whitespace

Comment last updated at 2021-08-31 13:23:41 UTC

@yacth
Copy link
Contributor Author

yacth commented Nov 4, 2020 •
edited

Hi @jni, I don't understand what is causing the failing checks in here, could you guide me through the understandement of it please.

You asked me to do a benchmark (#5033 (comment)), what are you expecting for it ? Could you please give me some details ?

@jni
Copy link
Member

jni commented Nov 4, 2020

The failure is super weird. Don't worry about it for now, it's almost certainly not due to your changes. We'll have to investigate.

Re: benchmarks: it would be good to add a benchmark for a sample image, say data.eagle(), that computes threshold_li, before and after the changes in this PR. (You can compare quickly by using data.eagle().astype(npfloat32) vs just data.eagle(), maybe, although there are some confounding factors in that case.) In short: my idea is that this approach will be faster, but we need to confirm that with measurements.

@yacth
Copy link
Contributor Author

yacth commented Nov 5, 2020

I don't know why it creates an error when I use data.eagle(), then I used data.camera() for now.

I had these results:

· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Benchmarking existing-py_home_yacth_anaconda3_envs_skimage-dev_bin_python3.8
[ 25.00%] ··· benchmark_threshold.ThresholdingLi.time_float                                                     13.5±0ms
[ 50.00%] ··· benchmark_threshold.ThresholdingLi.time_integer                                                   10.2±0ms

It looks faster with histogram as you expected, what do you think about it ?

@emmanuelle
Copy link
Member

Hi @yacth how long is the benchmark before and after your changes?

@emmanuelle
Copy link
Member

Also what is the error with data.eagle()? How can we reproduce it?

@yacth
Copy link
Contributor Author

yacth commented Nov 5, 2020

Hi @yacth how long is the benchmark before and after your changes?

Hi @emmanuelle, I hope I'm not mistaken, is the benchmark result given in computation time as written above ?
I am using the same data data.camera() but for integer I am using it as it is and for float I am transforming it using data.camera().astype(np.float32) . I have :

  • 13.5 ms for float
  • 10.2 ms for integer

(actually I re ran the benchmark and I got different results, sorry I don't understand yet completely how the benchmark is done

· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Benchmarking existing-py_home_yacth_anaconda3_envs_skimage-dev_bin_python3.8
[ 25.00%] ··· Running (benchmark_threshold.ThresholdingLi.time_float_image--)..
[ 75.00%] ··· benchmark_threshold.ThresholdingLi.time_float_image                                             11.7±0.1ms
[100.00%] ··· benchmark_threshold.ThresholdingLi.time_integer_image                                           8.00±0.2ms

)

Using the following benchmark_thresholding.py file:

import numpy as np
from skimage.filters.thresholding import threshold_li
from skimage.data import eagle
from skimage.data import camera

class ThresholdingLi:
    """Benchmark for threshold_li in scikit-image."""

    def setup(self):
        self.image = eagle()

    def time_integer_image(self):
        result1 = threshold_li(self.image)

    def time_float_image(self):
        result1 = threshold_li(self.image.astype(np.float32))

If for self.image I use camera() everything works correctly, if I use eagle() I get the following result using the command asv run -E existing -b ThresholdingLi:

· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Benchmarking existing-py_home_yacth_anaconda3_envs_skimage-dev_bin_python3.8
[ 25.00%] ··· Running (benchmark_threshold.ThresholdingLi.time_float_image--)..
[ 75.00%] ··· benchmark_threshold.ThresholdingLi.time_float_image                                              failed
[100.00%] ··· benchmark_threshold.ThresholdingLi.time_integer_image                                            failed

@jni
Copy link
Member

jni commented Nov 5, 2020

@yacth I think what Emma is saying is what I mentioned earlier, that there are confounding factors if you use float32 like I suggested. And so it would be good to compare fully the benchmark to the master branch by following the instructions in this section:

https://scikit-image.org/docs/dev/contribute.html#comparing-results-to-master

Regarding eagle, asv is obscuring the error message. Can you try python -C "from skimage.data import eagle; print(eagle().shape)" in a terminal to see what the error is?

@yacth
Copy link
Contributor Author

yacth commented Nov 5, 2020

@yacth I think what Emma is saying is what I mentioned earlier, that there are confounding factors if you use float32 like I suggested. And so it would be good to compare fully the benchmark to the master branch by following the instructions in this section:

https://scikit-image.org/docs/dev/contribute.html#comparing-results-to-master

Regarding eagle, asv is obscuring the error message. Can you try python -C "from skimage.data import eagle; print(eagle().shape)" in a terminal to see what the error is?

Using python -c "from skimage.data import eagle; print(eagle().shape)" I get :

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/yacth/github/scikit-image/skimage/data/__init__.py", line 385, in eagle
    return _load("data/eagle.png")
  File "/home/yacth/github/scikit-image/skimage/data/__init__.py", line 342, in _load
    return imread(_fetch(f), as_gray=as_gray)
  File "/home/yacth/github/scikit-image/skimage/data/__init__.py", line 212, in _fetch
    raise ModuleNotFoundError(
ModuleNotFoundError: The requested file is part of the scikit-image distribution, but requires the installation of an optional dependency, pooch. To install pooch, use your preferred python package manager. Follow installation instruction found at https://scikit-image.org/docs/stable/install.html

When I used the command asv continuous master -b TransformSuite to try and get the same result as shown in your documentation I get an error saying it failed to create the project and import the benchmark:

· Creating environments
· Discovering benchmarks
·· Uninstalling from conda-py3.7-cython-numpy1.15-scipy
·· Installing d88b92b7 <master> into conda-py3.7-cython-numpy1.15-scipy.
·· Error running /home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/bin/python /home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py discover /home/yacth/github/scikit-image/benchmarks /tmp/tmpp9wvng_8/result.json (exit status 1)
   STDOUT -------->

   STDERR -------->
   Traceback (most recent call last):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1315, in <module>
       main()
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1308, in main
       commands[mode](args)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1004, in main_discover
       list_benchmarks(benchmark_dir, fp)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 989, in list_benchmarks
       for benchmark in disc_benchmarks(root):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 887, in disc_benchmarks
       for module in disc_modules(root_name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 869, in disc_modules
       for item in disc_modules(name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 857, in disc_modules
       module = import_module(module_name)
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/importlib/__init__.py", line 127, in import_module
       return _bootstrap._gcd_import(name[level:], package, level)
     File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
     File "<frozen importlib._bootstrap>", line 983, in _find_and_load
     File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
     File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
     File "<frozen importlib._bootstrap_external>", line 728, in exec_module
     File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
     File "/home/yacth/github/scikit-image/benchmarks/benchmark_exposure.py", line 6, in <module>
       from skimage.transform import rescale
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/skimage/transform/__init__.py", line 1, in <module>
       from .hough_transform import (hough_line, hough_line_peaks,
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/skimage/transform/hough_transform.py", line 2, in <module>
       from scipy.spatial import cKDTree
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/__init__.py", line 155, in <module>
       from . import fft
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/__init__.py", line 79, in <module>
       from ._helper import next_fast_len
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_helper.py", line 3, in <module>
       from ._pocketfft import helper as _helper
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_pocketfft/__init__.py", line 3, in <module>
       from .basic import *
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_pocketfft/basic.py", line 6, in <module>
       from . import pypocketfft as pfft
   ImportError: /home/linuxbrew/.linuxbrew/lib/libstdc++.so.6: version `GLIBCXX_3.4.22' not found (required by /home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_pocketfft/pypocketfft.cpython-37m-x86_64-linux-gnu.so)

·· Failed: trying different commit/environment
·· Uninstalling from conda-py3.7-cython-numpy1.15-scipy
·· Installing 9ffad9bd <master~1> into conda-py3.7-cython-numpy1.15-scipy.
·· Error running /home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/bin/python /home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py discover /home/yacth/github/scikit-image/benchmarks /tmp/tmpoo_bp2_t/result.json (exit status 1)
   STDOUT -------->

   STDERR -------->
   Traceback (most recent call last):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1315, in <module>
       main()
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1308, in main
       commands[mode](args)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1004, in main_discover
       list_benchmarks(benchmark_dir, fp)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 989, in list_benchmarks
       for benchmark in disc_benchmarks(root):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 887, in disc_benchmarks
       for module in disc_modules(root_name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 869, in disc_modules
       for item in disc_modules(name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 857, in disc_modules
       module = import_module(module_name)
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/importlib/__init__.py", line 127, in import_module
       return _bootstrap._gcd_import(name[level:], package, level)
     File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
     File "<frozen importlib._bootstrap>", line 983, in _find_and_load
     File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
     File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
     File "<frozen importlib._bootstrap_external>", line 728, in exec_module
     File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
     File "/home/yacth/github/scikit-image/benchmarks/benchmark_exposure.py", line 6, in <module>
       from skimage.transform import rescale
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/skimage/transform/__init__.py", line 1, in <module>
       from .hough_transform import (hough_line, hough_line_peaks,
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/skimage/transform/hough_transform.py", line 2, in <module>
       from scipy.spatial import cKDTree
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/__init__.py", line 155, in <module>
       from . import fft
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/__init__.py", line 79, in <module>
       from ._helper import next_fast_len
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_helper.py", line 3, in <module>
       from ._pocketfft import helper as _helper
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_pocketfft/__init__.py", line 3, in <module>
       from .basic import *
     File "/home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_pocketfft/basic.py", line 6, in <module>
       from . import pypocketfft as pfft
   ImportError: /home/linuxbrew/.linuxbrew/lib/libstdc++.so.6: version `GLIBCXX_3.4.22' not found (required by /home/yacth/github/scikit-image/.asv/env/082d956b571f9c8e15dc9c91ad51627d/lib/python3.7/site-packages/scipy/fft/_pocketfft/pypocketfft.cpython-37m-x86_64-linux-gnu.so)

·· Failed: trying different commit/environment
·· Uninstalling from conda-py3.7-cython-numpy1.16-scipy.
·· Installing d88b92b7 <master> into conda-py3.7-cython-numpy1.16-scipy.
·· Error running /home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/bin/python /home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py discover /home/yacth/github/scikit-image/benchmarks /tmp/tmpiavu6i8x/result.json (exit status 1)
   STDOUT -------->

   STDERR -------->
   Traceback (most recent call last):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1315, in <module>
       main()
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1308, in main
       commands[mode](args)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1004, in main_discover
       list_benchmarks(benchmark_dir, fp)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 989, in list_benchmarks
       for benchmark in disc_benchmarks(root):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 887, in disc_benchmarks
       for module in disc_modules(root_name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 869, in disc_modules
       for item in disc_modules(name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 857, in disc_modules
       module = import_module(module_name)
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/importlib/__init__.py", line 127, in import_module
       return _bootstrap._gcd_import(name[level:], package, level)
     File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
     File "<frozen importlib._bootstrap>", line 983, in _find_and_load
     File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
     File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
     File "<frozen importlib._bootstrap_external>", line 728, in exec_module
     File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
     File "/home/yacth/github/scikit-image/benchmarks/benchmark_exposure.py", line 6, in <module>
       from skimage.transform import rescale
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/skimage/transform/__init__.py", line 1, in <module>
       from .hough_transform import (hough_line, hough_line_peaks,
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/skimage/transform/hough_transform.py", line 2, in <module>
       from scipy.spatial import cKDTree
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/__init__.py", line 155, in <module>
       from . import fft
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/__init__.py", line 79, in <module>
       from ._helper import next_fast_len
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_helper.py", line 3, in <module>
       from ._pocketfft import helper as _helper
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_pocketfft/__init__.py", line 3, in <module>
       from .basic import *
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_pocketfft/basic.py", line 6, in <module>
       from . import pypocketfft as pfft
   ImportError: /home/linuxbrew/.linuxbrew/lib/libstdc++.so.6: version `GLIBCXX_3.4.22' not found (required by /home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_pocketfft/pypocketfft.cpython-37m-x86_64-linux-gnu.so)

·· Failed: trying different commit/environment
·· Uninstalling from conda-py3.7-cython-numpy1.16-scipy
·· Installing 9ffad9bd <master~1> into conda-py3.7-cython-numpy1.16-scipy.
·· Error running /home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/bin/python /home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py discover /home/yacth/github/scikit-image/benchmarks /tmp/tmph6tn0tts/result.json (exit status 1)
   STDOUT -------->

   STDERR -------->
   Traceback (most recent call last):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1315, in <module>
       main()
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1308, in main
       commands[mode](args)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 1004, in main_discover
       list_benchmarks(benchmark_dir, fp)
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 989, in list_benchmarks
       for benchmark in disc_benchmarks(root):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 887, in disc_benchmarks
       for module in disc_modules(root_name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 869, in disc_modules
       for item in disc_modules(name, ignore_import_errors=ignore_import_errors):
     File "/home/yacth/anaconda3/envs/skimage-dev/lib/python3.8/site-packages/asv/benchmark.py", line 857, in disc_modules
       module = import_module(module_name)
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/importlib/__init__.py", line 127, in import_module
       return _bootstrap._gcd_import(name[level:], package, level)
     File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
     File "<frozen importlib._bootstrap>", line 983, in _find_and_load
     File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
     File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
     File "<frozen importlib._bootstrap_external>", line 728, in exec_module
     File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
     File "/home/yacth/github/scikit-image/benchmarks/benchmark_exposure.py", line 6, in <module>
       from skimage.transform import rescale
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/skimage/transform/__init__.py", line 1, in <module>
       from .hough_transform import (hough_line, hough_line_peaks,
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/skimage/transform/hough_transform.py", line 2, in <module>
       from scipy.spatial import cKDTree
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/__init__.py", line 155, in <module>
       from . import fft
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/__init__.py", line 79, in <module>
       from ._helper import next_fast_len
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_helper.py", line 3, in <module>
       from ._pocketfft import helper as _helper
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_pocketfft/__init__.py", line 3, in <module>
       from .basic import *
     File "/home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_pocketfft/basic.py", line 6, in <module>
       from . import pypocketfft as pfft
   ImportError: /home/linuxbrew/.linuxbrew/lib/libstdc++.so.6: version `GLIBCXX_3.4.22' not found (required by /home/yacth/github/scikit-image/.asv/env/8cc9d2a1d90c9fe2d422e9b3a773d6a6/lib/python3.7/site-packages/scipy/fft/_pocketfft/pypocketfft.cpython-37m-x86_64-linux-gnu.so)

·· Failed to build the project and import the benchmark suite.

@jni
Copy link
Member

jni commented Nov 6, 2020

@yacth ah, right!

(1) Regarding data.eagle, as the error message suggests, you should pip install pooch. Then things should work.

(2) The second problem can probably be fixed by running asv continuous master -b TransformSuite -E existing instead. It seems asv is trying to create a conda environment automatically and that is failing. -E existing tells it to use your current environment instead. We should probably modify the instructions to say this.

@yacth
Copy link
Contributor Author

yacth commented Nov 6, 2020

@yacth ah, right!

(1) Regarding data.eagle, as the error message suggests, you should pip install pooch. Then things should work.

(2) The second problem can probably be fixed by running asv continuous master -b TransformSuite -E existing instead. It seems asv is trying to create a conda environment automatically and that is failing. -E existing tells it to use your current environment instead. We should probably modify the instructions to say this.

Hi @jni, I installed pooch as suggested and I am using image = eagle()
asv dev -b ThresholdLi gives me :

· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Benchmarking existing-py_home_yacth_anaconda3_envs_skimage-dev_bin_python3.8
[ 25.00%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                   248±0ms
[ 50.00%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                 126±0ms

which seems to give an expected result.

asv run -E existing -b ThresholdLi gives me :

[ 25.00%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 75.00%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                   241±3ms
[100.00%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                 124±1ms

which giving something we could expect.

But asv continuous master -b ThresholdLi is failing as shown :

· Creating environments
· Discovering benchmarks
·· Uninstalling from conda-py3.7-cython-numpy1.15-scipy
·· Installing d88b92b7 <master> into conda-py3.7-cython-numpy1.15-scipy.
· Running 8 total benchmarks (2 commits * 2 environments * 2 benchmarks)
[  0.00%] · For scikit-image commit 9ffad9bd <master~1> (round 1/2):
[  0.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy..
[  0.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[  6.25%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 12.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy
[ 12.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 18.75%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 25.00%] · For scikit-image commit d88b92b7 <master> (round 1/2):
[ 25.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy..
[ 25.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[ 31.25%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 37.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy..
[ 37.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 43.75%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 50.00%] · For scikit-image commit d88b92b7 <master> (round 2/2):
[ 50.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy
[ 50.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[ 56.25%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                    failed
[ 62.50%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                  failed
[ 62.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy
[ 62.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 68.75%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                    failed
[ 75.00%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                  failed
[ 75.00%] · For scikit-image commit 9ffad9bd <master~1> (round 2/2):
[ 75.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy..
[ 75.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[ 81.25%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                    failed
[ 87.50%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                  failed
[ 87.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy...
[ 87.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 93.75%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                    failed
[100.00%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                  failed

BENCHMARKS NOT SIGNIFICANTLY CHANGED.

I don't know what is causing this failure, because with image = camera() it seems to work correctly:

· Creating environments
· Discovering benchmarks
·· Uninstalling from conda-py3.7-cython-numpy1.15-scipy.
·· Installing d88b92b7 <master> into conda-py3.7-cython-numpy1.15-scipy..
· Running 8 total benchmarks (2 commits * 2 environments * 2 benchmarks)
[  0.00%] · For scikit-image commit 9ffad9bd <master~1> (round 1/2):
[  0.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy...
[  0.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[  6.25%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 12.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy
[ 12.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 18.75%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 25.00%] · For scikit-image commit d88b92b7 <master> (round 1/2):
[ 25.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy..
[ 25.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[ 31.25%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 37.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy....
[ 37.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 43.75%] ··· Running (benchmark_threshold.ThresholdLi.time_float_image--)..
[ 50.00%] · For scikit-image commit d88b92b7 <master> (round 2/2):
[ 50.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy
[ 50.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[ 56.25%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                  19.6±1ms
[ 62.50%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                15.2±1ms
[ 62.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy
[ 62.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 68.75%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                  28.6±2ms
[ 75.00%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                              20.9±0.3ms
[ 75.00%] · For scikit-image commit 9ffad9bd <master~1> (round 2/2):
[ 75.00%] ·· Building for conda-py3.7-cython-numpy1.15-scipy..
[ 75.00%] ·· Benchmarking conda-py3.7-cython-numpy1.15-scipy
[ 81.25%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                  22.6±2ms
[ 87.50%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                              15.3±0.3ms
[ 87.50%] ·· Building for conda-py3.7-cython-numpy1.16-scipy..
[ 87.50%] ·· Benchmarking conda-py3.7-cython-numpy1.16-scipy
[ 93.75%] ··· benchmark_threshold.ThresholdLi.time_float_image                                                  29.3±2ms
[100.00%] ··· benchmark_threshold.ThresholdLi.time_integer_image                                                24.2±3ms

BENCHMARKS NOT SIGNIFICANTLY CHANGED.

(p.s. asv continuous master -b TransformSuite -E existing or asv continuous master -b ThresholdLi -E existing are giving me :

· No range spec may be specified if benchmarking in an existing environment

)

@jni
Copy link
Member

jni commented Nov 7, 2020

Hmm, yeah, asv continuous appears to be pretty annoying. I think probably the correct workflow when optimising code is to write the benchmarks first, then make the changes to the function. This way there is a commit that corresponds to master + benchmarks that one can always go back to and use asv dev on.

I worked around it by copying the benchmark file out of version control and then changing back to master. This allows me to run asv dev and compute the benchmarks on the master branch.

Results:

 ~/projects/scikit-image (master)
 $ asv dev -b ThresholdingLi
· Discovering benchmarks
· Running 2 total benchmarks (1 commits * 1 environments * 2 benchmarks)
[  0.00%] ·· Benchmarking existing-py_Users_jni_conda_envs_all_bin_python
[ 25.00%] ··· ...k_threshold2.ThresholdingLi.time_float_image            229±0ms
[ 50.00%] ··· ...threshold2.ThresholdingLi.time_integer_image            163±0ms
(all)
jni@beastie Sat Nov 07 14:20
 ~/projects/scikit-image (master)
 $ git switch threshold-li-changes
Switched to branch 'threshold-li-changes'
(all)
jni@beastie Sat Nov 07 14:20
 ~/projects/scikit-image (threshold-li-changes)
 $ asv dev -b ThresholdingLi
· Discovering benchmarks
· Running 4 total benchmarks (1 commits * 1 environments * 4 benchmarks)
[  0.00%] ·· Benchmarking existing-py_Users_jni_conda_envs_all_bin_python
[ 12.50%] ··· ...rk_threshold.ThresholdingLi.time_float_image            230±0ms
[ 25.00%] ··· ..._threshold.ThresholdingLi.time_integer_image            121±0ms
[ 37.50%] ··· ...k_threshold2.ThresholdingLi.time_float_image            235±0ms
[ 50.00%] ··· ...threshold2.ThresholdingLi.time_integer_image            110±0ms

So the speedup is about 50% for a large-ish image. It would be interesting to profile this to figure out whether there's any further easy speedups to be gained...

@jni
Copy link
Member

jni commented Nov 7, 2020

I installed line_profiler then ran it in IPython with:

from skimage import data, filters
eagle = data.eagle()
%lprun -f filters.threshold_li filters.threshold_li(eagle)

The output is here:

https://gist.github.com/jni/7e617f9dcbbb201c82265d82b983d789

The long-running lines are:

   597         1     197474.0 197474.0     72.8      tolerance = tolerance or np.min(np.diff(np.unique(image))) / 2

and

   631         2      50174.0  25087.0     18.5          hist, bin_centers = histogram(image.ravel(),
   632         1          1.0      1.0      0.0                                        nbins,
   633         1          3.0      3.0      0.0                                        source_range='image')

The slow np.unique() call can definitely be removed with regards to tolerance in integer images — it should be 0.5 in these cases!

@yacth
Copy link
Contributor Author

yacth commented Nov 7, 2020

I installed line_profiler then ran it in IPython with:

from skimage import data, filters
eagle = data.eagle()
%lprun -f filters.threshold_li filters.threshold_li(eagle)

The output is here:

https://gist.github.com/jni/7e617f9dcbbb201c82265d82b983d789

The long-running lines are:

   597         1     197474.0 197474.0     72.8      tolerance = tolerance or np.min(np.diff(np.unique(image))) / 2

and

   631         2      50174.0  25087.0     18.5          hist, bin_centers = histogram(image.ravel(),
   632         1          1.0      1.0      0.0                                        nbins,
   633         1          3.0      3.0      0.0                                        source_range='image')

The slow np.unique() call can definitely be removed with regards to tolerance in integer images — it should be 0.5 in these cases!

Sorry I don't understand what do you mean by removing ?
You want to replace np.unique() by a constant for integer images ?

@yacth
Copy link
Contributor Author

yacth commented Nov 9, 2020 •
edited

Hi @jni what about using the bins_centers of the histigram instead of np.unique(image) because for integer images it should be the same ?
It seems to calculate much faster as well...

@alexdesiqueira alexdesiqueira mentioned this pull request Nov 10, 2020
Closed
Base automatically changed from master to main February 18, 2021 18:23
@mkcor mkcor mentioned this pull request Feb 22, 2021
Closed
@grlee77
Merge remote-tracking branch 'upstream/main' into threshold-li-changes
d79c40f
@grlee77
precompute float32 image in threshold_li benchmark
837d49b
@grlee77
move threshold_li benchmark to benchmark_filters.py
188fc3e 
location consistent with other threshold benchmarks (Otsu, Sauvola)
@grlee77
BENCH: add try/except around data.eagle
96787c5 
The eagle image is not present in older commits (it was added for skimage 0.18)
@grlee77
Support fast path for all signed and unsigned integer types
310c2ad 
Remove nbins argument as it is ignored by exposure.histogram for integer image types
@grlee77
Avoid expensive initial tolerance computation for integer images
9f808cc 
Already know that np.diff cannot return a value smaller than 1, so just set to 0.5 directly
@grlee77
Copy link
Contributor

grlee77 commented Aug 21, 2021

I pushed a few small changes here to revive this nice PR

1.) moved the new benchmark into the existing benchmark_filters file since some other threshold benchmarks were already there
2.) no need to restrict the fast, histogram-based path to uint8 and uint16 only. It can work for all integer types

3.) updated with @jni's suggestion related to your question:

Hi @jni what about using the bins_centers of the histogram instead of np.unique(image) because for integer images it should be the same ?

I think the suggestion here was that we don't need to search for the minimum difference using np.diff for integer images (it cannot be smaller than 1). See update in commit 9f808cc

@grlee77
Copy link
Contributor

grlee77 commented Aug 21, 2021

Benchmarks after the change in 9f808cc are showing a factor 6 acceleration for the uint8 benchmark as compared to the current main branch (29.8 ms vs 182 ms).

Benchmark on main

[ 25.00%] ··· benchmark_temp.ThresholdLi.time_float32_image              214±0ms
[ 50.00%] ··· benchmark_temp.ThresholdLi.time_integer_image              182±0ms

Benchmark in this branch

[ 25.00%] ··· benchmark_filters.ThresholdLi.time_float32_image              213±0ms
[ 50.00%] ··· benchmark_filters.ThresholdLi.time_integer_image              29.8±0ms

@grlee77 grlee77 changed the title [ENH] Changing threshold_li using histogram for integer images threshold_li: fast, histogram-based code path for integer images Aug 21, 2021
grlee77
grlee77 approved these changes Aug 21, 2021
View reviewed changes
Copy link
Contributor

@grlee77 grlee77 left a comment

Choose a reason for hiding this comment

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

Thanks @yacth, this looks great!

@grlee77 grlee77 added this to the 0.19 milestone Aug 21, 2021
@grlee77
Copy link
Contributor

grlee77 commented Aug 21, 2021

closes #5033

rfezzani
rfezzani reviewed Aug 23, 2021
View reviewed changes
Copy link
Member

@rfezzani rfezzani left a comment

Choose a reason for hiding this comment

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

Thank you @yacth , I just left two suggestions 😉

skimage/filters/thresholding.py Outdated Show resolved Hide resolved
skimage/filters/thresholding.py Outdated Show resolved Hide resolved
@grlee77 @rfezzani
Apply suggestions from code review
a83ceaa 
Co-authored-by: Riadh Fezzani <rfezzani@gmail.com>
@grlee77
Copy link
Contributor

grlee77 commented Sep 2, 2021

@rfezzani, I merged your suggestions and this should be ready now if you want to approve & merge.

rfezzani
rfezzani approved these changes Sep 2, 2021
View reviewed changes
Copy link
Member

@rfezzani rfezzani left a comment

Choose a reason for hiding this comment

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

Thank you @yacth and @grlee77, merging 🎉

@rfezzani rfezzani merged commit fbc3840 into scikit-image:main Sep 2, 2021
@yacth
Copy link
Contributor Author

yacth commented Sep 2, 2021

Sorry for my late reply, I didn't have the possibily to contribute the past weeks.
Thank you @rfezzani and @grlee77 for finishing the job. 🎉

@rfezzani rfezzani mentioned this pull request Oct 18, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grlee77 grlee77 grlee77 approved these changes

@rfezzani rfezzani rfezzani approved these changes

Assignees
No one assigned
Labels
📈 type: Performance
Projects
None yet
Milestone
0.19
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@yacth @pep8speaks @jni @emmanuelle @grlee77 @rfezzani