-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: Use Kahan summation to calculate groupby.sum() #38903
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
We dispatch to numpy in case of 32bit systems, which returns the wrong result |
doc/source/whatsnew/v1.3.0.rst
Outdated
@@ -51,6 +51,7 @@ Other enhancements | |||
- :func:`pandas.read_sql_query` now accepts a ``dtype`` argument to cast the columnar data from the SQL database based on user input (:issue:`10285`) | |||
- Improved integer type mapping from pandas to SQLAlchemy when using :meth:`DataFrame.to_sql` (:issue:`35076`) | |||
- :func:`to_numeric` now supports downcasting of nullable ``ExtensionDtype`` objects (:issue:`33013`) | |||
- Improve numerical stability for :meth:`DataFrameGroupBy.sum()` and :meth:`SeriesGroupBy.sum()` through using Kahan summation (:issue:`38778`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no brackets after ...GroupBy.sum
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments, ping on green.
note that we have been seeing some recent work on combing the aggregation functions for 1d and groupby, this would be a great case here. (slightly complicated because for 1d we actually dispatch to bottleneck if installed), otherwise we use the np.nan*
functions so have to see if this is worth it).
doc/source/whatsnew/v1.3.0.rst
Outdated
@@ -51,6 +51,7 @@ Other enhancements | |||
- :func:`pandas.read_sql_query` now accepts a ``dtype`` argument to cast the columnar data from the SQL database based on user input (:issue:`10285`) | |||
- Improved integer type mapping from pandas to SQLAlchemy when using :meth:`DataFrame.to_sql` (:issue:`35076`) | |||
- :func:`to_numeric` now supports downcasting of nullable ``ExtensionDtype`` objects (:issue:`33013`) | |||
- Improve numerical stability for :meth:`DataFrameGroupBy.sum` and :meth:`SeriesGroupBy.sum` through using Kahan summation (:issue:`38778`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to group bug fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved
@jreback green |
thanks @phofl |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
This simplifies the op example.
I think
is dispatched to numpy? The result is wrong too.
100.0
Mean doesn't work either. I will look through the functions to determine which need Kahan summation too and open an issue to track these.