lizzieyzy icon indicating copy to clipboard operation
lizzieyzy copied to clipboard

Incorrect handling of SGF placements

Open lightvector opened this issue 2 years ago • 6 comments

It appears that lizzieyzy handles SGF placements incorrectly when those placements are used in SGF commentaries to demonstrate alternative board positions in the middle of a game. Here is an attached SGF file demonstrating this.

(;GM[1]FF[4]CA[UTF-8]AP[CGoban:3]ST[2]RU[Japanese]SZ[19]KM[6.50]PW[White]PB[Black];B[pd];W[pp];B[dd];W[dp];B[qn](;W[nq])(;AE[dp]AW[dd]AB[co][dq]))

After move 5, the board is changed from this position: image

To this position: image

However, in lizzieyzy, it looks like this, failing to change the upper left corner correctly:

image

lightvector avatar Dec 26 '22 16:12 lightvector

The AB/AW/AE tags are sometimes used in SGF commentary files to edit the board in the middle of a game, to demonstrate how a sequence that is failing might work if the position were slightly different, and so sometimes they are used to delete, add or change the color of existing stones. lizzieyzy appears to delete and add stones using these tags, but fails when the tags request to change the color of an existing stone.

lightvector avatar Dec 26 '22 16:12 lightvector

Additionally, it appears that deleting stones is not communicated properly to the GTP program:

image

You can see here that the white stone in the lower-left 4-4 point is still present on the board in the GTP program, even though it has been removed from Lizzieyzy's graphical display.

For general GTP programs, since GTP doesn't have an explicit way to delete stones from the board, the correct way to communicate the stone removal is to clear the board, and then re-place all stones on the board. For KataGo specifically, there is a "set_position" command documented at https://github.com/lightvector/KataGo/blob/master/docs/GTP_Extensions.md, which can be used to directly set an arbitrary position. It is also recommended to use "set_position" instead of a sequence of "play" commands, because modern bots may use the move history to affect their policy predictions, and using a sequence of "play" may give a ridiculous move history.

lightvector avatar Dec 26 '22 16:12 lightvector

Thanks for the detailed reporting. "set_position" only works in katago,so I'll re-synchronize all stones on board if goto or leave a move with "AE" flag in SGF,by sending "clear_board" and many "play". Maybe by this way,other AIs like leeazero will also work fine. I have made a not full-tested fix,hope it works fine.

yzyray avatar Jan 18 '23 03:01 yzyray

I tested the latest release - thank you, it is definitely better than before and some SGFs work for me successfully that failed before. There are still some issues however. The same SGF I posted at the start of this issue:

(;GM[1]FF[4]CA[UTF-8]AP[CGoban:3]ST[2]RU[Japanese]SZ[19]KM[6.50]PW[White]PB[Black];B[pd];W[pp];B[dd];W[dp];B[qn](;W[nq])(;AE[dp]AW[dd]AB[co][dq]))

It deletes the stone properly on D4, however it does not replace the stone on D16 with a white stone.

Initial state on move 5: image

Correct state of board after SGF placements happen: image

Current lizzieyzy (upper left corner is wrong): image

lightvector avatar Feb 19 '23 14:02 lightvector

@yzyray - If it is useful to you for testing, I created a file that does various patterns of large whole-board edits. Here it is:

(;GM[1]FF[4]CA[UTF-8]AP[CGoban:3]ST[2]
RU[Japanese]SZ[13]KM[0.00]
PW[White]PB[Black]
;B[jd]
;AW[ad][bd][cd][dd][ed][fd][gd][hd][id][jd][kd][ld][md][ae][be][ce][de][ee][fe][ge][he][ie][je][ke][le][me][ah][bh][ch][kh][lh][mh][ai][bi][ci][ki][li][mi][ej][fj][gj][hj][ij][ek][fk][gk][hk][ik][el][fl][gl][hl][il][em][fm][gm][hm][im]AB[aa][ba][ca][da][ea][fa][ga][ha][ia][ja][ka][la][ma][ab][bb][cb][db][eb][fb][gb][hb][ib][jb][kb][lb][mb][af][bf][cf][df][jf][kf][lf][mf][ag][bg][cg][dg][jg][kg][lg][mg][fh][gh][hh][fi][gi][hi][aj][bj][cj][dj][jj][kj][lj][mj][ak][bk][ck][dk][jk][kk][lk][mk][al][bl][cl][dl][jl][kl][ll][ml][am][bm][cm][dm][jm][km][lm][mm]PL[B]
;B[fg]
;W[ff]
;AE[aa][ha][ma][fd][kd][ce][af][jf][mh][ki][cj][hj][ek][jk][am][fm][mm]AW[ga][ia][ja][ka][db][fb][gb][hb][jb][ac][cc][ec][gc][kc][bf][df][ef][gf][hf][lf][ag][bg][cg][eg][fg][hg][ig][kg][lg][mg][fh][gh][hh][ih][jh][ei][fi][gi][ii][bj][dj][jj][lj][mj][ck][kk][mk][dl][jl][cm][dm]AB[bc][fc][hc][jc][lc][mc][cd][ed][gd][id][md][be][de][fe][je][le][me][if][ah][ch][eh][kh][ai][bi][di][ji][li][ej][gj][ij][fk][hk][el][il][hm][im]PL[W]
(;AE[ca][ia][ja][la][cb][db][eb][jb][ec][jc][kc][lc][ed][gd][ld][ae][be][ge][le][me][bf][gf][hf][if][bg][cg][dg][ig][ih][jh][kh][di][ei][fi][aj][fj][kj][lj][mj][ak][fk][gk][hk][mk][al][bl][cl][hl][ml][cm][hm][im][jm]AW[da][ea][fa][ab][kb][lb][mb][fc][hc][mc][cd][md][ce][je][cf][jf][jg][eh][ai][bi][li][gj][hj][ij][bk][dk][il][kl][fm][km]AB[aa][ga][gb][hb][cc][dc][ic][dd][jd][kd][ee][ke][af][ff][lf][ag][fg][gg][hg][mg][bh][hh][mh][ci][ii][cj][dj][jj][ek][jk][kk][fl][gl][am][gm][mm]
(;AE[fb][gb][hb][ib][dc][fc][gc][hc][ic][dd][hd][id][jd][fg][hg][lg][dj][ej][gj][hj][ij][jj][dk][ek][ik][jk][dl][el][fl][gl][il][jl]AW[de][ee][fe][ge][ff][gf][hf][if][dg][gg][ig][dh][hh][ih][jh][di][ei][fi][hi][ii][ji]AB[ca][da][ea][fa][ha][ia][ja][ka][la][ma][ab][cb][kb][lb][mb][ac][kc][lc][mc][ad][bd][cd][ld][md][ae][be][ce][le][me][bf][cf][cg][kg][kh][lh][ai][bi][ki][li][mi][aj][bj][kj][lj][mj][ak][bk][ck][mk][al][bl][cl][kl][ml][cm][dm][em][fm][hm][im][jm][km]
;B[gc])
(;AE[aa][ba][da][ea][fa][ga][ka][ab][bb][fb][gb][hb][ib][kb][lb][mb][ac][bc][cc][fc][gc][hc][ic][mc][ad][bd][dd][id][jd][md][ce][de][ee][fe][he][ie][af][df][ef][ff][jf][kf][lf][mf][ag][eg][fg][gg][hg][jg][kg][lg][mg][ah][bh][ch][eh][fh][gh][hh][lh][mh][ai][bi][gi][hi][ii][ji][li][mi][bj][dj][ej][gj][hj][jj][bk][ck][ek][kk][lk][dl][el][fl][gl][il][jl][kl][ll][am][bm][dm][em][fm][gm][km][lm][mm]AW[dc][ke][kh][ci][di][kj][jk]AB[ec][jc][ed][hd][fj][ij][dk][ik]
;W[gg]))
(;B[dc]
;W[am]
;B[jk]
;W[ma])
(;W[aa]
;B[gg]
;W[mm]))

Here is the the file uploaded to Eidogo, where you can see where the correct patterns should be at each node within the tree: http://eidogo.com/#GlUVaSmJ

Some screenshots of where specific points of the tree should look like: image image image image image

Thank you for all your work! Except for this issue being unable to handle these SGF files, this is definitely one of the most useful GUIs I've tried so far.

lightvector avatar Feb 19 '23 15:02 lightvector

@yzyray - The above bug that remains seems to be primarily because the program does not handle direct replacements of a black stone with a white stone or vice versa.

However, I found an another bug that only involves deletions, rather than direct replacements of the opponent's stone! Here is a very simple SGF that demonstrates the issue:

(;GM[1]FF[4]CA[UTF-8]AP[CGoban:3]ST[2]
RU[Japanese]SZ[19]KM[0.00]
PW[White]PB[Black]PL[B]
;B[pp]
(;AE[pp]AB[dc][cd][dd][de][df][dg][ch][dh][eh]PL[W]
;W[di])
(;AE[pp]AB[jc][kc][id][ld][le][kf][jg][ih][jh][kh][lh]PL[W]
;W[ji])
(;AB[dk][ek][cl][fl][fm][dn][en][fo][cp][fp][dq][eq]
;W[dr]))

The correct structure of this SGF is that there should be 3 separate variations, with black stones showing "1", "2", "3". Here is the result in other SGF editors: image


However, Lizzieyzy incorrectly merges the first two nodes and only produces 2 variations, with two white moves branching off of the merged node:

image

If you step on the merged node, then you see both the 1 and 2 at the same time!

image

If you step forward again, then the 2 vanishes and the board seems to correct on the UI, however, the GTP engine was not sent the correct information, it still has been told that the 2 is on the board:

image

So whenever an SGF file has multiple variations like this, it is sometimes impossible to analyze the separate variations because the stones from one variation are always contaminating the other variation that was incorrectly merged into the same node: :(

image

lightvector avatar Feb 26 '23 16:02 lightvector