Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm interested in whether there is a way to further improve a "fast" version of a function used in a homework assignment I received recently (I've already submitted the completed work).

from math import log

def func_fast(mass, density):
    return sum(map((log(mass * density)).__truediv__, range(1,10001)))

def func_slow(mass, density): 
    total = 0.0 
    for i in range(10000): 
        masslog = log(mass * density) 
        total += masslog/(i+1)

    return total

mass = 2.5 
density = 12.0

The fast version times in around 2-2.5ish seconds while the slow version nets 6-7 seconds.

share|improve this question
1  
Your numbers (performance ones) do not add up. When I run those functions I get results in 0.009 to 0.025 seconds depending on the run... My linux box (2 yr old), is not that much faster than yours.... is it? –  rolfl Jan 8 at 16:25

2 Answers 2

This continues from Jaime's post.

This can be trivially sped up to \$\mathcal{O}(1)\$ by caching sum(1/j for j in range(1, 10001)) = 9.787606036044348. Then one just does

def func_O1(mass, density):
    # 9.7876... = sum(1/j for j in range(1, 10001))
    return log(mass * density) * 9.787606036044348

This gives a speedup by a factor of 3000.

If the limit (10000) is varied, use an approximation for the harmonic series:

from math import log

def harmonic(n):
    gamma = 0.57721566490153286060651209008240243104215933593992
    return gamma + log(n) + 0.5/n - 1./(12*n**2) + 1./(120*n**4)

def func_fastest(mass, density):
    return log(mass * density) * harmonic(10000)

harmonic is inaccurate below \$n \approx 120\$ so it would make sense to cache exact values up to harmonic(120). Note that whilst this is an approximation, for \$n\$ greater than \$120\$ it's exact \$\pm 1\text{eps}\$ whereas the summation gets steadily worse.

This only speeds up the function by a factor of 500.

share|improve this answer
1  
I don't hear "only by a factor of 500" too often, made my day! ;-) I suppose the divisions are what's taking the most time in your harmonic call, so it may not make much of a difference, but the premature optimizer in me would rewrite that as n2 = n*n; n4 = n2*n2 and use those in the series expansion. Or if willing to give up some numerical precision, even better, 1_2n = 1 / (2*n); 1_4n2 = 1_2n*1_2n; 1_16n4 = 1_4n2*1_4n2 and then hardcode the decimal expansions of 1/3 and 2/15 for the last two terms. –  Jaime Jan 8 at 20:18
1  
@Jaime Best I could figure out is ((6.*n - 1.) * n2 + 0.1) / (12. * n4) (courtesy of WolframAlpha). That makes it more like a factor of ~1000. I'll think I'll leave it in the comments, though! –  Veedrac Jan 8 at 20:49
    
Amazing. Well done! –  gaborous May 22 at 18:25

The speed-up you are seeing is mostly from removing the calculation of the logarithm from inside the inner loop, and instead reusing the value. This performs only about 10% slower than your fast_func, and is infinitely more readable:

def func_slow(mass, density):
    total = 0
    masslog = log(mass*density)
    for j in range(1, 10001):
        total += 1. / j
    return masslog * total

And if you want to squeeze those last drops of performance, the speed of the following code is marginally, if at all, slower than fast_func, and is also much more Pythonic and readable:

def func(mass, density):
    return log(mass * density) * sum(1/j for j in range(1, 10001))

While they stayed in the language, and are therefore fair game, reading Guido's 10 year old essay on how he planned to get rid of reduce, map, lambda and filter for Python 3 will probably have you looking at those a little differently: I for one always try to come up with more Pythonic alternatives when tempted to use them.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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