Further Optimisation of Project Euler problem 14 (Collatz Sequence)

Question

When I first starting trying the question, my code would take over a minute to even finish running and give me the answer. I have already tried dynamic programming and storing previous numbers so it doesn't have to run the same number multiple times. I have also tried compacting (n3)+1 and n / 2 into a single line with ((n3)+1) but both of these has only managed to cut my code to 10 seconds. Is there anything else I can try to speed up my code?

def Collatz(n):
  dic = {a: 0 for a in range(1,1000000)}
  dic[1] = 0
  dic[2] = 1
  number,length = 1,1
  for i in range(3,n,1):
    z = i
    testlength = 0
    loop = "T"
    while loop == "T":
      if z % 2 == 0:
        z = z / 2
        testlength += 1
      else:
        z = ((z*3)+1) / 2
        testlength += 2
      if z < i:
        testlength += dic[z]
        loop = "F"
    dic[i] = testlength
    if testlength > length:
      print(i,testlength)
      number,length = i,testlength
  return number,length
print(Collatz(1000000))

Why are you using the strings "T" and "F" instead of the booleans True and False? — Mad Physicist, Jan 18 '22 at 04:06
Alright I have done that and it seem to have helped speed up the ode a bit, thanks. Is there anything else I can do or should change? — HelloEveryone, Jan 18 '22 at 04:09
Take out the print statement. It slows things down massively — Mad Physicist, Jan 18 '22 at 04:22
I have tried taking out the printing every time the largest number updates, but it doesn't seem to speed things up too much(maybe around 1 second faster) and I don't get updates so I'm not too such if the code is running sometimes. Is there anything else or is that everything I can do to speed up the code? — HelloEveryone, Jan 18 '22 at 04:30
I deleted my answer because it's slower than yours. I was recording a list of all the steps I made, and adding them into the dictionary, but that's overkill — Mad Physicist, Jan 18 '22 at 05:00
Oh. So there isnt anything else I can do to make the code run faster? — HelloEveryone, Jan 18 '22 at 05:11
@HelloEveryone : I've improved the speed a bit, making use of some of MattTimmermans' ideas but sticking to a recursive solution. I prefer my solution a bit, in so much as `collatz(n)` returns the length of the sequence for given `n` and the logic of finding the maximum is not inside the function. Also it cuts the time by about 10%. — vorrade -, Feb 06 '22 at 15:10

Matt Timmermans · Answer 1 · 2022-01-18T05:21:19.703

1

When you calculate the sequence for one input, you find out the sequence length for all the intermediate values. It helps to remember all of these in the dictionary so you never have to calculate a sequence twice of any number < n.

I also started at (n-1)//2, since there's no point testing any number x if 2x is going to be tested later, because 2x will certainly have a longer sequence:

def Collatz(n):
    dic = [-1]*n
    dic[1] = 0
    bestlen = 0
    bestval = 1
    q=[]
    for i in range((n-1)//2,n,1):
        q.clear()
        z = i
        while z >= n or dic[z] < 0:
            q.append(z)
            if z % 2 == 0:
                z = z//2
            else:
                z = z*3+1
        testlen = len(q)+dic[z]
        if testlen > bestlen:
            bestlen = testlen
            bestval = i
            print (bestval, bestlen)
        for j in range(0,len(q)):
            z = q[j]
            if z < n:
                dic[z] = testlen-j
    return bestval, bestlen

print(Collatz(1000000))

edited Jan 18 '22 at 05:21

answered Jan 18 '22 at 05:10

Matt Timmermans

53,709
3
46
87

Did you run `%timeit` on this? – Mad Physicist Jan 18 '22 at 05:13
Interesting. Very similar to my delete approach, but 22% faster than OP's code, while mine was 35% slower. – Mad Physicist Jan 18 '22 at 05:15
Oh I didnt think about that before. Thanks! Also, would it be faster to test x, check if its (n-1)//2, then if it is then have the key 2x have the value +1. So instead of not running x because its going to be run under 2x, you dont run 2x because its just x with a +1 value. – HelloEveryone Jan 18 '22 at 05:33
I don't think that will help – Matt Timmermans Jan 18 '22 at 05:52
@MattTimmermans : Your dictionary is actually an array. – vorrade - Feb 06 '22 at 10:33
@vorrade, yes. An array is a lot faster, and if you have dense integer keys, it takes less space, too. – Matt Timmermans Feb 06 '22 at 14:08

vorrade - · Answer 2 · 2022-02-06T11:08:03.680

Although the answer from Matt Timmermanns is fast, it is not quite as easy to understand as a recursive function. Here is my attempt that is actually faster for n = 10*million and perhaps easier to understand...

f = 10000000

def collatz(n):
    if n>=collatz.bounds:
        if (n % 4) == 0:
            return collatz(n//4)+2
        if (n % 2) == 0:
            return collatz(n//2)+1
        return collatz((3*n+1)//2)+2
    if collatz.memory[n]>=0:
        return collatz.memory[n]
    if (n % 2) == 0:
        count = collatz(n//2)+1
    else:
        count = collatz((3*n+1)//2)+2
    collatz.memory[n] = count
    return count

collatz.memory = [-1]*f
collatz.memory[1] = 0
collatz.bounds = f

highest = max(collatz(i) for i in range(f//2, f+1))
highest_n = collatz.memory.index(highest)

print(f"collatz({highest_n}) is {highest}")

My results:

$ time /usr/bin/python3 collatz.py 
collatz(8400511) is 685

real    0m9.445s
user    0m9.375s
sys     0m0.060s

Compared to

$ time /usr/bin/python3 mattsCollatz.py 
(8400511, 685)

real    0m10.672s
user    0m10.599s
sys     0m0.066s

Further Optimisation of Project Euler problem 14 (Collatz Sequence)

2 Answers2

Linked