Bug while implementing Monte Carlo Markov Chain in Python

Question

I'm trying to implement a simple Markov Chain Monte Carlo in Python 2.7, using numpy. The goal is to find the solution to the "Knapsack Problem," where given a set of m objects of value vi and weight wi, and a bag with holding capacity b, you find the greatest value of objects that can be fit into your bag, and what those objects are. I started coding in the Summer, and my knowledge is extremely lopsided, so I apologize if I'm missing something obvious, I'm self-taught and have been jumping all over the place.

The code for the system is as follows (I split it into parts in an attempt to figure out what's going wrong).

import numpy as np
import random


def flip_z(sackcontents): 
    ##This picks a random object, and changes whether it's been selected or not.
    pick=random.randint(0,len(sackcontents)-1)
    clone_z=sackcontents
    np.put(clone_z,pick,1-clone_z[pick])
    return clone_z

def checkcompliance(checkedz,checkedweights,checkedsack):
    ##This checks to see whether a given configuration is overweight
    weightVector=np.dot(checkedz,checkedweights)
    weightSum=np.sum(weightVector)
    if (weightSum > checkedsack):
        return False
    else:
        return True

def getrandomz(length):
    ##I use this to get a random starting configuration.
    ##It's not really important, but it does remove the burden of choice.       
    z=np.array([])
    for i in xrange(length):
        if random.random() > 0.5:
            z=np.append(z,1)
        else:
            z=np.append(z,0)
    return z

def checkvalue(checkedz,checkedvalue):
    ##This checks how valuable a given configuration is.
    wealthVector= np.dot(checkedz,checkedvalue)
    wealthsum= np.sum(wealthVector)
    return wealthsum

def McKnapsack(weights, values, iterations,sack): 
    z_start=getrandomz(len(weights))
    z=z_start
    moneyrecord=0.
    zrecord=np.array(["error if you see me"])
    failures=0.
    for x in xrange(iterations):
        current_z= np.array ([])
        current_z=flip_z(z)
        current_clone=current_z
        if (checkcompliance(current_clone,weights,sack))==True:
            z=current_clone
            if checkvalue(current_z,values)>moneyrecord:
                moneyrecord=checkvalue(current_clone,values)
                zrecord= current_clone
        else:failures+=1
    print "The winning knapsack configuration is %s" %(zrecord)
    print "The highest value of objects contained is %s" %(moneyrecord)

testvalues1=np.array([3,8,6])
testweights1= np.array([1,2,1])

McKnapsack(testweights1,testvalues1,60,2.)

What should happen is the following: With a maximum carrying capacity of 2, it should randomly switch between the different potential bag carrying configurations, of which there are 2^3=8 with the test weights and values I've fed it, with each one or zero in the z representing having or not having a given item. It should discard the options with too much weight, while keeping track of the ones with the highest value and acceptable weight. The correct answer would be to see 1,0,1 as the configuration, with 9 as the maximized value. I get nine for value every time when I use even moderately high amounts of iterations, but the configurations seem completely random, and somehow break the weight rule. I've double-checked my "checkcompliance" function with a lot of test arrays, and it seems to work. How are these faulty, overweight configurations getting past my if statements and into my zrecord ?

Answer 1

The trick is that z (and therefore also current_z and also zrecord) end up always referring to the exact same object in memory. flip_z modifies this object in-place via np.put.

Once you find a new combination that increases your moneyrecord, you set a reference to it -- but then in the subsequent iteration you go ahead and change the data at that same reference.

In other words, lines like

current_clone=current_z
zrecord= current_clone

do not copy, they only make yet another alias to the same data in memory.

One way to fix this is to explicitly copy that combination once you find it's a winner:

if checkvalue(current_z, values) > moneyrecord:
    moneyrecord = checkvalue(current_clone, values)
    zrecord = current_clone.copy()