Re: [gnugo-devel] defend both, attack either, worms[], dragons[]

[Gunnar Farneback]

Arend wrote:
> Hence I finally decided to kill both the worms[] and the dragons[]
> array, together with find_worm() and find_dragon(). They seem to be
> a relict from 2D-board times to me.

I see you didn't manage to break free from one of the
micro-optimizations in estimate_strategical_value(). There isn't
really any compelling reason to index the dragon_value[] array
differently or to dynamically allocate its memory. This patch gets rid
of the use of dragon id in estimate_strategical_value().

/Gunnar

Index: engine/move_reasons.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/move_reasons.c,v
retrieving revision 1.104
diff -u -r1.104 move_reasons.c
--- engine/move_reasons.c       11 Dec 2002 07:42:19 -0000      1.104
+++ engine/move_reasons.c       11 Dec 2002 15:11:43 -0000
@@ -646,9 +646,9 @@
 void
 add_attack_move(int pos, int ww, int code)
 {
+  ASSERT_ON_BOARD1(ww);
   ww = worm[ww].origin;
 
-  ASSERT_ON_BOARD1(ww);
   if (code == WIN)
     add_move_reason(pos, ATTACK_MOVE, ww);
   else if (code == KO_A)
@@ -664,9 +664,9 @@
 void
 add_defense_move(int pos, int ww, int code)
 {
+  ASSERT_ON_BOARD1(ww);
   ww = worm[ww].origin;
 
-  ASSERT_ON_BOARD1(ww);
   if (code == WIN)
     add_move_reason(pos, DEFEND_MOVE, ww);
   else if (code == KO_A)
Index: engine/value_moves.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/value_moves.c,v
retrieving revision 1.71
diff -u -r1.71 value_moves.c
--- engine/value_moves.c        11 Dec 2002 07:42:19 -0000      1.71
+++ engine/value_moves.c        11 Dec 2002 15:11:48 -0000
@@ -1912,8 +1912,6 @@
   int l;
   int aa = NO_MOVE;
   int bb = NO_MOVE;
-  int a_id;
-  int b_id;
   float aa_value = 0.0;
   float bb_value = 0.0;
   
@@ -1921,11 +1919,10 @@
   float tot_value = 0.0;
 
   /* Strategical value of connecting or cutting dragons. */
-  float *dragon_value = malloc(sizeof(float) * number_of_dragons);
-  gg_assert(dragon_value);
+  float dragon_value[BOARDMAX];
 
-  for (k = 0; k < number_of_dragons; k++)
-    dragon_value[k] = 0.0;
+  for (aa = BOARDMIN; aa < BOARDMAX; aa++)
+    dragon_value[aa] = 0.0;
   
   for (k = 0; k < MAX_REASONS; k++) {
     int r = move[pos].reason[k];
@@ -1984,7 +1981,6 @@
        for (l = 0; l < next_lunch; l++)
          if (lunch_worm[l] == aa) {
            bb = lunch_dragon[l];
-           b_id = dragon[bb].id;
 
            /* FIXME: This value cannot be computed without some measurement
             * of how the actual move affects the dragon.  The dragon safety
@@ -2024,11 +2020,11 @@
                    || DRAGON2(bb).safety == INVINCIBLE))
              this_value = 0.0;
            
-           if (this_value > dragon_value[b_id]) {
+           if (this_value > dragon_value[bb]) {
              DEBUG(DEBUG_MOVE_REASONS,
                    "  %1m:   %f - %1m attacked/defended\n",
                    pos, this_value, bb);
-             dragon_value[b_id] = this_value;
+             dragon_value[bb] = this_value;
            }
          }
 
@@ -2126,8 +2122,8 @@
            else
              this_value = 1.7 * dragon[cc].effective_size;
            
-           if (this_value > dragon_value[dragon[cc].id]) {
-             dragon_value[dragon[cc].id] = this_value;
+           if (this_value > dragon_value[dragon[cc].origin]) {
+             dragon_value[dragon[cc].origin] = this_value;
              DEBUG(DEBUG_MOVE_REASONS,
                    "  %1m:   %f - connect %1m and %1m to attack thrashing 
dragon %1m\n",
                    pos, this_value, aa, bb, cc);
@@ -2148,17 +2144,14 @@
        if (aa == bb)
          continue;
 
-       a_id = dragon[aa].id;
-       b_id = dragon[bb].id;
-       
        /* If we are ahead by more than 20, value connections more strongly */
        if ((color == WHITE && score > 20.0)
            || (color == BLACK && score < -20.0))
          this_value = connection_value(aa, bb, pos, gg_abs(score));
        else
          this_value = connection_value(aa, bb, pos, 0);
-       if (this_value > dragon_value[a_id]) {
-         dragon_value[a_id] = this_value;
+       if (this_value > dragon_value[aa]) {
+         dragon_value[aa] = this_value;
           DEBUG(DEBUG_MOVE_REASONS,
                "  %1m:   %f - %1m cut/connect strategic value\n",
                pos, this_value, aa);
@@ -2170,8 +2163,8 @@
          this_value = connection_value(bb, aa, pos, gg_abs(score));
        else
          this_value = connection_value(bb, aa, pos, 0);
-       if (this_value > dragon_value[b_id]) {
-         dragon_value[b_id] = this_value;
+       if (this_value > dragon_value[bb]) {
+         dragon_value[bb] = this_value;
           DEBUG(DEBUG_MOVE_REASONS,
                "  %1m:   %f - %1m cut/connect strategic value\n",
                pos, this_value, bb);
@@ -2244,7 +2237,6 @@
         * FIXME: Improve the implementation.
         */
        aa = move_reasons[r].what;
-       a_id = dragon[aa].id;
 
        /* FIXME: This value cannot be computed without some
         * measurement of how the actual move affects the dragon. The
@@ -2279,8 +2271,8 @@
          }
        }
                
-       if (this_value > dragon_value[a_id]) {
-         dragon_value[a_id] = this_value;
+       if (this_value > dragon_value[aa]) {
+         dragon_value[aa] = this_value;
           DEBUG(DEBUG_MOVE_REASONS,
                "  %1m:   %f - %1m strategic attack/defend\n",
                pos, this_value, aa);
@@ -2290,7 +2282,6 @@
 
       case UNCERTAIN_OWL_DEFENSE:
        aa = move_reasons[r].what;
-       a_id = dragon[aa].id;
        
        /* If there is an adjacent dragon which is critical we should
         * skip this type of move reason, since attacking or defending
@@ -2316,8 +2307,8 @@
        else 
          this_value = gg_min(2*dragon[aa].effective_size, gg_abs(0.65*score));
        
-       if (this_value > dragon_value[a_id]) {
-         dragon_value[a_id] = this_value;
+       if (this_value > dragon_value[aa]) {
+         dragon_value[aa] = this_value;
          DEBUG(DEBUG_MOVE_REASONS,
                "  %1m:   %f - %1m uncertain owl defense bonus\n",
                pos, this_value, aa);
@@ -2327,11 +2318,11 @@
     }
   }
   
-  for (k = 0; k < number_of_dragons; k++) {
-    if (dragon_value[k] == 0.0)
+  for (aa = BOARDMIN; aa < BOARDMAX; aa++) {
+    if (dragon_value[aa] == 0.0)
       continue;
 
-    aa = dragon2[k].origin;
+    ASSERT1(dragon[aa].origin == aa, aa);
 
     /* If this dragon is critical but not attacked/defended by this
      * move, we ignore the strategic effect.
@@ -2353,7 +2344,7 @@
        && (attack_move_reason_known(pos, aa)
            || defense_move_reason_known(pos, aa))) {
       TRACE("  %1m:   %f - %1m strategic value already counted - A.\n",
-           pos, dragon_value[k], aa);
+           pos, dragon_value[aa], aa);
       continue;
     }
     /* If the dragon has been owl captured, owl defended, or involved
@@ -2367,7 +2358,7 @@
        * value (e.g. because connecting to strong dragon) we award the
        * excess value as a bonus.
        */
-      float excess_value = (dragon_value[k] - 
+      float excess_value = (dragon_value[aa] - 
                            2 * dragon[aa].effective_size);
       if (excess_value > 0.0) {
        TRACE("  %1m: %f - strategic bonus for %1m\n", pos, excess_value, aa);
@@ -2375,19 +2366,17 @@
       }
       else {
        TRACE("  %1m:   %f - %1m strategic value already counted - B.\n",
-             pos, dragon_value[k], aa);
+             pos, dragon_value[aa], aa);
       }
       
       continue;
     }
 
     TRACE("  %1m: %f - strategic effect on %1m\n",
-         pos, dragon_value[k], aa);
-    tot_value += dragon_value[k];
+         pos, dragon_value[aa], aa);
+    tot_value += dragon_value[aa];
   }
 
-  free(dragon_value);
-  
   /* Finally, subtract penalty for invasion type moves. */
   this_value = strategic_penalty(pos, color);
   if (this_value > 0.0) {