Re: [gnugo-devel] overlaps/duplications of move reasons

[Arend Bayer]

> > This move reason is often greatly over-valued:
> >   defends against combination attack on several worms
> 
> The valuation of these sometimes overlaps with other values, but yes,
As for overlaps in general, I suggest the following change in
move_reasons.c to centralize the handling of such duplications.

(I have currently only included overlaps of tactical attacks with
owl-attacks, so this is not yet for CVS. )
As this patch's only purpose is to make maintaining the code easier, I
would like to get a public opinion whether this scheme looks convenient.
If others agree, I will convert any existing duplication handling (and
some inessentiality criteria) and send a completed patch. I'd hope this
makes value_move_reasons somewhat quicker to read and hack.
Then I would send another patch with suggested additions to the list
of move reasons to be discarded.

-Arend

(NOT for CVS)
Index: engine/move_reasons.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/move_reasons.c,v
retrieving revision 1.46
diff -u -r1.46 move_reasons.c
--- engine/move_reasons.c       10 Dec 2001 17:07:18 -0000      1.46
+++ engine/move_reasons.c       11 Dec 2001 23:10:15 -0000
@@ -80,6 +80,18 @@
 /* Point redistribution */
 static int replacement_map[BOARDMAX];
 
+
+/* Helper functions to check conditions for 
+ * move reasons.
+ */
+typedef int (*discard_condition_fn_ptr)(int pos, int what);
+
+struct discard_rule {
+  int reason_type;
+  discard_condition_fn_ptr condition;
+  char trace_message[80];
+};
+
 /* Initialize move reason data structures. */
 void
 clear_move_reasons(void)
@@ -2389,6 +2401,72 @@
   return adjusted_value;
 }
 
+/* Deletes a move reason by number, keeping the order of the rest. */
+void
+delete_move_reason(int pos, int k)
+{
+  int i;
+  for (i = k; (move[pos].reason[i] >= 0) && (i < MAX_REASONS); i++) 
+    move[pos].reason[i] = move[pos].reason[i+1];
+}
+
+#define NUM_DISCARD_RULES 18
+/* This array lists rules according to which we discard move reasons.
+ * This is intended e.g. for move reasons that only
+ * duplicate another reason (like a tactical attack on a worm that
+ * belongs to a dragon that is owl-attacked by the same move), and
+ * for moves that attack/defend an inessential worm.
+ * The format is: {Rule to be discarded, pointer to function that
+ * checks the reason is obsolete, trace message}
+ */
+static struct discard_rule discard_rules[NUM_DISCARD_RULES] =
+{
+  { ATTACK_MOVE, owl_attack_move_reason_known,
+    " - attack on %1m (owl attacked as well)\n" },
+  { ATTACK_MOVE_GOOD_KO, owl_attack_move_reason_known,
+    " - attack on %1m (owl attacked as well)\n"},
+  { ATTACK_MOVE_BAD_KO,  owl_attack_move_reason_known,
+    " - attack on %1m (owl attacked as well)\n"},
+  { DEFEND_MOVE, owl_defense_move_reason_known, 
+    " - defense of %1m (owl defended as well)\n"},
+  { DEFEND_MOVE_GOOD_KO, owl_defense_move_reason_known,
+    " - defense of %1m (owl defended as well)\n"},
+  { DEFEND_MOVE_BAD_KO, owl_defense_move_reason_known,
+    " - defense of %1m (owl defended as well)\n"},
+};
+
+/* This function checks the list of move reasons for duplications,
+ * and discards the obsolete ones. Note that it assumes that the
+ * move reasons are sorted (by type) as done in value_move_reasons
+ * directly before discard_duplicated_move_reasons is called.
+ */
+void
+discard_obsolete_move_reasons(int pos)
+{
+  int k, l;
+  for (k = 0; k < NUM_DISCARD_RULES; k++) {
+    for (l = 0; l < MAX_REASONS; l++) {
+
+      int r = move[pos].reason[l];
+      if (move_reasons[r].type < discard_rules[k].reason_type)
+        continue;
+      else
+        if (move_reasons[r].type > discard_rules[k].reason_type)
+          break;
+
+      /* Otherwise move[pos].reason[l] fits the reason in our
+       * inessentiality rule. So we check the condition.
+       */
+      if (discard_rules[k].condition(pos, move_reasons[r].what)) {
+        DEBUG(DEBUG_MOVE_REASONS,
+              strcat("%1m: 0.0 - ",discard_rules[k].trace_message),
+              pos, move_reasons[r].what); 
+        delete_move_reason(pos, l);
+        l--;
+      } 
+    }
+  }
+}
 
 /*
  * Estimate the direct territorial value of a move at (pos).
@@ -2456,15 +2534,6 @@
        secondary_value += 0.2 * this_value;
        break;
       }
-      
-      /* If the move also owl attacks the same stones, count points
-       * for that move reason instead.
-       */
-      if (owl_attack_move_reason_known(pos, find_dragon(aa))) {
-       DEBUG(DEBUG_MOVE_REASONS,
-             "  %1m: 0.0 - attack on %1m (owl attacked as well)\n", pos, aa);
-       break;
-      }
 
       /* Mark the string as captured, for evaluation in the influence code. */
       mark_string(aa, saved_stones, INFLUENCE_CAPTURED_STONE);
@@ -2526,15 +2595,6 @@
              "  %1m: 0.0 - defense of %1m (inessential)\n", pos, aa);
        break;
       }
-      
-      /* If the move also owl defends the same stones, count points
-       * for that move reason instead.
-       */
-      if (owl_defense_move_reason_known(pos, find_dragon(aa))) {
-       DEBUG(DEBUG_MOVE_REASONS,
-             "  %1m: 0.0 - defense of %1m (owl defended as well)\n", pos, aa);
-       break;
-      }
 
       /* Mark the string as saved, for evaluation in the influence code. */
       mark_string(aa, saved_stones, INFLUENCE_SAVED_STONE);
@@ -3553,6 +3613,8 @@
 
     /* Sort the move reasons. This makes it easier to visually compare
      * the reasons for different moves in the trace outputs.
+     * Also, discard_obsolete_move_reasons assumes the move reasons
+     * being sorted.
      */
     num_reasons = 0;
     while (move[pos].reason[num_reasons] >= 0)
@@ -3560,6 +3622,8 @@
     qsort(&(move[pos].reason[0]), num_reasons, 
          sizeof(move[pos].reason[0]),
          compare_move_reasons);
+    /* Discard move reasons that only duplicate another. */
+    discard_obsolete_move_reasons(pos, color);
 
     /* Estimate the value of various aspects of the move. The order
      * is significant. Territorial value must be computed before