From: Joost VandeVondele Date: Tue, 11 Feb 2020 19:42:32 +0000 (+0100) Subject: Fix for incorrect VALUE_MATE_IN_MAX_PLY usage. X-Git-Url: https://git.sesse.net/?p=stockfish;a=commitdiff_plain;h=be5a2f015e45886e32867b4559ef51dd694a3cec;ds=inline 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 2ed6ebc2..6745ee26 100644 --- a/src/endgame.cpp +++ b/src/endgame.cpp @@ -137,7 +137,7 @@ Value Endgame::operator()(const Position& pos) const { ||(pos.count(strongSide) && pos.count(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::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 c8a35a36..f1416a74 100644 --- 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 902c2cfc..7ab7560a 100644 --- 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,