-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REGR: Avoid overflow with groupby sum #48059
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
I would be okay with this version of the change |
Do you have a rough estimate of how much slower? I certainly think this version is good enough (in the end, before we were casting as well, to float64), but if it is a significant difference, we can still table it as a future possible improvement if we find a better way to avoid the many compilation combinations |
Around 5% of the Memory footprint is something that would make it worth, if we can avoid the duplications. But for now, this should be sufficient since we were castint to float anyway |
Will open an issue, after this is merged. |
Thanks joris If ok, I would merge when ci is greenish |
Go ahead! |
Failures unrelated |
* REGR: Avoid overflow with groupby sum * Add comment
* REGR: Avoid overflow with groupby sum * Add comment
This is the alternative to #48044.
I would very much prefer this one, even though it is a bit slower.
cc @mroeschke cc @jorisvandenbossche