| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Probably better to describe this as a feature request rather than a bug: there's never been any intention to support things other than int, and if this were implemented it wouldn't be something that should be backported to older Python versions.
That said, math.log does seem to be an outlier here; almost everything else in mathmodule.c that does explicit integer-handling does use PyNumber_Index.
There are several large groups of function in math: these which work with real numbers and convert arguments to C double using PyFloat_AsDouble(), these which work with integer numbers and use PyNumber_Index(), and these which call a special method (like __ceil__). math.log is an outlier because it works with real numbers, but has also a special case for integers to support values larger than the maximal Python float.
If we make math.log supporting types with __index__, we should do this in other functions too. The simplest way is to make PyFloat_AsDouble() using nb_index as a fallback if nb_float is not provided. The float() constructor does this.
If we make math.log supporting types with __index__, we should do this in other functions too. The simplest way is to make PyFloat_AsDouble() using nb_index as a fallback if nb_float is not provided.
I'm not sure about this. PyFloat_AsDouble needs to return a double so there is no advantage in trying __index__ over __float__. I think it is better for types to provide __float__ if that is what they want. Any type that already defines __index__ can easily define __float__ if desired.
The problem for math.log(gmpy2.mpz(...)) is that the calculation is more accurate/complete for an integer rather than a float. In particular __index__ needs to be given a separate codepath rather than being used as a fallback in float conversion. For functions that actually want to handle integers and floats separately it is more useful if PyFloat_AsDouble does not convert integers into floats. Then PyNumber_Index can be tried as a fallback with an integer handling codepath.
The simplest way is to make PyFloat_AsDouble() using nb_index as a fallback if nb_float is not provided.
I'm confused. Doesn't it do that already?
>>> class A: ... def __index__(self): return 13 ... >>> from math import cos >>> cos(A()) 0.9074467814501962
Oh, I overlooked this.
Then this issue is more like a bug.
>>> import math
>>> math.log10(10**1000)
1000.0
>>> class A:
... def __index__(self): return 10**1000
...
>>>
>>> math.log10(A())
Traceback (most recent call last):
File "<python-input-6>", line 1, in <module>
math.log10(A())
~~~~~~~~~~^^^^^
OverflowError: int too large to convert to float
Then this issue is more like a bug.
True - it's definitely inconsistent that math.log works for arbitrarily large ints and for small integer-likes, but not for large integer-likes. I'm still not convinced that we would want to backport a change to 3.12 and 3.13, though.
#121011 is a simple solution for this inconsistency. It is not optimal for large int-like argument, because some work is done twice. Making it more efficient in this corner case needs inlining PyFloat_AsDouble and math_1. I am not sure that it is worth it.
There are other functions in the math for which we can compute a finite result for large integer argument instead of raising an OverflowError: acosh, asinh, atan, atan2, cbrt, copysign, erf, erfc, log1p, sqrt, tanh. cmath.log doesn't have a special case for large int. If we go this way, it is a long way.
If we go this way, it is a long way.
Agreed; I don't think we want to go this way at all. If the large int support for math.log weren't already there, I don't think there'd be a strong case for adding it. IIUC, many of the originally-intended use-cases would now be covered by something like int.bit_length.
I don't think there'd be a strong case for adding it
Then, maybe it's a good opportunity to deprecate this support?
(History: 7852616)
Maybe if there is a special integer handling path in math.log it should check for index before float
@oscarbenjamin, I'm just curious if you worry only about inconsistency (wrt builtin ints) or had in mind some applications for this feature that couldn't be realized with existing int's methods?
I'm just curious if you worry only about inconsistency (wrt builtin ints) or had in mind some applications for this feature that couldn't be realized with existing int's methods?
I ran into a bug in SymPy where under certain conditions math.log is called with integers but the arguments might be gmpy2.mpz or flint.fmpz which would then overflow as above. What I realised then though is that under more typical conditions this is implicitly converting mpz to float meaning different behaviour for mpz vs int. It is not clear to me if the original authors of this code in SymPy knew that math.log had a special path for handling integers (the code perhaps predates that feature).
The code in question could probably use .bit_length() instead although I haven't reviewed in detail to see how straight-forward that would be. You can see an example here and there are many other calls to log and log2 in this file:
https://github.com/sympy/sympy/blob/a9a6f150383de85a70a19e91e88bca40ceb093ba/sympy/ntheory/factor_.py#L866
That particular line uses int(math.log(B, p)). Possibly it is impossible for rounding errors in float(B) or float(p) to result in the call to int rounding what should be an integer downwards but I haven't analysed that in detail:
>>> print([int(math.log(float(10**(2*n)), float(10**(n)))) for n in range(10,100)])
[2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2]Compared to the status quo I would prefer if either math.log raised an error always for mpz or otherwise just converted all inputs (including int) to float so that behaviour for mpz and int is equivalent. If mpz is not going to be handled the same as int then from my perspective it is invalid to pass an mpz in place of an int and so a conversion is needed before calling math.log. This is awkward though because it means a lot of mostly redundant conversions everywhere.
I would also be happier making use of a function with a different name whose stated purpose was to compute logs accurately with large integers and e.g. having a documented guarantee that ilog(a,b) gives integer values when b is an exact integer power of a or perhaps returning an upper or lower bound for log(a,b) as an integer. SymPy has such a function:
>>> integer_log(100, 10)
(2, True)
>>> integer_log(101, 10)
(2, False)I assume this is not used in the code shown because the integers are expected to be small and math.log is faster. I actually recently searched for a related function because of a discussion elsewhere and found this:
https://stackoverflow.com/questions/39281632/check-if-a-number-is-a-perfect-power-of-another-number
Apparently the top-rated answer for how to find if one integer is a power of another in Python is to use math.log. The docs don't say anything about this feature but it is advertised on StackOverflow by the authors instead. I would prefer that the docs clearly state:
Apparently PyPy and CPython give different results here:
>>>> math.log(10**1000).hex()
'0x1.1fd2b914f1517p+11'
>>> math.log(10**1000).hex()
'0x1.1fd2b914f1516p+11'Should that be considered a bug? CPython's behaviour I presume does not depend on libm here so differences in the C math library are not the cause of the difference.
It seems that PyPy also has:
>>>> math.log(10**100, 10**10000)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OverflowError: int too large to convert to floatSo I guess it does not have this feature.
If we were starting from scratch here I would say that math.log should always use floats and a separate function should be used for integers. I guess we are too far down this road to do that though and this feature needs to be kept now.
Since there is apparently a feature that math.log handles large integers as a special case I would prefer that integer-like types use that path as well rather than being treated like floats. I don't like the idea that accuracy or correctness is silently affected by passing types that should usually be interchangeable. We can't just duck-type over the difference between integers and floats by calling __float__. In general if a function accepts arbitrary types as inputs and wants to separate integers and floats then __index__ is the way to do that.
in the absence of a compelling reason not to, when working with giant ints I favor matching what gmpy2 does.
JFR, in gmpy2 log*() functions are just wrappers for appropriate MPFR/MPC functions.
Like Python's, they also accept giant ints.
That works in a slightly different way (and I'm not sure if the bigfloat package works like that). All gmpy2 functions accept giant ints, because they are converted exactly (not using current context settings, like precision, exponent bounds, etc):
>>> from gmpy2 import *
>>> set_context(ieee(64))
>>> get_context()
context(precision=53, real_prec=Default, imag_prec=Default,
round=RoundToNearest, real_round=Default, imag_round=Default,
emax=1024, emin=-1073,
subnormalize=True,
trap_underflow=False, underflow=False,
trap_overflow=False, overflow=False,
trap_inexact=False, inexact=False,
trap_invalid=False, invalid=False,
trap_erange=False, erange=False,
trap_divzero=False, divzero=False,
allow_complex=False,
rational_division=False,
allow_release_gil=False)
>>> log(10**1000)
mpfr('2302.5850929940457')
>>> mpfr(10**1000)
mpfr('inf')
>>> # WAT? Here is what happens under the hood:
>>> mpfr(10**1000, precision=1)
mpfr('10000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000.0',3322)
>>> log(_)
mpfr('2302.5850929940457')In that sense gmpy2 has foolish consistency.
By "matching" I mean API much more than implementation, but your point stands. For example, gmpy2.sin(10**50000) also works fine. Of course Python doesn't have potentially unbounded floats under the covers, so won't/shouldn't ever support that, although that could fly as a new sin function in the decimal module
Serhiy's suggested alternative for math.log(large integer) is not suitable because it doesn't actually work for large integers
@oscarbenjamin, I forgot to mention that you shouldn't take Serhiy's example literally. Rather, it could be like this:
def mylog(n):
nbits = n.bit_length()
# use integer division, rather multiplication to a float:
return math.log(n/(1<<nbits)) + nbits/math.log2(math.e)This should work in same range as current math.log.
I think that this fits another use case, mentioned by @tim-one:
It's not true that all uses want only the integer part. The result of log10(n) not only gives a clue about the number of decimal digits, but also about what the leading digits of n are.
From my quick test, above simple version looks not much worse than the current math.log (stats taken for randint(2, 2**n), where n=1000 or 10**8), esp. for bit_length >> 1000:
===== log for 1000 ================= math.log mylog================ -1.00 0 0.00% 185 4.52% +0.00 4094 99.95% 2843 69.41% +1.00 1 0.02% 1067 26.05% ===== log for 100000000 ================= math.log mylog================ -1.00 1563 38.16% 773 18.87% +0.00 2441 59.59% 2522 61.57% +1.00 91 2.22% 800 19.53%
# a.py
import math, mpmath
from collections import defaultdict
from random import randint, seed
from math import ulp, floor
from sys import argv
ULP_SCALE = 2.0
bins1 = defaultdict(int)
bins2 = defaultdict(int)
name = 'log'
ref = mpmath.log
lib = math.log
def lib2(n):
nbits = n.bit_length()
return math.log(n/(1<<nbits)) + nbits/math.log2(math.e)
seed(1)
count = 1
nbits = eval(argv[1])
while count & 0xfff:
arg = randint(2, 2**nbits)
with mpmath.extraprec(100):
try:
got1 = lib(arg)
except:
continue
expected = float(mpmath.autoprec(ref)(arg))
got2 = lib2(arg)
ULP = ulp(expected)
diff1 = (got1 - expected)/ULP
diff2 = (got2 - expected)/ULP
diff1 = floor(float(diff1) * ULP_SCALE)
diff2 = floor(float(diff2) * ULP_SCALE)
bins1[diff1] += 1
bins2[diff2] += 1
count += 1
header = f"===== {name} for {nbits} ================="
print(f"{header:=<35}", flush=True)
print(f"{'math.log':<20} {'mylog':=<21}", flush=True)
for k in sorted(set(bins1) | set(bins2)):
v1 = bins1[k]
v2 = bins2[k]
print(f"{k/ULP_SCALE:+5.2f} {v1:7} "
f"{v1/count:6.2%} {v2:7} {v2/count:6.2%}",
flush=True)I think that with the PEP 791 - this issue might be postponed. The ilog2() looks to be a good candidate for this module. But given above, I agree with Serhiy that current logic for integers in loghelper() should be deprecated.
The ilog() (like SymPy's integer_log()) might be a reasonable addition, but I doubt it's can be easily implemented efficiently, e.g.:
https://github.com/sympy/sympy/blob/2e7baea39cd0d891433bdc2ef2222e445c381ca3/sympy/core/intfunc.py#L131-L134
I have merged my PR. Now, if we add math.integer.ilog(), we could deprecate support of large integers in math.log(). But only many years after adding the former().
If you mean for ilog to be a function from int to float then I don't think it belongs in the math.integer module. If there are mixed mode functions then it is better to organise them based on the output type rather than input type.
What would make sense in the math.integer module is a 2-argument log function log: Z x Z -> Z that computes something like floor(log(a, b)).
If you mean for ilog to be a function from int to float then I don't think it belongs in the math.integer module.
In principle, PEP doesn't specify the output type, it's intentional. Floats are tricky because of (possible) dependency on hardware, but the constraints are 1) input type 2) behavior (exact computations).
I meant an int to int function, like isqrt. This is why prefix i.
For integers we usually need to know an exact lower or upper integer bound, so using an itermediate float is not safe.
Thanks for fixing this Serhiy.
I assume by int to int you mean
math.integer.ilog(a, b) -> floor(log(a, b))
If so then I agree that function should go in the math.integer module. I'm not sure it can replace math.log(large integer) -> float in general though.
| Back | FazBrowse Home | New Git URL |
Bug report
Bug description:
The math.log function has a special path for integers that only works for the builtin int type:
cpython/Modules/mathmodule.c
Lines 2220 to 2223 in ac61d58
That means that it does not work for 3rd party integer types e.g.:
Maybe if there is a special integer handling path in math.log it should check for __index__ before __float__.
Related gh-106502 is about math.log with Decimal.
CPython versions tested on:
3.13
Operating systems tested on:
No response
Linked PRs