Greggan
Greggan

Reputation: 11

How to structure python programs? Tried making it more structured, now runs 13 times slower

Im very new to programming, I wrote a simple program for a school project and wanted to make the code "prettier" by not just having the program be one giant function but instead be made up of multiple smaller functions with a singe purpose. I seemed to have messed up royally since the program now runs 13 times slower. How should I structured the program to make it run faster and just in general make programs easier to write, read and edit?

Here are the two programs:

First program (for reference values runs in ≈0:20):

import numpy as np 
import matplotlib.pyplot as plt

def graf(a,b,H,p):
    GM =  39.5216489684
    x_0 = a + np.sqrt(a**2 - b**2)
    v_0 = np.sqrt(GM*(2/x_0 - 1/a))
    konstant_period = np.sqrt(a**3)*H
    h = 1/H

    '''starting position given by an elliptic orbit '''
    stor_x_lista = [x_0]
    stor_y_lista = [0]

    hastighet_x = [0]
    hastighet_y = [v_0]

    liten_x_lista = []
    liten_y_lista = []

    ''' a loop that approximates the points of the orbit'''

    t = 0
    tid_lista = []
    n = 0
    while n < konstant_period:
        hastighet_x.append(hastighet_x[n] - h*GM* stor_x_lista[n]/(np.sqrt(stor_x_lista[n]**2 + stor_y_lista[n]**2))**3)
        stor_x_lista.append(stor_x_lista[n] + h*hastighet_x[n])

        hastighet_y.append(hastighet_y[n] - h*GM*stor_y_lista[n]/(np.sqrt(stor_x_lista[n]**2 + stor_y_lista[n]**2))**3)
        stor_y_lista.append(stor_y_lista[n] + h*hastighet_y[n])

    '''smaller list of points to run faster'''
    if n % p == 0:
            liten_x_lista.append(stor_x_lista[n])
            liten_y_lista.append(stor_y_lista[n])
            tid_lista.append(t)

    n += 1    
    t += h 
    ''' function that finds the angle'''
    vinkel = []
    siffra = 0
    while siffra < len(liten_x_lista):
        if liten_y_lista[siffra ] >= 0:
            vinkel.append( np.arccos( liten_x_lista[siffra]/np.sqrt( liten_x_lista[siffra]**2 + liten_y_lista[siffra]**2)))
            siffra += 1

        elif liten_y_lista[siffra] < 0 :
            vinkel.append( np.pi + np.arccos( -liten_x_lista[siffra]/np.sqrt( liten_x_lista[siffra]**2 + liten_y_lista[siffra]**2) ))
            siffra += 1

    '''get rid of line to find periodic function'''
    mod_lista = []

    modn = 0
    while modn < len(vinkel):
        mod_lista.append(vinkel[modn] - (2*np.pi*tid_lista[modn])/np.sqrt(a**3))
        modn += 1

    '''make all inputs have period 1'''
    squeeze_tid = []
    squeezen = 0
    while squeezen < len(tid_lista):
        squeeze_tid.append(tid_lista[squeezen]/np.sqrt(a**3))
        squeezen += 1

    del mod_lista[-1:]
    del tid_lista[-1:]
    del squeeze_tid[-1:]

    plt.plot(squeeze_tid,mod_lista)  
    plt.title('p(t) där a = ' + str(a) + ' och b = ' + str(b))
    plt.show               

Second more split up program (for reference values runs in ≈4:20):

import numpy as np 
import matplotlib.pyplot as plt

'''function that generates the points of the orbit'''
def punkt(a,b,H,p):
    GM =  39.5216489684
    x_0 = a + np.sqrt(a**2 - b**2)
    v_0 = np.sqrt(GM*(2/x_0 - 1/a))
    konstant_period = np.sqrt(a**3)*H
    h = 1/H

    '''starting position given by an elliptic orbit '''
    stor_x_lista = [x_0]
    stor_y_lista = [0]

    hastighet_x = [0]
    hastighet_y = [v_0]

    liten_x_lista = []
    liten_y_lista = []

    ''' a loop that approximates the points of the orbit'''
    t = 0
    tid_lista = []
    n = 0
    while n < konstant_period:
        hastighet_x.append(hastighet_x[n] - h*GM* stor_x_lista[n]/(np.sqrt(stor_x_lista[n]**2 + stor_y_lista[n]**2))**3)
        stor_x_lista.append(stor_x_lista[n] + h*hastighet_x[n])

        hastighet_y.append(hastighet_y[n] - h*GM*stor_y_lista[n]/(np.sqrt(stor_x_lista[n]**2 + stor_y_lista[n]**2))**3)
        stor_y_lista.append(stor_y_lista[n] + h*hastighet_y[n])


        '''smaller list of points to run faster'''
        if n % p == 0:
            liten_x_lista.append(stor_x_lista[n])
            liten_y_lista.append(stor_y_lista[n])
            tid_lista.append(t)

        n += 1    
        t += h 

    return (liten_x_lista,liten_y_lista,tid_lista)

''' function that finds the angle'''

def vinkel(a,b,H,p):
    '''import lists'''
    liten_x_lista = punkt(a,b,H,p)[0]
    liten_y_lista = punkt(a,b,H,p)[1]
    tid_lista = punkt(a,b,H,p)[2]

    '''find the angle'''
    vinkel_lista = []
    siffra = 0
    while siffra < len(liten_x_lista):
        if liten_y_lista[siffra ] >= 0:
            vinkel_lista.append( np.arccos( liten_x_lista[siffra]/np.sqrt( liten_x_lista[siffra]**2 + liten_y_lista[siffra]**2)))
            siffra += 1

        elif liten_y_lista[siffra] < 0 :
            vinkel_lista.append( np.pi + np.arccos( -liten_x_lista[siffra]/np.sqrt( liten_x_lista[siffra]**2 + liten_y_lista[siffra]**2) ))
            siffra += 1
    return (vinkel_lista, tid_lista)

def periodisk(a,b,H,p):
    '''import lists'''
    tid_lista = vinkel(a,b,H,p)[1]
    vinkel_lista = vinkel(a,b,H,p)[0]

    '''get rid of linear line to find p(t)'''
    mod_lista = []

    modn = 0
    while modn < len(vinkel_lista):
        mod_lista.append((vinkel_lista[modn] - (2*np.pi*tid_lista[modn])/np.sqrt(a**3)))
        modn += 1

    '''make all inputs have period 1'''
    squeeze_tid = []
    squeezen = 0
    while squeezen < len(tid_lista):
        squeeze_tid.append(tid_lista[squeezen]/np.sqrt(a**3))
        squeezen += 1

    del mod_lista[-1:]
    del tid_lista[-1:]
    del squeeze_tid[-1:]

    return (squeeze_tid,mod_lista)

'''fixa 3d-punkt av p(a,b) a är konstant b varierar??? '''

def hitta_amp(a):
    x_b = []
    y_b = []
    n_b = 0.1
    while n_b <= a: 
        x_b.append(n_b)        
        y_b.append(punkt(a,n_b,10**5,10**3))

    return 0

def graf(a,b,H,p):
    plt.plot(periodisk(a,b,H,p)[0],periodisk(a,b,H,p)[1])
    plt.show

I would assume the thing that is going wrong is that the program is running the same, slow code multiple times instead of just running it once and then accessing the data. Is the problem that everything is done locally and nothing is stored globally or is it something else? Just as a heads up, the only thing I know about programming is basic syntax, I have no clue how to actually write and run programs. I ran all the code in spyder if that affects anything.

Upvotes: 1

Views: 62

Answers (2)

Pablo
Pablo

Reputation: 446

You are calling the functions once per returned list, you should only call them once.

When a method returns multiple variables, (e.g. punkt):

def punkt(a,b,H,p):

    # Here is all your code

    return (liten_x_lista,liten_y_lista,tid_lista)

You must be careful to only call the function once:

   result = punkt(a,b,H,p)
   liten_x_lista = result[0]
   liten_y_lista = result[1]
   tid_lista = result[2]

   # As opposed to:
   liten_x_lista = punkt(a,b,H,p)[0] # 1st call, ignoring results 2 and 3
   liten_y_lista = punkt(a,b,H,p)[1] # 2nd call, ignoring results 1 and 3
   tid_lista = punkt(a,b,H,p)[2] # 3rd call, ignoring results 1 and 2

Note: I would personally not return a list, but use python's unpacking:

def punkt(a,b,H,p):

    # Here is all your code

    return liten_x_lista, liten_y_lista, tid_lista

And you'd access it:

   liten_x_lista, liten_y_lista, tid_lista = punkt(a,b,H,p)

Upvotes: 0

h4z3
h4z3

Reputation: 5478

plt.plot(periodisk(a,b,H,p)[0],periodisk(a,b,H,p)[1])

This code runs periodisk twice with the same arguments, thus at this point we know we run things at least 2 times slower.

You should do some_var = periodisk(a,b,H,p) and then some_var[0], some_var[1]. Or just use unpacking:

plt.plot(*periodisk(a,b,H,p))

tid_lista = vinkel(a,b,H,p)[1]
vinkel_lista = vinkel(a,b,H,p)[0]

Again doing the same thing twice (total: 4*time of (current) vinkel function). Again, smart assignment to fix this:

vinkel_lista, tid_lista = vinkel(a,b,H,p)

liten_x_lista = punkt(a,b,H,p)[0]
liten_y_lista = punkt(a,b,H,p)[1]
tid_lista = punkt(a,b,H,p)[2]

And now you repeat yourself thrice. (total: 12 * time of current punkt function)

liten_x_lista, liten_y_lista, tid_lista = punkt(a,b,H,p)

punkt function is like in original, so we arrived as total being 12 times slower - which quite matches your time estimations. :)

Upvotes: 1

Related Questions