Re: [gnugo-devel] changed board_hash after reading

[Arend Bayer]

On Wed, 17 Aug 2005,  wrote:

> Martin wrote:
> > I digged a little deeper to find out that 
> > 
> >   if (komaster_trymove(...))
> >     popgo();
> > 
> > can alter the board_hash. I blame this on the invocations of
> > set_new_komaster() and set_new_kom_pos(). Before I start figuring out
> > what komaster_trymove does (and how to fix it): Is this indeed a problem
> > to be fixed or am I overlooking something and making wrong assumptions?
> 
> This looks like a bug. As far as I can tell the problem is that
> board_hash is stored in a stack in do_trymove() but in the case of
> komaster_trymove() it would need to be stored already at the start of
> komaster_trymove() rather than when trymove()/tryko() is eventually
> called. More specifically it needs to be stored before it is updated
> in set_new_komaster()/set_new_kom_pos().

It is a little more subtle than this I think. 

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.

In particular, note that not only board_hash, but also the komaster
state is altered by the above komster_trymove()/poggo() pair, and
the board_hash is actually correctly matching board state+komaster state.

The first set_new_...() pair looks unproblematic to me:

  /* 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);
  }

While it's extremely bad style to do this as a side effect of 
komaster_trymove(), there is nothing buggy with changing the
komaster (and accordingly board_hash) back to EMPTY when there is no ko
around any more.

However, the following is clearly a bug, and a pretty bad one actually:

  *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);
    }
  }

It's only migitated by the fact that WEAK_KO is rare, I believe.
(It has the effect of resetting the komaster from WEAK_KO to EMPTY
for all siblings of any tried non-ko move.)

I will take care of fixing the bug in the 2nd quoted code excerpt.
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.


Arend

-- 
http://guests.mpim-bonn.mpg.de/bayer/