benchmark: add some warping benchmarks to create a baseline #3260

hmaarrfk · 2018-07-10T06:54:02Z

PR #3253 is about speeding up computation when you want to.

I figured I would add some benchmarks to create a baseline.

It considers 2 cases:

The case where want to go to "float64"
The case where we don't care and just want the warped result as the final image.

I really wish I could test this on my own machine but I tried to install asv:

pip install git+https://github.com/airspeed-velocity/asv.git@v0.2.2rc1#egg=asv

and

pip install git+https://github.com/airspeed-velocity/asv.git#egg=asv

but all failed with

Traceback (most recent call last):
               File "/home/mark/miniconda3/envs/sk/lib/python3.6/site-packages/asv/benchmark.py", line 795, in <module>
                 commands[mode](args)
               File "/home/mark/miniconda3/envs/sk/lib/python3.6/site-packages/asv/benchmark.py", line 740, in main_discover
                 list_benchmarks(benchmark_dir, fp)
               File "/home/mark/miniconda3/envs/sk/lib/python3.6/site-packages/asv/benchmark.py", line 725, in list_benchmarks
                 for benchmark in disc_benchmarks(root):
               File "/home/mark/miniconda3/envs/sk/lib/python3.6/site-packages/asv/benchmark.py", line 710, in disc_benchmarks
                 for module in disc_files(root, os.path.basename(root) + '.'):
               File "/home/mark/miniconda3/envs/sk/lib/python3.6/site-packages/asv/benchmark.py", line 699, in disc_files
                 module = import_module(package + filename)
               File "/home/mark/git/scikit-image/.asv/env/eef864a9fed06fd59ca8023f9f5b02ad/lib/python3.6/importlib/__init__.py", line 126, in import_module
                 return _bootstrap._gcd_import(name[level:], package, level)
               File "<frozen importlib._bootstrap>", line 994, in _gcd_import
               File "<frozen importlib._bootstrap>", line 971, in _find_and_load
               File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
               File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
               File "<frozen importlib._bootstrap_external>", line 678, in exec_module
               File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
               File "/home/mark/git/scikit-image/benchmarks/transform_warp.py", line 2, in <module>
                 from skimage.transform import SimilarityTransform, warp

Checklist

[It's fine to submit PRs which are a work in progress! But before they are merged, all PRs should provide:]

Clean style in the spirit of PEP8
Docstrings for all functions
Gallery example in ./doc/examples (new features only)
Benchmark in ./benchmarks, if your changes aren't covered by an
existing benchmark
Unit tests

[For detailed information on these and other aspects see scikit-image contribution guidelines]

References

[If this is a bug-fix or enhancement, it closes issue # ]
[If this is a new feature, it implements the following paper: ]

For reviewers

(Don't remove the checklist below.)

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.
Consider backporting the PR with @meeseeksdev backport to v0.14.x

pep8speaks · 2018-07-10T06:54:04Z

Hello @hmaarrfk! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on August 06, 2018 at 00:07 Hours UTC

lagru · 2018-07-14T12:44:10Z

benchmarks/transform_warp.py

+                                         translation=(0, 4))
+        self.order = order
+
+    def same_type(self):


I think you'll need to prefix these functions with time_ otherwise asv won't run / find them. And the signature must be able to accept all parameters as positional arguments because asv passes them implicitly to all setup, time_x and peakmem_ methods of the class.

lagru · 2018-07-14T12:51:45Z

I really wish I could test this on my own machine but I tried to install asv [...] but all failed with...

Is the traceback shown during installation of asv or when you try to run it?

I managed to install asv using either conda (conda-forge channel) or providing pip with the path to the cloned repository (which is similar to your attempts).

lagru · 2018-07-14T12:55:34Z

benchmarks/transform_warp.py

+        precision data type."""
+        result = warp(self.image, self.tform, order=self.order,
+                      preserve_range=True)
+        convert(result, dtype=self.image.dtype, copy=False)


Right now you are measuring the time it takes to execute warp and convert together. Is this intended? Wouldn't it be better to keep these measurements separate?

hmaarrfk · 2018-07-14T14:16:53Z

@lagru. I’ll try to install again tonight. I do want to benchmark conversion and warping together. Through conversations with @jni, he mentionnés that it was crucial to try and keep everything in the floating point representation during intermediate processing steps for multi step algorithms. That said, the original data that you might ty and warp might be in uint data types. I’m wondering if the CPU implicit casts are faster than converting everything to a given data type upfront before calling warp.

…

On Sat, Jul 14, 2018 at 5:55 AM Lars G. ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In benchmarks/transform_warp.py <#3260 (comment)> : > + [0, 1, 3] + ) + param_names = ['dtype_in', 'N', 'order'] + + def setup(self, dtype_in, N, order): + self.image = convert(np.random.rand(N, N), dtype=dtype_in) + self.tform = SimilarityTransform(scale=1, rotation=np.pi / 10, + translation=(0, 4)) + self.order = order + + def same_type(self): + """Test the case where the users wants to preserve their same low + precision data type.""" + result = warp(self.image, self.tform, order=self.order, + preserve_range=True) + convert(result, dtype=self.image.dtype, copy=False) Right now you are measuring the time it takes to execute warp *and* convert together. Is this intended? Wouldn't it be better to keep these measurements separate? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3260 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFfmEpV8cftN6N8g1X9jvW8eKt0YAKPks5uGepKgaJpZM4VI21C> .

jni · 2018-07-23T08:21:59Z

@hmaarrfk yes, floating point values are critical for precision, but it is still useful to measure the time for each step separately. Therefore I would recommend benchmarks with and without conversion.

codecov-io · 2018-07-23T14:34:38Z

Codecov Report

Merging #3260 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3260      +/-   ##
==========================================
- Coverage   86.81%   86.75%   -0.06%     
==========================================
  Files         341      339       -2     
  Lines       27406    27356      -50     
==========================================
- Hits        23792    23733      -59     
- Misses       3614     3623       +9

Impacted Files	Coverage Δ
skimage/io/_plugins/simpleitk_plugin.py	`63.63% <0%> (-18.19%)`	⬇️
skimage/io/_plugins/gtk_plugin.py	`25% <0%> (-5.56%)`	⬇️
skimage/io/tests/test_simpleitk.py	`70.17% <0%> (-3.51%)`	⬇️
skimage/_shared/_warnings.py	`90.9% <0%> (-2.28%)`	⬇️
skimage/io/_plugins/util.py	`58.92% <0%> (-1.79%)`	⬇️
skimage/measure/_moments.py	`97.5% <0%> (-1.25%)`	⬇️
skimage/draw/_random_shapes.py	`95.78% <0%> (-1.06%)`	⬇️
skimage/data/__init__.py	`96% <0%> (-0.08%)`	⬇️
skimage/measure/_regionprops.py	`98.66% <0%> (-0.01%)`	⬇️
skimage/filters/tests/test_thresholding.py	`100% <0%> (ø)`	⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7549015...d49ecc5. Read the comment docs.

hmaarrfk · 2018-07-23T14:47:17Z

@lagru @jni thank you for your comments.

I was treating the convert step as a "nop" in the time_to_float64. I feel like a common use case (at least for myself) is to cast things back to the original type after using many scikit-image functions.

My hypothesis:

time_to_float64 test the current performance where the output is a float 64. This should improve for cases where the dtype of the image is not float64 with PR Enable warping with more than just doubles. #3253 as the computation will implicitly upcast to float64 (hopefully) but stay the same when the dtype is float64.
time_same_type: should show improvements for the case when users need lower precision than 64bit. That said, the warping coefficients are still double. I should experiment to see if there are any gains in setting the warping coefficients to single precision when low precision types are used.

I had erroneously used convert as opposed to astype within the benchmarks.

jni

@hmaarrfk great, LGTM. Only a minor suggestion: I think it is preferred form to use np.random.random((N, N)) over np.random.rand(N, N) for consistency with other random functions, as well as the standard NumPy array creation functions (zeros, ones, full, etc).

hmaarrfk · 2018-07-24T02:24:52Z

@jni, why can't I find the documentation for numpy.random.random

hmaarrfk · 2018-07-24T02:26:55Z

~~It isn't on this main page, but it does have its own doc page~~ i'm just blind.

hmaarrfk · 2018-07-24T02:31:07Z

@jni done!

jni · 2018-07-24T03:10:32Z

@hmaarrfk

i'm just blind.

I suspected that might be the case as I saw your comments rolling in... =D

hmaarrfk · 2018-07-24T15:53:25Z

@jni, I realize i needed to install asv with system python3 and run it with bash or some combination thereof.

~~doesn't seem to be running correctly on my computer. mrg-1 for now.~~ working now.

hmaarrfk · 2018-09-15T17:13:20Z

Closing in favour of just includeing this straight in #3253

lagru reviewed Jul 14, 2018

View reviewed changes

jni approved these changes Jul 24, 2018

View reviewed changes

hmaarrfk force-pushed the warp_benchmark branch from 3cdb55e to e4e1009 Compare July 24, 2018 02:31

jni added the status: mrg+1 label Jul 24, 2018

soupault added the 📈 type: Performance label Jul 28, 2018

soupault added this to the 0.15 milestone Jul 28, 2018

hmaarrfk force-pushed the warp_benchmark branch 2 times, most recently from e2f3c8e to c597859 Compare August 6, 2018 00:07

hmaarrfk added 2 commits September 7, 2018 06:32

benchmark: add some warping benchmarks to create a baseline

548a84a

Fixed the benchmarks for warping

d49ecc5

hmaarrfk force-pushed the warp_benchmark branch from c597859 to d49ecc5 Compare September 7, 2018 10:32

hmaarrfk closed this Sep 15, 2018

hmaarrfk deleted the warp_benchmark branch November 4, 2018 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

benchmark: add some warping benchmarks to create a baseline #3260

benchmark: add some warping benchmarks to create a baseline #3260

hmaarrfk commented Jul 10, 2018

pep8speaks commented Jul 10, 2018 •

edited

Loading

lagru Jul 14, 2018

lagru commented Jul 14, 2018

lagru Jul 14, 2018

hmaarrfk commented Jul 14, 2018 via email

jni commented Jul 23, 2018

codecov-io commented Jul 23, 2018 •

edited

Loading

hmaarrfk commented Jul 23, 2018

jni left a comment

hmaarrfk commented Jul 24, 2018

hmaarrfk commented Jul 24, 2018 •

edited

Loading

hmaarrfk commented Jul 24, 2018

jni commented Jul 24, 2018

hmaarrfk commented Jul 24, 2018 •

edited

Loading

hmaarrfk commented Sep 15, 2018

benchmark: add some warping benchmarks to create a baseline #3260

benchmark: add some warping benchmarks to create a baseline #3260

Conversation

hmaarrfk commented Jul 10, 2018

Checklist

References

For reviewers

pep8speaks commented Jul 10, 2018 • edited Loading

Comment last updated on August 06, 2018 at 00:07 Hours UTC

lagru Jul 14, 2018

Choose a reason for hiding this comment

lagru commented Jul 14, 2018

lagru Jul 14, 2018

Choose a reason for hiding this comment

hmaarrfk commented Jul 14, 2018 via email

jni commented Jul 23, 2018

codecov-io commented Jul 23, 2018 • edited Loading

Codecov Report

hmaarrfk commented Jul 23, 2018

jni left a comment

Choose a reason for hiding this comment

hmaarrfk commented Jul 24, 2018

hmaarrfk commented Jul 24, 2018 • edited Loading

hmaarrfk commented Jul 24, 2018

jni commented Jul 24, 2018

hmaarrfk commented Jul 24, 2018 • edited Loading

hmaarrfk commented Sep 15, 2018

pep8speaks commented Jul 10, 2018 •

edited

Loading

codecov-io commented Jul 23, 2018 •

edited

Loading

hmaarrfk commented Jul 24, 2018 •

edited

Loading

hmaarrfk commented Jul 24, 2018 •

edited

Loading