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.

Sign up for GitHub

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

Jump to bottom

gh-122234: fix accuracy issues for sum() #122236

Merged
merged 4 commits into from
Jul 29, 2024
Merged

gh-122234: fix accuracy issues for sum() #122236

merged 4 commits into from
Jul 29, 2024
+22 −11

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Jul 24, 2024 •
edited by bedevere-app bot
Loading

@skirpichev
pythongh-122234: fix accuracy issues for sum()
a458432 
* Use compensated summation for complex sums with floating-point items.
  This amends python#121176.

* sum() specializations for floats and complexes now use
  PyLong_AsDouble() instead of PyLong_AsLongAndOverflow() and
  compensated summation as well.
@bedevere-app bedevere-app bot mentioned this pull request Jul 24, 2024
Closed
@ericsnowcurrently ericsnowcurrently removed their request for review July 24, 2024 16:38
@picnixz
Copy link
Contributor

picnixz commented Jul 24, 2024 •
edited
Loading

I'm asking Victor and Raymond since I know that Victor has been reviewing Sergey's PR (and I think you also merged it) and Raymond since Sergey asked him a question on the issue. Are there other people I should ask (maybe Serhiy?)

ericsnowcurrently
ericsnowcurrently requested changes Jul 24, 2024
View reviewed changes
Python/bltinmodule.c Show resolved Hide resolved
Python/bltinmodule.c Show resolved Hide resolved
@bedevere-app
Copy link

bedevere-app bot commented Jul 24, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@skirpichev

This comment was marked as outdated.

Sign in to view
@bedevere-app
Copy link

bedevere-app bot commented Jul 24, 2024

Thanks for making the requested changes!

@ericsnowcurrently: please review the changes made to this pull request.

ericsnowcurrently
ericsnowcurrently approved these changes Jul 24, 2024
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Seems okay to me. However, you should have an expert on numeric stuff also take a look before this gets merged.

@vstinner vstinner merged commit 169e713 into python:main Jul 29, 2024
38 of 41 checks passed
@vstinner
Copy link
Member

Merged, thank you.

@skirpichev skirpichev deleted the fix-sum-122234 branch July 29, 2024 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhettinger rhettinger rhettinger approved these changes

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

@vstinner vstinner Awaiting requested review from vstinner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@skirpichev @picnixz @vstinner @ericsnowcurrently @rhettinger