Performance increase rolling min max #19549

hexgnu · 2018-02-06T15:15:44Z

closes PERF: improve perf of variable window rolling_min/max #19521
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

In my testing the performance of

import pandas as pd
import timeit

df = pd.DataFrame({"a": 0}, index=pd.date_range('2017-01-01', '2019-01-01', freq='1T'))

timeit.timeit(lambda: df.rolling('1d').max(), number=1)

Went from 1.8 sec to 0.3 sec on my machine (lenovo laptop).

pep8speaks · 2018-02-06T15:15:56Z

Hello @hexgnu! Thanks for updating the PR.

In the file asv_bench/benchmarks/rolling.py, following are the PEP8 issues :

Line 25:1: E302 expected 2 blank lines, found 1

Comment last updated on February 12, 2018 at 04:38 Hours UTC

This reverts commit 08ff553.

jreback · 2018-02-06T23:31:08Z

pls add a whatsnew, and I think we have asv's for this, so run and post those as well (of course need passing too :>)

approach looks good.

codecov · 2018-02-07T09:07:03Z

Codecov Report

Merging #19549 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #19549      +/-   ##
==========================================
+ Coverage   91.58%   91.59%   +<.01%     
==========================================
  Files         150      150              
  Lines       48807    48807              
==========================================
+ Hits        44702    44704       +2     
+ Misses       4105     4103       -2

Flag	Coverage Δ
#multiple	`89.96% <ø> (ø)`	⬆️
#single	`41.73% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/util/testing.py	`83.85% <0%> (+0.2%)`	⬆️

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 7a5634e...65c0dbe. Read the comment docs.

jreback · 2018-02-07T11:30:09Z

pandas/_libs/src/headers/math.h

@@ -1,11 +0,0 @@
-#ifndef _PANDAS_MATH_H_


I wouldn't remove this. otherwise windows builds will fail.

I see you are trying to fix this using the cpp library below. not sure how this will work out. windows is a bit funky on its incudes.

yea I think I'm going to revert it back to math.h even though it's a tad wonky, it was actually working quite well that way.

oh ugh I just realized I didn't checkin src/headers/cmath which is why this whole thing is failing.

…ase_rolling_min_max

hexgnu · 2018-02-08T10:35:44Z

Ok pretty sure this will pass this time since I went through the trouble of setting up windows and testing it on MSVC 9.0.

jreback

lgtm. just a little docs. ping on green.

jreback · 2018-02-08T11:22:29Z

pandas/_libs/window.pyx

@@ -1242,32 +1244,43 @@ cdef _roll_min_max(ndarray[numeric] input, int64_t win, int64_t minp,

    output = np.empty(N, dtype=input.dtype)



can you add a description of the algorithm and a link (if available)

jreback · 2018-02-08T11:23:36Z

pandas/_libs/src/headers/cmath

@@ -0,0 +1,13 @@
+#ifndef _PANDAS_MATH_H_
+#define _PANDAS_MATH_H_
+


can you add commets here on why we have this file (so the next person doesn't go thru the same as you :>)

chris-b1 · 2018-02-08T21:39:41Z

I'm not sure it matters in practice with the toolchains that actually get used but historically we haven't used c++ at all inside pandas - maybe has implications adding it?

chris-b1 · 2018-02-08T21:46:41Z

pandas/_libs/src/headers/cmath

+#if defined(_MSC_VER) && (_MSC_VER < 1800)
+#include <cmath>
+namespace std {
+  __inline int signbit(double num) { return _copysign(1.0, num) < 0; }


I would revert this to use math.h - technically extending namespace std is undefined behavior so you'd need to do it a different way

I don't think that's the right direction to go, cmath is what you use when you write C++ code. math.h doesn't work either when compiling in clang. This works in all three compilers even though it's not the most ideal (MSVC doesn't have this defined for older versions which is annoying).

hexgnu · 2018-02-09T00:12:45Z

Here are the ASVs I ran. But I'm noticing 1 an increase in time_pairwise and also idk if there's a variable window rolling benchmark. So I'll look into that.

       before           after         ratio
     [36f90528]       [42f8fdfd]
+      5.86±0.2ms       7.29±0.6ms     1.24  rolling.Pairwise.time_pairwise(10, 'corr', False)
-     2.30±0.08ms      2.08±0.03ms     0.91  rolling.Methods.time_rolling('Series', 1000, 'float', 'mean')
-        38.7±2ms       33.4±0.6ms     0.86  rolling.Methods.time_rolling('DataFrame', 10, 'float', 'median')
-      3.56±0.3ms      2.88±0.03ms     0.81  rolling.Quantile.time_quantile('Series', 10, 'float', 1)
-      3.46±0.2ms       2.80±0.2ms     0.81  rolling.Quantile.time_quantile('DataFrame', 1000, 'float', 0)
-      3.79±0.2ms      2.95±0.07ms     0.78  rolling.Methods.time_rolling('Series', 10, 'float', 'min')
-      2.61±0.2ms      1.98±0.03ms     0.76  rolling.Methods.time_rolling('Series', 10, 'int', 'sum')
-        73.3±5ms         55.7±1ms     0.76  rolling.Methods.time_rolling('DataFrame', 1000, 'int', 'median')
-      3.96±0.2ms      2.77±0.07ms     0.70  rolling.Quantile.time_quantile('Series', 1000, 'float', 0)
-      4.69±0.6ms       3.07±0.1ms     0.65  rolling.Methods.time_rolling('Series', 10, 'float', 'std')
-      9.06±0.3ms       5.49±0.2ms     0.61  rolling.Methods.time_rolling('Series', 10, 'int', 'count')
-      3.63±0.2ms      2.13±0.08ms     0.59  rolling.Methods.time_rolling('Series', 10, 'float', 'mean')
-        9.37±1ms       5.42±0.4ms     0.58  rolling.Methods.time_rolling('Series', 10, 'float', 'count')
-      3.96±0.4ms      2.27±0.08ms     0.57  rolling.Methods.time_rolling('Series', 10, 'int', 'kurt')
-     3.55±0.08ms      2.00±0.03ms     0.56  rolling.Methods.time_rolling('Series', 10, 'float', 'sum')
-      4.22±0.2ms       2.17±0.3ms     0.51  rolling.Methods.time_rolling('Series', 10, 'int', 'skew')
-      5.75±0.4ms      2.89±0.03ms     0.50  rolling.Methods.time_rolling('Series', 10, 'float', 'max')
-        4.27±1ms      2.11±0.04ms     0.49  rolling.Methods.time_rolling('Series', 1000, 'int', 'min')

jreback · 2018-02-09T00:18:00Z

we have other c++ code so this is not a big deal

hexgnu · 2018-02-09T00:19:17Z

@chris-b1 also there is C++ code in the repo look at msgpack

hexgnu · 2018-02-09T06:52:48Z

So in reading this most of the benchmarks for variable rolling windows has gone down but some haven't. How accurate is ASV? Or is it perhaps me doing something else impacting it, cause I was doing some work in a jupyter notebook.

      before           after         ratio
     [36f90528]       [060dfb77]
+      4.78±0.1ms       10.5±0.8ms     2.19  rolling.VariableWindowMethods.time_rolling('Series', '50s', 'float', 'min')
+      3.03±0.2ms       5.99±0.7ms     1.98  rolling.Methods.time_rolling('DataFrame', 1000, 'int', 'min')
+      3.66±0.1ms       7.09±0.6ms     1.94  rolling.Quantile.time_quantile('Series', 10, 'float', 1)
+      2.26±0.1ms       4.34±0.7ms     1.92  rolling.Methods.time_rolling('Series', 10, 'int', 'sum')
+         129±3ms          159±5ms     1.24  rolling.VariableWindowMethods.time_rolling('DataFrame', '1d', 'float', 'median')
+     4.74±0.07ms       5.61±0.3ms     1.18  rolling.VariableWindowMethods.time_rolling('DataFrame', '1d', 'int', 'kurt')
-        395±50ms          340±4ms     0.86  rolling.Quantile.time_quantile('DataFrame', 1000, 'float', 0.5)
-      5.76±0.1ms       4.90±0.3ms     0.85  rolling.VariableWindowMethods.time_rolling('DataFrame', '1h', 'float', 'std')
-      6.17±0.2ms      5.10±0.06ms     0.83  rolling.VariableWindowMethods.time_rolling('Series', '50s', 'int', 'skew')
-      4.20±0.2ms      3.47±0.07ms     0.83  rolling.VariableWindowMethods.time_rolling('DataFrame', '50s', 'int', 'sum')
-      23.9±0.9ms       19.2±0.8ms     0.80  rolling.Pairwise.time_pairwise(1000, 'cov', True)
-      4.08±0.4ms      3.20±0.05ms     0.78  rolling.Quantile.time_quantile('Series', 1000, 'float', 1)
-        6.22±1ms       4.72±0.1ms     0.76  rolling.VariableWindowMethods.time_rolling('DataFrame', '50s', 'int', 'kurt')
-        4.46±1ms      3.21±0.08ms     0.72  rolling.Methods.time_rolling('Series', 1000, 'float', 'kurt')
-      4.12±0.3ms       2.91±0.2ms     0.71  rolling.Quantile.time_quantile('DataFrame', 1000, 'float', 1)
-        223±20ms          152±9ms     0.68  rolling.Quantile.time_quantile('Series', 10, 'float', 0.5)
-      6.47±0.7ms       4.42±0.2ms     0.68  rolling.VariableWindowMethods.time_rolling('DataFrame', '1h', 'float', 'skew')
-        4.90±1ms       3.28±0.2ms     0.67  rolling.Methods.time_rolling('DataFrame', 10, 'int', 'count')
-        11.2±1ms         7.40±1ms     0.66  rolling.Pairwise.time_pairwise(None, 'corr', False)
-        7.08±2ms       4.47±0.2ms     0.63  rolling.VariableWindowMethods.time_rolling('DataFrame', '50s', 'float', 'skew')
-      5.83±0.7ms       3.66±0.3ms     0.63  rolling.VariableWindowMethods.time_rolling('DataFrame', '50s', 'float', 'mean')
-      4.41±0.4ms       2.71±0.3ms     0.61  rolling.Quantile.time_quantile('Series', 10, 'int', 0)
-        5.22±1ms      3.16±0.07ms     0.60  rolling.Methods.time_rolling('Series', 1000, 'float', 'skew')
-        8.02±1ms       4.57±0.2ms     0.57  rolling.Pairwise.time_pairwise(1000, 'cov', False)
-      5.74±0.5ms      3.13±0.07ms     0.55  rolling.Methods.time_rolling('Series', 1000, 'float', 'max')
-      3.73±0.4ms       1.92±0.2ms     0.52  rolling.Methods.time_rolling('DataFrame', 1000, 'float', 'sum')
-      3.76±0.3ms       1.88±0.2ms     0.50  rolling.Quantile.time_quantile('DataFrame', 1000, 'int', 1)
-      4.55±0.5ms      2.25±0.06ms     0.49  rolling.Quantile.time_quantile('Series', 1000, 'int', 1)
-      4.62±0.6ms      2.27±0.01ms     0.49  rolling.Methods.time_rolling('Series', 1000, 'float', 'mean')
-      7.25±0.5ms       3.48±0.2ms     0.48  rolling.VariableWindowMethods.time_rolling('DataFrame', '50s', 'float', 'count')
-      6.37±0.3ms       2.84±0.2ms     0.45  rolling.Quantile.time_quantile('DataFrame', 1000, 'float', 0)
-        325±20ms         138±10ms     0.43  rolling.VariableWindowMethods.time_rolling('Series', '1d', 'float', 'median')
-        821±20ms          330±5ms     0.40  rolling.Quantile.time_quantile('Series', 1000, 'float', 0.5)
-        98.9±5ms         6.45±1ms     0.07  rolling.VariableWindowMethods.time_rolling('Series', '1h', 'int', 'max')
-         102±3ms       6.56±0.4ms     0.06  rolling.VariableWindowMethods.time_rolling('Series', '1h', 'int', 'min')
-         107±5ms       5.99±0.4ms     0.06  rolling.VariableWindowMethods.time_rolling('Series', '1h', 'float', 'max')
-         117±2ms       6.44±0.6ms     0.06  rolling.VariableWindowMethods.time_rolling('Series', '1h', 'float', 'min')
-         104±6ms       5.11±0.2ms     0.05  rolling.VariableWindowMethods.time_rolling('DataFrame', '1h', 'int', 'max')
-         106±2ms       5.09±0.1ms     0.05  rolling.VariableWindowMethods.time_rolling('DataFrame', '1h', 'float', 'max')
-       117±0.6ms       5.04±0.4ms     0.04  rolling.VariableWindowMethods.time_rolling('DataFrame', '1h', 'float', 'min')
-         126±5ms       5.35±0.1ms     0.04  rolling.VariableWindowMethods.time_rolling('DataFrame', '1h', 'int', 'min')
-           2.26s       7.99±0.8ms     0.00  rolling.VariableWindowMethods.time_rolling('DataFrame', '1d', 'int', 'max')
-           2.29s         6.95±1ms     0.00  rolling.VariableWindowMethods.time_rolling('Series', '1d', 'float', 'max')
-           2.90s         8.39±2ms     0.00  rolling.VariableWindowMethods.time_rolling('DataFrame', '1d', 'int', 'min')
-           2.09s       5.58±0.3ms     0.00  rolling.VariableWindowMethods.time_rolling('DataFrame', '1d', 'float', 'min')
-           2.52s         6.63±1ms     0.00  rolling.VariableWindowMethods.time_rolling('Series', '1d', 'int', 'max')
-           2.41s         6.09±1ms     0.00  rolling.VariableWindowMethods.time_rolling('Series', '1d', 'int', 'min')
-           2.45s       5.60±0.2ms     0.00  rolling.VariableWindowMethods.time_rolling('Series', '1d', 'float', 'min')
-           2.61s       5.63±0.1ms     0.00  rolling.VariableWindowMethods.time_rolling('DataFrame', '1d', 'float', 'max')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.

jreback · 2018-02-10T16:27:00Z

can you add a whatsnew note in perf. lgtm otherwise.

I suspect that a very short time window has a small perf regression here, but we win really big for bigger windows. So this is a nice tradeoff (in theory we could use the original method for short windows but that adds a lot to code complexity). You could add this in the code as a comment.

…ase_rolling_min_max

jreback · 2018-02-12T11:35:20Z

i restarted. ping on green.

jreback · 2018-02-14T11:13:24Z

thanks @hexgnu nice patch!

…21704) User reported that `df.rolling(to_offset('3D'), closed='left').max()` segfaults when df has a datetime index. The bug was in PR pandas-dev#19549. In that PR, in https://github.com/pandas-dev/pandas/blame/master/pandas/_libs/window.pyx#L1268 `i` is initialized to `endi[0]`, which is 0 when `closed=left`. So in the next line when it tries to set `output[i-1]` it goes out of bounds. In addition, there are 2 more bugs in the `roll_min_max` code. The second bug is that for variable size windows, the `nobs` is never updated when elements leave the window. The third bug is at the end of the fixed window where all output elements up to `minp` are initialized to 0 if the input is not float. This PR fixes all three of the aforementioned bugs, at the cost of casting the output array to floating point even if the input is integer. This is less than ideal if the output has no NaNs, but is still consistent with roll_sum behavior.

…21704) User reported that `df.rolling(to_offset('3D'), closed='left').max()` segfaults when df has a datetime index. The bug was in PR pandas-dev#19549. In that PR, in https://github.com/pandas-dev/pandas/blame/master/pandas/_libs/window.pyx#L1268 `i` is initialized to `endi[0]`, which is 0 when `closed=left`. So in the next line when it tries to set `output[i-1]` it goes out of bounds. In addition, there are 2 more bugs in the `roll_min_max` code. The second bug is that for variable size windows, the `nobs` is never updated when elements leave the window. The third bug is at the end of the fixed window where all output elements up to `minp` are initialized to 0 if the input is not float. This PR fixes these three bugs, at the cost of casting the output array to floating point even if the input is integer. This is less than ideal if the output has no NaNs, but is consistent with roll_sum behavior.

hexgnu added 2 commits February 6, 2018 21:43

First stab at using deque over full iterations

737c033

Working deque implementation of min/max

15d2563

hexgnu added 4 commits February 6, 2018 22:17

oops

06f2658

Remove some extraneous variables

8089e67

Get rid of some of the branches in the code

08ff553

Revert "Get rid of some of the branches in the code"

6ef87b2

This reverts commit 08ff553.

jreback added Performance Memory or execution speed performance Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels Feb 6, 2018

hexgnu added 5 commits February 7, 2018 10:23

Prefer cmath over math.h for cpp

6e8c041

Change to std namespace

b0a0ef6

Fix issue with variable window size

b19774e

Oh right variable window size ;)

92857ee

Use std namespace for windows compilation

832ff9d

jreback reviewed Feb 7, 2018

View reviewed changes

hexgnu added 5 commits February 7, 2018 18:49

Add cmath so build will complete

f00e994

Merge remote-tracking branch 'upstream/master' into performance_incre…

0d713be

…ase_rolling_min_max

Fix linting error in window.pyx

1ab4e21

I think this will fix MSVC build

d5b60cd

I think this is what I want to do

42f8fdf

jreback added this to the 0.23.0 milestone Feb 8, 2018

jreback requested changes Feb 8, 2018

View reviewed changes

jreback reviewed Feb 8, 2018

View reviewed changes

chris-b1 reviewed Feb 8, 2018

View reviewed changes

hexgnu added 4 commits February 9, 2018 10:31

Add documentation

38e3f70

Add another benchmark to test variable window methods

7f4abf9

Ugh use // not #

23fe816

New better benchmark

060dfb7

jreback approved these changes Feb 10, 2018

View reviewed changes

hexgnu added 2 commits February 12, 2018 10:21

Add whatsnew entry

aeb9b9b

Merge remote-tracking branch 'upstream/master' into performance_incre…

65c0dbe

…ase_rolling_min_max

jreback merged commit 39e7b69 into pandas-dev:master Feb 14, 2018

harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018

Performance increase rolling min max (pandas-dev#19549)

93bfede

chris-b1 mentioned this pull request May 23, 2018

Pandas 0.23.0 gives ImportError: DLL load failed #21106

Closed

changhiskhan mentioned this pull request Jul 11, 2018

BUG: datetime rolling min/max segfaults when closed=left (#21704) #21853

Merged

4 tasks

WillAyd mentioned this pull request Nov 28, 2018

build failure with Xcode 10 - libstdc++ not supported anymore #23424

Closed

ghost mentioned this pull request Jul 26, 2019

CLN: refactor roll_generic to call roll_generic_with_indexer #27523

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance increase rolling min max #19549

Performance increase rolling min max #19549

hexgnu commented Feb 6, 2018 •

edited

Loading

pep8speaks commented Feb 6, 2018 •

edited

Loading

jreback commented Feb 6, 2018

codecov bot commented Feb 7, 2018 •

edited

Loading

jreback Feb 7, 2018

jreback Feb 7, 2018

hexgnu Feb 7, 2018

hexgnu Feb 7, 2018

hexgnu commented Feb 8, 2018

jreback left a comment

jreback Feb 8, 2018

jreback Feb 8, 2018

chris-b1 commented Feb 8, 2018

chris-b1 Feb 8, 2018

hexgnu Feb 9, 2018

hexgnu commented Feb 9, 2018 •

edited

Loading

jreback commented Feb 9, 2018

hexgnu commented Feb 9, 2018

hexgnu commented Feb 9, 2018

jreback commented Feb 10, 2018

jreback commented Feb 12, 2018

jreback commented Feb 14, 2018

		@@ -1242,32 +1244,43 @@ cdef _roll_min_max(ndarray[numeric] input, int64_t win, int64_t minp,

		output = np.empty(N, dtype=input.dtype)

		@@ -0,0 +1,13 @@
		#ifndef _PANDAS_MATH_H_
		#define _PANDAS_MATH_H_

Performance increase rolling min max #19549

Performance increase rolling min max #19549

Conversation

hexgnu commented Feb 6, 2018 • edited Loading

pep8speaks commented Feb 6, 2018 • edited Loading

Comment last updated on February 12, 2018 at 04:38 Hours UTC

jreback commented Feb 6, 2018

codecov bot commented Feb 7, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexgnu commented Feb 8, 2018

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-b1 commented Feb 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexgnu commented Feb 9, 2018 • edited Loading

jreback commented Feb 9, 2018

hexgnu commented Feb 9, 2018

hexgnu commented Feb 9, 2018

jreback commented Feb 10, 2018

jreback commented Feb 12, 2018

jreback commented Feb 14, 2018

hexgnu commented Feb 6, 2018 •

edited

Loading

pep8speaks commented Feb 6, 2018 •

edited

Loading

codecov bot commented Feb 7, 2018 •

edited

Loading

hexgnu commented Feb 9, 2018 •

edited

Loading