Fix issues from using adjustedDepth too broadly
authorJoost VandeVondele <Joost.VandeVondele@gmail.com>
Sun, 28 Oct 2018 14:25:15 +0000 (15:25 +0100)
committerStéphane Nicolet <cassio@free.fr>
Thu, 1 Nov 2018 15:00:56 +0000 (16:00 +0100)
The recently committed Fail-High patch (081af9080542a0d076a5482da37103a96ee15f64)
had a number of changes beyond adjusting the depth of search on fail high, with
some undesirable side effects.

1) Decreasing depth on PV output, confusing GUIs and players alike as described in
   issue #1787. The depth printed is anyway a convention, let's consider adjustedDepth
   an implementation detail, and continue to print rootDepth. Depth, nodes, time and
   move quality all increase as we compute more. (fixing this output has no effect on
   play).

2) Fixes go depth output (now based on rootDepth again, no effect on play), also
   reported in issue #1787

3) The depth lastBestDepth is used to compute how long a move is stable, a new move
   found during fail-high is incorrectly considered stable if based on adjustedDepth
   instead of rootDepth (this changes time management). Reverting this passed STC
   and LTC:

   STC
   LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
   Total: 82982 W: 17810 L: 17808 D: 47364
   http://tests.stockfishchess.org/tests/view/5bd391a80ebc595e0ae1e993

   LTC
   LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
   Total: 109083 W: 17602 L: 17619 D: 73862
   http://tests.stockfishchess.org/tests/view/5bd40c820ebc595e0ae1f1fb

4) In the thread voting scheme, the rank of the fail-high thread is now artificially
   low, incorrectly since the quality of the move is much better than what adjustedDepth
   suggests (e.g. if it takes 10 iterations to find VALUE_KNOWN_WIN, it has very low
   depth). Further evidence comes from a test that showed that the move of highest
   depth is not better than that of the last PV (which is potentially of much lower
   adjustedDepth).

   I.e. this test http://tests.stockfishchess.org/tests/view/5bd37a120ebc595e0ae1e7c3
   failed SPRT[0, 5]:

   LLR: -2.95 (-2.94,2.94) [0.00,5.00]
   Total: 10609 W: 2266 L: 2345 D: 5998

   In a running 5+0.05 th 8 test (more than 10000 games) a positive Elo estimate is
   shown (strong enough for a [-3,1], possibly not [0,4]):

   http://tests.stockfishchess.org/tests/view/5bd421be0ebc595e0ae1f315
   LLR: -0.13 (-2.94,2.94) [0.00,4.00]
   Total: 13644 W: 2573 L: 2532 D: 8539
   Elo 1.04 [-2.52,4.61] / LOS 71%

Thus, restore old behavior as a bugfix, keeping the core of the fail-high patch
idea as resolving scheme. This is non-functional for bench, but changes searches
via time management and in the threaded case.

Bench: 3556672

src/search.cpp

index 2678f7d..c9526ec 100644 (file)
@@ -368,7 +368,6 @@ void Thread::search() {
 
       size_t pvFirst = 0;
       pvLast = 0;
-      Depth adjustedDepth = rootDepth;
 
       // MultiPV loop. We perform a full root search for each PV line
       for (pvIdx = 0; pvIdx < multiPV && !Threads.stop; ++pvIdx)
@@ -405,7 +404,7 @@ void Thread::search() {
           int failedHighCnt = 0;
           while (true)
           {
-                 adjustedDepth = std::max(ONE_PLY, rootDepth - failedHighCnt * ONE_PLY);
+              Depth adjustedDepth = std::max(ONE_PLY, rootDepth - failedHighCnt * ONE_PLY);
               bestValue = ::search<PV>(rootPos, ss, alpha, beta, adjustedDepth, false);
 
               // Bring the best move to the front. It is critical that sorting
@@ -428,7 +427,7 @@ void Thread::search() {
                   && multiPV == 1
                   && (bestValue <= alpha || bestValue >= beta)
                   && Time.elapsed() > 3000)
-                  sync_cout << UCI::pv(rootPos, adjustedDepth, alpha, beta) << sync_endl;
+                  sync_cout << UCI::pv(rootPos, rootDepth, alpha, beta) << sync_endl;
 
               // In case of failing low/high increase aspiration window and
               // re-search, otherwise exit the loop.
@@ -463,15 +462,15 @@ void Thread::search() {
 
           if (    mainThread
               && (Threads.stop || pvIdx + 1 == multiPV || Time.elapsed() > 3000))
-              sync_cout << UCI::pv(rootPos, adjustedDepth, alpha, beta) << sync_endl;
+              sync_cout << UCI::pv(rootPos, rootDepth, alpha, beta) << sync_endl;
       }
 
       if (!Threads.stop)
-          completedDepth = adjustedDepth;
+          completedDepth = rootDepth;
 
       if (rootMoves[0].pv[0] != lastBestMove) {
          lastBestMove = rootMoves[0].pv[0];
-         lastBestMoveDepth = adjustedDepth;
+         lastBestMoveDepth = rootDepth;
       }
 
       // Have we found a "mate in x"?