add ASV (continued) by grlee77 · Pull Request #264 · PyWavelets/pywt

grlee77 · 2016-12-31T01:12:14Z

This is a revival of @kwohlfahrt's work in #158 (which has not been updated in several months).

I have rebased that PR on current master and made some updates. The first new commit here addresses minor comments @rgommers made in the old PR.

I added several other benchmarks for the majority of the remaining transforms. I have tried to keep the run time down to around 5 minutes for current master when calling via:

asv run --python=same --quick

I have also attempted to make tests skip functions that were not present on older commits by raising a NotImplementedError whenever a function under test cannot be imported. As is, for v0.3.0, several benchmarks get skipped and there are only a handful of test cases that result in failures (i.e. some tests use complex dtypes which was not supported then).

No benchmarks setup, but ASV builds correctly

Interestingly, 1D dwt is significantly slower on python 3.

Benchmarking these functions should hopefully give a closer view of how performance in the C core changes without requiring a lot of effort to instrument them separately. The different modes should have somewhat different performance, especially for small sizes/large filters. If this doesn't happen, there is too much overhead to benchmark the convolution directly and this needs to be done separately. modes are only set for 1D, since n-dimensional path is not affected by the mode (it is just passed through to the 1D).

raises NotImplementedErrors on various functions that may not be present on older commits so that those tests are skipped. special case to support the now outdated pywt.MODES syntax from the 0.3.0 API

codecov-io · 2016-12-31T01:25:52Z

Current coverage is 86.17% (diff: 100%)

Merging #264 into master will decrease coverage by 0.89%

@@             master       #264   diff @@
==========================================
  Files            20         20          
  Lines          2962       3045    +83   
  Methods           0         45    +45   
  Messages          0          0          
  Branches        297        521   +224   
==========================================
+ Hits           2579       2624    +45   
- Misses          333        371    +38   
  Partials         50         50

Powered by Codecov. Last update 45314ce...01733e2

rgommers · 2016-12-31T08:17:06Z

asv/benchmarks/cwt_benchmarks.py

+        try:
+            from pywt import cwt
+        except ImportError:
+            raise NotImplementedError("cwt not available")


Is there an advantage to doing this within setup and raising an exception? SciPy does it like I described at #158 (comment). Here's an example for lgmres, https://github.com/scipy/scipy/blob/master/benchmarks/benchmarks/sparse_linalg_solve.py#L15, works fine and seems simpler.

The difference seems to be that with my approach the test shows up as "n/a" instead of as "failed". I used NotImplementedError in setup based on the following text in the ASV docs

If setup raises a NotImplementedError, the benchmark is marked as skipped for the parameter values in question.

see for example the following output when running the CWT tests on 0.4.0 where it is not present:
(scroll right to see the n/a)

12:58 $ asv run --python=3.5 --quick --bench="Cwt*" v0.4.0^! · No `environment_type` specified in asv.conf.json. This will be required in the future. · Creating environments · Discovering benchmarks ·· Uninstalling from conda-py3.5-Cython-numpy ·· Building for conda-py3.5-Cython-numpy....... ·· Installing into conda-py3.5-Cython-numpy.. · Running 1 total benchmarks (1 commits * 1 environments * 1 benchmarks) [ 0.00%] · For pywt commit hash 3f4f46a9: [ 0.00%] ·· Building for conda-py3.5-Cython-numpy.^[[1;2C.... [ 0.00%] ·· Benchmarking conda-py3.5-Cython-numpy [100.00%] ··· Running cwt_benchmarks.CwtTimeSuite.time_cwt n/a;...

Interesting, didn't know that. +1 for NotImplemented

rgommers · 2016-12-31T08:17:59Z

asv/benchmarks/cwt_benchmarks.py

+
+class CwtTimeSuite(CwtTimeSuiteBase):
+    def time_cwt(self, n, wavelet, max_scale):
+        pywt.cwt(self.data, self.scales, self.wavelet)


this should use the max_scale and wavelet parameters that are passed to time_cwt it looks like.

In this benchmark, self.scales is a range while max_scale is an integer

for self.wavelet vs. wavelet I think it depends which code path you want to benchmark. For small sizes, the benchmark runs a bit faster if the input is already a Wavelet object, but for larger transforms the overhead of converting a string to a Wavelet object is negligable.

That said, I am fine with just passing the wavelet string instead of the self.wavelet object. The same goes for the mode parameter in some of the other tests.

rgommers · 2016-12-31T08:18:41Z

asv/benchmarks/dwt_benchmarks.py

+
+class DwtTimeSuite(DwtTimeSuiteBase):
+    def time_dwt(self, n, wavelet, mode):
+        pywt.dwt(self.data, self.wavelet, self.mode)


same here, remove self. to use the parameters passed to time_dwt.

and same again for time_idwt below

and more cases further on

rgommers · 2016-12-31T08:20:34Z

asv/benchmarks/dwt_benchmarks.py

+
+
+class Wavedec2TimeSuite(Wavedec2TimeSuiteBase):
+    def time_wavedec2(self, n, wavelet, dtype):


dtype isn't used

and n isn't either

The benchmarks fail if I remove any of these unused parameters that were defined in the parent class. (They are used, but only in the setup method of the parent).

I do not see this clearly stated in the docs: https://asv.readthedocs.io/en/latest/writing_benchmarks.html#parameterized-benchmarks

ah that makes sense. not much to be done about it then

rgommers · 2016-12-31T08:21:32Z

asv/benchmarks/dwt_benchmarks.py

+
+
+class WavedecnTimeSuite(WavedecnTimeSuiteBase):
+    def time_wavedecn(self, D, n, wavelet, dtype):


more unused parameters

true for multiple benchmarks below

rgommers · 2016-12-31T08:26:19Z

Thanks for picking this up, would be nice to get these in!

rgommers · 2016-12-31T08:27:51Z

A couple more review comments from gh-158:

rename asv/ to benchmarks/
add a benchmarks/README with basic instructions (a simple pip install asv and then asv run --python=same will do)

@pv

credit to @pv who wrote the corresponding README for the scipy project, upon which this one is based.

grlee77 · 2016-12-31T18:58:13Z

I have addressed most comments, but see my responses above on the inability to remove unused parameters and on the logic behind raising a NotImplementedError in setup.py

on my macbook, the quick version of the benchmarks took 4:32 on current master. I am running it now without --quick to see how long this takes. It is also possible that benchmarks on old revisions may take much longer (e.g. dwtn/idwtn are more than an order of magnitude faster on current master vs. 0.3.0).

grlee77 · 2016-12-31T19:22:43Z

full duration was 36:15 for current master without `--quick`` (for a single commit).

rgommers · 2017-01-01T22:33:00Z

LGTM. In it goes. Thanks @grlee77!

Kai Wohlfahrt and others added 8 commits December 30, 2016 15:41

ENH: Add asv

76b7b92

No benchmarks setup, but ASV builds correctly

ENH: Add initial benchmarks for dwt

26d0f05

Interestingly, 1D dwt is significantly slower on python 3.

ENH: Benchmarks for (i)swt

c41097f

address PyWaveletsgh-158 review comments

21e0e9b

ENH: add benchmarks for additional DWT and SWT functions

0ad3670

ENH: add CWT benchmark

287e076

make benchmarks compatible with older commits back to v0.3.0

219a1c7

raises NotImplementedErrors on various functions that may not be present on older commits so that those tests are skipped. special case to support the now outdated pywt.MODES syntax from the 0.3.0 API

grlee77 added the enhancement label Dec 31, 2016

grlee77 added this to the v1.0 milestone Dec 31, 2016

rgommers reviewed Dec 31, 2016

View reviewed changes

rgommers mentioned this pull request Dec 31, 2016

Add ASV #158

Closed

grlee77 added 3 commits December 31, 2016 13:21

use wavelet strings instead of wavelet objects in benchmarks

74e3958

rename asv folder as benchmarks

958d1d8

DOC: add a README for the benchmarks

01733e2

credit to @pv who wrote the corresponding README for the scipy project, upon which this one is based.

rgommers merged commit 45cf2a5 into PyWavelets:master Jan 1, 2017

grlee77 deleted the asv_v2 branch March 9, 2017 04:46



		class Wavedec2TimeSuite(Wavedec2TimeSuiteBase):
		def time_wavedec2(self, n, wavelet, dtype):



		class WavedecnTimeSuite(WavedecnTimeSuiteBase):
		def time_wavedecn(self, D, n, wavelet, dtype):

Uh oh!

Conversation

grlee77 commented Dec 31, 2016

Uh oh!

codecov-io commented Dec 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 86.17% (diff: 100%)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rgommers commented Dec 31, 2016

Uh oh!

rgommers commented Dec 31, 2016

Uh oh!

grlee77 commented Dec 31, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grlee77 commented Dec 31, 2016

Uh oh!

rgommers commented Jan 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-io commented Dec 31, 2016 •

edited

Loading

grlee77 commented Dec 31, 2016 •

edited

Loading