[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:

Re-closing; we should be good now.

--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:


New changeset beb3a835dae3cd940b93e7caa32450890c4cd539 by Miss Islington (bot) 
in branch '3.9':
bpo-44698: Restore complex pow behaviour for small integral exponents 
(GH-27772) (GH-27797)
https://github.com/python/cpython/commit/beb3a835dae3cd940b93e7caa32450890c4cd539


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:


New changeset 3f81e9628f6f104c103d0e38adb84c51e5261626 by Miss Islington (bot) 
in branch '3.10':
bpo-44698: Restore complex pow behaviour for small integral exponents 
(GH-27772) (GH-27796)
https://github.com/python/cpython/commit/3f81e9628f6f104c103d0e38adb84c51e5261626


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[miss-islington]

Change by miss-islington :


--
pull_requests: +26265
pull_request: https://github.com/python/cpython/pull/27796

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[miss-islington]

Change by miss-islington :


--
pull_requests: +26266
pull_request: https://github.com/python/cpython/pull/27797

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:


New changeset 4b9a2dcf19e5d13c3bc2afea2de1f65cd994c699 by Mark Dickinson in 
branch 'main':
bpo-44698: Restore complex pow behaviour for small integral exponents (GH-27772)
https://github.com/python/cpython/commit/4b9a2dcf19e5d13c3bc2afea2de1f65cd994c699


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:

Thanks, Pablo!

> could you add me as a reviewer for the backport

Will do! I'll waiting on review for the PR against the main branch, but hope to 
get the backport PR up by this coming weekend at the latest.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I'm keeping track to the 3.10 backports to the second rc so could you add me as 
a reviewer for the backport?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> GH-27772 reverts the behaviour change but keeps the fix for the undefined 
> behaviour. Can the backport of GH-27772 to the 3.10 branch go in before 
> 3.10.0 final?

Thanks for checking with me! 

Given that this is fixing a bug that we introduced in rc1, I think it makes 
sense to backport to fix it. I have reviewed the code in GH-27772 and is 
contained enough, so I think is a good idea to go ahead.

Thanks Mark for all the work!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:

@Pablo: I'm adding you to the nosy list to get your opinion on whether (the 
backport of) GH-27772 can go into 3.10.0.rc2.

Summary: GH-27278 (which I reviewed and approved) and its backports fixed some 
undefined behaviour in complex exponentiation, but also introduced a behaviour 
change that affected NumPy: some exponentiation expressions that returned a 
result in 3.10.0b4 raise an OverflowError in 3.10.0.rc1.

GH-27772 reverts the behaviour change but keeps the fix for the undefined 
behaviour. Can the backport of GH-27772 to the 3.10 branch go in before 3.10.0 
final?

--
nosy: +pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Change by Mark Dickinson :


--
pull_requests: +26246
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/27772

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Sebastian Berg]

Sebastian Berg  added the comment:

Thanks for looking into it!

`cpow` is indeed complicated.  We had some discussion in NumPy about `0**y` 
branch cuts (I did yet not finish it, because thinking about the branch cuts is 
tricky).

It is almost reassuring that the C standard also hedges out :).  Although, it 
means that there is no reliance on math libraries if we want to treat the 
special cases more carefully :(.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:

> brave or foolhardy enough to try to figure out what all of the special case 
> results *should* be for complex pow

Addendum: even the C standards give up at this point. For everything else the 
special cases are spelled out in detail, but for cpow, from §G.6.4.1 of C17 (in 
Annex G), we have:

> The cpow functions raise floating-point exceptions if appropriate for the 
> calculation of the parts of the result, and may also raise spurious 
> floating-point exceptions.

And that's it. (Well, not quite: there's a footnote, which says:

> This allows cpow(z, c) to be implemented as cexp(cclog(z)) without precluding 
> implementations that treat special cases more carefully.

)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Change by Mark Dickinson :


--
assignee: twouters -> mark.dickinson
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:

> a double-check that the behaviour change is desirable

I'm not sure I'm brave or foolhardy enough to try to figure out what all of the 
special case results *should* be for complex pow. The special cases seem 
reasonably clear cut for everything else in the cmath module, but for pow it's 
pretty much going to be a nightmare. I'm not sure there's much Python can do 
here in the way of guarantees, other than "not obviously horribly wrong". (And 
while I have reasonable confidence we can achieve the "not obviously horribly 
wrong" goal for everything else, I still have doubts for pow.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Mark Dickinson]

Mark Dickinson  added the comment:

Thanks, @seberg. I'll take a look and see if we can restore the old behaviour, 
at least for Python 3.9 and 3.10. I'm not convinced that the new behaviour is 
objectively wrong, but I agree that the *change* in behaviour is problematic.

I do find it a little odd that we're special-casing integer exponents for 
complex numbers when no such special-casing exists for floats. It's not purely 
for optimization (despite what the code might say). And all the messing around 
with trying to figure out exactly _which_ doubles can safely be cast to long 
(which was the original cause of this issue) seems like a symptom of a deeper 
problem, which is that it doesn't make a whole lot of sense to be 
distinguishing in the first place. But if we're going to change the behaviour, 
we should only be doing that in 3.11.

OTOH, there *is* real value in z**2 being fast and accurate; we definitely want 
to keep that. (Perhaps the math and cmath modules should grow a `square` 
function.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Sebastian Berg]

Sebastian Berg  added the comment:

(Sorry for the spam.  I think we can/should just hard-code the expected values 
in the NumPy test-suite.  So this is not actually an issue for NumPy and 
probably just warrants a double-check that the behaviour change is desirable.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Sebastian Berg]

Sebastian Berg  added the comment:

Hmm, sorry, I overshot/misread :(.

The thing that the NumPy test-suite trips over is that:

c_powi(inf+0j, 3)

seems to not raise, but:

_Py_c_pow(inf+0j, 3.+0j)

(or nan+0.j rather then inf+0j)

does seem to raise (returning `nan+nanj` in both cases).  If this is the `inf` 
code path, raising an error may actually be fix and the integer code path 
should maybe also do it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Sebastian Berg]

Sebastian Berg  added the comment:

The fix broke NumPy (see also https://github.com/numpy/numpy/pull/19612)

It seems incorrect.  After all, it doesn't matter much whether the float can be 
converted to an integer correctly (or even if it returns an undefined value), 
so long `int_value == float_value` remains sensible.

The old cast cast integers to complex when they were out of range (which is 
fine), the new code raises an error instead.

--
nosy: +seberg

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset 85ac81499ec5fb156a57408bcd95b06de4531488 by Miss Islington (bot) 
in branch '3.9':
bpo-44698: Fix undefined behaviour in complex exponentiation. (GH-27278) 
(GH-27367)
https://github.com/python/cpython/commit/85ac81499ec5fb156a57408bcd95b06de4531488


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset 256d97c8a3151ec7caba6c06bf2a1e682008c304 by Miss Islington (bot) 
in branch '3.10':
bpo-44698: Fix undefined behaviour in complex exponentiation. (GH-27278) 
(#27366)
https://github.com/python/cpython/commit/256d97c8a3151ec7caba6c06bf2a1e682008c304


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Łukasz Langa]

Change by Łukasz Langa :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[miss-islington]

Change by miss-islington :


--
pull_requests: +25905
pull_request: https://github.com/python/cpython/pull/27367

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 5.0 -> 6.0
pull_requests: +25904
pull_request: https://github.com/python/cpython/pull/27366

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Łukasz Langa]

Łukasz Langa  added the comment:


New changeset 1d582bbc969e05896addf97844ddf17ce9830e5e by T. Wouters in branch 
'main':
bpo-44698: Fix undefined behaviour in complex exponentiation. (GH-27278)
https://github.com/python/cpython/commit/1d582bbc969e05896addf97844ddf17ce9830e5e


--
nosy: +lukasz.langa

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +mark.dickinson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Thomas Wouters]

Change by Thomas Wouters :


--
keywords: +patch
pull_requests: +25823
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/27278

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue44698] Undefined behaviour in Objects/complexobject.c's complex_pow

[Thomas Wouters]

New submission from Thomas Wouters :

Objects/complexobject.c's complex_pow uses undefined behaviour, by casting a 
float of unknown magnitude to a long:

int_exponent = (long)exponent.real;

At Google we build with clang and -fsanitize=float-cast-overflow by default, 
which catches this particular kind of undefined behaviour. We didn't notice, 
however, because the only code we've come across that exercises this behaviour 
was a commented-out test in test_complex, which was uncommented in 3.8. Running 
the test, or just '1e19+1j ** 1e19', is enough to trigger the undefined 
behaviour. I'll prepare a PR to fix it.

--
assignee: twouters
messages: 397947
nosy: gregory.p.smith, twouters
priority: normal
severity: normal
status: open
title: Undefined behaviour in Objects/complexobject.c's complex_pow
type: behavior
versions: Python 3.10, Python 3.11, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com