MAINT: Rewrite shape normalization in pad function by lagru · Pull Request #11966 · numpy/numpy

lagru · 2018-09-16T19:03:36Z

This rewrite joins the previous functions _normalize_shape and _validate_length as an intro to larger changes suggested in gh-11126. This function accelerates the benchmarks for np.pad by a factor of
0.57 to 0.90.

cc @eric-wieser

Benchmarks

$ asv continuous -E conda:3.6 -b Pad master pad-as-pairs
       before           after         ratio
     [88c01b8a]       [3a6182a]
     <master>         <pad-as-pairs>
-         134±4μs          120±3μs     0.90  bench_lib.Pad.time_pad((10, 100), 1, 'mean')
-      73.7±0.4μs         63.9±2μs     0.87  bench_lib.Pad.time_pad((10, 10, 10), 1, 'edge')
-         102±2μs         87.9±3μs     0.86  bench_lib.Pad.time_pad((10, 10, 10), 3, 'constant')
-        88.4±2μs         74.0±3μs     0.84  bench_lib.Pad.time_pad((10, 10, 10), 1, 'constant')
-        145±10μs          121±3μs     0.84  bench_lib.Pad.time_pad((10, 100), 3, 'mean')
-        57.2±2μs         47.7±2μs     0.83  bench_lib.Pad.time_pad((10, 10, 10), 3, 'wrap')
-         116±7μs         96.4±5μs     0.83  bench_lib.Pad.time_pad((10, 100), (0, 5), 'linear_ramp')
-      54.4±0.7μs         45.1±1μs     0.83  bench_lib.Pad.time_pad((10, 10, 10), 1, 'reflect')
-        81.1±2μs         67.1±2μs     0.83  bench_lib.Pad.time_pad((1000,), 1, 'mean')
-        42.3±1μs         34.0±2μs     0.80  bench_lib.Pad.time_pad((10, 100), 1, 'reflect')
-      29.1±0.9μs         23.4±1μs     0.80  bench_lib.Pad.time_pad((1000,), 3, 'wrap')
-        92.1±2μs         73.6±5μs     0.80  bench_lib.Pad.time_pad((10, 100), (0, 5), 'mean')
-        64.5±2μs       50.4±0.9μs     0.78  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'reflect')
-      38.3±0.6μs         29.7±1μs     0.78  bench_lib.Pad.time_pad((10, 100), 3, 'wrap')
-        70.6±5μs         54.2±1μs     0.77  bench_lib.Pad.time_pad((10, 10, 10), 3, 'reflect')
-        70.1±5μs         53.6±2μs     0.76  bench_lib.Pad.time_pad((10, 100), 3, 'constant')
-        36.5±2μs       27.5±0.6μs     0.75  bench_lib.Pad.time_pad((1000,), 1, 'edge')
-        32.7±1μs       24.6±0.9μs     0.75  bench_lib.Pad.time_pad((1000,), 1, 'reflect')
-        69.5±9μs         51.8±2μs     0.75  bench_lib.Pad.time_pad((10, 100), 1, 'constant')
-      46.1±0.9μs         34.3±2μs     0.74  bench_lib.Pad.time_pad((10, 100), (0, 5), 'reflect')
-        36.7±2μs       27.3±0.5μs     0.74  bench_lib.Pad.time_pad((1000,), 3, 'edge')
-      29.2±0.4μs         21.5±1μs     0.74  bench_lib.Pad.time_pad((1000,), 1, 'wrap')
-        72.4±2μs         53.1±5μs     0.73  bench_lib.Pad.time_pad((1000,), (0, 5), 'linear_ramp')
-      32.3±0.8μs       23.6±0.6μs     0.73  bench_lib.Pad.time_pad((1000,), 3, 'reflect')
-        57.5±3μs       41.9±0.6μs     0.73  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'wrap')
-      48.7±0.8μs       34.5±0.8μs     0.71  bench_lib.Pad.time_pad((1000,), 1, 'constant')
-        77.9±6μs         54.5±2μs     0.70  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'constant')
-        70.5±4μs       48.7±0.8μs     0.69  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'edge')
-      42.5±0.6μs       29.3±0.8μs     0.69  bench_lib.Pad.time_pad((10, 100), (0, 5), 'wrap')
-        44.5±1μs         30.4±1μs     0.68  bench_lib.Pad.time_pad((10, 100), (0, 5), 'edge')
-      48.8±0.6μs       33.2±0.5μs     0.68  bench_lib.Pad.time_pad((1000,), 3, 'constant')
-      35.5±0.7μs       24.0±0.8μs     0.68  bench_lib.Pad.time_pad((1000,), (0, 5), 'reflect')
-      32.4±0.6μs       20.1±0.5μs     0.62  bench_lib.Pad.time_pad((1000,), (0, 5), 'wrap')
-        33.2±1μs       20.5±0.6μs     0.62  bench_lib.Pad.time_pad((1000,), (0, 5), 'edge')
-        60.0±3μs       36.3±0.9μs     0.61  bench_lib.Pad.time_pad((10, 100), (0, 5), 'constant')
-      45.1±0.5μs       25.7±0.7μs     0.57  bench_lib.Pad.time_pad((1000,), (0, 5), 'constant')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.

numpy/lib/arraypad.py

eric-wieser · 2018-09-16T19:19:13Z

numpy/lib/arraypad.py

I think these lines are the important part of the patch

numpy/lib/arraypad.py

eric-wieser · 2018-09-16T23:38:00Z

I'm with mhvk here - this function doesn't seem important enough to justify a constant-time micro-optimization. Let's just fix the int/intp bug, and remove the weird list comprehensions that are simpler as a call to .min.

numpy/lib/arraypad.py

lagru · 2018-09-17T13:33:44Z

Removing the optimizations results (923ce12) slows down the benchmarks noticeably compared to (88c01b8). At this rate I would say the extra code paths for size == 1 and size == 2 may be worth it, considering that those are the most likely input. But if your experience tells you otherwise I'll gladly defer to your decision. :)

In the meantime I'll write some unit tests for _as_pairs.

New Benchmarks

$ asv continuous -s -E conda:3.6 -b Pad master pad-as-pairs
       before           after         ratio
     [88c01b8a]       [923ce12b]
     <master>         <pad-as-pairs>
+      31.7±0.4μs       39.2±0.6μs     1.24  bench_lib.Pad.time_pad((1000,), 1, 'reflect')
+      42.1±0.4μs       52.0±0.4μs     1.23  bench_lib.Pad.time_pad((10, 100), 1, 'reflect')
+      38.5±0.6μs         47.4±1μs     1.23  bench_lib.Pad.time_pad((10, 100), 3, 'wrap')
+      54.1±0.4μs       66.5±0.9μs     1.23  bench_lib.Pad.time_pad((10, 10, 10), 1, 'reflect')
+      31.8±0.7μs       39.0±0.6μs     1.23  bench_lib.Pad.time_pad((1000,), 3, 'reflect')
+      43.2±0.6μs       52.9±0.5μs     1.23  bench_lib.Pad.time_pad((10, 100), 3, 'reflect')
+      28.9±0.9μs       35.3±0.1μs     1.22  bench_lib.Pad.time_pad((1000,), 3, 'wrap')
+      29.0±0.5μs       35.4±0.3μs     1.22  bench_lib.Pad.time_pad((1000,), 1, 'wrap')
+      37.7±0.3μs       45.9±0.3μs     1.22  bench_lib.Pad.time_pad((10, 100), 1, 'wrap')
+      48.1±0.5μs         58.2±1μs     1.21  bench_lib.Pad.time_pad((10, 10, 10), 1, 'wrap')
+      35.3±0.7μs       42.3±0.7μs     1.20  bench_lib.Pad.time_pad((1000,), 1, 'edge')
+        46.8±1μs       55.6±0.5μs     1.19  bench_lib.Pad.time_pad((10, 100), (0, 5), 'reflect')
+         236±2μs          280±6μs     1.19  bench_lib.Pad.time_pad((10, 10, 10), 1, 'linear_ramp')
+      35.5±0.6μs       42.1±0.2μs     1.18  bench_lib.Pad.time_pad((1000,), 3, 'edge')
+        42.2±1μs       49.6±0.5μs     1.18  bench_lib.Pad.time_pad((10, 100), (0, 5), 'wrap')
+         158±2μs          185±2μs     1.17  bench_lib.Pad.time_pad((10, 100), 1, 'linear_ramp')
+     63.8±0.09μs       74.7±0.7μs     1.17  bench_lib.Pad.time_pad((10, 10, 10), 3, 'reflect')
+        32.3±2μs       37.7±0.2μs     1.17  bench_lib.Pad.time_pad((1000,), (0, 5), 'wrap')
+      57.3±0.4μs         66.8±1μs     1.16  bench_lib.Pad.time_pad((10, 10, 10), 3, 'wrap')
+        95.4±2μs          109±2μs     1.15  bench_lib.Pad.time_pad((1000,), 3, 'linear_ramp')
+      52.9±0.7μs       60.4±0.3μs     1.14  bench_lib.Pad.time_pad((10, 100), 1, 'edge')
+        65.0±1μs       74.2±0.3μs     1.14  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'reflect')
+      54.6±0.3μs       62.2±0.3μs     1.14  bench_lib.Pad.time_pad((10, 100), 3, 'edge')
+         181±1μs          206±2μs     1.14  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'linear_ramp')
+         115±2μs        131±0.4μs     1.14  bench_lib.Pad.time_pad((10, 100), (0, 5), 'linear_ramp')
+       294±0.4μs          334±5μs     1.14  bench_lib.Pad.time_pad((10, 10, 10), 3, 'linear_ramp')
+        74.1±1μs         84.1±2μs     1.14  bench_lib.Pad.time_pad((10, 10, 10), 1, 'edge')
+        72.7±3μs       82.2±0.5μs     1.13  bench_lib.Pad.time_pad((1000,), (0, 5), 'linear_ramp')
+        47.8±2μs       54.0±0.3μs     1.13  bench_lib.Pad.time_pad((1000,), 1, 'constant')
+        57.5±1μs       64.9±0.4μs     1.13  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'wrap')
+        68.1±2μs       76.9±0.5μs     1.13  bench_lib.Pad.time_pad((10, 100), 1, 'constant')
+        98.4±4μs          111±1μs     1.13  bench_lib.Pad.time_pad((1000,), 1, 'linear_ramp')
+        90.9±1μs          102±2μs     1.12  bench_lib.Pad.time_pad((10, 10, 10), 3, 'edge')
+        48.8±1μs       54.6±0.5μs     1.12  bench_lib.Pad.time_pad((1000,), 3, 'constant')
+        81.4±2μs       90.9±0.3μs     1.12  bench_lib.Pad.time_pad((1000,), 3, 'mean')
+        91.0±2μs        102±0.2μs     1.12  bench_lib.Pad.time_pad((10, 10, 10), 1, 'constant')
+        44.7±2μs       49.9±0.3μs     1.12  bench_lib.Pad.time_pad((10, 100), (0, 5), 'edge')
+        70.5±1μs         78.5±1μs     1.11  bench_lib.Pad.time_pad((10, 100), 3, 'constant')
+        33.6±2μs       37.3±0.2μs     1.11  bench_lib.Pad.time_pad((1000,), (0, 5), 'edge')
+        65.6±2μs       72.4±0.9μs     1.10  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'edge')
+        44.8±3μs       49.4±0.2μs     1.10  bench_lib.Pad.time_pad((1000,), (0, 5), 'constant')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.

eric-wieser · 2018-09-17T15:29:07Z

lows down the benchmarks noticeably

I wonder whether .tolist() was actually a performance improvement?

numpy/lib/arraypad.py

eric-wieser · 2018-09-24T02:46:33Z

numpy/lib/arraypad.py

Would be great to add a failing unit tests for this. Here's one that fails:

# pad the non-empty axis so that no memory allocation is needed assert_equal(np.pad(np.zeros((1, 0)), [(0, 2**31), (0, 0)], 'constant').shape, (2**31+1, 0)) assert_equal(np.pad(np.zeros((1, 0)), [(0, 2**32), (0, 0)], 'constant').shape, (2**32+1, 0))

Written up as #12023

Will do. :)

Would be great to add a failing unit tests for this.

You mean marked with pytest's xfail? Considering the explanation in #12023 (comment): how would I mark a unit test as xfailing only on Windows Systems?

I think what I meant by this, is test the above since they no longer fail with this patch. More rigorous I suppose would be to use np.iinfo(np.intp).max instead of 2**32

Wouldn't it be simpler to test _as_pairs directly if the unit test in question would be introduced alongside this anyway?

E.g.:

_as_pairs([np.iinfo(np.intp).max], 2, as_index=True) # -> passes _as_pairs([np.iinfo(np.intp).max + 1], 2, as_index=True) # -> raises "ValueError: index can't contain negative values"

I'm still not sure what this test is supposed to accomplish. You want to document the behavior for the edge case around the maximum of intp, correct?

Also

np.pad(np.zeros((1, 0)), [(0, np.iinfo(np.intp).max), (0, 0)], 'constant')

will raise a different error which may not be what you intended...

ValueError: array is too big; `arr.size * arr.dtype.itemsize` is larger than the maximum possible size.

@mattip, @mhvk Before you consider merging this I just want to highlight this thread in case you missed it. From what I gathered @eric-wieser suggested this as an optional improvement. However I still don't know how to proceed.

Of course I'm always happy to add the test as a separate PR if / when this thread is resolved.

This rewrite joins the previous functions _normalize_shape and _validate_length as an intro to larger changes suggested in numpygh-11126.

lagru · 2018-09-24T09:50:58Z

@eric-wieser

Some comments & observations:

New unit tests, hopefully testing _as_pairs inside and out.
For now I included the two extra code paths again. But only to give you the chance to evaluate a revised version of these. I tried to improve the comments and the logic as well. This should have exactly the same behavior as just using broadcast_to and is in any case now covered by the tests. This version is faster than the old one (0.91 to 0.54). Despite being a constant time improvement this may be significant if padding small arrays repeatly. Concerning @mhvk's valid concern about multiple code paths, I hope that could be mitigated by covering this with unit tests.
Removing the two code paths leads to a slow down compared to master by roughly 1.10 to 1.30. tolist is indeed a small improvement compared to returning the array.

I hope that is enough to for you to decide which version to prefer. Obviously I'm still prefering the faster version which I tried to improve. I hope this doesn't seem too strong-headed. In any case I'll defer to your final judgment. ;)

Test coverage of _as_pairs is 100%.

rgommers · 2018-09-25T15:24:44Z

Very interesting issue, CI passing but local runtests failing:

$ python runtests.py 
Building, see build.log...
Build OK
NumPy version 1.16.0.dev0+59be3b8
NumPy relaxed strides checking option: True

=================================================== ERRORS ====================================================
________ ERROR collecting build/testenv/lib/python3.6/site-packages/numpy/core/tests/test_overrides.py ________
numpy/core/tests/test_overrides.py:9: in <module>
    from numpy.core.overrides import (
numpy/core/overrides.py:9: in <module>
    _NDARRAY_ARRAY_FUNCTION = ndarray.__array_function__
E   AttributeError: type object 'numpy.ndarray' has no attribute '__array_function__'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
167 deselected, 1 error in 3.58 seconds

Looking into this. Probably unrelated to this particular PR.

EDIT: in-place build works fine, looks like a runtests.py issue.

numpy/lib/arraypad.py

Addresses reviews: - The test "x.ndim < 3" now covers both optimizations as it should. - Extend comments and improve formatting as well.

numpy/lib/tests/test_arraypad.py

eric-wieser

Looks good now, thanks - hopefully the micro-optimizations here won't bite us in future.

Can you show a final before / after benchmark?

lagru · 2018-11-03T14:37:25Z

Can you show a final before / after benchmark? -- @eric-wieser

Here you go :

New benchmarks

I made a few temporary additions to the benchmarks to hopefully cover more use cases:

--- a/benchmarks/benchmarks/bench_lib.py
+++ b/benchmarks/benchmarks/bench_lib.py
@@ -13,9 +13,10 @@ class Pad(Benchmark):

     param_names = ["shape", "pad_width", "mode"]
     params = [
-        [(1000,), (10, 100), (10, 10, 10)],
-        [1, 3, (0, 5)],
-        ["constant", "edge", "linear_ramp", "mean", "reflect", "wrap"],
+        [(1,), (1000,), (10, 100), (10, 10, 10)],
+        [1, 3, (0, 5), (20, 0)],
+        ["constant", "edge", "linear_ramp", "mean", "minimum", "maximum",
+         "reflect", "symmetric", "wrap", "empty"],
     ]

Results using a single core while keeping the OS as idle as possible:

$ taskset -c 1 asv continuous -E conda:3.6 -b Pad master pad-as-pairs
       before           after         ratio
     [28c53260]       [a0608385]
     <master>         <pad-as-pairs>
-         262±1μs          237±1μs     0.90  bench_lib.Pad.time_pad((10, 10, 10), (20, 0), 'constant')
-         163±3μs          146±1μs     0.90  bench_lib.Pad.time_pad((10, 10, 10), 3, 'minimum')
-         157±2μs          141±2μs     0.90  bench_lib.Pad.time_pad((10, 100), 3, 'mean')
-         135±3μs          120±2μs     0.89  bench_lib.Pad.time_pad((10, 10, 10), 1, 'maximum')
-      58.4±0.4μs       51.5±0.5μs     0.88  bench_lib.Pad.time_pad((1,), (20, 0), 'symmetric')
-       164±0.4μs        144±0.9μs     0.88  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'mean')
-      49.5±0.3μs       43.3±0.4μs     0.88  bench_lib.Pad.time_pad((1,), (20, 0), 'wrap')
-      88.1±0.2μs       77.0±0.4μs     0.87  bench_lib.Pad.time_pad((10, 10, 10), 3, 'edge')
-         134±3μs          117±2μs     0.87  bench_lib.Pad.time_pad((10, 10, 10), 1, 'minimum')
-         165±2μs          144±1μs     0.87  bench_lib.Pad.time_pad((10, 100), (20, 0), 'linear_ramp')
-         102±2μs       88.6±0.3μs     0.87  bench_lib.Pad.time_pad((1000,), 1, 'linear_ramp')
-      98.3±0.6μs         84.1±1μs     0.85  bench_lib.Pad.time_pad((1000,), 3, 'linear_ramp')
-         129±2μs          110±3μs     0.85  bench_lib.Pad.time_pad((10, 100), (0, 5), 'linear_ramp')
-        96.8±2μs         82.2±1μs     0.85  bench_lib.Pad.time_pad((1,), 1, 'linear_ramp')
-        90.1±1μs       76.1±0.9μs     0.84  bench_lib.Pad.time_pad((1000,), 1, 'mean')
-      72.6±0.4μs      61.3±0.09μs     0.84  bench_lib.Pad.time_pad((10, 10, 10), 1, 'edge')
-      62.1±0.3μs      52.2±0.08μs     0.84  bench_lib.Pad.time_pad((10, 10, 10), 3, 'reflect')
-      93.6±0.2μs         78.6±1μs     0.84  bench_lib.Pad.time_pad((1,), 3, 'linear_ramp')
-        89.5±2μs         75.1±1μs     0.84  bench_lib.Pad.time_pad((10, 100), 3, 'maximum')
-        86.3±2μs         72.1±1μs     0.84  bench_lib.Pad.time_pad((10, 100), 1, 'maximum')
-        81.7±2μs       68.1±0.3μs     0.83  bench_lib.Pad.time_pad((1,), 3, 'mean')
-        91.8±5μs       76.4±0.6μs     0.83  bench_lib.Pad.time_pad((1000,), 3, 'mean')
-        88.9±2μs         73.9±2μs     0.83  bench_lib.Pad.time_pad((10, 100), 3, 'minimum')
-        81.8±1μs       67.9±0.6μs     0.83  bench_lib.Pad.time_pad((1,), 1, 'mean')
-      61.9±0.3μs       51.0±0.2μs     0.82  bench_lib.Pad.time_pad((10, 10, 10), 3, 'symmetric')
-        85.6±2μs         70.5±1μs     0.82  bench_lib.Pad.time_pad((10, 100), 1, 'minimum')
-      45.3±0.8μs         37.1±2μs     0.82  bench_lib.Pad.time_pad((1,), 1, 'maximum')
-      35.3±0.1μs       28.9±0.2μs     0.82  bench_lib.Pad.time_pad((1,), 3, 'symmetric')
-      53.3±0.6μs       43.6±0.2μs     0.82  bench_lib.Pad.time_pad((10, 10, 10), 1, 'reflect')
-       102±0.7μs         83.3±3μs     0.82  bench_lib.Pad.time_pad((10, 100), (0, 5), 'mean')
-        98.4±2μs       80.2±0.2μs     0.81  bench_lib.Pad.time_pad((10, 10, 10), 3, 'constant')
-     45.7±0.04μs       37.2±0.4μs     0.81  bench_lib.Pad.time_pad((1,), (0, 5), 'symmetric')
-      56.6±0.5μs       46.0±0.2μs     0.81  bench_lib.Pad.time_pad((10, 100), (20, 0), 'reflect')
-      55.3±0.3μs       44.9±0.3μs     0.81  bench_lib.Pad.time_pad((10, 10, 10), 3, 'wrap')
-      52.2±0.3μs       42.2±0.3μs     0.81  bench_lib.Pad.time_pad((10, 100), 3, 'edge')
-       113±0.9μs       91.2±0.3μs     0.81  bench_lib.Pad.time_pad((10, 100), (20, 0), 'mean')
-      40.0±0.3μs       32.3±0.2μs     0.81  bench_lib.Pad.time_pad((1,), (0, 5), 'wrap')
-      31.0±0.1μs      24.9±0.09μs     0.80  bench_lib.Pad.time_pad((1,), 3, 'wrap')
-       101±0.7μs       80.9±0.5μs     0.80  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'minimum')
-      53.1±0.2μs       42.5±0.1μs     0.80  bench_lib.Pad.time_pad((10, 10, 10), 1, 'symmetric')
-         102±1μs         81.8±1μs     0.80  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'maximum')
-      41.8±0.2μs       33.4±0.1μs     0.80  bench_lib.Pad.time_pad((10, 100), 3, 'reflect')
-      57.7±0.1μs       46.0±0.2μs     0.80  bench_lib.Pad.time_pad((10, 100), (20, 0), 'symmetric')
-      50.3±0.3μs       40.1±0.2μs     0.80  bench_lib.Pad.time_pad((10, 100), 1, 'edge')
-      62.0±0.3μs       49.0±0.1μs     0.79  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'reflect')
-      53.6±0.8μs       42.4±0.5μs     0.79  bench_lib.Pad.time_pad((1000,), 1, 'maximum')
-      47.0±0.2μs       37.0±0.3μs     0.79  bench_lib.Pad.time_pad((10, 10, 10), 1, 'wrap')
-      54.0±0.7μs       42.3±0.9μs     0.78  bench_lib.Pad.time_pad((1000,), 3, 'maximum')
-      53.5±0.8μs       41.9±0.5μs     0.78  bench_lib.Pad.time_pad((1000,), 1, 'minimum')
-      41.0±0.1μs       32.0±0.2μs     0.78  bench_lib.Pad.time_pad((10, 100), 1, 'reflect')
-        86.3±1μs       67.3±0.5μs     0.78  bench_lib.Pad.time_pad((10, 10, 10), 1, 'constant')
-      53.9±0.5μs       42.0±0.4μs     0.78  bench_lib.Pad.time_pad((1000,), 3, 'minimum')
-      51.1±0.1μs       39.7±0.1μs     0.78  bench_lib.Pad.time_pad((10, 100), (20, 0), 'wrap')
-      37.1±0.1μs       28.7±0.1μs     0.77  bench_lib.Pad.time_pad((10, 100), 3, 'wrap')
-      41.9±0.2μs      32.4±0.08μs     0.77  bench_lib.Pad.time_pad((10, 100), 3, 'symmetric')
-      61.4±0.1μs       47.2±0.3μs     0.77  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'symmetric')
-      41.2±0.2μs      31.4±0.04μs     0.76  bench_lib.Pad.time_pad((10, 100), 1, 'symmetric')
-     36.4±0.09μs       27.8±0.1μs     0.76  bench_lib.Pad.time_pad((10, 100), 1, 'wrap')
-        44.8±1μs       34.2±0.1μs     0.76  bench_lib.Pad.time_pad((1,), 1, 'minimum')
-      70.6±0.2μs         53.6±3μs     0.76  bench_lib.Pad.time_pad((1,), (0, 5), 'linear_ramp')
-      45.7±0.8μs       34.5±0.5μs     0.75  bench_lib.Pad.time_pad((1,), 3, 'maximum')
-      55.1±0.1μs      41.5±0.07μs     0.75  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'wrap')
-      63.0±0.3μs       47.3±0.4μs     0.75  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'edge')
-        45.4±1μs       34.0±0.8μs     0.75  bench_lib.Pad.time_pad((1,), 3, 'minimum')
-      64.7±0.7μs       48.3±0.1μs     0.75  bench_lib.Pad.time_pad((1000,), (0, 5), 'mean')
-      32.0±0.1μs       23.8±0.9μs     0.75  bench_lib.Pad.time_pad((1,), 1, 'reflect')
-      31.2±0.2μs       23.2±0.6μs     0.74  bench_lib.Pad.time_pad((1000,), 3, 'reflect')
-      34.5±0.1μs       25.6±0.2μs     0.74  bench_lib.Pad.time_pad((1000,), 1, 'edge')
-      64.5±0.3μs       47.7±0.6μs     0.74  bench_lib.Pad.time_pad((10, 100), 1, 'constant')
-     34.8±0.09μs       25.7±0.2μs     0.74  bench_lib.Pad.time_pad((1000,), 3, 'edge')
-     45.1±0.06μs       33.0±0.2μs     0.73  bench_lib.Pad.time_pad((10, 100), (0, 5), 'reflect')
-      72.4±0.9μs       53.0±0.4μs     0.73  bench_lib.Pad.time_pad((10, 100), (20, 0), 'minimum')
-      67.0±0.6μs       49.1±0.2μs     0.73  bench_lib.Pad.time_pad((10, 100), 3, 'constant')
-      65.8±0.9μs       48.1±0.5μs     0.73  bench_lib.Pad.time_pad((1000,), (20, 0), 'mean')
-      31.1±0.2μs      22.7±0.08μs     0.73  bench_lib.Pad.time_pad((1000,), 1, 'reflect')
-      73.8±0.6μs       53.7±0.1μs     0.73  bench_lib.Pad.time_pad((1000,), (20, 0), 'linear_ramp')
-      30.5±0.4μs      22.2±0.07μs     0.73  bench_lib.Pad.time_pad((1,), 3, 'edge')
-      73.8±0.7μs       53.6±0.4μs     0.73  bench_lib.Pad.time_pad((10, 100), (20, 0), 'maximum')
-     32.3±0.07μs      23.4±0.05μs     0.72  bench_lib.Pad.time_pad((1,), 3, 'reflect')
-      74.2±0.4μs       53.7±0.1μs     0.72  bench_lib.Pad.time_pad((1000,), (0, 5), 'linear_ramp')
-      59.3±0.6μs       42.8±0.5μs     0.72  bench_lib.Pad.time_pad((1,), (0, 5), 'mean')
-     30.8±0.08μs      22.2±0.08μs     0.72  bench_lib.Pad.time_pad((1,), 1, 'edge')
-      63.8±0.9μs         45.9±1μs     0.72  bench_lib.Pad.time_pad((10, 100), (0, 5), 'minimum')
-      30.6±0.2μs       21.9±0.2μs     0.72  bench_lib.Pad.time_pad((1000,), 3, 'symmetric')
-      64.2±0.7μs       46.0±0.8μs     0.72  bench_lib.Pad.time_pad((10, 100), (0, 5), 'maximum')
-      70.7±0.2μs      50.5±0.09μs     0.71  bench_lib.Pad.time_pad((1,), (20, 0), 'linear_ramp')
-     28.0±0.08μs      20.0±0.06μs     0.71  bench_lib.Pad.time_pad((1000,), 3, 'wrap')
-      30.8±0.2μs      21.9±0.04μs     0.71  bench_lib.Pad.time_pad((1000,), 1, 'symmetric')
-     48.2±0.08μs      34.3±0.06μs     0.71  bench_lib.Pad.time_pad((10, 100), (20, 0), 'edge')
-        60.7±1μs       43.0±0.2μs     0.71  bench_lib.Pad.time_pad((1,), (20, 0), 'mean')
-      45.2±0.4μs       31.9±0.2μs     0.71  bench_lib.Pad.time_pad((10, 100), (0, 5), 'symmetric')
-      28.4±0.2μs       20.1±0.1μs     0.71  bench_lib.Pad.time_pad((1000,), 1, 'wrap')
-      40.7±0.3μs      28.5±0.03μs     0.70  bench_lib.Pad.time_pad((10, 100), (0, 5), 'wrap')
-     28.0±0.03μs      19.4±0.04μs     0.69  bench_lib.Pad.time_pad((1,), 1, 'symmetric')
-     25.2±0.03μs       17.3±0.1μs     0.69  bench_lib.Pad.time_pad((1,), 1, 'wrap')
-     42.5±0.09μs       29.0±0.3μs     0.68  bench_lib.Pad.time_pad((10, 100), (0, 5), 'edge')
-      74.0±0.9μs       49.4±0.1μs     0.67  bench_lib.Pad.time_pad((10, 10, 10), (0, 5), 'constant')
-      34.2±0.3μs      22.5±0.07μs     0.66  bench_lib.Pad.time_pad((1000,), (0, 5), 'reflect')
-      43.6±0.7μs       28.6±0.2μs     0.66  bench_lib.Pad.time_pad((1000,), (0, 5), 'maximum')
-     46.4±0.09μs      30.3±0.05μs     0.65  bench_lib.Pad.time_pad((1000,), 1, 'constant')
-      44.2±0.3μs       28.8±0.2μs     0.65  bench_lib.Pad.time_pad((1000,), (20, 0), 'maximum')
-      46.4±0.1μs      30.2±0.08μs     0.65  bench_lib.Pad.time_pad((1000,), 3, 'constant')
-      33.6±0.1μs         21.8±1μs     0.65  bench_lib.Pad.time_pad((1000,), (20, 0), 'symmetric')
-      43.5±0.7μs      28.2±0.08μs     0.65  bench_lib.Pad.time_pad((1000,), (0, 5), 'minimum')
-      33.7±0.1μs      21.8±0.09μs     0.65  bench_lib.Pad.time_pad((1000,), (0, 5), 'symmetric')
-      34.0±0.1μs       22.0±0.2μs     0.65  bench_lib.Pad.time_pad((1000,), (20, 0), 'reflect')
-      44.2±0.7μs       28.5±0.2μs     0.64  bench_lib.Pad.time_pad((1000,), (20, 0), 'minimum')
-      42.6±0.2μs      27.2±0.07μs     0.64  bench_lib.Pad.time_pad((1,), 1, 'constant')
-     31.2±0.06μs       19.8±0.1μs     0.64  bench_lib.Pad.time_pad((1000,), (20, 0), 'wrap')
-      31.3±0.3μs      19.9±0.05μs     0.63  bench_lib.Pad.time_pad((1000,), (0, 5), 'wrap')
-      42.6±0.3μs       26.9±0.2μs     0.63  bench_lib.Pad.time_pad((1,), 3, 'constant')
-      60.1±0.5μs      38.0±0.08μs     0.63  bench_lib.Pad.time_pad((10, 100), (20, 0), 'constant')
-      31.8±0.1μs       19.8±0.7μs     0.62  bench_lib.Pad.time_pad((1000,), (0, 5), 'edge')
-      31.3±0.3μs       19.4±0.3μs     0.62  bench_lib.Pad.time_pad((1000,), (20, 0), 'edge')
-      37.0±0.2μs      22.7±0.05μs     0.61  bench_lib.Pad.time_pad((1,), (0, 5), 'minimum')
-      30.2±0.2μs      18.4±0.07μs     0.61  bench_lib.Pad.time_pad((1,), (20, 0), 'reflect')
-      36.9±0.1μs       22.5±0.1μs     0.61  bench_lib.Pad.time_pad((1,), (0, 5), 'maximum')
-      55.8±0.2μs       33.8±0.1μs     0.61  bench_lib.Pad.time_pad((10, 100), (0, 5), 'constant')
-      37.6±0.2μs      22.8±0.09μs     0.61  bench_lib.Pad.time_pad((1,), (20, 0), 'maximum')
-      28.5±0.2μs      17.2±0.06μs     0.60  bench_lib.Pad.time_pad((1,), (20, 0), 'edge')
-      30.5±0.1μs      18.4±0.06μs     0.60  bench_lib.Pad.time_pad((1,), (0, 5), 'reflect')
-     28.7±0.04μs      17.1±0.05μs     0.60  bench_lib.Pad.time_pad((1,), (0, 5), 'edge')
-      37.9±0.2μs       22.5±0.1μs     0.59  bench_lib.Pad.time_pad((1,), (20, 0), 'minimum')
-     42.9±0.08μs         25.1±1μs     0.58  bench_lib.Pad.time_pad((1000,), (0, 5), 'constant')
-      43.4±0.2μs       23.7±0.1μs     0.55  bench_lib.Pad.time_pad((1000,), (20, 0), 'constant')
-      39.8±0.1μs       20.9±0.1μs     0.53  bench_lib.Pad.time_pad((1,), (20, 0), 'constant')
-     40.4±0.08μs       21.2±0.1μs     0.52  bench_lib.Pad.time_pad((1,), (0, 5), 'constant')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.

mattip · 2018-11-15T18:42:52Z

@mhvk you also looked at this, any last comments?

Also improve description of the rounding behavior when using `as_index=True`.

mhvk · 2018-11-15T20:11:15Z

No, no last comments. Thanks, @lagru!

mattip · 2018-11-15T22:24:01Z

Thanks @lagru and the reviewers

pllim · 2019-01-16T14:30:49Z

What Numpy version will this patch be in? 1.16.0? This broke something in scikit-image/scikit-image#3551 and affects Astropy testing, so I am trying to figure out what is compatible with what in the test matrix. Thanks.

seberg · 2019-01-16T15:14:45Z

Yes, seems to be in 1.16.0 can you open an issue?

lagru · 2019-01-16T15:14:50Z

@pllim Yes. This patch was introduced between 1.15.4 and 1.16.0rc1.

pllim · 2019-01-16T15:30:40Z

can you open an issue?

I am not sure what issue to open. Seems to be between scikit-image and numpy. And the former already merged a patch to handle this. It is just that now scikit-image < 0.14.2 cannot use a newer version of Numpy.

seberg · 2019-01-16T15:49:14Z

@pllim OK, nevermind. If you scikit-image fixed it, I think lets just not worry about it. This was usage of a private internal function, so removing is not really a bug or anything.

That said, if this was a real issue, we can work around it of course...

pllim · 2019-01-16T15:54:32Z

@seberg , just to clarify, I am not involved with scikit-image dev, I am just a user in this case. Yes, I agree that it is not a bug.

eric-wieser reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated

Copy link
Copy Markdown

Member

eric-wieser Sep 16, 2018

Choose a reason for hiding this comment

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

I think these lines are the important part of the patch

eric-wieser reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

lagru force-pushed the pad-as-pairs branch from f82d8d6 to 3a6182a Compare September 16, 2018 19:43

mhvk reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Sep 16, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

charris added component: numpy.lib 03 - Maintenance labels Sep 17, 2018

eric-wieser reviewed Sep 24, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Sep 24, 2018

View reviewed changes

MAINT: Rewrite shape normalization in pad function

b7e8560

This rewrite joins the previous functions _normalize_shape and _validate_length as an intro to larger changes suggested in numpygh-11126.

lagru force-pushed the pad-as-pairs branch from 923ce12 to 9cc9ee1 Compare September 24, 2018 09:26

TST: Add new tests for _as_pairs

59be3b8

Test coverage of _as_pairs is 100%.

lagru force-pushed the pad-as-pairs branch from 9cc9ee1 to 59be3b8 Compare September 24, 2018 10:03

eric-wieser reviewed Oct 12, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Oct 12, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

eric-wieser reviewed Oct 12, 2018

View reviewed changes

numpy/lib/arraypad.py Outdated Show resolved Hide resolved

MAINT: Improve logic, format and comments

a060838

Addresses reviews: - The test "x.ndim < 3" now covers both optimizations as it should. - Extend comments and improve formatting as well.

lagru force-pushed the pad-as-pairs branch from ca6d234 to a060838 Compare October 27, 2018 12:54

eric-wieser reviewed Oct 30, 2018

View reviewed changes

numpy/lib/tests/test_arraypad.py Outdated Show resolved Hide resolved

eric-wieser approved these changes Oct 30, 2018

View reviewed changes

TST: Improve _as_pairs test for two values

c318afb

Also improve description of the rounding behavior when using `as_index=True`.

lagru force-pushed the pad-as-pairs branch from 6556e98 to c318afb Compare November 15, 2018 19:33

mattip merged commit a4b96ad into numpy:master Nov 15, 2018

pllim mentioned this pull request Nov 16, 2018

_validate_lengths replaced in numpy 1.16 leading to ImportError exception in scikit-image less than 0.14.2 scikit-image/scikit-image#3551

Closed

5 tasks

liwt31 pushed a commit to liwt31/numpy that referenced this pull request Nov 19, 2018

MAINT: Rewrite shape normalization in pad function (numpy#11966)

c7d3964

lagru mentioned this pull request Nov 22, 2018

MAINT: Rewrite numpy.pad without concatenate #11358

Merged

5 tasks

lagru deleted the pad-as-pairs branch November 23, 2018 16:47

abhiTronix mentioned this pull request Dec 11, 2018

Bug: Scikit-image 0.14.1 broken with numpy 1.16 scikit-image/scikit-image#3586

Closed

lagru mentioned this pull request Dec 19, 2018

add a fast path for constant padding lagru/numpy#3

Closed

Uh oh!

Conversation

lagru commented Sep 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eric-wieser Sep 16, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eric-wieser commented Sep 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

lagru commented Sep 17, 2018 • edited by eric-wieser Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-wieser commented Sep 17, 2018

Uh oh!

Uh oh!

eric-wieser Sep 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

lagru Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

lagru Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

eric-wieser Oct 28, 2018

Choose a reason for hiding this comment

Uh oh!

lagru Nov 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lagru Nov 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lagru commented Sep 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgommers commented Sep 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eric-wieser left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lagru commented Nov 3, 2018

Uh oh!

mattip commented Nov 15, 2018

Uh oh!

mhvk commented Nov 15, 2018

Uh oh!

mattip commented Nov 15, 2018

Uh oh!

pllim commented Jan 16, 2019

Uh oh!

seberg commented Jan 16, 2019

Uh oh!

lagru commented Jan 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Jan 16, 2019

Uh oh!

seberg commented Jan 16, 2019

Uh oh!

lagru commented Sep 16, 2018 •

edited

Loading

eric-wieser commented Sep 16, 2018 •

edited

Loading

lagru commented Sep 17, 2018 •

edited by eric-wieser

Loading

eric-wieser Sep 24, 2018 •

edited

Loading

lagru Nov 3, 2018 •

edited

Loading

lagru Nov 15, 2018 •

edited

Loading

lagru commented Sep 24, 2018 •

edited

Loading

rgommers commented Sep 25, 2018 •

edited

Loading

eric-wieser left a comment •

edited

Loading

lagru commented Jan 16, 2019 •

edited

Loading