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

cmath module documentation is misleading on branch cuts #85417

Closed
mdickinson opened this issue Jul 8, 2020 · 9 comments · Fixed by #102046
Closed

cmath module documentation is misleading on branch cuts #85417

mdickinson opened this issue Jul 8, 2020 · 9 comments · Fixed by #102046
Assignees
Labels
3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir

Comments

@mdickinson
Copy link
Member

mdickinson commented Jul 8, 2020

BPO 41245
Nosy @rhettinger, @mdickinson

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-07-08.17:19:22.882>
labels = ['3.7', '3.8', '3.9', '3.10', 'docs']
title = 'cmath module documentation is misleading on branch cuts'
updated_at = <Date 2020-07-08.18:52:27.472>
user = 'https://github.com/mdickinson'

bugs.python.org fields:

activity = <Date 2020-07-08.18:52:27.472>
actor = 'mark.dickinson'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation']
creation = <Date 2020-07-08.17:19:22.882>
creator = 'mark.dickinson'
dependencies = []
files = []
hgrepos = []
issue_num = 41245
keywords = []
message_count = 5.0
messages = ['373323', '373325', '373328', '373329', '373330']
nosy_count = 3.0
nosy_names = ['rhettinger', 'mark.dickinson', 'docs@python']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue41245'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

Linked PRs

@mdickinson
Copy link
Member Author

The documentation for the cmath module is misleading on the behaviour near branch cuts. For example, the documentation for cmath.acos says:

Return the arc cosine of x. There are two branch cuts: One
extends right from 1 along the real axis to ∞, continuous
from below. The other extends left from -1 along the real
axis to -∞, continuous from above.

That "continuous from below" and "continuous from above" language is misleading; in fact what happens on the vast majority of systems (those for which the floating-point format used is IEEE 754 binary64), if the imaginary part of x is zero, the sign of x is used to determine which side of the branch cut x lies.

@mdickinson
Copy link
Member Author

the sign of x is used [...]

Correction: That should say "the sign of the imaginary part of x is used [...]"

@mdickinson mdickinson added docs Documentation in the Doc dir 3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes labels Jul 8, 2020
@rhettinger
Copy link
Contributor

+1 for changing the language to match the actual mechanics.

"the sign of the imaginary part of x is used [...]"

I'm trying to see where this happens. Is this part of cmath_sqrt?

    if (z.real >= 0.) {
        r.real = s;
        r.imag = copysign(d, z.imag); 
    } else {
        r.real = d;
        r.imag = copysign(s, z.imag);
    }

"continuous from below" and "continuous from above"
language is misleading;

I'm curious, is that language incorrect? My mental image of a branch cut is a helical graph with the edge cases being continuous from above and below. Likewise, my mental model for branch cut logic is it resolves multiple possible output values in a way preserves continuity from one side or the other.

In other words, I think about branch cuts in terms of continuity rather than sign preservation. Is that incorrect?

@mdickinson
Copy link
Member Author

Yes, that looks like the right part of the sqrt code.

For the acos docstring, "continuous from below" implies that for any complex number z that lies exactly _on_ the branch cut, acos(z) is close to acos(w) for a nearby value w just _below_ the branch cut. But that's demonstrably not true: see the change of sign in the real part below:

>>> acos(complex(2.3, -1e-10))  # value just "below" the branch cut
(4.828045495852677e-11+1.475044781241425j)
>>> acos(complex(2.3, 0.0))  # nearby value exactly _on_ the branch cut
-1.475044781241425j

In effect, for a branch cut along the real axis, the sign of the zero in the imaginary part of the argument allows us to be continuous from both sides at once.

@mdickinson
Copy link
Member Author

[...] see the change of sign in the real part below [...]

Grr. Stupid fingers. That should say "imaginary part", not "real part"

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@skirpichev
Copy link
Member

in fact what happens on the vast majority of systems (those for which the floating-point format used is IEEE 754 binary64), if the imaginary part of x is zero, the sign of x is used to determine which side of the branch cut x lies.

@mdickinson, it seems that some MP libraries use different convention, e.g.:

>>> from mpmath import acos
>>> acos(2 + 1e-10j)
mpc(real='5.773502691896258e-11', imag='-1.3169578969248168')
>>> acos(complex(2))
mpc(real='0.0', imag='1.3169578969248168')
>>> acos(2 - 1e-10j)
mpc(real='5.773502691896258e-11', imag='1.3169578969248168')
>>> acos(-2 + 1e-10j)
mpc(real='3.1415926535320584', imag='-1.3169578969248168')
>>> acos(complex(-2))
mpc(real='3.1415926535897931', imag='-1.3169578969248168')
>>> acos(-2 - 1e-10j)
mpc(real='3.1415926535320584', imag='1.3169578969248168')

Ditto Mathematica. That's exactly "continuous from below ... and ... continuous from above." from the cmath docs.

Shouldn't we adjust the implementation instead?

@mdickinson
Copy link
Member Author

Shouldn't we adjust the implementation instead?

Definitely not! The implementation was very deliberately written this way. It's based on Kahan's "Branch Cuts for Complex Elementary Functions" paper, and it agrees with C99's Annex G and many other languages.

@skirpichev
Copy link
Member

skirpichev commented Feb 19, 2023

Ok, I see. Looks a bit surprising for complex analysis, but I see why it does make sense with signed zero floating-point arithmetics.

But it seems, that the current docs contains (since 2008) a note ("On platforms with hardware and system-level support for signed zeros, functions involving branch cuts are continuous on both sides of the branch cut: the sign of the zero distinguishes one side of the branch cut from the other. On platforms that do not support signed zeros the continuity is as specified below."). With this remark in mind - the acos docs seems correct, isn't?

Edit: So far, after 5ea052b - IEEE 754 support is a strict requirement. IIUIC, we can just leave the above remark (or like C11's Annex G "The functions are continuous onto both sides of their branch cuts, taking into account the sign of
zero. For example, csqrt (−2±i0) = ±i sqrt(2).") and skip all "continuous from" notes for all functions. Does it make sense as pr?

@mdickinson
Copy link
Member Author

the current docs contains (since 2008) a note [...]

Indeed - I wrote that note. :-)

the acos docs seems correct, isn't

Technically correct, yes, but I think unhelpful in its current form: it relies on someone reading the documentation also reading that note, rather than just reading the direct description of the relevant function.

It's been almost 15 years since I wrote that note, and in those 15 years IEEE 754 has only increased its overwhelming dominance. The descriptions should describe what happens on the 100% of systems whose C double has signed zeros, rather than describing the behaviour on the 0% of systems where the C double doesn't support signed zeros. We could then update the note to note what happens on systems where the double doesn't support signed zeros, or we could just throw it out altogether. I'd probably go for the latter - the non-signed-zero systems are pretty much entirely hypothetical at this point.

mdickinson added a commit to mdickinson/cpython that referenced this issue Feb 19, 2023
@mdickinson mdickinson self-assigned this Feb 19, 2023
skirpichev added a commit to skirpichev/cpython that referenced this issue Feb 19, 2023
…orms without signed zero

IIUIC, after 5ea052b - IEEE 754 support is a strict requirement.
mdickinson added a commit that referenced this issue Feb 19, 2023
This PR updates the cmath module documentation to reflect the reality that Python is almost always (and as far as I can tell, that "almost" can be omitted) running on a machine whose C double supports signed zeros.

* Removes misleading references to functions being continuous from above / below / the left / the right at branch cuts
* Expands the note on branch cuts at the top of the module documentation to explain the double-sided sign-of-zero-based behaviour
jaraco pushed a commit to jaraco/cpython that referenced this issue Feb 20, 2023
…hon#102046)

This PR updates the cmath module documentation to reflect the reality that Python is almost always (and as far as I can tell, that "almost" can be omitted) running on a machine whose C double supports signed zeros.

* Removes misleading references to functions being continuous from above / below / the left / the right at branch cuts
* Expands the note on branch cuts at the top of the module documentation to explain the double-sided sign-of-zero-based behaviour
carljm added a commit to carljm/cpython that referenced this issue Feb 20, 2023
* main: (60 commits)
  pythongh-102056: Fix a few bugs in error handling of exception printing code (python#102078)
  pythongh-102011: use sys.exception() instead of sys.exc_info() in docs where possible (python#102012)
  pythongh-101566: Sync with zipp 3.14. (pythonGH-102018)
  pythonGH-99818: improve the documentation for zipfile.Path and Traversable (pythonGH-101589)
  pythongh-88233: zipfile: handle extras after a zip64 extra (pythonGH-96161)
  pythongh-101981: Apply HOMEBREW related environment variables (pythongh-102074)
  pythongh-101907: Stop using `_Py_OPCODE` and `_Py_OPARG` macros (pythonGH-101912)
  pythongh-101819: Adapt _io types to heap types, batch 1 (pythonGH-101949)
  pythongh-101981: Build macOS as recommended by the devguide (pythonGH-102070)
  pythongh-97786: Fix compiler warnings in pytime.c (python#101826)
  pythongh-101578: Amend PyErr_{Set,Get}RaisedException docs (python#101962)
  Misc improvements to the float tutorial (pythonGH-102052)
  pythongh-85417: Clarify behaviour on branch cuts in cmath module (python#102046)
  pythongh-100425: Update tutorial docs related to sum() accuracy (FH-101854)
  Add missing 'is' to `cmath.log()` docstring (python#102049)
  pythongh-100210: Correct the comment link for unescaping HTML (python#100212)
  pythongh-97930: Also include subdirectory in makefile. (python#102030)
  pythongh-99735: Use required=True in argparse subparsers example (python#100927)
  Fix incorrectly documented attribute in csv docs (python#101250)
  pythonGH-84783: Make the slice object hashable (pythonGH-101264)
  ...
carljm added a commit to carljm/cpython that referenced this issue Feb 22, 2023
* main: (225 commits)
  pythongh-102056: Fix a few bugs in error handling of exception printing code (python#102078)
  pythongh-102011: use sys.exception() instead of sys.exc_info() in docs where possible (python#102012)
  pythongh-101566: Sync with zipp 3.14. (pythonGH-102018)
  pythonGH-99818: improve the documentation for zipfile.Path and Traversable (pythonGH-101589)
  pythongh-88233: zipfile: handle extras after a zip64 extra (pythonGH-96161)
  pythongh-101981: Apply HOMEBREW related environment variables (pythongh-102074)
  pythongh-101907: Stop using `_Py_OPCODE` and `_Py_OPARG` macros (pythonGH-101912)
  pythongh-101819: Adapt _io types to heap types, batch 1 (pythonGH-101949)
  pythongh-101981: Build macOS as recommended by the devguide (pythonGH-102070)
  pythongh-97786: Fix compiler warnings in pytime.c (python#101826)
  pythongh-101578: Amend PyErr_{Set,Get}RaisedException docs (python#101962)
  Misc improvements to the float tutorial (pythonGH-102052)
  pythongh-85417: Clarify behaviour on branch cuts in cmath module (python#102046)
  pythongh-100425: Update tutorial docs related to sum() accuracy (FH-101854)
  Add missing 'is' to `cmath.log()` docstring (python#102049)
  pythongh-100210: Correct the comment link for unescaping HTML (python#100212)
  pythongh-97930: Also include subdirectory in makefile. (python#102030)
  pythongh-99735: Use required=True in argparse subparsers example (python#100927)
  Fix incorrectly documented attribute in csv docs (python#101250)
  pythonGH-84783: Make the slice object hashable (pythonGH-101264)
  ...
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 26, 2023
…honGH-102046)

This PR updates the cmath module documentation to reflect the reality that Python is almost always (and as far as I can tell, that "almost" can be omitted) running on a machine whose C double supports signed zeros.

* Removes misleading references to functions being continuous from above / below / the left / the right at branch cuts
* Expands the note on branch cuts at the top of the module documentation to explain the double-sided sign-of-zero-based behaviour
(cherry picked from commit b513c46)

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Feb 26, 2023
…honGH-102046)

This PR updates the cmath module documentation to reflect the reality that Python is almost always (and as far as I can tell, that "almost" can be omitted) running on a machine whose C double supports signed zeros.

* Removes misleading references to functions being continuous from above / below / the left / the right at branch cuts
* Expands the note on branch cuts at the top of the module documentation to explain the double-sided sign-of-zero-based behaviour
(cherry picked from commit b513c46)

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
mdickinson added a commit that referenced this issue Feb 26, 2023
…-102046) (#102275)

gh-85417: Clarify behaviour on branch cuts in cmath module (GH-102046)

This PR updates the cmath module documentation to reflect the reality that Python is almost always (and as far as I can tell, that "almost" can be omitted) running on a machine whose C double supports signed zeros.

* Removes misleading references to functions being continuous from above / below / the left / the right at branch cuts
* Expands the note on branch cuts at the top of the module documentation to explain the double-sided sign-of-zero-based behaviour
(cherry picked from commit b513c46)

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
mdickinson added a commit that referenced this issue Feb 26, 2023
…-102046) (#102276)

gh-85417: Clarify behaviour on branch cuts in cmath module (GH-102046)

This PR updates the cmath module documentation to reflect the reality that Python is almost always (and as far as I can tell, that "almost" can be omitted) running on a machine whose C double supports signed zeros.

* Removes misleading references to functions being continuous from above / below / the left / the right at branch cuts
* Expands the note on branch cuts at the top of the module documentation to explain the double-sided sign-of-zero-based behaviour
(cherry picked from commit b513c46)

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants