-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ASV: add benchmarks for groupby cython aggregations #39846
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
Conversation
And the output of a run of those benchmarks (to give an idea of timings):
Only |
Do you see any way to refactor GroupByMethods to make it more useful? The benchmarks in this file take a very long time to run as is (in real time) so wondering if we can streamline them |
The issues with fitting it into GroupByMethods: it's also benchmarking different dtypes of the key column, which I am not interested in here. It also benchmarks more methods than just the cython aggregations. It also has a different number of groups / size of groups trade-off, and it is generally useful to have a bit varying benchmarks on this aspect. To be clear, I agree that we should be careful about adding benchmarks as they already take a long time (that's also the reason that for now I didn't add float32, because I don't think it would add much value in addition to float64, as there is already a specific benchmark for float32 for a single case) |
What might be more useful to trim down the time to run the groupby benchmarks is to look a bit more into |
To be more specific, this is the quick
So most are small benchmarks, but eg the |
+1 |
thanks @jorisvandenbossche (happy to have a followup for fixing those identified slow benchmarks). maybe even create an issue to generally reduce time on longish benchmarks (as a good first issue). |
I noticed that we currently don't really have a groupby benchmark that specifically targets the cython aggregations. We have of course benchmarks that call those, but eg
GroupByMethods
is setup in such a way that it's mostly benchmarking the factorization and post-processing (also useful of course), while eg for sum only 7% is spent in the actualgroupby_add
algorithm.So therefore this PR is adding an additional benchmark more targetted at the cython aggregation (where 30-50% of the time is spent in the actual aggregation function, so we will more easily catch regressions/improvements there)
For now I only added "float64". I could also add "float32", but not sure how useful that is (since they are all using fused types, it's the same implementation for both dtypes.