3
\$\begingroup\$

I have this function called error, which estimates value based on previous value and rewards in trials. The difference between the estimated value and true value is then calculated, giving the error. I need to run this code 1000 times with different lists of trials, but currently it runs too slowly. How would I optimize this code?

Additional information: alpha is a parameter. Reward and true_value are lists. Value always starts at 0.5, but I remove this starting number before calculating error

def loopfunction(alpha, reward, true_value):
    difference = []
    value = [0.5]
    for index, lr in enumerate(reward):
        value.append(lever_update(alpha, value[-1], reward[index]))
    value.pop(0)
    zip_object = zip(value, true_value)
    for value_i, true_value_i in zip_object:
        difference.append(value_i-true_value)
    absdiff = [abs(number) for number in difference]
    
    return absdiff

The lever_update function, which calculates value is here:

def lever_update(alpha, value, reward):
   value += alpha * (reward - value) 
return(value)
\$\endgroup\$
1
  • 1
    \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How to Ask for examples, and revise the title accordingly. \$\endgroup\$
    – Mast
    May 8 '21 at 10:26
1
\$\begingroup\$
  1. Using list comprehensions instead of numerous .append calls is often times faster and more readable.

  2. A list of rewards should be called rewards instead of reward. Same goes for values and true_values.

  3. We don't need to enumerate rewards, since we don't need the index itself. We can simply iterate over rewards.

    def loopfunction(alpha, rewards, true_values):
    
        values = [0.5]
        for reward in rewards:
            values.append(lever_update(alpha, values[-1], reward))
    
        values.pop(0)
    
        return [abs(value - true_value) for value, true_value in zip(values, true_values)]
    

I sadly can't test performance right now, but I would assume the first for loop takes up most of your runtime. This is what I'd focus on for further improvements. In general, making code more concise and readable also allows for easier refactoring / performance improvements in the future.


We can also make lever_update more concise:

def lever_update(alpha, value, reward):
   return value + alpha * (reward - value)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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