-
-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature suggestion: Display territory analyze for KataGo #619
Comments
Documentation about the feature in KataGo: https://github.com/lightvector/KataGo/blob/master/docs/GTP_Extensions.md
|
I have to agree with this feature request. As a beginner It would help my game to see the territory change dynamically |
similar #589 |
Sabaki can't integrate with features that are only available in |
Except for the very fact of including more values to report, in what way does it not conform right now? Is it the fact that winrate is reported as a float rather than multiplied by 100 and rounded to an integer? That's the only difference I can think of right now and it seems like a rather trivial one, so maybe I'm missing something obvious. If necessary, I can add an additional input option (following after color and interval) that changes this behavior. Edit: There is also the fact that ownership is only output once, rather than per-move, because it applies to all board locations rather than just moves (for example, locations that are covered by stones or are illegal suicides). In reviewing KataGo's GTP-extension documentation (https://github.com/lightvector/KataGo/blob/master/docs/GTP_Extensions.md) I realized that this wasn't the most clearly documented, so I updated the documentation just now to be clearer about how ownership is placed within the "grammar tree" of the output format. But even if you don't support this, it shouldn't stop one from at least reporting values like the score, which are simply additional fields within the original |
Exactly. Not only the winrate, but also lcb, prior are different from As for ownership, it doesn't quite fit into the key value pair format, since the "value" actually consists of multiple values separated by spaces. You need to know the number of values before you're able to parse the whole directive correctly. |
Thanks for the clarifications! I think I understand. Specific thoughts:
What do you think? (edit: minor clarifications, some formatting improvements, note about future-proofness) |
Thanks for explaining your rationale! About the Frankly, I don't like the idea of a Obviously, changing the default behavior of Now I'm also out of good ideas. |
This comment has been minimized.
This comment has been minimized.
@lightvector I've tested out a few scenarios; it seems it's unlikely that KataGo reports a board position with a 100% winrate, so I'm going forward with the idea of looking if a winrate has a I will update the documentation as follows:
What do you think? |
I constructed a board position where the winrate from kata-analyze on a move is exactly 100%, and currently it does look like the floating point formatter currently used by the code ( Depending on the presence of a decimal point for scaling feels a little fragile. Given that KataGo already fully conforms to Sabaki's analysis output format for both the "analyze" and "lz-analyze" commands, can we not simply just say either that "kata-analyze" needs a slightly different numeric parser, or to allow me to implement an optional flag that would bring it in line with the other two analysis commands on precisely the fields that currently overlap between the two? The semantics of the command is rigidly documented (https://github.com/lightvector/KataGo/blob/master/docs/GTP_Extensions.md) and I plan to never break compatibility in any future changes, so there shouldn't need to be some dependence on subtle formatting details. Sorry for my part in the mess. If I had realized that lz-analyze itself was based around something from Sabaki, I might have considered more carefully for output formatting. Until more recently I wasn't aware that it was anything other than a one-off Leela-Zero-specific command with no prior precedent, the same way GnuGo or Pachi have a ton of idiosyncratic extensions as well, so I didn't see the harm in picking a scaling that made it more natural to have higher precision, given that I wanted to add a bunch of other new values as well. |
Maybe you can introduce a decimal point for 100% for the future? I could also forgo The syntax of My goal here is to avoid idiosyncratic GTP extensions and let any engine to utilize Sabaki's analysis visualization features without me having to implement engine specific logic continuously. I'm sure a consistent syntax will benefit everyone in the end. |
The decimal point is fragile and would be hacky to force. I don't think this is a good idea - relying on the way a float is written or trying to get it to format exactly right is going to be a source of potential bugs. Introducing One note if you're thinking about implementing ownership prediction reporting API: a toggle to turn it on/off in the bot itself is necessary, because unlike score-related outputs, which are just a single additional number, ownership predictions are a value per every board location, and computing and reporting it has nontrivial cost and will slow down the search, so it should be computed only if actually needed. If Sabaki is in a mode where it will not be displayed, the API should support a way to also tell the bot in the analysis command so that the bot can stop computing it. (kata-analyze indeed currently implements an argument-based method of controlling whether it reports ownership or not). |
Oh, does Benefit of modifying |
AFAIK there was no precedent to I'm fine with having a |
Okay, well let me know what you want to do. If you're still really a fan of the float decimal point thing, I'll try to safeguard KataGo's output for next release. Otherwise, I'll add the appropriate fields in whatever specification you wish to either |
I've thought about it, and I'd like to keep my specification like it is now (different parsing based on whether decimal point is available or not), because I do like specifying percentages as decimal numbers, and I also don't want to lose compatibility with On your side, if you don't want to introduce a "hack" to have a decimal point for |
The thing I don't like is that this choice deliberately introduces a bug that did not exist before between existing versions (namely, Sabaki post-decimal-point change and Kata v1.3.3), in the face of pre-existing documentation that warns that such an implementation may have such a bug (or at least, does not preclude that it may have a bug). But maybe it's so rare that it doesn't matter, and I agree compatibility is nice. Anyways, I'll update the floating point output in the next version then. |
And I'll modify the plain "analyze" command to also have the appropriate fields too, now that I know that this is a Sabaki-specific command rather than one that was decided on and fixed by someone else. (Edit: Thanks!) |
It turns out to be a bit messy to ensure that floats always have decimal points for So I stuck with solely modifying |
Thank you for the update! |
KataGo v.1.3.4 is released with these changes now. Let me know if there is anything else. Thanks! |
Thanks @yishn and @lightvector for collaborating on improving Sabaki and KataGo interoperability! 🍾 |
Any news here? |
May I ask the progress? Such an amazing feature we are waiting for so long time. 😀 @yishn |
KataGo can assume the territory ownership through the game. It would be nice to display it on the board as Lizzie does:
The text was updated successfully, but these errors were encountered: