Resolve #11: Fixing Fox's komi values #16
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is just the functional part and two test files. Test cases coming next as well as documentation. Adjusting Komi on fox is added as an option you can pass to ParseToken. This handles fox's komi in 3 cases by following kaTrain's fixup: 1. Handicap games it adjusts to 0.5 Komi 2. Even games using chinese rules it adjusts to 7.5 Komi (from 375) 3. From other rulesets it adjusts to (6.5 from 325) Note: probably a bug here as some games have 0 as Komi and we shouldn't add Komi in those cases.
After finding a number of test cases, I think this is more likely to get the various scenarios across handicap stones, rulesets, and what not correct. Update test cases to cover. Added documentation. Handling komi for fox is trickier than it seems. I settled on detecting actual handicap stones to determine 0.5 scenarios and then used a combination of rulesets and handicap setting to determine between 6.5 and 7.5 handicap. Note: The "Better" way for some of this would be to walk the nodes searching for the data we see. IE instead of nesting into the data structure directly in search of Fox's AB nodes for adding black stones, we should walk the tree searching for that node. However, since this solution is about fixing up a specific go client I went with what is faster. For the record some servers like Pandanet (IGS) do put their handicap stones with the rest of the game metadata in the root node and not in the first child. The logic is to assume a common handicap, 6.5 which is used on many game formats and servers. Then I check for fox specific scenarios around handicap and different rulesets. After that, I use the presence of handicap stones to figure out if the game should be 0.5 handicap and I assume the actual presence of handicap stones trumps everything else on Fox.
loarabia
changed the title
apetresc
reviewed
Aug 13, 2023
| KM[11] | ||
| RU[Japanese] | ||
| ;B[pq] | ||
| (;W[dc] |
There was a problem hiding this comment.
Out of curiosity, why is every single move a new branch?
There was a problem hiding this comment.
I'm not sure.
I think we'd have to ask @anoek over in OGS why the SGF is generated like that. IIRC, OGS only publicly publishes frontend code and not backend code so I don't think I can investigate why OGS SGF generation works that way.
I grabbed a game from my record on OGS and used it. I could pull something from KGS I suppose.
There was a problem hiding this comment.
@apetresc, It turns out you aren’t the only person to ask this question. online-go/online-go.com#2005 I just piled on and asked there as well.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds solution, test cases, and documentation.
First implementation followed KaTrain's model. Second implementation is based on my own logic after studying a number of files and debugging through parsing them.
Handling komi for Fox is trickier than it seems. I settled on detecting actual handicap stones to determine 0.5 scenarios and then used a combination of rulesets and handicap setting to determine between 6.5 and 7.5 handicap. There is likely more that I'm missing for other rulesets because I don't have good examples.
The logic is:
Note: The "Better" way for some of this would be to walk the nodes searching for the data we seek. E.g. instead of nesting into the data structure directly in search of Fox's AB nodes for adding black handicap stones, we should walk the tree searching for that node. However, since this solution is about fixing up a specific go client that likely writes it out consistently, I went with what was easier to write.
For the record, some servers like OGS and Pandanet (IGS) do put their handicap stone nodes with the rest of the game metadata in the root node and not in the first child.