pasky / pachi

A fairly strong Go/Baduk/Weiqi playing program
http://pachi.or.cz/
GNU General Public License v2.0
506 stars 117 forks source link

Emit LZ style analysis to stdout for compatibility #109

Closed bernds closed 5 years ago

bernds commented 5 years ago

LZ emits analysis to stdout, therefore q5go expects to see it there. This changes pachi to match Leela's behaviour at least in this respect.

lemonsqueeze commented 5 years ago

Thanks for spotting this,

I think we can't just replace stderr with stdout though, this is called from genmove also... With last PRs there's a report_fh to say where things should be logged: Could you try this branch, confirm it works for you ?

bernds commented 5 years ago

I had to comment out DCNN=1 and also add BOARDSIZE=19 when calling make to produce a binary that didn't crash on startup. Then it seems to work - in the sense that evaluations are seen. The behaviour is still pretty different from LZ (especially the habit of stopping once some arbitrary limit is reached is problematic), but that's something for a separate issue.

There is one serious problem though, in that sometimes q5go seems to receive a string like this: "pondering) info move P5 visits 291686 winrate 9991 prior 10000 order 0 pv P5 R14 O7 T14 M14 J19 K13" where the extra "pondering)" is unexpected and it almost seems like two different messages are mixed up somehow.

lemonsqueeze commented 5 years ago

I had to comment out DCNN=1 and also add BOARDSIZE=19 when calling make to produce a binary that didn't crash on startup.

Complained about missing dcnn files ? golast19.prototxt, golast.trained was renamed to detlef54.prototxt, detlef54.trained. Should be fine without BOARDSIZE=19 though, could you describe what happened ?

Then it seems to work - in the sense that evaluations are seen. The behaviour is still pretty different from LZ (especially the habit of stopping once some arbitrary limit is reached is problematic), but that's something for a separate issue.

Yes, tree memory is preallocated right now. Can be increased by setting max_tree_size but for analyze would be nice to have it dynamic. I'm adding a gtp command to change Pachi settings, something q5go could use in the meantime. What else is different from LZ ?

There is one serious problem though, in that sometimes q5go seems to receive a string like this: "pondering) info move P5 visits 291686 winrate 9991 prior 10000 order 0 pv P5 R14 O7 T14 M14 J19 K13" where the extra "pondering)" is unexpected and it almost seems like two different messages are mixed up somehow.

Yes, I don't think we need this anymore. Can get rid of the extra "(pondering)" now.

bernds commented 5 years ago

I got this: pachi: board.c:238: board_clear: Assertion `size > 1 && size <= BOARD_MAX_SIZE' failed.

Only seems to happen with DCNN enabled and no BOARD_SIZE defined. With DCNN and BOARD_SIZE defined it just segfaults.

The other major difference is that it does not understand arguments to lz-analyze. q5go passes an interval that makes it receive an evaluation every second (in batch analysis mode this is used to control the amount of search time), and pachi sends them much faster.

lemonsqueeze commented 5 years ago

Updated analyze_fix branch:

Dumb prototype for now, will crash after a few hundred lz-analyze commands.

lemonsqueeze commented 5 years ago

I got this: pachi: board.c:238: board_clear: Assertion `size > 1 && size <= BOARD_MAX_SIZE' failed.

Only seems to happen with DCNN enabled and no BOARD_SIZE defined. With DCNN and BOARD_SIZE defined it just segfaults.

Strange, it works for me. Which command line / gtp commands did you use ?

bernds commented 5 years ago

Just plain ./pachi in the build directory.

(gdb) bt

0 0x0000000000000000 in ?? ()

1 0x000000000040c3ee in dcnn_init (b=b@entry=0x9eddb0) at dcnn.c:120

bernds commented 5 years ago

... oh great, mustn't paste backtraces into github issues :(

lemonsqueeze commented 5 years ago

Something weird going on here, try make clean ; make If it's still happening make fast instead will give better debug info.

lemonsqueeze commented 5 years ago

gtp_setoption + analyze_fix PR getting ready, Let me know if there are still caveats from q5go point of view.

lemonsqueeze commented 5 years ago

analyze_fix PR merged, closing this one. If there are issues remaining pls create an issue.