Re: [gnugo-devel] changed board_hash after reading

[Gunnar Farnebäck]

Arend wrote:
> It is a little more subtle than this I think. 

Right you are.

> The problems are the set_new_komaster()/set_new_kom_pos() calls in
> komaster_trymove() that happen before any (successful) trymove() or tryko()
> -- that is the point where BEGIN_CHANGE_RECORD is started (important because
> anything before that is not undone before next popgo()), and where the
> board_hash is saved.

I totally forgot about BEGIN_CHANGE_RECORD. So the effect is not that
the board hash gets out of sync but that all siblings are affected,
which as you say is unproblematic for the first calls to change
komaster as they are valid for all siblings.

> I will take care of fixing the bug in the 2nd quoted code excerpt.

The fix in Martin's patch looks good to me.

> About the first: We could instead automatically reset komaster
> to EMPTY in do_trymove() when a ko is resolved (and remove the above
> code from komaster_trymove()). I am not sure what is best here.

This is also what Martin does. But is there actually any need for an
automatic reset? It seems to me that this check could be done in
komaster_trymove() after doing the move and that it's only meaningful
for non-ko moves.

What about the appended patch? Regression results are

ninestones:570  FAIL A2 [A9|B10|D11|F11|J15|A16]
1 FAIL
Total nodes: 1653508667 3157394 13008534 (-0.084% -0.052% -0.0061%)

Compared to Martin's patch the failure is the same, there is no pass,
and there's a slight decrease in reading nodes instead of a 2.2%
increase.

/Gunnar

Index: engine/board.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/board.c,v
retrieving revision 1.111
diff -u -r1.111 board.c
--- engine/board.c      12 Jun 2005 09:34:13 -0000      1.111
+++ engine/board.c      1 Sep 2005 07:45:30 -0000
@@ -1226,35 +1226,10 @@
   int kpos;
   int previous_board_ko_pos = board_ko_pos;
 
-  /* First we check whether the ko claimed by komaster has been
-   * resolved. If that is the case, we revert komaster to EMPTY.
-   *
-   * The ko has been resolved in favor of the komaster if it has
-   * been filled, or if it is no longer a ko and an opponent move
-   * there is suicide.
-   */
-  if (((komaster == WHITE || komaster == GRAY_WHITE)
-       && (IS_STONE(board[kom_pos])
-          || (!is_ko(kom_pos, BLACK, NULL)
-              && is_suicide(kom_pos, BLACK))))
-      || ((komaster == BLACK || komaster == GRAY_BLACK)
-         && (IS_STONE(board[kom_pos])
-             || (!is_ko(kom_pos, WHITE, NULL)
-                 && is_suicide(kom_pos, WHITE))))) {
-    set_new_komaster(EMPTY);
-    set_new_kom_pos(NO_MOVE);
-  }
-
   *is_conditional_ko = 0;
   ko_move = is_ko(pos, color, &kpos);
 
-  if (!ko_move) {
-    if (komaster == WEAK_KO) {
-      set_new_komaster(EMPTY);
-      set_new_kom_pos(NO_MOVE);
-    }
-  }
-  else {
+  if (ko_move) {
     /* If opponent is komaster we may not capture his ko. */
     if (komaster == other && pos == kom_pos)
       return 0;
@@ -1293,8 +1268,31 @@
     }
   }
 
-  if (!ko_move)
+  if (!ko_move) {
+    /* If we are komaster, check whether the ko was resolved by the
+     * current move. If that is the case, revert komaster to EMPTY.
+     *
+     * The ko has been resolved in favor of the komaster if it has
+     * been filled, or if it is no longer a ko and an opponent move
+     * there is suicide.
+     */
+    if (((komaster == color
+         || (komaster == GRAY_WHITE && color == WHITE)
+         || (komaster == GRAY_BLACK && color == BLACK))
+        && (IS_STONE(board[kom_pos])
+            || (!is_ko(kom_pos, other, NULL)
+                && is_suicide(kom_pos, other))))) {
+      set_new_komaster(EMPTY);
+      set_new_kom_pos(NO_MOVE);
+    }
+
+    if (komaster == WEAK_KO) {
+      set_new_komaster(EMPTY);
+      set_new_kom_pos(NO_MOVE);
+    }
+    
     return 1;
+  }
 
   if (komaster == other) {
     if (color == WHITE)