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.

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-69639: add mixed-mode rules for complex arithmetic (C-like) #124829

Merged
merged 29 commits into from
Nov 26, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Oct 1, 2024

"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+1j
1j
>>> 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/

"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 (in particular,
there is no special version for the true division with real lhs
operand).  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).
Objects/complexobject.c Outdated Show resolved Hide resolved
Objects/complexobject.c Outdated Show resolved Hide resolved
@skirpichev

This comment was marked as resolved.

@ericsnowcurrently ericsnowcurrently removed their request for review October 1, 2024 16:04
Copy link
Member

@serhiy-storchaka serhiy-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.

Objects/floatobject.c Outdated Show resolved Hide resolved
Objects/complexobject.c Outdated Show resolved Hide resolved
Objects/complexobject.c Outdated Show resolved Hide resolved
Objects/complexobject.c Outdated Show resolved Hide resolved
Lib/test/test_complex.py Outdated Show resolved Hide resolved
* more tests for multiplication
* rename to _Py_convert_int_to_double
* rename to real_to_float/complex
* slightly optimize code
@skirpichev
Copy link
Member Author

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

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

>>> -0.0+1j
1j
>>> 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 like skirpichev#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
Member Author

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

Objects/complexobject.c Outdated Show resolved Hide resolved
@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 GSL does), 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 of complexobject.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
Member Author

And it solves a half of problems.

And that 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?

@skirpichev skirpichev marked this pull request as draft October 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 and versionchanged directives.

@skirpichev skirpichev marked this pull request as ready for review October 2, 2024 11:46
@skirpichev skirpichev requested a review from willingc as a code owner October 2, 2024 11:46
@skirpichev skirpichev requested a review from picnixz October 31, 2024 13:16
Copy link
Member

@picnixz picnixz 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!

Objects/complexobject.c Outdated Show resolved Hide resolved
Co-authored-by: Bénédikt Tran <[email protected]>
@skirpichev
Copy link
Member Author

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

Copy link
Member

@serhiy-storchaka serhiy-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. 👍

Python/bltinmodule.c Show resolved Hide resolved
Lib/test/test_builtin.py Outdated Show resolved Hide resolved
serhiy-storchaka and others added 2 commits November 26, 2024 11:44
Co-authored-by: Sergey B Kirpichev <[email protected]>
…USE WEB-EDITOR...

I'LL NEVER USE WEB-EDITOR...
pull bot pushed a commit to Ucg2c3/cpython that referenced this pull request Nov 26, 2024
…ythonGH-124829)

"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 (in particular,
there is no special version for the true division with real lhs
operand).  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).
@serhiy-storchaka serhiy-storchaka merged commit f0d3f10 into python:main Nov 26, 2024
40 checks passed
@skirpichev skirpichev deleted the complex-float-arith-69639 branch November 27, 2024 00:19
@skirpichev
Copy link
Member Author

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+1j
1j
>>> 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 the previous 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 like 1j e.g. to trigger errors, where 1+1j will work just as fine.)

ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…ythonGH-124829)

"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 (in particular,
there is no special version for the true division with real lhs
operand).  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).
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…ythonGH-124829)

"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 (in particular,
there is no special version for the true division with real lhs
operand).  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).
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…ythonGH-124829)

"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 (in particular,
there is no special version for the true division with real lhs
operand).  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).
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
…ythonGH-124829)

"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 (in particular,
there is no special version for the true division with real lhs
operand).  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).
@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
Member Author

@vstinner, these functions follows existing convention for comlexobject.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() to PyComplex_Add() and pass PyComplex 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
Member Author

FYI: #128813

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants