pnprog / goreviewpartner

A tool to help analyse and review your game of go (weiqi, baduk) using strong bots.
GNU General Public License v3.0
285 stars 66 forks source link

Support for PhoenixGo #99

Open pnprog opened 5 years ago

pnprog commented 5 years ago

Hi @wonderingabout ,

I'm opening an issue here to discuss about the bug in issue 86 to avoid polluting PhoenixGo.

Looking at the RSGF file you share, I can notice that only three variations are provided (width=3), and they are always 1 move deep (depth=1) => this is probably another issue, but just to be sure, can you double check the values inprint_tree_depth and print_tree_width in mcts_something.conf ?

On my own computer, I got 6 variations (most of the time) and most of them are 2 or 3 moves deep, with a very small number or playout. Also, the win/rate graph is quite similar to that of LeelaZero with PhoenixGo weights. So this is definitely a parsing issue. Maybe the output is a bit different when using GPU or CPU?

So, I need to see by myself what is the output of PhoenixGo from your computer. Here is what I propose:

Hopefully, I can find something :)

pnprog commented 5 years ago

Maybe I found the problem: it could be related to pondering. Can you disable pondering and restart the analysis again? In the mctf conf file: enable_background_search: 0 Apparently, when pondering is on, when clear_board GTP instruction is used, PhoenixGo dumps the current result of the tree search, and those extra data (formatted as the other data) is confusing GRP.

pnprog commented 5 years ago

Yes, I can confirm that my GRP has similar symptoms when pondering is ON. At least, this is something easy to fix.

wonderingabout commented 5 years ago

i was thinking about pondering too, but i thought maybe phoenixgo is smart

i used 6x6 in .conf file of phoenixgo

but on grp settings i used max variations 3, with 25 depth

i think it was fine to pollute the phoenixgo github, after all its related

i'll link you my conf file when i finish updating the PR

pnprog commented 5 years ago

i think it was fine to pollute the phoenixgo github, after all its related

That probably ok, but I don't want to have my bugs advertised all over github as well :P

I just pushed a commit that should help with this pondering issue. But try first without pondering, to see if everything else is OK. I will be running a test with pondering during the night.

wonderingabout commented 5 years ago

then i'll retry with same version and ponderingoff this time, same as this file :

https://github.com/wonderingabout/PhoenixGo/blob/faqv2-bazel-master/etc/mcts_1gpu_grp.conf

wonderingabout commented 5 years ago

oh wow @pnprog !!!

its 1000 times better with pondering off lol

http://www.mediafire.com/file/vcenr3jzq2vaxqs/game2v3.rsgf

native phoenixgo 3200 simulations per move :

graph3

compare it with LZ-phoenixgo-orig -v 3200 :

lzphoenixgogrp

now i need to check if they actually suggest the same moves with same variations

wonderingabout commented 5 years ago

@alreadydone

i have yet to check if they suggest the same moves and variations though

wonderingabout commented 5 years ago

compare it to lz-phoenixgo-orig : https://github.com/Tencent/PhoenixGo/issues/86#issuecomment-456697268

wonderingabout commented 5 years ago

now i want to test width 3 + depth 15

(i think seeing 3 alternate moves is reasonable + with enough diversity + they will be meaningful enough (enough simulations spent on them))

i'll try again now with this :

debugger {
    print_tree_depth: 15
    print_tree_width: 3
}
wonderingabout commented 5 years ago

result v4 :+1:

http://www.mediafire.com/file/83ajv4mytdw3ld3/game2v4.rsgf/file

graph4

compare it with LZ-phoenixgo-orig -v 3200 :

lzphoenixgogrp

pnprog commented 5 years ago

Good to see it's starts to be ok :)

i think seeing 3 alternate moves is reasonable + with enough diversity + they will be meaningful enough (enough simulations spent on them)

What can be done is to filter the variations to display (and among the variations, the number of moves) using the number of playouts done. I did something similar with the implementation for Pachi.

wonderingabout commented 5 years ago

isnt it what is already done here ?

when i set variations to display to 3 ?

wonderingabout commented 5 years ago

btw i sent you this yesterday @pnprog :+1:

https://github.com/pnprog/goreviewpartner/pull/100

i think it will help clear away many of the confusion :)

wonderingabout commented 5 years ago

so, i'm closing the PhoenixGo github issues in : https://github.com/Tencent/PhoenixGo/issues/86

to avoid duplicates,

let's continue here whenever we can : i hope that i will be ready when we can continue this :+1: (but it's working quite good already)

btw PhoenixGo now has second move path and third move path too @pnprog , see : https://github.com/Tencent/PhoenixGo/issues/85

pnprog commented 5 years ago

Hi @wonderingabout ,

I see your PR for PhoenixGo have been accepted. Can you update/push again your PR for GRP? (I made some changes in config.ini since then, so the PR cannot be merged as it is)

wonderingabout commented 5 years ago

hi too @pnprog :+1:

ok, let me look