2

I am trying to implement a priority queue of my class type BBNode, but it doesn't seem to sift up the new nodes the way I expect it. Rather than have the smallest be at the head of the queue (how it works now), I want the largest to be there, but I can't figure out how to make this work. Here's my BBNode class.

public class BBNode implements Comparable<BBNode>{
    public int level; //level on the tree
    public int value; //value up to that node
    public int weight; //weight up to that node
    public double bound; //bound of that node

    //...constructors
    public int compareTo(BBNode node){
        if(this.bound > node.bound) return -1;
        if(this.bound < node.bound) return 1;
        return 0;
    }
}

And here's where I use the PQ.

PriorityQueue<BBNode> pq = new PriorityQueue<BBNode>();
//..other variables
while(pq.peek() != null){
  v = pq.poll();
  //System.out.println(v.weight + " " + v.value + " " + v.bound);
  if(v.bound >= bestVal && v.level < sortedItems.length-1){
     //left branch: take the next item
     u.level = v.level+1;
     u.weight = v.weight + sortedItems[u.level].weight;
     u.value = v.value + sortedItems[u.level].value;
     u.bound = bound(u);
     //System.out.println(u.bound);
     if(u.bound > bestVal){
        pq.add(u);
        System.out.println("adding " + u.bound);
        System.out.println("top of pq is " + pq.peek().bound);
     }
     if(u.weight <= maxWt && u.value > bestVal){
        bestVal = u.value;
        bestWt = u.weight;
        //System.out.println(bestVal + " " + bestWt);
        takeList[arrIdx++] = sortedItems[u.level].item+1;
     }
     //right branch: don't take the next item
     u.weight = v.weight;
     u.value = v.value;
     u.bound = bound(u);
     if(u.bound > bestVal){
        pq.add(u);
        System.out.println("adding " + u.bound);
        System.out.println("top of pq is " + pq.peek().bound);
     }
  }
}

Sorry the formatting at the end sucks. The last bracket corresponds to the while loop.

I've also tried switching around the -1 and 1 in the compare method, and I've also tried implementing a Comparator, but with the same results.

Any help is appreciated :)

3
  • 1
    Your BBNode is correct, and inserting them into a priority queue works just fine (with the largest bound being first). So why do you think its wrong? What are a couple of values that you insert that end up being not properly inserted? Commented Jun 4, 2013 at 19:44
  • Can you add the section of code where you set up 'u'? Commented Jun 4, 2013 at 20:03
  • 1
    In the code as quoted, you are not assigning u to point to a different object on each iteration, just changing fields in the object u references. If that is the case in your real code, there is only one object ever added, and its value changes based on the latest v. Commented Jun 4, 2013 at 20:09

2 Answers 2

3

What's likely happening is that you are modifying the bound of instances that are currently in the PriorityQueue when you do things like u.bound = bound(u);. First time through the loop, you set the u.bound and put it in, next time through you change u.bound again without pulling it off the queue first.

If you're using an organized collection (HashSet/Map, TreeSet/Map, PriorityQueue, etc) and you change an elements value in such a way as to affect how the collection is organized, you are breaking the assumptions on which that collection is organized, and they will fail in various interesting ways. I think that's what's happening here.

Some questions that talk about this for other collection types:

Sign up to request clarification or add additional context in comments.

1 Comment

This was exactly it. As soon as I instantiated each new object before putting in the PQ, it worked! The funny thing is this "pointer" logic gave me trouble in a C++ program about a month ago, but it didn't occur to me this might be the problem again :(
0

Also note that the PriorityQueue (java 1.6) appears to sort best effort.
I found this out in a unit test using either the PriorityQueue and the ConcurrentSkipListSet while using the same Comparator.
Where the PriorityQueue tries to find the right spot for a newly added item (it seems to walk the nodes but once an item is greater after all previous items were smaller, and vice versa the queue stops looking and puts the item next to the last inspected).
In comparison to the binary search algorithm on an array it looks as if the upper and lower bounds during search are closing towards the right spot but a gap remains. When I repeat the same test using the ConcurrentSkipListSet with the same comparator, the set is actually sorted properly!

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.