Why does my memoization code not save the correct keys with the values?

Question

I'm currently learning dynamic programming with Python to write a rather simple Branch & Bound algorithm. I found this video on Youtube with kind of good explanations in the first place. The examples there are written in JavaScript. Only few changes were necessary to make in run in Python also.

But when I came along to the part with "bestSum" (1:52:26 in video), as soon as I bring the memoization in there comes the problem:

My code doesn't seem to save correct keys with the values in the "memo" dictionary. In the forwarded parts of the videos it worked fine every time but in this case not.

What's the difference here? I tried putting the "memo" code snippet to different parts in the code section but nothing worked.

Here a working Python code with memoization for how to make out possibilities to add up a number from different other numbers:

def howSum(targetSum, numbers, memo={}):
    # print("new howSum")
    if targetSum in memo:
        # print("memo returned")
        return memo[targetSum]
    if targetSum == 0:
        return []
    if targetSum < 0:
        return 0
    
    for num in numbers:
        remainder = targetSum - num
        remainderResult = howSum(remainder, numbers, memo)
        if remainderResult != 0:
            remainderResult.append(num)
            memo[targetSum] = remainderResult
            return remainderResult
    
    memo[targetSum] = 0
    return 0

print(howSum(7, [2, 3], memo={}))
print(howSum(7, [5, 3, 4, 7], memo={}))
print(howSum(7, [2, 4], memo={}))
print(howSum(8, [2, 3, 5], memo={}))

Correct result:

[3, 2, 2]
[4, 3]
0
[2, 2, 2, 2]

And the not working code for how to get the smallest amount of available numbers to add up for a target number.

def bestSum(targetSum, numbers, memo={}):
    if targetSum in memo:
        return memo[targetSum]
    if targetSum == 0:
        return []
    if targetSum < 0:
        return 0
    
    shortestCombination = 0
    
    for num in numbers:
        remainder = targetSum - num 
        remainderCombination = bestSum(remainder, numbers, memo)
        if remainderCombination != 0:
            remainderCombination.append(num)
            combination = remainderCombination
            if shortestCombination == 0 or len(combination) < len(shortestCombination):
                shortestCombination = combination
                memo[targetSum] = shortestCombination
    
    memo[targetSum] = shortestCombination
    return shortestCombination
    
print(bestSum(7, [5, 4, 3, 7]))
print(bestSum(8, [2, 3, 5]))
print(bestSum(8, [1, 4, 5]))

Incorrect results:

[7]
[5, 3]
[5, 3]

To safe place here: if you print out the "memo" dictionary for the function

print(bestSum(8, [1, 4, 5]))

you will get

{..., 8: [4,1,4]}

Telling me the best/shortest possibility to add up 8 out of [1,4,5] is [4,1,4] instead of [4,4]. If I run the code without the memoization section if works absolutely correct, so it's something about the memoization (I guess).

score 0 · Answer 1 · answered Jan 30 '22 at 22:05

A few things that I notice:

combination = remainderCombination is not doing what's intended here. This makes combination an alias for remainderCombination, i.e. changing one will change the other. To make a copy, use copy().
shortestCombination = combination; memo[targetSum] = shortestCombination Setting memo here is unnecessary. memo is a cache for storing the final answers. At this point in the loop, you still haven't decided on a final shortestCombination value, so why set a memoization value?
Check out this article: Mutable Default Arguments
Returning 0 and a list from the same function is not ideal though allowed in Python. You can consider returning None instead of 0. In that case remember that != and is not are two different things in Python.

Taking these into account:

def bestSum(targetSum, numbers, memo):
    if targetSum in memo:
        return memo[targetSum]
    if targetSum == 0:
        return []
    if targetSum < 0:
        return None
    
    shortestCombination = None
    
    for num in numbers:
        remainder = targetSum - num 
        remainderCombination = bestSum(remainder, numbers, memo)

        if remainderCombination is not None:
            combination = remainderCombination.copy()
            combination.append(num)
            
            if (shortestCombination is None) or (len(combination) < len(shortestCombination)):
                shortestCombination = combination

    memo[targetSum] = shortestCombination
    return shortestCombination


print(bestSum(7, [5, 4, 3, 7], {}))
print(bestSum(8, [2, 3, 5], {}))
print(bestSum(8, [1, 4, 5], {}))
print(bestSum(100, [1, 2, 5, 25], {}))

Rather than make the caller of `bestSum` provide an empty dict each time, make `bestSum` a two-argument function that acts as a wrapper around a second, private method that actually handles the dynamic programming: `def bestSum(targetSum, numbers): return _bestSum(targetSum, numbers, {})`. (I assume you are avoiding the standard `... memo=None): if memo is None: memo = {}; ...` trick in order to avoid the conditional on every recursive call. — chepner, Jan 30 '22 at 22:13
Thank you very much. There is still much basic knowlegde I need to learn. I`m very glad there are people like you answering the questions of people like me, trying to learn programming on their own. Your advices already did it for me. — Luca Graebenteich, Mar 28 '22 at 17:36

Why does my memoization code not save the correct keys with the values?

1 Answers1