Skip to content

BUG: fix corner cases in complex division (were nan+nanj)#26595

Closed
skirpichev wants to merge 3 commits intonumpy:mainfrom
skirpichev:fix-complex-tdiv-26560
Closed

BUG: fix corner cases in complex division (were nan+nanj)#26595
skirpichev wants to merge 3 commits intonumpy:mainfrom
skirpichev:fix-complex-tdiv-26560

Conversation

@skirpichev
Copy link
Copy Markdown

The numpy uses Smith's algorithm to do complex true division. In some cases (see below) it produces (nan+nanj), but meaningful components could be recovered. See e.g. C11 Annex G.5.2, _Cdivd()'s example.

>>> 1/complex128('(inf+infj)')  # should be 0j
<stdin>:1: RuntimeWarning: invalid value encountered in scalar divide
(nan+nanj)
>>> complex128(inf, -inf)/complex128(1+0j)  # should be (inf-infj)
<stdin>:1: RuntimeWarning: invalid value encountered in scalar divide
np.complex128(nan+nanj)

Closes #26560


FYI, related PR in CPython: python/cpython#119457

The numpy uses Smith's algorithm to do complex true division.  In some
cases (see below) it produces (nan+nanj), but meaningful components
could be recovered.  See e.g. C11 Annex G.5.2, _Cdivd()'s example.

>>> 1/complex128('(inf+infj)')  # should be 0j
<stdin>:1: RuntimeWarning: invalid value encountered in scalar divide
(nan+nanj)
>>> complex128(inf, -inf)/complex128(1+0j)  # should be (inf-infj)
<stdin>:1: RuntimeWarning: invalid value encountered in scalar divide
np.complex128(nan+nanj)

Closes #26560
@mattip
Copy link
Copy Markdown
Member

mattip commented Jun 5, 2024

Since this touches the inner loops, we should benchmark to make sure any change is acceptable. This will also need a release note.

@mattip mattip added 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes labels Jun 5, 2024
@skirpichev
Copy link
Copy Markdown
Author

skirpichev commented Jun 6, 2024

Since this touches the inner loops, we should benchmark to make sure any change is acceptable.

It seems I can't built numpy locally (on my old notebook:D), not sure how to do this...

This will also need a release note.

Done.

@mattip mattip removed the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Jun 10, 2024
@skirpichev
Copy link
Copy Markdown
Author

Since this touches the inner loops, we should benchmark to make sure any change is acceptable.

In usual case (Smith's division algorithm produces meaningful output) this adds just one extra npy_isnan() call. I would expect you worry about this case.

BTW, here some benchmarks on CPython. Current main, with similar patch:

$ python -m pyperf timeit -q -s 'a, b = 1+1j, 2+1j' 'a/b'
Mean +- std dev: 155 ns +- 1 ns

Without:

$ python -m pyperf timeit -q -s 'a, b = 1+1j, 2+1j' 'a/b'
Mean +- std dev: 155 ns +- 1 ns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

BUG: Invalid corner cases (resulting in nan+nanj) in complex division

2 participants