maxpumperla / deep_learning_and_the_game_of_go

Code and other material for the book "Deep Learning and the Game of Go"
https://www.manning.com/books/deep-learning-and-the-game-of-go
987 stars 390 forks source link

Wrong ko detection by setting _grid value to None instead of deleting whole key #21

Closed Efaq closed 5 years ago

Efaq commented 5 years ago

When doing some study with the code, I may have found a bug regarding the ko detection when working with goboard_slow:

Given a specific board state, if one piece is placed and then removed, the initial and final board configurations are the same, but in the dictionary _grid of the first one there will be no entry for the Point where the stone was placed, while in the _grid of the second one there will be an entry pointing to None. This makes the equality check of situation for ko to return False, since the dictionaries are different, but the states are the same and there should be a ko. I came across examples of this behavior.

I believe that this correction should take place:

In goboard_slow.py, the _remove_string method should look like:

def _remove_string(self, string):
    for point in string.stones:
        for neighbor in point.neighbors():  # <1>
            neighbor_string = self._grid.get(neighbor)
            if neighbor_string is None:
                continue
            if neighbor_string is not string:
                neighbor_string.add_liberty(point)
        del(self._grid[point])

That is, the last line is changed so the whole key is deleted instead of it being linked to None.

Efaq commented 5 years ago

Alternatively, the dictionary _grid should be initialized with keys for all possible Point objects in the board pointing to None.

maxpumperla commented 5 years ago

@Efaq very good catch. mind sending a pull request against our master branch?

Efaq commented 5 years ago

@maxpumperla Sure, will do!