You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
The return value of function math.isclose can be somewhat misleading #126165
I think, the reason for this is that the default value of parameter abs_tol for function math.isclose is too small. Here are the definitions of the two functions mentioned above:
or increase the value of the default parameter abs_tol appropriately.
I fail to see any arguments for this in your text. "Do as numpy does"?
In the PEP 485 the default value was chosen because: "there is no value that is appropriate for the general case. It is impossible to know an appropriate value without knowing the likely values expected for a given use case. If all the values tested are on order of one, then a value of about 1e-9 might be appropriate, but that would be far too large if expected values are on order of 1e-9 or smaller."
I'll relabel this as docs issue. Please describe how you would like to improve sphinx docs/docstrings.
If you are going for change defaults - this will be a huge compatibility break. I think this should be discussed first on https://discuss.python.org/.
Personally I think a default value should be set with the most common or most intuitive use cases in mind, and the OP's example of math.isclose(math.sin(math.pi), 0) is False demonstrates well why the current default value of the abs_tol parameter doesn't meet that expectation.
I find numpy.isclose's default value of 1e-08 for atol to be a reasonable choice because it is one digit more precise than a single-precision floating point number.
But of course, as @skirpichev pointed out, changing the default value at this point would cause backwards compatibility issues, so it's probably best that we make math.isclose(math.sin(math.pi), 0, abs_tol=1e-08) a usage example in the documentation.
Not at all. Maybe it's a valid documentation issue and we can explicitly mention defaults in the docstring/sphinx docs (not just via function signature). I'm -0 on this, but we do such things in some functions, e.g.:
>>> help(math.prod)
Help on built-in function prod in module math:
prod(iterable, /, *, start=1)
Calculate the product of all the elements in the input iterable.
The default start value for the product is 1.
When the iterable is empty, return the start value. This function is
intended specifically for use with numeric values and may reject
non-numeric types.
So, if you have a concrete suggestion on how to improve docs - feel free to reopen.
I think a default value should be set with the most common or most intuitive use cases in mind, and the OP's example of math.isclose(math.sin(math.pi), 0) is False
I don't think we can appeal to one single case. What's "most intuitive use cases"?! The PEP has much more generic argumentation.
reasonable choice because it is one digit more precise than a single-precision floating point number.
(IEEE single-precision floats have 6-9 decimal digits precision.)
Why single-precision floats are special here at all? BTW probably now in most implementations Python's floats are double-precision IEEE floats.
best that we make math.isclose(math.sin(math.pi), 0, abs_tol=1e-08) a usage example in the documentation.
Most math's functions have no examples, but maybe it worth.
It was suggested that comparing to zero is, in fact, a common use case (evidence suggest that the numpy functions are often used with zero). In this case, it would be desirable to have a “useful” default. Values around 1e-8 were suggested, being about half of floating point precision for values of around value 1.
But then the PEP explains why in the end it chooses not to guess what the users most likely want, in the spirit of The Zen. I personally think it's better to have a more useful default but I can understand where the PEP's argument is coming from.
(IEEE single-precision floats have 6-9 decimal digits precision.)
Single-precision floats with 24 bits of precision:
>>> math.log10(2**24)
7.224719895935548
>>> n = 1
>>> while ctypes.c_float(1 + 10 ** -n).value != 1:
... n += 1
...
>>> n
8
Most math's functions have no examples, but maybe it worth.
There are a few math functions that do have examples, and since the PEP also mentions that comparing to zero is a rather common use case, I think it is indeed worth adding an example of such a common use case to the doc.
The current abs_tol lead sentence assume a lot of user understanding of the meaning of 'useful for comparisons near zero' and the implication of the (not repeated) 0.0 default, given the function definition in the next paragraph: abs(a-b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol). I suggest replacing
abs_tol is the minimum absolute tolerance – useful for comparisons near zero. abs_tol must be at least zero.
abs_tol is the minimum absolute tolerance; it must be at least 0.0. When comparing to 0.0, the default abs_tol of 0.0 means that only 0.0 is close to 0.0. Pass a larger tolerance to allow for floating point inaccuracies.
>>> from math import sin, pi, isclose
>>> sin(pi)
1.2246467991473532e-16 # May vary slightly on different systems.
>>> isclose(sin(pi), 0.0)
False
>>> isclose(sin(pi), 0.0, abs_tol=1e-15) # Define 'close to zero' as needed or desired.
True
@skirpichev To me a negative rel_tol makes no sense. The function returns abs(a-b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol). Changing that would require a PEP.
I'm not proposing that nonsense. Rather we should adjust the error check (now it's common for rel_tol and abs_tol) per documentation. It should be rel_tol <= 0 -> raises rather than rel_tol < 0 -> raises, as it's now.
Sorry, I was mislead by 'docs also slightly inaccurate', implying to me 'doc should be changed'. I now understand you saying 'doc slightly conflicts with implementation, and doc is correct'. rel_tol = 0 with default abs_tol = 0 make isclose same as ==. Do you approve of proposed doc change?
Activity
skirpichev commentedon Oct 30, 2024
State what, eh? the math.isclose() docstring has a complete description of parameters and their meaning. Ditto the sphinx docs: https://docs.python.org/3.14/library/math.html#math.isclose.
I fail to see any arguments for this in your text. "Do as numpy does"?
In the PEP 485 the default value was chosen because: "there is no value that is appropriate for the general case. It is impossible to know an appropriate value without knowing the likely values expected for a given use case. If all the values tested are on order of one, then a value of about 1e-9 might be appropriate, but that would be far too large if expected values are on order of 1e-9 or smaller."
I'll relabel this as docs issue. Please describe how you would like to improve sphinx docs/docstrings.
If you are going for change defaults - this will be a huge compatibility break. I think this should be discussed first on https://discuss.python.org/.
Xiaokang2022 commentedon Oct 30, 2024
Well, it was me who was negligent. Thank you for pointing out my problem.
blhsing commentedon Oct 30, 2024
Personally I think a default value should be set with the most common or most intuitive use cases in mind, and the OP's example of
math.isclose(math.sin(math.pi), 0) is Falsedemonstrates well why the current default value of theabs_tolparameter doesn't meet that expectation.I find
numpy.isclose's default value of1e-08foratolto be a reasonable choice because it is one digit more precise than a single-precision floating point number.But of course, as @skirpichev pointed out, changing the default value at this point would cause backwards compatibility issues, so it's probably best that we make
math.isclose(math.sin(math.pi), 0, abs_tol=1e-08)a usage example in the documentation.skirpichev commentedon Oct 30, 2024
Not at all. Maybe it's a valid documentation issue and we can explicitly mention defaults in the docstring/sphinx docs (not just via function signature). I'm -0 on this, but we do such things in some functions, e.g.:
>>> help(math.prod) Help on built-in function prod in module math: prod(iterable, /, *, start=1) Calculate the product of all the elements in the input iterable. The default start value for the product is 1. When the iterable is empty, return the start value. This function is intended specifically for use with numeric values and may reject non-numeric types.So, if you have a concrete suggestion on how to improve docs - feel free to reopen.
skirpichev commentedon Oct 30, 2024
I don't think we can appeal to one single case. What's "most intuitive use cases"?! The PEP has much more generic argumentation.
(IEEE single-precision floats have 6-9 decimal digits precision.)
Why single-precision floats are special here at all? BTW probably now in most implementations Python's floats are double-precision IEEE floats.
Most math's functions have no examples, but maybe it worth.
blhsing commentedon Oct 30, 2024
I'll quote the PEP as follows:
https://peps.python.org/pep-0485/#absolute-tolerance-default
But then the PEP explains why in the end it chooses not to guess what the users most likely want, in the spirit of The Zen. I personally think it's better to have a more useful default but I can understand where the PEP's argument is coming from.
Single-precision floats with 24 bits of precision:
There are a few math functions that do have examples, and since the PEP also mentions that comparing to zero is a rather common use case, I think it is indeed worth adding an example of such a common use case to the doc.
skirpichev commentedon Oct 30, 2024
This is an example with IEEE doubles.
Details
Edit:
Try that with CPython's float's;)
Feel free to open a pr.
terryjreedy commentedon Oct 30, 2024
The current abs_tol lead sentence assume a lot of user understanding of the meaning of 'useful for comparisons near zero' and the implication of the (not repeated) 0.0 default, given the function definition in the next paragraph:
abs(a-b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol). I suggest replacinghttps://docs.python.org/3.14/library/math.html#math.isclose
with the more explicit:
3 remaining items
terryjreedy commentedon Oct 31, 2024
@skirpichev To me a negative rel_tol makes no sense. The function returns
abs(a-b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol). Changing that would require a PEP.skirpichev commentedon Oct 31, 2024
I'm not proposing that nonsense. Rather we should adjust the error check (now it's common for rel_tol and abs_tol) per documentation. It should be
rel_tol <= 0 -> raisesrather thanrel_tol < 0 -> raises, as it's now.terryjreedy commentedon Oct 31, 2024
Sorry, I was mislead by 'docs also slightly inaccurate', implying to me 'doc should be changed'. I now understand you saying 'doc slightly conflicts with implementation, and doc is correct'. rel_tol = 0 with default abs_tol = 0 make isclose same as ==. Do you approve of proposed doc change?
skirpichev commentedon Oct 31, 2024
On another hand with abs_tol>0 - rel_tol=0 does make sense...
#126165 (comment)
?
See PR.
hauntsaninja commentedon Nov 2, 2024
Strong +1 on not making rel_tol=0 raise. rel_tol=0 abs_tol>0 is indeed useful.
gh-126165: Improve docs of function `math.isclose` (#126215)
pythongh-126165: Improve docs of function `math.isclose` (pythonGH-12…
pythongh-126165: Improve docs of function `math.isclose` (pythonGH-12…
[3.13] gh-126165: Improve docs of function `math.isclose` (GH-126215) (…
[3.12] gh-126165: Improve docs of function `math.isclose` (GH-126215) (…
skirpichev commentedon Nov 4, 2024
Unfortunately, merged commit lacks note about rel_tol. We should document it's nonnegative.
terryjreedy commentedon Nov 4, 2024
Checkin diff says "rel_tol must be greater than zero." was changed to "rel_tol must be nonnegative and less than
1.0." in both places.pythongh-126165: Improve docs of function `math.isclose` (python#126215)
pythongh-126165: Improve docs of function `math.isclose` (python#126215)