Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-121149: improve accuracy of builtin sum() for complex inputs#121176
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Not much to say on that PR except some style comments.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
item=PyIter_Next(iter);
has an overhead (which can be non trivial in certain cases) for each iteration compared to more direct
iternext=*Py_TYPE(it)->tp_iternext;item=iternext(it);
and StopIteration handler in the end
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
That's out of the scope for this pr. |
Uh oh!
There was an error while loading.Please reload this page.
Overall this patch looks basically sound. If you're open to change, I wish it was in more of a functional style than a mutate in place style. I find the latter harder to read and harder to debug. Also the name I suggest something like this: It could be used very plainly and readably, making it obvious what is changing: I'm not attached to the names only the core concept. Use |
Thanks, I like new names.
That was chosen for compatibility with the referenced wiki page, but the later variant does make sense too. And it's already used in the fsum(). 2698be6 - renaming part
Does make sense. I did this change in the second commit,5242bd6 (with a minor change: don't introduce temporary As we inline all helpers - this variant should have same performance as in the commit above. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
* Use compensated summation for complex sums with floating-point items. This amendspython#121176.* sum() specializations for floats and complexes now use PyLong_AsDouble() instead of PyLong_AsLongAndOverflow() and compensated summation as well.
Uh oh!
There was an error while loading.Please reload this page.
📚 Documentation preview 📚:https://cpython-previews--121176.org.readthedocs.build/