Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-120010: fix invalid (nan+nanj) results in _Py_c_prod()#120287
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
gh-120010: fix invalid (nan+nanj) results in _Py_c_prod() #120287
Uh oh!
There was an error while loading. Please reload this page.
Conversation
skirpichev commented Jun 9, 2024 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
In some cases, previously computed as (nan+nanj), we could recover meaningful component values in the result, see e.g. the C11, Annex G.5.2, routine _Cmultd(): >>> z = 1e300+1j >>> z*(nan+infj) # was (nan+nanj) (-inf+infj) That also fix some complex powers for small integer exponents, computed with optimised algorithm (by squaring): >>> z**5 # was (nan+nanj) Traceback (most recent call last): File "<python-input-1>", line 1, in <module> z**5 ~^^~ OverflowError: complex exponentiation
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Jun 10, 2024
cc @mdickinson |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Misc/NEWS.d/next/Core and Builtins/2024-06-04-08-26-25.gh-issue-120010._z-AWz.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
…e-120010._z-AWz.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz left a comment
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.
Looks good!
vstinner left a comment
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.
vstinner commented Nov 15, 2024
Thanks for this change is beyond my IEEE-754 skills :-( Maybe @serhiy-storchaka can have a look? |
skirpichev commented Nov 27, 2024
This should be adjusted after merging #124829. |
| Py_complex | ||
| _Py_c_prod(Py_complexa, Py_complexb) | ||
| _Py_c_prod(Py_complexz, Py_complexw) |
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.
Should be ok now. This doesn't require a mixed-mode variant.
Yet, maybe I should change variable names back for consistency with the rest? Current naming here follows to the C standard.
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.
@serhiy-storchaka what do you think about renaming of arguments?
I think mapping local variables "abcd" -> "tuvw" will hot hurt readability and then we can preserve original argument names.
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.
I am fine with both variants. Preserving names from the old code can make the diff smaller, but in this case the new code is too different from the old code. Using names from the C standard examples helps comparing our code with the original code. The latter looks more important here.
serhiy-storchaka left a comment
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.
LGTM. But this is G.5.1, not G.5.2.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
8b7c194 into python:mainUh oh!
There was an error while loading. Please reload this page.
…onGH-120287) In some cases, previously computed as (nan+nanj), we could recover meaningful component values in the result, see e.g. the C11, Annex G.5.1, routine _Cmultd(): >>> z = 1e300+1j >>> z*(nan+infj) # was (nan+nanj) (-inf+infj) That also fix some complex powers for small integer exponents, computed with optimized algorithm (by squaring): >>> z**5 # was (nan+nanj) Traceback (most recent call last): File "<python-input-1>", line 1, in <module> z**5 ~^^~ OverflowError: complex exponentiation
In some cases, previously computed as (nan+nanj), we could recover meaningful component values in the result, see e.g. the C11, Annex G.5.2, routine _Cmultd():
That also fix some complex powers for small integer exponents, computed with optimised algorithm (by squaring):