Skip to content
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

Introduce a switch for reverting twofold draw visits #1513

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

Naphthalin
Copy link
Contributor

When #1375 was merged, we decided to revert all visits to twofold draws if they were made into non terminals to keep the tree visit consistent. However, this introduced two problematic interactions:

  1. In rare cases it's possible that the repetition line was the only non-losing line for one side, and reverting all visits to that line specifically will make first move of that line look much worse (or better) than it actually is.
  2. In fast time controls, reverting the visits will lead to other root moves having more visits, therefore being selected accidentally. This can by made even more likely by an interaction with 1, and happened in Evaluation Blindness to forced 3-fold repetition #1466

This PR introduces a switch whether to revert twofold visits when the twofold draw node is made non terminal; default behaviour is --revert-two-fold-visits=false for testing purposes, =true is equivalent to the current behaviour.

@Naphthalin
Copy link
Contributor Author

from Discord, thx to trre:

decided to take a stab at it since it seems like MakeNotTerminal(terminal_visits) was doubling the visits the node should have. changed it to MakeNotTerminal(0) and kept the if (!revert_two_fold_visits) break; in there so nothing is reverted

@trre123
Copy link
Contributor

trre123 commented Dec 15, 2021

With changes listed above, this test was run at 6s+0.1s

   # PLAYER        :  RATING  ERROR  POINTS  PLAYED   (%)  CFS(%)    W    D    L  D(%)
   1 lc0-pr1513    :     2.1   10.6   503.0    1000    50      65  133  740  127    74
   2 lc0-master    :     0.0   ----   497.0    1000    50     ---  127  740  133    74

@Naphthalin Naphthalin added the testing required Feature/bug fix needs more testing. Implies not for merge. label Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing required Feature/bug fix needs more testing. Implies not for merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants