Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-69639: add mixed-mode rules for complex arithmetic (C-like)#124829

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

Merged

Conversation

@skirpichev
Copy link
Contributor

@skirpichevskirpichev commentedOct 1, 2024•
edited
Loading

"Generally, mixed-mode arithmetic combining real and complex variables should be performed directly, not by first coercing the real to complex, lest the sign of zero be rendered uninformative; the same goes for combinations of pure imaginary quantities with complex variables." (c) Kahan, W: Branch cuts for complex elementary functions.

This patch implements mixed-mode arithmetic rules, combining real and complex variables as specified by C standards since C99. Most C compilers implementing C99+ Annex G have only these special rules (without support for imaginary type, which is going to be deprecated in C2y).

New rules allow to use complex arithmetic for implementation of mathematical functions without "corner cases" for special numbers (like signed zero or infinities). Well, at least it works now for more cases;)

Examples:

>>>2-0j# was (2+0j)(2-0j)>>> z=complex(-0.0,2)>>> cmath.asinh(z)(-1.3169578969248166+1.5707963267948966j)>>> cmath.log(z+ cmath.sqrt(1+ z*z))# real component had wrong sign before(-1.3169578969248164+1.5707963267948966j)>>> (cmath.inf+1j)*2# imaginary component was nan before(inf+2j)

That doesn't work (requires support for imaginary type):

>>>-0.0+1j1j>>> z=complex(-0.0,2)>>> cmath.atan(z)(-1.5707963267948966+0.5493061443340549j)>>>1j*(cmath.log(1-1j*z)- cmath.log(1+1j*z))/2# wrong real part(1.5707963267948966+0.5493061443340549j)

Notes for reviewers

📚 Documentation preview 📚:https://cpython-previews--124829.org.readthedocs.build/

EDM115 reacted with eyes emoji
"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
@skirpichev

This comment was marked as resolved.

@ericsnowcurrentlyericsnowcurrently removed their request for reviewOctober 1, 2024 16:04
serhiy-storchaka
serhiy-storchaka previously approved these changesOct 1, 2024
Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM in general. Very LGTM! I only left a few suggestions for style.

* more tests for multiplication* rename to _Py_convert_int_to_double* rename to real_to_float/complex* slightly optimize code
@skirpichev
Copy link
ContributorAuthor

@serhiy-storchaka, what do you think on fixingrepr(complex(-0.0, 1)) or adding special case for the true division (this is what you did in#124243 or I did inskirpichev#1)?

I also worry that this is a half-way solution: this neither fixesall eval(repr) round-trip issues or allows to useany analytical identity from textbook, e.g.:

>>>-0.0+1j1j>>> z=complex(-0.0,2)>>>1j*(cmath.log(1-1j*z)- cmath.log(1+1j*z))/2(1.5707963267948966+0.5493061443340549j)>>> cmath.atan(z)(-1.5707963267948966+0.5493061443340549j)

Proper fix seems to be only something likeskirpichev#1. Does it look complicated for you? (Note, that arithmetic methods on the complex type are mostly unchanged wrt the current pr.)

@skirpichev
Copy link
ContributorAuthor

I also was thinking about using additional C-API helper functions like_Py_c_sum_real() (as GSLdoes), instead of inlining that stuff. Does it make sense for you?

@serhiy-storchaka
Copy link
Member

I think that the idea of special handling of mixed complex-real arithmetic is much easier to "sell" than the idea of the imaginary number class. And it solves a half of problems. It will help to convince in necessarily to support pure imaginaries. Let's eat the elephant piece by piece.

adding special case for the true division

I was surprised you did not include it. What does C99+ say about it? It looks natural and is useful in some cases, so I would implement it even if the C standard omits it.

I also was thinking about using additional C-API helper functions like_Py_c_sum_real() (as GSLdoes), instead of inlining that stuff. Does it make sense for you?

It makes sense to me. The status of functions like_Py_c_sum() is unclear -- they are private, but documented. Some of them are used outside ofcomplexobject.c, and some of them are non-trivial, -- this is likely the reason of exposing them. I think that this all would also be true for the new functions.

Of course, the changes in arithmetic and the new C API (even if it is private) should be documented in many places.

@skirpichev
Copy link
ContributorAuthor

And it solves a half of problems.

Andthat is a problem.

I was surprised you did not include it. What does C99+ say about it?

It seems, there is no such special version in the C standard. Not sure why. And such case miss in implementations, e.g. clang:
https://github.com/llvm/llvm-project/blob/496187b3b81ea76a8b67d796609d7f09992cf96d/libcxx/include/complex#L835-L841

On another hand, GSL or MPC libraries implement such case.

I would implement it even if the C standard omits it.

Ok, I'll add this with naming scheme like in your pr.

Of course, the changes in arithmetic and the new C API (even if it is private) should be documented in many places.

New arithmetic rules documented in stdtypes.rst, C API stuff will go to Doc/c-api/complex.rst. Did I miss something else?

@skirpichevskirpichev marked this pull request as draftOctober 2, 2024 07:47
@serhiy-storchaka
Copy link
Member

New arithmetic rules documented in stdtypes.rst, C API stuff will go to Doc/c-api/complex.rst. Did I miss something else?

In Doc/reference/expressions.rst: "If either argument is a complex number, the other is converted to complex". There may be other leftovers.

You missed a What's New entry andversionchanged directives.

@skirpichevskirpichev marked this pull request as ready for reviewOctober 2, 2024 11:46
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

One last missing modification and LGTM! Thanks for your patience Sergey!

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@skirpichev
Copy link
ContributorAuthor

@serhiy-storchaka, are you ok with current wording in docs?

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

skirpichev reacted with hooray emoji
Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
pullbot pushed a commit to Ucg2c3/cpython that referenced this pull requestNov 26, 2024
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
@serhiy-storchakaserhiy-storchaka merged commitf0d3f10 intopython:mainNov 26, 2024
40 checks passed
@skirpichevskirpichev deleted the complex-float-arith-69639 branchNovember 27, 2024 00:19
@skirpichev
Copy link
ContributorAuthor

Thanks to all reviewers, especially Serhiy!

What's next,@serhiy-storchaka? This pr, as per description, address#69639 only partially. It's still easy to smash sign of zero or make some component to be nan, e.g.:

>>>-0.0+1j1j>>>float('inf')*1j(nan+infj)>>> z=complex(-0.0,2)>>>import cmath>>> cmath.atan(z)(-1.5707963267948966+0.5493061443340549j)>>>1j*(cmath.log(1-1j*z)- cmath.log(1+1j*z))/2(1.5707963267948966+0.5493061443340549j)

and we still have funny integer zeros in repr:

>>>complex(-0.0,1)(-0+1j)

Probably, it doesn't make sense to change repr alone. And I think that theprevious discussion showed - this set of problems can be solved only with new imaginary type.

Should I prepare a PEP draft, would you like to sponsor (or be PEP co-author) such idea? Or it's better to introduce this first on the d.p.o/ideas?

PS:
skirpichev#1 rebased and adjusted. (BTW it can be made smaller: some tests in the main just use imaginary literals like1j e.g. to trigger errors, where1+1j will work just as fine.)

ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
@vstinner
Copy link
Member

This change adds 6 private functions and documents them:

  • _Py_cr_diff()
  • _Py_cr_prod()
  • _Py_cr_quot()
  • _Py_cr_sum()
  • _Py_rc_diff()
  • _Py_rc_quot()

I would prefer to move these functions to the internal C API, or to make them public. I dislike adding private functions (functions with a name prefixed by_Py).

@skirpichev
Copy link
ContributorAuthor

@vstinner, these functions follows existing convention forcomlexobject.c.

move these functions to the internal C API

I don't think it's a good idea. These functions are extensively used in the cmath module and outside of the CPython it happens too.

make them public

This does make sense for me. We could just drop_ prefixes or do you suggest some more severe renaming? Should I open issue in the C-API repo or this seems to be a minor issue for you?

@vstinner
Copy link
Member

@vstinner
Copy link
Member

We could just drop _ prefixes or do you suggest some more severe renaming?

I would prefer to rename_Py_c_sum() toPyComplex_Add() and passPyComplex struct by reference (pointer) instead of passing it by value. So it would be a new C API.

Note:_Py_c_abs() is not documented.

Should I open issue in the C-API repo or this seems to be a minor issue for you?

If you agree with the idea of adding a public C API, you can open a new issue in this project (cpython) first.

@skirpichev
Copy link
ContributorAuthor

FYI:#128813

vstinner reacted with thumbs up emoji

Florents-Tselai pushed a commit to Florents-Tselai/cpython that referenced this pull requestAug 16, 2025
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
Florents-Tselai pushed a commit to Florents-Tselai/cpython that referenced this pull requestAug 16, 2025
…ythonGH-124829)"Generally, mixed-mode arithmetic combining real and complex variables shouldbe performed directly, not by first coercing the real to complex, lest the signof zero be rendered uninformative; the same goes for combinations of pureimaginary quantities with complex variables." (c) Kahan, W: Branch cuts forcomplex elementary functions.This patch implements mixed-mode arithmetic rules, combining real andcomplex variables as specified by C standards since C99 (in particular,there is no special version for the true division with real lhsoperand).  Most C compilers implementing C99+ Annex G have only thesespecial rules (without support for imaginary type, which is going to bedeprecated in C2y).
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

@willingcwillingcwillingc approved these changes

@picnixzpicnixzpicnixz approved these changes

+1 more reviewer

@rruuaanngrruuaanngrruuaanng left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@skirpichev@serhiy-storchaka@vstinner@willingc@picnixz@rruuaanng
[8]ページ先頭
©2009-2025 Movatter.jp