Fix a major bug in dynamic pclasses. One of thse little things that

was sooo wrong you wonder how it worked in the first place... Took
me days to find this one!

Also added a new switch, '-o', that lets assign try out solutions
that over-load a pnode. This helps a lot with topologies where the
optimal solution is a best-fit onto multiplexed pnodes.

The end result is that Mike's snake maps much better - it used to get
an essentially random mapping, but now it gets something acceptible.

assign/anneal.cc

@@ -58,7 +58,7 @@ tb_pnode *find_pnode(tb_vnode *vn)
   int num_types = tt.first;
   pclass_vector *acceptable_types = tt.second;
   
-  tb_pnode *newpnode;
+  tb_pnode *newpnode = NULL;
 
   /*
   int enabled_pclasses = 0;
@@ -77,6 +77,13 @@ tb_pnode *find_pnode(tb_vnode *vn)
   int first = i;
   for (;;) {
     i = (i+1)%num_types;
+
+    // Skip pclasses that have been disabled
+    if ((*acceptable_types)[i]->disabled) {
+	i = std::rand()%num_types;
+	continue;
+    }
+
 #ifdef PCLASS_SIZE_BALANCE
     int acceptchance = 1000 * (*acceptable_types)[i]->size * 1.0 /
 	npnodes;
@@ -84,11 +91,15 @@ tb_pnode *find_pnode(tb_vnode *vn)
 	continue;
     }
 #endif
-#ifdef LOAD_BALANCE
+
+    // For load balancing only
 REDO_SEARCH:
     tb_pnode* firstmatch = NULL;
-#endif
-#ifdef FIND_PNODE_SEARCH
+
+#ifndef FIND_PNODE_SEARCH
+    // If not searching for the pnode, just grab the front one
+    newpnode = (*acceptable_types)[i]->members[vn->type]->front();
+#else
 #ifdef PER_VNODE_TT
     // If using PER_VNODE_TT and vclasses, it's possible that there are
     // some pclasses in this node's type table that can't be used right now,
@@ -98,10 +109,6 @@ REDO_SEARCH:
 	    (*acceptable_types)[i]->members.end()) {
 	continue;
     }
-    if ((*acceptable_types)[i]->disabled) {
-	i = std::rand()%num_types;
-	continue;
-    }
 #endif
     list<tb_pnode*>::iterator it = (*acceptable_types)[i]->members[vn->type]->L.begin();
 #ifdef LOAD_BALANCE
@@ -110,7 +117,7 @@ REDO_SEARCH:
     for (int j = 0; j < skip; j++) {
 	it++;
     }
-#endif
+#endif // LOAD_BALANCE
     while (it != (*acceptable_types)[i]->members[vn->type]->L.end()) {
 #ifdef LOAD_BALANCE
 	if ((*it)->typed) {
@@ -133,15 +140,16 @@ REDO_SEARCH:
 	} else {
 	    break;
 	}
-#else
+#else // LOAD_BALANCE
 	if ((*it)->typed && ((*it)->current_type.compare(vn->type) ||
-		    ((*it)->current_type_record->current_load >=
-		     (*it)->current_type_record->max_load))) {
+		 (!allow_overload &&
+		  ((*it)->current_type_record->current_load >=
+		   (*it)->current_type_record->max_load)))) {
 	    it++;
 	} else {
 	    break;
 	}
-#endif
+#endif // LOAD_BALANCE
     }
     if (it == (*acceptable_types)[i]->members[vn->type]->L.end()) {
 #ifdef LOAD_BALANCE
@@ -151,15 +159,13 @@ REDO_SEARCH:
 	} else {
 	    newpnode = NULL;
 	}
-#else
+#else // LOAD_BALANCE
 	newpnode = NULL;
-#endif
+#endif // LOAD_BALANCE
     } else {
 	newpnode = *it;
     }
-#else
-    newpnode = (*acceptable_types)[i]->members[vn->type]->front();
-#endif
+#endif // FIND_PNODE_SEARCH
 #ifdef PCLASS_DEBUG
     cerr << "Found pclass: " <<
       (*acceptable_types)[i]->name << " and node " <<

assign/anneal.h

@@ -60,6 +60,7 @@ extern pclass_list pclasses;
 extern pnode_pvertex_map pnode2vertex;
 extern double absbest;
 extern int absbestviolated, iters, iters_to_best;
+extern bool allow_overload;
 
 #ifdef PER_VNODE_TT
 extern pclass_types vnode_type_table;

assign/assign.cc

@@ -101,6 +101,9 @@ bool prune_pclasses = false;
 
 // Whether or not we should use the experimental support for dynamic pclasses
 bool dynamic_pclasses = false;
+
+// Whether or not to allow assign to temporarily over-subscribe pnodes
+bool allow_overload = false;
   
 // XXX - shouldn't be in this file
 double absbest;
@@ -307,6 +310,7 @@ void print_help()
 #endif
   cerr << "  -T          - Doing some scoring self-testing." << endl;
   cerr << "  -H <float>  - Try <float> times harder." << endl;
+  cerr << "  -o          - Allow overloaded pnodes to be considered." << endl;
   exit(2);
 }
  
@@ -538,7 +542,7 @@ int main(int argc,char **argv)
   char ch;
   timelimit = 0.0;
   timetarget = 0.0;
-  while ((ch = getopt(argc,argv,"s:v:l:t:rpPTdH:")) != -1) {
+  while ((ch = getopt(argc,argv,"s:v:l:t:rpPTdH:o")) != -1) {
     switch (ch) {
     case 's':
       if (sscanf(optarg,"%d",&seed) != 1) {
@@ -579,6 +583,8 @@ int main(int argc,char **argv)
 	print_help();
       }
       break;
+    case 'o':
+      allow_overload = true; break;
     default:
       print_help();
     }

assign/pclass.cc

@@ -149,7 +149,7 @@ int generate_pclasses(tb_pgraph &PG, bool pclass_for_each_pnode,
 	if (pclass_equiv(PG,curP,(*dit).second)) {
 	  // found the right class
 	  found_class=1;
-	  curclass->add_member(curP);
+	  curclass->add_member(curP,false);
 	  break;
 	} 
       }
@@ -161,7 +161,7 @@ int generate_pclasses(tb_pgraph &PG, bool pclass_for_each_pnode,
       pclasses.push_back(n);
       canonical_members[n]=curP;
       n->name = curP->name;
-      n->add_member(curP);
+      n->add_member(curP,false);
     }
   }
 
@@ -191,9 +191,8 @@ int generate_pclasses(tb_pgraph &PG, bool pclass_for_each_pnode,
       tb_pclass *n = new tb_pclass;
       pclasses.push_back(n);
       n->name = pnode->name + "-own";
-      n->add_member(pnode);
+      n->add_member(pnode,true);
       n->disabled = true;
-      pnode->my_own_class = n;
     }
   }
 
@@ -232,7 +231,7 @@ int generate_pclasses(tb_pgraph &PG, bool pclass_for_each_pnode,
   return 0;
 }
 
-int tb_pclass::add_member(tb_pnode *p)
+int tb_pclass::add_member(tb_pnode *p, bool is_own_class)
 {
   tb_pnode::types_map::iterator it;
   for (it=p->types.begin();it!=p->types.end();++it) {
@@ -243,7 +242,11 @@ int tb_pclass::add_member(tb_pnode *p)
     members[type]->push_back(p);
   }
   size++;
-  p->my_class=this;
+  if (is_own_class) {
+      p->my_own_class=this;
+  } else {
+      p->my_class=this;
+  }
   return 0;
 }
 
@@ -277,6 +280,11 @@ void assert_own_class_invariant(tb_pnode *p) {
     pclass_debug();
     abort();
   }
+  if (!own_class && !other_class) {
+    cerr << "In neither class!" << endl;
+    pclass_debug();
+    abort();
+  }
 }
 
 // should be called after add_node
@@ -356,7 +364,7 @@ int pclass_unset(tb_pnode *p)
 #endif
   }
 
-  if (p->my_own_class) {
+  if (p->my_own_class && (p->total_load == 1)) {
     p->my_own_class->disabled = true;
   }
 

assign/pclass.h

@@ -75,7 +75,7 @@ public:
   typedef hash_set<tb_pnode*,hashptr<tb_pnode*> > tb_pnodeset;
   typedef hash_map<crope,tb_pnodeset*> pclass_members_set;
 
-  int add_member(tb_pnode *p);
+  int add_member(tb_pnode *p, bool is_own_class);
 
   crope name;			// purely for debugging
   int size;

assign/score.cc

@@ -352,8 +352,6 @@ void remove_node(vvertex vv)
   } else if (tr->current_load >= tr->max_load) {
     // If the pnode is still over its max load, reduce the penalty to adjust
     // for the new load.
-    // XXX - This shouldn't ever happen, right? I don't think we can ever
-    // over-assign to a pnode.
     SDEBUG(cerr << "  reducing penalty, new load=" <<
 	pnode->current_type_record->current_load <<
 	   " (>= " << pnode->current_type_record->max_load << ")" << endl);
@@ -524,18 +522,6 @@ int add_node(vvertex vv,pvertex pv, bool deterministic)
 	  return 1;
 	} else {
 	  SDEBUG(cerr << "  compatible types" << endl);
-	  if (pnode->current_type_record->current_load
-	      == pnode->current_type_record->max_load) {
-	    /* XXX - We could ignore this check and let the code
-	       at the end of the routine penalize for going over
-	       load.  Failing here seems to work better though. */
-
-	    // XXX is this a bug?  do we need to revert the pnode/vnode to
-	    // it's initial state.
-	    SDEBUG(cerr << "  node is full" << endl);
-	    cerr << "  node is full" << endl;
-	    return 1;
-	  }
 	}
       }
     }