gh-117999: Fix small integer powers of complex numbers #124243
Conversation
serhiy-storchaka
commented
Sep 19, 2024
•
serhiy-storchaka
commented
- Fix the sign of zero components in the result. E.g. complex(1,-0.0)**2 now evaluates to complex(1,-0.0) instead of complex(1,0.0).
- Fix negative small integer powers of infinite complex numbers. E.g. complex(inf)**-1 now evaluates to complex(0,-0.0) instead of complex(nan,nan).
- Powers of infinite numbers no longer raise OverflowError. E.g. complex(inf)**1 now evaluates to complex(inf) and complex(inf)**0.5 now evaluates to complex(inf,nan).
- Issue: Small integer powers of real±0j are invalid #117999
* Fix the sign of zero components in the result. E.g. complex(1,-0.0)**2 now evaluates to complex(1,-0.0) instead of complex(1,-0.0). * Fix negative small integer powers of infinite complex numbers. E.g. complex(inf)**-1 now evaluates to complex(0,-0.0) instead of complex(nan,nan). * Powers of infinite numbers no longer raise OverflowError. E.g. complex(inf)**1 now evaluates to complex(inf) and complex(inf)**0.5 now evaluates to complex(inf,nan).
|
I'll take look, but probably already tomorrow :( Meanwhile, I'm not sure if fix for division should be included here. That does make sense for a separate pr, that will provide mixed-mode arithmetic for complex. I was thinking (In fact, I did, patch just lacks more tests: skirpichev#4) on splitting out from skirpichev#1 just cases for real op complex, as specified by C99+ (note, that there is no special case for |
There was a problem hiding this comment.
That looks close to #118000. I doubt there is an easy way to fix powi() algorithm to match generic one (same signs, etc).
Lib/test/test_complex.py
Outdated
| self.assertComplexesAreIdentical(c**0, complex(1, +0.0)) | ||
| self.assertComplexesAreIdentical(c**1, c) | ||
| self.assertComplexesAreIdentical(c**2, c*c) | ||
| self.assertComplexesAreIdentical(c**3, c*(c*c)) | ||
| self.assertComplexesAreIdentical(c**4, (c*c)*(c*c)) | ||
| self.assertComplexesAreIdentical(c**5, c*((c*c)*(c*c))) |
There was a problem hiding this comment.
I would rather worry if lhs will not match not with some random arrangements of parens on rhs, but with the generic algorithm (i.e. _testcapi._py_c_pow()).
But, e.g.
>>> import ctypes, _testcapi
>>> libm = ctypes.CDLL('libm.so.6')
>>> libm.cpow.argtypes = [ctypes.c_double_complex, ctypes.c_double_complex]
>>> libm.cpow.restype = ctypes.c_double_complex
>>> z = -1-1j
>>> z**6
(-0-8j)
>>> _testcapi._py_c_pow(z, 6)
((4.408728476930473e-15-8.000000000000004j), 0)
>>> libm.cpow(z, 6)
(4.408728476930471e-15-7.999999999999998j)There was a problem hiding this comment.
I added new tests to test that the result has zero component with the same sign as the result for small non-zero component.
| assert(n > 0); | ||
| while ((n & 1) == 0) { | ||
| x = _Py_c_prod(x, x); | ||
| n >>= 1; | ||
| } | ||
| Py_complex r = x; | ||
| n >>= 1; | ||
| while (n) { | ||
| x = _Py_c_prod(x, x); | ||
| if (n & 1) { | ||
| r = _Py_c_prod(r, x); | ||
| } | ||
| n >>= 1; |
There was a problem hiding this comment.
This looks longer than in #118000, but is there a difference (up to handling non-negative exponents in my case)?
There was a problem hiding this comment.
#118000 always has additional multiplication by c, even if n is the power of 2.
For example, if n=8, c**16 is calculated as:
(((c**2)**2)**2)**2in this PR -- 4 multiplication.c * c * c**2 * c**4 * c**8in gh-117999: fixed small nonnegative integer powers of complex numbers #118000 -- 7 multiplications,
| if (isfinite(a.real) && isfinite(a.imag)) { | ||
| _Py_ADJUST_ERANGE2(p.real, p.imag); | ||
| } |
There was a problem hiding this comment.
Hmm, we don't call math functions in c_powi. Is this workaround needed at all?
There was a problem hiding this comment.
Without this, complex(inf)**1 raises an OverflowError.
| if (isfinite(a.real) && isfinite(a.imag) | ||
| && isfinite(b.real) && isfinite(b.imag)) | ||
| { | ||
| _Py_ADJUST_ERANGE2(r.real, r.imag); | ||
| } |
There was a problem hiding this comment.
That looks correct for me, given description of _Py_ADJUST_ERANGE2(). But what we gain here?
There was a problem hiding this comment.
Without this, complex(inf)**101 raised an OverflowError.
There is no test for this, because the result ((inf+nanj)) is still incorrect -- it should be (inf+0j), as for smaller exponents.
