3
\$\begingroup\$

I've used Stirling's approximation to compute a factorial but I want to know if it is a cleaner and more concise way to solve this kind of problem.

Here is my code:

from numba import jit
from decimal import Decimal
from math import pi

getcontext().prec = 128

@jit
def stirling(n):
    return Decimal((2.0*n+1.0/3.0)*pi).sqrt()*Decimal(n**n)/Decimal(n).exp()

if __name__ == "__main__":
    print(stirling(255144))

Also , how can I get rid off decimal.Overflow error?

\$\endgroup\$

closed as off-topic by Peilonrayz, Ludisposed, Vogel612, Peter Taylor, Graipher Dec 18 '17 at 15:58

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – Peilonrayz, Ludisposed, Vogel612, Peter Taylor, Graipher
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 6
    \$\begingroup\$ The code as currently provided raises "decimal.Overflow: [<class 'decimal.Overflow'>]". Since you ask "how can I get rid off decimal.Overflow error?" this is not working as you intend. And so is off-topic. \$\endgroup\$ – Peilonrayz Dec 18 '17 at 13:03
  • \$\begingroup\$ I don't think you need the .0s on the ends of the numbers in stirling \$\endgroup\$ – 13ros27 Dec 18 '17 at 13:14
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Post a follow-up question instead. \$\endgroup\$ – Mast Dec 18 '17 at 16:42
8
\$\begingroup\$

The biggest problem of your code (while being very short itself) is that hard to read and complicated formula. One way to improve readability of the formula is by using meaningful variables to name parts of the formula. This also makes it easier to find errors, because you can first look in what part of the formula the error occurs, and debug more easily.

Furthermore, adding spaces around operators inside the formula makes it more readable.

However, Python already has a solution for that in the standard library. Since your question implies that your goal is not to implement Stirling's approximation, but rather a pythonic solution, I suggest the following:

from math import factorial

print(factorial(1337))

This is probably the most pythonic way of doing it. Using math.factorial from the standard library, your code becomes more readable and you get around hard to find errors by avoiding complicated formulas.

\$\endgroup\$
2
\$\begingroup\$

While D. Everhard's answer gives good tips for making your code more readable, I think he misses the purpose of your code. Stirling's formula provides a good approximation for factorials when the operand is very large. Unless math.factorial applies Stirling's approximation for large n, it will likely overflow much sooner than your code as n increases.

Regarding the overflow error, factorials of large numbers are very large, and you are likely exceeding the maximum value that a decimal.Decimal can support. Here is a solution I have used previously using numpy.longdouble. Note there are still large values of n where this code will break.

from math import factorial, pow, sqrt, pi, e
import numpy as np

def stirling_factorial(n):
    if n == 0:
        return 1
    else:
        return np.longdouble(sqrt(2*pi*n))*np.power(np.longdouble(n/e), np.longdouble(n))
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.