Allow lizzie to work with alreadydone's dynamic komi. #329
Conversation
|
Why is it important to display the dynamic komi? isn't it best hidden from the user? Also I was able to use lizzie successfully before with the dynamic komi branch. What was going wrong? |
| @@ -425,6 +435,13 @@ public void shutdown() { | |||
| } | |||
| } | |||
|
|
|||
| public String getKomi() { | |||
| if (komi != null && oppKomi != null) { | |||
| return komi.substring(5,9) + " / " + oppKomi.substring(5+4, 9+4); | |||
There was a problem hiding this comment.
If displaying the komi is indeed important, this needs to be reworked. you haven't done range checking in the komi for substring; and you are using unnamed constants. Additionally, what if the komi is multiple digits? eg 17.5? Would this code still work?
There was a problem hiding this comment.
The dynamic komi changes during the game. The bot is trying to first set the dynamic komi so that the winrate is higher than desired threshold. This gives some indication of how close is the handicap game as you can observer the komi going down from 50 till 7.5.
Yes, I have used magical numbers as I have assumed that the komi and oop_komi strings will only be reported by this particular leelaz.exe. Note that komi always starts with "komi " abd oopKomi with "oop_komi" so here is 5 and 5+4 constant.
The numbers reported by the version with komi are with super-high precision and I thought printing just 4 first characters is enough (e.g. 7.50 or 53.4 or 123.).
However I am also fine with abandoning the visual change whatsoever if you prefer to not clunk the UI with that (however it only shows if the leelaz is reporting this komi).
There was a problem hiding this comment.
I still don't think we should rely on being given 4 digits of precision. I guess the user who is using this version of leelaZ probably knows about the dynamic komi, so, it's ok if:
- you add string length checking in case they change their format
- add a label to the komi such as dynamic komi:
|
|
||
| variation = new ArrayList<>(Arrays.asList(data)); | ||
| variation = variation.subList(9, variation.size()); | ||
| for (int i=0; i<data.length; i++) { |
There was a problem hiding this comment.
change looks good if it's necessary but let's extract it to a new method, I miss the simplicity of the old code that was here.
There was a problem hiding this comment.
The old code is simple because it has magical numbers 3,5,7 and 9 assuming constant positions of certain values in leela-zero output and some forks add some extra information that can be read by some tools.
Old output: move C4 visits 274 winrate 4633 order 4 pv C4 Q16 R17
New output: move D17 visits 109 winrate 4630 N 0.881185 order 10 pv D17 Q4 R3 R4 Q3 O3 P3 P4 O2 D4 Q16
Would you elaborate on the kind of a method you'd like to extract here?
Something like this?
HashMap<String, String> ParseLine(line); //key,value pairs
and in the constructor:
(...)playouts = parsed["visits"]
There was a problem hiding this comment.
OK thanks for clarifying what went wrong. I guess the code is simple enough since that's all this constructor does! Not really important.
|
Thank you for taking time to review my changes. |
|
I have refactored the code for parsing komi and oop_komi from the leelaz.exe process. |
|
probably going under the board is ok until we have a better solution. then it's good! :) |
|
In an optimistic hope that this pull will be accepted I also added a commit to add the missing trailing space after "Pondering" string in resources. |
|
I added the show-dynamic-komi to the config file as well. |
|
thanks for working on this commit :) |
|
thank you for accepting my code! |
There is a fork (https://github.com/alreadydone/lz) of leela-zero that introduces a concept of dynamic komi.
In order for lizzie to work with that it's necessary to fix leelaz output parsing in MoveData.java.
The second step is to keep track of the current komi reported by leelaz and print it on the screen.
This pull request has no effect on lizzie when used with vanilla leelaz.exe
