Fix for incorrect VALUE_MATE_IN_MAX_PLY usage.

Fixes #2533, fixes #2543, fixes #2423.

the code that prevents false mate announcements depending on the TT
state (GHI), incorrectly used VALUE_MATE_IN_MAX_PLY. The latter
constant, however, also includes, counterintuitively, the TB win range.

This patch fixes that, by restoring the behavior for TB win scores,
while retaining the false mate correctness, and improving the mate
finding ability. In particular

no alse mates are announced with the poisened hash testcase
```
position fen 8/8/8/3k4/8/8/6K1/7R w - - 0 1
go depth 40
position fen 8/8/8/3k4/8/8/6K1/7R w - - 76 1
go depth 20
ucinewgame
```

mates are found with the testcases reported in #2543
```
position fen 4k3/3pp3/8/8/8/8/2PPP3/4K3 w - - 0 1
setoption name Hash value 1024
go depth 55
ucinewgame
```
and
```
position fen 4k3/4p3/8/8/8/8/3PP3/4K3 w - - 0 1
setoption name Hash value 1024
go depth 45
ucinewgame
```

furthermore, on the mate finding benchmark (ChestUCI_23102018.epd),
performance improves over master, roughly reaching performance with the
false mate protection reverted
```
Analyzing 6566 mate positions for best and found mates:

                 ----------------best ---------------found
           nodes master revert  fixed master revert  fixed
        16000000   4233   4236   4235   5200   5201   5199
        32000000   4583   4585   4585   5417   5424   5418
        64000000   4852   4853   4855   5575   5584   5579
       128000000   5071   5068   5066   5710   5720   5716
       256000000   5280   5282   5279   5819   5827   5826
       512000000   5471   5468   5468   5919   5935   5932
```

On a testcase with TB enabled, progress is made consistently, contrary
to master
```
setoption name SyzygyPath value ../../../syzygy/3-4-5/
setoption name Hash value 2048
position fen 1R6/3k4/8/K2p4/4n3/2P5/8/8 w - - 0 1
go depth 58
ucinewgame
```

The PR (prior to a rewrite for clarity)

passed STC:
LLR: 2.94 (-2.94,2.94) {-1.50,0.50}
Total: 65405 W: 12454 L: 12384 D: 40567
Ptnml(0-2): 920, 7256, 16285, 7286, 944
http://tests.stockfishchess.org/tests/view/5e441a3be70d848499f63d15

passed LTC:
LLR: 2.94 (-2.94,2.94) {-1.50,0.50}
Total: 27096 W: 3477 L: 3413 D: 20206
Ptnml(0-2): 128, 2215, 8776, 2292, 122
http://tests.stockfishchess.org/tests/view/5e44e277e70d848499f63d63

The incorrectly named VALUE_MATE_IN_MAX_PLY and VALUE_MATED_IN_MAX_PLY
were renamed into VALUE_TB_WIN_IN_MAX_PLY and VALUE_TB_LOSS_IN_MAX_PLY,
and correclty defined VALUE_MATE_IN_MAX_PLY and VALUE_MATED_IN_MAX_PLY
were introduced.

One further (corner case) mistake using these constants was fixed (go
mate X), which could lead to a premature return if X > MAX_PLY / 2,
but TB were present.

Thanks to @svivanov72 for one of the reports and help fixing the issue.

closes https://github.com/official-stockfish/Stockfish/pull/2552

Bench: 4932981

diff --git a/src/endgame.cpp b/src/endgame.cpp
index 2ed6ebc27b78a2a9de0428886ec7503cdb233c0f..6745ee2664c612689646c283f78833355d509dcf 100644 (file)
--- a/src/endgame.cpp
+++ b/src/endgame.cpp
@@ -137,7 +137,7 @@ Value Endgame<KXK>::operator()(const Position& pos) const {
       ||(pos.count<BISHOP>(strongSide) && pos.count<KNIGHT>(strongSide))
       || (   (pos.pieces(strongSide, BISHOP) & ~DarkSquares)
           && (pos.pieces(strongSide, BISHOP) &  DarkSquares)))
-      result = std::min(result + VALUE_KNOWN_WIN, VALUE_MATE_IN_MAX_PLY - 1);
+      result = std::min(result + VALUE_KNOWN_WIN, VALUE_TB_WIN_IN_MAX_PLY - 1);
 
   return strongSide == pos.side_to_move() ? result : -result;
 }
@@ -162,7 +162,7 @@ Value Endgame<KBNK>::operator()(const Position& pos) const {
                 + PushClose[distance(winnerKSq, loserKSq)]
                 + PushToCorners[opposite_colors(bishopSq, SQ_A1) ? ~loserKSq : loserKSq];
 
-  assert(abs(result) < VALUE_MATE_IN_MAX_PLY);
+  assert(abs(result) < VALUE_TB_WIN_IN_MAX_PLY);
   return strongSide == pos.side_to_move() ? result : -result;
 }
 

diff --git a/src/search.cpp b/src/search.cpp
index c8a35a36d2f079906bf8f38a46775c95b7727262..f1416a74091e1db17c21571618eb1a892721c266 100644 (file)
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -290,13 +290,13 @@ void MainThread::search() {
           votes[th->rootMoves[0].pv[0]] +=
               (th->rootMoves[0].score - minScore + 14) * int(th->completedDepth);
 
-          if (bestThread->rootMoves[0].score >= VALUE_MATE_IN_MAX_PLY)
+          if (bestThread->rootMoves[0].score >= VALUE_TB_WIN_IN_MAX_PLY)
           {
               // Make sure we pick the shortest mate
               if (th->rootMoves[0].score > bestThread->rootMoves[0].score)
                   bestThread = th;
           }
-          else if (   th->rootMoves[0].score >= VALUE_MATE_IN_MAX_PLY
+          else if (   th->rootMoves[0].score >= VALUE_TB_WIN_IN_MAX_PLY
                    || votes[th->rootMoves[0].pv[0]] > votes[bestThread->rootMoves[0].pv[0]])
               bestThread = th;
       }
@@ -755,9 +755,10 @@ namespace {
 
                 int drawScore = TB::UseRule50 ? 1 : 0;
 
-                value =  wdl < -drawScore ? -VALUE_MATE + MAX_PLY + ss->ply + 1
-                       : wdl >  drawScore ?  VALUE_MATE - MAX_PLY - ss->ply - 1
-                                          :  VALUE_DRAW + 2 * wdl * drawScore;
+                // use the range VALUE_MATE_IN_MAX_PLY to VALUE_TB_WIN_IN_MAX_PLY to score
+                value =  wdl < -drawScore ? VALUE_MATED_IN_MAX_PLY + ss->ply + 1
+                       : wdl >  drawScore ? VALUE_MATE_IN_MAX_PLY - ss->ply - 1
+                                          : VALUE_DRAW + 2 * wdl * drawScore;
 
                 Bound b =  wdl < -drawScore ? BOUND_UPPER
                          : wdl >  drawScore ? BOUND_LOWER : BOUND_EXACT;
@@ -863,7 +864,7 @@ namespace {
         if (nullValue >= beta)
         {
             // Do not return unproven mate scores
-            if (nullValue >= VALUE_MATE_IN_MAX_PLY)
+            if (nullValue >= VALUE_TB_WIN_IN_MAX_PLY)
                 nullValue = beta;
 
             if (thisThread->nmpMinPly || (abs(beta) < VALUE_KNOWN_WIN && depth < 13))
@@ -890,7 +891,7 @@ namespace {
     // much above beta, we can (almost) safely prune the previous move.
     if (   !PvNode
         &&  depth >= 5
-        &&  abs(beta) < VALUE_MATE_IN_MAX_PLY)
+        &&  abs(beta) < VALUE_TB_WIN_IN_MAX_PLY)
     {
         Value raisedBeta = std::min(beta + 189 - 45 * improving, VALUE_INFINITE);
         MovePicker mp(pos, ttMove, raisedBeta - ss->staticEval, &thisThread->captureHistory);
@@ -996,7 +997,7 @@ moves_loop: // When in check, search starts from here
       // Step 13. Pruning at shallow depth (~200 Elo)
       if (  !rootNode
           && pos.non_pawn_material(us)
-          && bestValue > VALUE_MATED_IN_MAX_PLY)
+          && bestValue > VALUE_TB_LOSS_IN_MAX_PLY)
       {
           // Skip quiet moves if movecount exceeds our FutilityMoveCount threshold
           moveCountPruning = moveCount >= futility_move_count(improving, depth);
@@ -1494,7 +1495,7 @@ moves_loop: // When in check, search starts from here
       // Detect non-capture evasions that are candidates to be pruned
       evasionPrunable =    inCheck
                        &&  (depth != 0 || moveCount > 2)
-                       &&  bestValue > VALUE_MATED_IN_MAX_PLY
+                       &&  bestValue > VALUE_TB_LOSS_IN_MAX_PLY
                        && !pos.capture(move);
 
       // Don't search moves with negative SEE values
@@ -1560,28 +1561,47 @@ moves_loop: // When in check, search starts from here
   }
 
 
-  // value_to_tt() adjusts a mate score from "plies to mate from the root" to
-  // "plies to mate from the current position". Non-mate scores are unchanged.
+  // value_to_tt() adjusts a mate or TB score from "plies to mate from the root" to
+  // "plies to mate from the current position". standard scores are unchanged.
   // The function is called before storing a value in the transposition table.
 
   Value value_to_tt(Value v, int ply) {
 
     assert(v != VALUE_NONE);
 
-    return  v >= VALUE_MATE_IN_MAX_PLY  ? v + ply
-          : v <= VALUE_MATED_IN_MAX_PLY ? v - ply : v;
+    return  v >= VALUE_TB_WIN_IN_MAX_PLY  ? v + ply
+          : v <= VALUE_TB_LOSS_IN_MAX_PLY ? v - ply : v;
   }
 
 
-  // value_from_tt() is the inverse of value_to_tt(): It adjusts a mate score
+  // value_from_tt() is the inverse of value_to_tt(): It adjusts a mate or TB score
   // from the transposition table (which refers to the plies to mate/be mated
-  // from current position) to "plies to mate/be mated from the root".
+  // from current position) to "plies to mate/be mated (TB win/loss) from the root".
+  // However, for mate scores, to avoid potentially false mate scores related to the 50 moves rule,
+  // and the graph history interaction, return an optimal TB score instead.
 
   Value value_from_tt(Value v, int ply, int r50c) {
 
-    return  v == VALUE_NONE             ? VALUE_NONE
-          : v >= VALUE_MATE_IN_MAX_PLY  ? VALUE_MATE - v > 99 - r50c ? VALUE_MATE_IN_MAX_PLY  : v - ply
-          : v <= VALUE_MATED_IN_MAX_PLY ? VALUE_MATE + v > 99 - r50c ? VALUE_MATED_IN_MAX_PLY : v + ply : v;
+    if (v == VALUE_NONE)
+        return VALUE_NONE;
+
+    if (v >= VALUE_TB_WIN_IN_MAX_PLY)  // TB win or better
+    {
+        if (v >= VALUE_MATE_IN_MAX_PLY && VALUE_MATE - v > 99 - r50c)
+            return VALUE_MATE_IN_MAX_PLY - 1; // do not return a potentially false mate score
+
+        return v - ply;
+    }
+
+    if (v <= VALUE_TB_LOSS_IN_MAX_PLY) // TB loss or worse
+    {
+        if (v <= VALUE_MATED_IN_MAX_PLY && VALUE_MATE + v > 99 - r50c)
+            return VALUE_MATED_IN_MAX_PLY + 1; // do not return a potentially false mate score
+
+        return v + ply;
+    }
+
+    return v;
   }
 
 
@@ -1767,7 +1787,7 @@ string UCI::pv(const Position& pos, Depth depth, Value alpha, Value beta) {
       Depth d = updated ? depth : depth - 1;
       Value v = updated ? rootMoves[i].score : rootMoves[i].previousScore;
 
-      bool tb = TB::RootInTB && abs(v) < VALUE_MATE - MAX_PLY;
+      bool tb = TB::RootInTB && abs(v) < VALUE_MATE_IN_MAX_PLY;
       v = tb ? rootMoves[i].tbScore : v;
 
       if (ss.rdbuf()->in_avail()) // Not at first line

diff --git a/src/types.h b/src/types.h
index 902c2cfce87f49881f4183bc9107e9b085dedbce..7ab7560a7ed5d8fb494b96bd9f95e6f61a1971ef 100644 (file)
--- a/src/types.h
+++ b/src/types.h
@@ -176,8 +176,10 @@ enum Value : int {
   VALUE_INFINITE  = 32001,
   VALUE_NONE      = 32002,
 
-  VALUE_MATE_IN_MAX_PLY  =  VALUE_MATE - 2 * MAX_PLY,
-  VALUE_MATED_IN_MAX_PLY = -VALUE_MATE + 2 * MAX_PLY,
+  VALUE_TB_WIN_IN_MAX_PLY  =  VALUE_MATE - 2 * MAX_PLY,
+  VALUE_TB_LOSS_IN_MAX_PLY = -VALUE_MATE + 2 * MAX_PLY,
+  VALUE_MATE_IN_MAX_PLY  =  VALUE_MATE - MAX_PLY,
+  VALUE_MATED_IN_MAX_PLY = -VALUE_MATE + MAX_PLY,
 
   PawnValueMg   = 128,   PawnValueEg   = 213,
   KnightValueMg = 781,   KnightValueEg = 854,