Implementing branch and bound for knapsack

Question

I'm having a headache implementing this (awful) pseudo-java code (I wonder: why the hell people do that?) for the b&b knapsack problem. This is my implementation so far, which outputs a maximum of 80 (when it should print 90, for the items on the textbook sample). I created a Comparator (on a LinkedList) to sort the elements by Pi/Wi before passing them to the algorithm, but on this input is already presorted. I'm debugging right now (and updating the posted code), cause I guess it's an array indexing problem... or is there a mistake on the bounding function?

input:

4 16 //# items maxWeight
40 2 // profit weight
30 5
50 10
10 5


class Node
{
    int level;
    int profit;
    int weight;
    double bound;

}

public class BranchAndBound {
    static int branchAndBound (LinkedList<Item> items, int W) {
        int n = items.size();

        int [] p= new int[n];
        int [] w= new int[n];

        for (int i=0; i<n;i++){

            p [i]= (int)items.get(i).value;
            w [i]= (int)items.get(i).weight;

        }

        Node u = new Node();
        Node v = new Node(); // tree root

        int maxProfit=0;

        LinkedList <Node> Q = new LinkedList<Node>();

        v.level=-1;
        v.profit=0;
        v.weight=0; // v initialized to -1, dummy root

        Q.offer(v); // place the dummy at the root

        while(!Q.isEmpty()){
            v = Q.poll();

            if (v.level==-1){
              u.level=0;
            }
            else if(v.level != (n - 1))
            {
               u.level = v.level+1; // set u to be a child of v
            }

            u = new Node();
            u.weight = v.weight + w[u.level];// set u to the child
            u.profit = v.profit + p[u.level]; // that includes the
                                             //next item
            double bound = bound(u, W, n, w, p);
            u.bound=bound;

            if(u.weight<=W && u.profit>maxProfit){
                maxProfit = u.profit;
            }

            if(bound>maxProfit){
                Q.add(u);
            }

            u = new Node();
            u.weight = v.weight; // set u to the child that
            u.profit = v.profit;// does NOT include the next item

            bound = bound(u, W, n, w, p);
            u.bound = bound;

            if (bound>maxProfit){
                Q.add(u);
            }
        }

        return maxProfit;
    }

    public static float bound(Node u, int W, int n, int [] w, int [] p){
        int j=0; int k=0;
        int totWeight=0;
        float result=0;

        if(u.weight>=W)
            return 0;
        else {
            result = u.profit;
            j= u.level +1;
            totWeight = u.weight;

            while ((j < n) && (totWeight + w[j]<=W)){
                totWeight = totWeight + w[j]; // grab as many items as possible
                 result = result + p[j];
                j++;
            }
            k=j; // use k for consistency with formula in text
            if (k<n)
                result = result + (W-totWeight) * p[k]/w[k];// grab fraction of kth item

            return result;
        }
    }
}

Answer 1

I have only tested it with the given example, but it looks like that wherever the pseudocode says

enqueue(Q, u)

you should add a copy of u to the linked list, rather than passing a reference to u and continue manipulating it.

In other words, define a copy constructor for the class Node and do

Q.offer(new Node(u));

instead of

Q.offer(u);

In fact, the code you give above only allocates two instances of the class Node per call to branchAndBound(..)