Lets break down the code and improve the parts of code that is taking so much time.
1-
If you replace if amicable(m, n) == True and m != n:
with if m != n and amicable(m, n) == True:
, it will save you 10000 calls to amicable method (the most expensive method) for which m != n
will be false
.
2- In the amicable
method you are looping 1 to n to find all the factors for both of the numbers. You need a better algorithm to find the factors. You can use the one mentioned here. It will reduce your O(n)
complexity to O(sqrt(n))
for finding factors.
def factors(n):
return set(reduce(list.__add__,
([i, n//i] for i in range(1, int(n**0.5) + 1) if n % i == 0)))
Considering both the points above your code will be
def amicable(a, b):
if sum(factors(a) - {a}) == b and sum(factors(b) - {b}) == a:
return True
return False
sum_of_amicables = 0
for m in range (1, 10001):
for n in range (1, 10001):
if m!= n and amicable(m, n) == True:
sum_of_amicables = sum_of_amicables + m + n
This final code took 10 minutes to run for me, which is half the time you have mentioned.
I was further able to optimize it to 1:30 minutes by optimizing factors
method.
There are 10000 * 10000 calls to factors
method. And factors
is called for each number 10000 times. That is, it calculates factors 10000 times for the same number. So we can optimize it by caching the results of previous factors calculation instead of calculating them at every call.
Here is how I modified factors
to cache the results.
def factors(n, cache={}):
if cache.get(n) is not None:
return cache[n]
cache[n] = set(reduce(list.__add__,
([i, n//i] for i in range(1, int(n**0.5) + 1) if n % i == 0)))
return cache[n]
Full Code: (Runtime 1:30 minutes)
So the full and final code becomes
def factors(n, cache={}):
if cache.get(n) is not None:
return cache[n]
cache[n] = set(reduce(list.__add__,
([i, n//i] for i in range(1, int(n**0.5) + 1) if n % i == 0)))
return cache[n]
def amicable(a, b):
if sum(factors(a) - {a}) == b and sum(factors(b) - {b}) == a:
return True
return False
sum_of_amicables = 0
for m in range (1, 10001):
for n in range (1, 10001):
if m!= n and amicable(m, n) == True:
sum_of_amicables = sum_of_amicables + m + n
You can still further improve it.
Hint: sum
is also called 10000 times for each number.