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.

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

PERF: Rolling/Expanding.cov/corr #39591

Merged
merged 22 commits into from
Feb 5, 2021

Conversation

mroeschke
Copy link
Member

@mroeschke mroeschke commented Feb 4, 2021

  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

Also I think I stumbled on some bugs along the way; added whatsnew and modified the existing tests where necessary.

$ asv_bench % asv continuous -f 1.1 upstream/master HEAD -b rolling.Pairwise.time_groupby

       before           after         ratio
     [f79468b8]       [6a1e8592]
     <master>         <ref/groupby_rolling_pairwise>
-        299±50ms         178±20ms     0.59  rolling.Pairwise.time_groupby(1000, 'corr', True)
-         272±7ms         157±20ms     0.58  rolling.Pairwise.time_groupby(1000, 'cov', True)
-         262±1ms          146±4ms     0.56  rolling.Pairwise.time_groupby(None, 'cov', True)
-        303±10ms          158±6ms     0.52  rolling.Pairwise.time_groupby(10, 'cov', True)
-        279±20ms        139±0.7ms     0.50  rolling.Pairwise.time_groupby(None, 'corr', True)
-        315±10ms         149±10ms     0.47  rolling.Pairwise.time_groupby(10, 'corr', True)
-         137±2ms       34.0±0.8ms     0.25  rolling.Pairwise.time_groupby(10, 'cov', False)
-         127±5ms         28.2±1ms     0.22  rolling.Pairwise.time_groupby(None, 'cov', False)
-         134±5ms       28.1±0.4ms     0.21  rolling.Pairwise.time_groupby(1000, 'cov', False)
-         135±2ms         27.7±1ms     0.21  rolling.Pairwise.time_groupby(None, 'corr', False)
-        178±10ms         33.7±2ms     0.19  rolling.Pairwise.time_groupby(10, 'corr', False)
-        159±20ms         29.9±2ms     0.19  rolling.Pairwise.time_groupby(1000, 'corr', False)

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE INCREASED.

@mroeschke mroeschke added this to the 1.3 milestone Feb 4, 2021
@mroeschke mroeschke added Performance Memory or execution speed performance Window rolling, ewma, expanding labels Feb 4, 2021
@@ -591,6 +599,79 @@ def _apply(
result.index = result_index
return result

def _apply_pairwise(self, target, other, pairwise, func):
Copy link
Contributor

Choose a reason for hiding this comment

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

wow. can you type the input / output args and add a doc-string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done (a testament to how complex groupby apply is)

@jreback jreback merged commit b2671cc into pandas-dev:master Feb 5, 2021
@jreback
Copy link
Contributor

jreback commented Feb 5, 2021

thanks @mroeschke

@mroeschke mroeschke deleted the ref/groupby_rolling_pairwise branch February 5, 2021 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Memory or execution speed performance Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants