LeelaWatcher reports illegal moves on legal ones (reason for #9 pinpointed, and is unrelated to fixes suggested)

[grolich]

It seems there are still holes in game logic. I think the problems reported in #4 and #9 were closed too soon (assuming because of the fix suggested in #5 and thus writing this here again) as I have managed to pinpoint the source of #9:

LeelaWatcher doesn't even get to capture logic in the problem mentioned there, which is why the fix won't affect it.

The problem is that isLegalMove() returns false on moves which are legal in all rule systems, before any such logic is reached.

Example cases found:
(To stick to the issue and not get sidetracked, I have constrained myself to look ONLY at the first occurrence of the problem encountered in a game, as further strange artifacts encountered with illegal moves could be an entirely different issue)

example 1:
WatcherOutput1.zip
2nd-262-2017-11-29T13_59_20.908Z.sgf.zip

Move 261 is Black's A10. The next move received, as is seen in WatcherOutput1, is white's S12, which is, while a horrible move, legal in any rule system.

example2:
WatcherOutput2.zip
1st-419-2017-11-29T13_54_19.179Z.sgf.zip

Move 418 is white's Q16, and the next move received is black's m1, which isLegalMove() says is illegal, and therefore is skipped, despite it being a perfectly legal move.

I do realize the very next move placed by LeelaWatcher in the SGF betrays another potential bug, but it's not healthy to consider multiple issues in 1 place...

Not only does the code never get to capture logic at all, but, as example 1 shows, it sometimes isn't even related to it. I will try to pinpoint it further by testing those exact positions on the legality functions when I have the time.

[fsparv]

Can you confirm that you are using the new 1.0.1 release that contains the fix?

[fsparv]

When the new version declares a move illegal it should print out the position. Can you please paste in the move/position (preferable to zip files)

[grolich]

You were right, I seem to have had a version mismatch (only part of the commits before 1.0.1).
The older version indeed reached the aforementioned "illegal move" because of a bad internal state related to the fixes.

I think the project could definitely use an SGF loader. Would both allow people to test new features on actual problematic game positions encountered, and would be great for error testing.

(I used a hacky 10 minute version I wrote which assumes too much about the SGF).

Sorry for the mix up in versions, not sure how I missed that.

[fsparv]

No problem :) Sounds like we can close this... reading SGF and scoring positions are things I had meant to do long ago. I'll have to look but I know scoring didn't get done and I suspect there's not much for SGF parsing happening. I didn't know much about parsers in those days so that might be a good thing :)