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

Improve docs of math.isclose(0, x) when x = 0 #131094

Closed
guyj-s1 opened this issue Mar 11, 2025 · 8 comments
Closed

Improve docs of math.isclose(0, x) when x = 0 #131094

guyj-s1 opened this issue Mar 11, 2025 · 8 comments
Labels
docs Documentation in the Doc dir

Comments

@guyj-s1
Copy link

guyj-s1 commented Mar 11, 2025

Documentation

In the documentation of math.isclose it states

When comparing x to 0.0, isclose(x, 0) is computed as abs(x) <= rel_tol * abs(x), which is False for any x and rel_tol less than 1.0.

The statement isn't correct since x = 0 will give True

Linked PRs

@guyj-s1 guyj-s1 added the docs Documentation in the Doc dir label Mar 11, 2025
@hoodmane
Copy link
Contributor

Seems the correction is to add a single word:

When comparing x to 0.0, isclose(x, 0) is computed as abs(x) <= rel_tol * abs(x), which is False for any nonzero x and rel_tol less than 1.0.

Would you mind making a PR?

@eendebakpt
Copy link
Contributor

When making the PR also update the formatting (the parameters should be emphasized). I think the suggested change to the doc is an improvement, although with floating point arithmetic there are some corner cases:

import math

x = 1e-320
rel_tol = 1e-9
abs_tol = math.nextafter(1, 0)
print(math.isclose(x, 0, rel_tol=rel_tol, abs_tol=abs_tol))

print(abs(x) <= rel_tol * abs(x))
print(abs(x) <= abs(rel_tol * x))

results in

True
False
False

@plashchynski
Copy link

plashchynski commented Mar 11, 2025

@eendebakpt it's true because the difference is less than abs_tol:

import math

x = 1e-320
rel_tol = 1e-9
abs_tol = math.nextafter(1, 0)

diff = abs(0 - x);
print(diff <= abs_tol) # returns True

I think this statement in the documentation holds only for abs_tol being zero (which is the default).

@eendebakpt
Copy link
Contributor

The relevant code is:

cpython/Modules/mathmodule.c

Lines 3202 to 3211 in ad90c5f

/* now do the regular computation
this is essentially the "weak" test from the Boost library
*/
diff = fabs(b - a);
return (((diff <= fabs(rel_tol * b)) ||
(diff <= fabs(rel_tol * a))) ||
(diff <= abs_tol));
}

@plashchynski I agree the statement is valid if f abs_tol is zero. The way the documentation is written suggest the statement is true in general though (if b=0).

@plashchynski
Copy link

@eendebakpt imho this statements sounds more like a reason to set abs_tol to a value greater than zero:

*abs_tol* is the absolute tolerance; it defaults to ``0.0`` and it must be
nonnegative. When comparing ``x`` to ``0.0``, ``isclose(x, 0)`` is computed
as ``abs(x) <= rel_tol * abs(x)``, which is ``False`` for any ``x`` and
rel_tol less than ``1.0``. So add an appropriate positive abs_tol argument
to the call.

@plashchynski
Copy link

Btw, if a = math.nextafter(0, 1) then math.isclose(a, 0) will always be true for any rel_tol > 0:

import math
a = math.nextafter(0, 1)
print(math.isclose(a, 0, rel_tol=0.9)) # True

because:

print(math.nextafter(0, 1) * 0.9 == math.nextafter(0, 1)) # True

which is contrary to this statement and to pure mathematics as well.

Is there a policy for such corner cases? Should they be considered and handled?

@skirpichev
Copy link
Member

@eendebakpt, I think you misread documentation. It looks like abs_tol disappear in equation of the paragraph, which deals with comparing x to 0, but it's because abs(x) <= rel_tol * abs(x) is valid only in case of abs_tol=0.0 (isclose(x, 0) means the default value of abs_val). Last sentence suggests what to do in this case: "So add an appropriate positive abs_tol argument to the call."

For nonzero abs_tol you must use the generic equation, mentioned right at the beginning of docs: abs(a-b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol).

So, in your example:

>>> import math
... 
... x = 1e-320
... rel_tol = 1e-9
... abs_tol = math.nextafter(1, 0)
... 
>>> math.isclose(x, 0, rel_tol=rel_tol, abs_tol=abs_tol)
True
>>> abs(x) <= max(rel_tol * abs(x), abs_tol)
True

@hoodmane fix looks good for me. Though, I'm not sure we need such pedancy in docs.

guyj-s1 added a commit to guyj-s1/cpython that referenced this issue Mar 12, 2025
Corrected the statement about any *x* and *rel_tol* always producing ``False`` to exclude ``x = 0``
guyjacoby added a commit to guyjacoby/cpython that referenced this issue Mar 12, 2025
Corrected the statement about any *x* and *rel_tol* always producing ``False`` to exclude ``x = 0``.
@picnixz picnixz changed the title math.isclose(0, x) will be True if x = 0 Improve docs of math.isclose(0, x) when x = 0 Mar 12, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 18, 2025
(cherry picked from commit 3f50f96)

Co-authored-by: Guy Jacoby <49398101+guyjacoby@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 18, 2025
(cherry picked from commit 3f50f96)

Co-authored-by: Guy Jacoby <49398101+guyjacoby@users.noreply.github.com>
@picnixz picnixz closed this as completed Mar 18, 2025
@picnixz
Copy link
Member

picnixz commented Mar 18, 2025

Thank you for the report and the fix!

picnixz pushed a commit that referenced this issue Mar 18, 2025
gh-131094: Refine `math.isclose` docs (GH-131139)
(cherry picked from commit 3f50f96)

Co-authored-by: Guy Jacoby <49398101+guyjacoby@users.noreply.github.com>
picnixz pushed a commit that referenced this issue Mar 18, 2025
gh-131094: Refine `math.isclose` docs (GH-131139)
(cherry picked from commit 3f50f96)

Co-authored-by: Guy Jacoby <49398101+guyjacoby@users.noreply.github.com>
colesbury pushed a commit to colesbury/cpython that referenced this issue Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
Status: Todo
Development

No branches or pull requests

6 participants