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.

Sign up for GitHub

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

Jump to bottom

Update search.cc to implement more aggressive time prune threshold to CLOP tune #123

Merged
merged 19 commits into from
Jul 29, 2018
Merged

Update search.cc to implement more aggressive time prune threshold to CLOP tune #123

merged 19 commits into from
Jul 29, 2018

Conversation

jjoshua2
Copy link
Contributor

Implement EARLY-C strategy from https://pdfs.semanticscholar.org/a2e6/299fd3c8ab17e3a1a783d518688b55bb2363.pdf. I came up with this independently and tried it in lczero as well https://github.com/glinscott/leela-chess/pull/556/files and found it helped CPU but not GPU for some reason...

@jjoshua2
Update search.cc
58853a3 
Implement EARLY-C strategy from https://pdfs.semanticscholar.org/a2e6/299fd3c8ab17e3a1a783d518688b55bb2363.pdf. I came up with this independently and tried it in lczero as well https://github.com/glinscott/leela-chess/pull/556/files and found it helped CPU but not GPU for some reason...
@jjoshua2 jjoshua2 requested a review from Tilps June 30, 2018 02:44
mooskagh
mooskagh reviewed Jun 30, 2018
View reviewed changes
src/mcts/search.cc Outdated
@@ -41,6 +41,7 @@ const char* Search::kTempDecayMovesStr = "Moves with temperature decay";
const char* Search::kNoiseStr = "Add Dirichlet noise at root node";
const char* Search::kVerboseStatsStr = "Display verbose move stats";
const char* Search::kSmartPruningStr = "Enable smart pruning";
const char* Search::pEarlyExit = "Aggressive smart pruning threshold";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pEarlyExitStr

src/mcts/search.cc Outdated
@@ -72,6 +73,8 @@ void Search::PopulateUciParams(OptionsParser* options) {
options->Add<BoolOption>(kNoiseStr, "noise", 'n') = false;
options->Add<BoolOption>(kVerboseStatsStr, "verbose-move-stats") = false;
options->Add<BoolOption>(kSmartPruningStr, "smart-pruning") = true;
options->Add<FloatOption>(p_early_exit, 0.1f, 10.0f,
"p_early_exit") = 1.0f;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dashes in flag name instead of underscores.

src/mcts/search.cc Outdated
@@ -578,7 +581,7 @@ SearchWorker::NodeToProcess SearchWorker::PickNodeToExtend() {
// To ensure we have at least one node to expand, always include
// current best node.
if (child != search_->best_move_node_ &&
search_->remaining_playouts_ <
p_early_exit*search_->remaining_playouts_ <
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this variable defined?
Also it's one additional float multiplication per visit. Probably not noticeable at all, but would be interesting to check with --backend=random

src/mcts/search.cc Outdated
@@ -72,6 +73,8 @@ void Search::PopulateUciParams(OptionsParser* options) {
options->Add<BoolOption>(kNoiseStr, "noise", 'n') = false;
options->Add<BoolOption>(kVerboseStatsStr, "verbose-move-stats") = false;
options->Add<BoolOption>(kSmartPruningStr, "smart-pruning") = true;
options->Add<FloatOption>(p_early_exit, 0.1f, 10.0f,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pEarlyExitStr

jjoshua2 added 5 commits June 30, 2018 10:23
@jjoshua2
Update search.cc
b371595 
Fixed comments except I dont know how to initalize p_early_exit yet. I was hoping all the options stuff did that...
@jjoshua2
Update search.cc
6b2b3a6 
I think this is closer...
@jjoshua2
Update search.h
0b36893 
declare two parameters in .h file
@jjoshua2
Update search.cc
242aaad 
I think this is how I get it
@jjoshua2
Update search.cc
7cca459 
got yah!
mooskagh
mooskagh reviewed Jul 15, 2018
View reviewed changes
src/mcts/search.cc Outdated
@@ -72,6 +73,8 @@ void Search::PopulateUciParams(OptionsParser* options) {
options->Add<BoolOption>(kNoiseStr, "noise", 'n') = false;
options->Add<BoolOption>(kVerboseStatsStr, "verbose-move-stats") = false;
options->Add<BoolOption>(kSmartPruningStr, "smart-pruning") = true;
options->Add<FloatOption>(pEarlyExitStr, 0.1f, 10.0f,
"p-early-exit") = 1.0f;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "p" mean here?

src/mcts/search.cc Outdated
@@ -41,6 +41,7 @@ const char* Search::kTempDecayMovesStr = "Moves with temperature decay";
const char* Search::kNoiseStr = "Add Dirichlet noise at root node";
const char* Search::kVerboseStatsStr = "Display verbose move stats";
const char* Search::kSmartPruningStr = "Enable smart pruning";
const char* Search::pEarlyExitStr = "Aggressive smart pruning threshold";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kEarlyExitStr or kPEarlyExitStr

@jjoshua2
Copy link
Contributor Author

The names of the constant p_early_exit is taken from the paper. It's the probability or really percentage that you prune at. If you prune at 1.0 (or above) its 100% chance of success not missing anything given a max_time, whereas the paper actually find optimal was p = 0.4 where you prune at only 40% of the nodes required to change it's mind, and increase scale_mover from 2 to 2.5.

@jjoshua2
Copy link
Contributor Author

jjoshua2 commented Jul 15, 2018 •
edited
Loading

I'm still trying different values but it seems like a 20 elo gain is possible given my two results
30s+.33 p .45 scale 2.5

Score of lc0_PR123 tuned vs lc0_PR123 d: 96 - 80 - 232 [0.520]
Elo difference: 13.63 +/- 22.14

and 18s+.2 default scale p 0.815

Score of lc0_PR123 tuned vs lc0_PR123 default: 129 - 102 - 418 [0.521]
Elo difference: 14.46 +/- 15.92

@jjoshua2
Copy link
Contributor Author

jjoshua2 commented Jul 28, 2018 •
edited
Loading

0.72 kEarly and 2.5 scale time with latest at the time main net 520 and 5+2 TC

Rank Name                          Elo     +/-   Games   Score   Draws
   0 Ethereal10.66 16CPU 4GB TB       8      46     130   51.2%   40.8%
   1 lc0_PR123 520 tuned             5      62      66   50.8%   47.0%
   2 lc0_PR123 520                 -22      70      64   46.9%   34.4%

jjoshua2 added 2 commits July 28, 2018 09:32
@jjoshua2
Update search.cc
4d4223d 
Average of mean and max from CLOP tune 7-28
@jjoshua2
Update engine.cc
bc6809e 
Updated from CLOP tune 7-28
@jjoshua2 jjoshua2 requested a review from dubslow July 28, 2018 16:44
@jjoshua2
Copy link
Contributor Author

jjoshua2 commented Jul 28, 2018 •
edited
Loading

I put in the average for the max and mean from the clop tune this morning. There about 25 elo +- 21 elo gain. I'm not planning on CLOP tuning any more until after TCEC starts probably as I'm testing latest testnets to send now.

dubslow
dubslow approved these changes Jul 29, 2018
View reviewed changes
Copy link
Member

@dubslow dubslow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Change is very minimal, I'm satisfied by jjosh's data that it's plus elo in both self play and against AB engines, and I believe crem's RFCs have been satisfied. Worst case scenario, we submit a further PR to rename it before next release (and we can do that now, rename things before releases! hooray!)

@dubslow dubslow merged commit b1301aa into master Jul 29, 2018
@jjoshua2 jjoshua2 deleted the jjoshua2-patch-3 branch July 30, 2018 16:04
@jjoshua2
Copy link
Contributor Author

jjoshua2 commented Aug 19, 2018 •
edited
Loading

Another test using roy's PR testing .68 vs 1.0 pruning

Score of lc0 10810 leroy j vs lc0 10810 1.0 asp: 49 - 31 - 402 [0.519]
Elo difference: 12.98 +/- 12.59

@borg323 borg323 mentioned this pull request Aug 20, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mooskagh mooskagh mooskagh left review comments

@dubslow dubslow dubslow approved these changes

@Tilps Tilps Awaiting requested review from Tilps

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jjoshua2 @dubslow @mooskagh