tensorflow / minigo

An open-source implementation of the AlphaGoZero algorithm
Apache License 2.0
3.47k stars 561 forks source link

Moves considered logging is broken at times #363

Closed sethtroisi closed 6 years ago

sethtroisi commented 6 years ago

screenshot from 2018-08-08 02-19-31

sethtroisi commented 6 years ago

PV not matching "top" listed move screenshot from 2018-08-08 02-23-14

tommadams commented 6 years ago

This is the same issue that PR #328 addresses.

The move list is sorted by (N, action_score, move) while the first move in the principle variation is chosen as by the first move with the highest N, effectively sorting by (N, move).

I can fix this, or if you want to add the fix to #328 you're more than welcome. This is the line that needs fixing: https://github.com/tensorflow/minigo/blob/2dbc02645b1ce4e1f0bd6ad0049627f26fb119cb/cc/mcts_player.cc#L169

Add a call to CalculateChildActionScores, then I guess you'd have to iterate over root_->edges manually instead of calling ArgMax since you need to index into two separate arrays.

sethtroisi commented 6 years ago

I found the problem.

We describe twice. One when verbose >= 2, one on stderr three guesses which one is more useful.

amj commented 6 years ago

This is not quite accurate. When running the python engine via gtp.py, we always install the minigui gtp handlers, even when we are not launching the engine via minigui/serve.py

The minigui genmove handler calls describe on the board after the move has been played (so the scraper picks it up). This results in the list of moves being the ones for the child move (i.e., only the re-used subtree).

tommadams commented 6 years ago

I'm going to fix the problem about move selection I mentioned in my earlier comment over in issue #260