From 74a0a73715322608332038f7c0151ddf0609a59a Mon Sep 17 00:00:00 2001 From: "Robert Nurnberg @ elitebook" <28635489+robertnurnberg@users.noreply.github.com> Date: Sun, 14 Jun 2026 09:52:10 +0200 Subject: [PATCH] Final fix for MultiPV mate PV corner cases This PR fixes the remaining corner cases in the treatment of MultiPV mated-in PVs, as well as an oversight in #6886. See the discussion in In particular: 1. `previousScore` and `previousPV` can only be trusted, if that rootmove was indeed fully searched in the previous iteration. 2. A move beyond `pvIdx` (that was hence not fully searched) may have an exact loss score that cannot be trusted. So if a MultiPV search gets aborted while searching `pvIdx`, we mark all the following loss scores as bounds. 3. The forgotten mate logic also got broken in #6886, because the `previousPV` of the forgotten mate's bestmove can only be trusted if that move was fully searched in the previous iteration, something that is not guaranteed. So we now store both `lastBestMoveScore` and `lastBestMovePV`. Here some scenarios for MultiPV = 8 that explain how master was broken: 1. Move A with an inexact mated-in-2 score from the previous iteration (so outside the top8 moves) gets flushed into the top8 moves for the current iteration, because the previous top8 move B is now scored as a mated-in-1. Hence we cannot trust `previousScore` or `previousPV` for move A, if the search gets aborted while it is being searched. 2. In the scenario above, move B has `Score != -VALUE_INFINITE` and a mated-in-1 score, which cannot be trusted as it was not fully searched. 3. Iteration N has bestmove A with mated-in-10, which gets recorded in `lastBestMoveScore` (renamed from `lastIterationScore`). Iteration 11 forgets the mate and has bestmove B with a cp score, move A may have an incomplete PV, and may even have a non-mate score. Iteration 12 gets aborted, and in trying to remember the forgotten mate, master recovers the `previousScore` and `previousPV` of move A, which may be neither mate nor complete. Passed STC non-reg: LLR: 2.94 (-2.94,2.94) <-1.75,0.25> Total: 69728 W: 17748 L: 17573 D: 34407 Ptnml(0-2): 143, 7571, 19274, 7720, 156 https://tests.stockfishchess.org/tests/view/6a2c40c60d5d4b19d08052f2 closes https://github.com/official-stockfish/Stockfish/pull/6906 No functional change --- src/search.cpp | 107 +++++++++++++++++++++++++++---------------------- src/search.h | 21 +++++----- 2 files changed, 69 insertions(+), 59 deletions(-) diff --git a/src/search.cpp b/src/search.cpp index 30d83c712..8e8cbcbc4 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -269,13 +269,13 @@ bool Search::Worker::iterative_deepening() { PVMoves pv; - Move lastBestMove = Move::none(); - Depth lastBestMoveDepth = 0; + PVMoves lastBestMovePV; + Depth lastBestMoveDepth = 0; + Value lastBestMoveScore = -VALUE_INFINITE; Value alpha, beta; - Value bestValue = -VALUE_INFINITE; - Value lastIterationScore = -VALUE_INFINITE; - Color us = rootPos.side_to_move(); + Value bestValue = -VALUE_INFINITE; + Color us = rootPos.side_to_move(); double timeReduction = 1, totBestMoveChanges = 0; int delta, iterIdx = 0; @@ -340,10 +340,11 @@ bool Search::Worker::iterative_deepening() { // Save the last iteration's scores before the first PV line is searched and // all the move scores except the (new) PV are set to -VALUE_INFINITE. - for (RootMove& rm : rootMoves) + for (usize i = 0; i < rootMoves.size(); ++i) { - rm.previousScore = rm.score; - rm.previousPV = rm.pv; + rootMoves[i].previousScore = rootMoves[i].score; + rootMoves[i].previousPV = rootMoves[i].pv; + rootMoves[i].previousScoreExact = i < multiPV; } usize pvFirst = 0; @@ -437,44 +438,52 @@ bool Search::Worker::iterative_deepening() { assert(alpha >= -VALUE_INFINITE && beta <= VALUE_INFINITE); } - // In multiPV analysis we do not let aborted searches spoil mated-in/ - // TB loss scores from a completed search in an earlier PV line. - // Hence we guard against an aborted pvIdx line overtaking pvIdx - 1 - // when pvIdx - 1 is a proven loss. - // Moreover, we do not trust an exact loss score from an aborted search. - if (threads.stop && pvIdx - && ((is_loss(rootMoves[pvIdx - 1].score) && rootMoves[pvIdx] < rootMoves[pvIdx - 1]) - || rootMoves[pvIdx].score_is_exact_loss())) + if (threads.stop && pvIdx) { - // If the previous score is worse than pvIdx - 1, we can safely use it. - // If it is equal, we make sure it cannot overtake pvIdx - 1. - if (rootMoves[pvIdx].previousScore != -VALUE_INFINITE - && rootMoves[pvIdx].previousScore <= rootMoves[pvIdx - 1].score) + // In multiPV analysis we do not let aborted searches spoil mated-in/ + // TB loss scores from a completed search in an earlier PV line. + // Hence we guard against an aborted pvIdx line overtaking pvIdx - 1 + // when pvIdx - 1 is a proven loss. + // Moreover, we do not trust an exact loss score from an aborted search. + if ((is_loss(rootMoves[pvIdx - 1].score) && rootMoves[pvIdx] < rootMoves[pvIdx - 1]) + || rootMoves[pvIdx].score_is_exact_loss()) { - rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore = - rootMoves[pvIdx].previousScore; - rootMoves[pvIdx].previousScore = -VALUE_INFINITE; - rootMoves[pvIdx].pv = rootMoves[pvIdx].previousPV; - rootMoves[pvIdx].unset_bound_flags(); - } - - // Otherwise, if we can, we cap the score to the best possible, and mark - // the score as a bound (also a valid excuse for the incomplete PV.) - else - { - if (is_loss(rootMoves[pvIdx - 1].score)) + // If previousScore is exact and worse than pvIdx - 1, we can safely use it. + // If it is equal, we make sure it cannot overtake pvIdx - 1. + if (rootMoves[pvIdx].previousScore != -VALUE_INFINITE + && rootMoves[pvIdx].previousScoreExact + && rootMoves[pvIdx].previousScore <= rootMoves[pvIdx - 1].score) { rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore = - rootMoves[pvIdx - 1].score; + rootMoves[pvIdx].previousScore; rootMoves[pvIdx].previousScore = -VALUE_INFINITE; - rootMoves[pvIdx].pv.resize(1); - rootMoves[pvIdx].scoreUpperbound = true; + rootMoves[pvIdx].pv = rootMoves[pvIdx].previousPV; + rootMoves[pvIdx].unset_bound_flags(); } - else - rootMoves[pvIdx].scoreUpperbound = false; - rootMoves[pvIdx].scoreLowerbound = !rootMoves[pvIdx].scoreUpperbound; + // Otherwise, if we can, we cap the score to the best possible, and mark + // the score as a bound (also a valid excuse for the incomplete PV.) + else + { + if (is_loss(rootMoves[pvIdx - 1].score)) + { + rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore = + rootMoves[pvIdx - 1].score; + rootMoves[pvIdx].previousScore = -VALUE_INFINITE; + rootMoves[pvIdx].pv.resize(1); + rootMoves[pvIdx].scoreUpperbound = true; + } + else + rootMoves[pvIdx].scoreUpperbound = false; + + rootMoves[pvIdx].scoreLowerbound = !rootMoves[pvIdx].scoreUpperbound; + } } + + // Finally, we mark all loss scores from partially searched moves as a bound. + for (usize i = pvIdx + 1; i < multiPV; ++i) + if (rootMoves[i].score_is_exact_loss()) + rootMoves[i].scoreLowerbound = true; } // Sort the PV lines searched so far and update the GUI @@ -490,21 +499,21 @@ bool Search::Worker::iterative_deepening() { break; } - const bool forgottenMate = lastIterationScore != -VALUE_INFINITE - && is_mate_or_mated(lastIterationScore) - && (std::abs(rootMoves[0].score) < std::abs(lastIterationScore) + const bool forgottenMate = lastBestMoveScore != -VALUE_INFINITE + && is_mate_or_mated(lastBestMoveScore) + && (std::abs(rootMoves[0].score) < std::abs(lastBestMoveScore) || rootMoves[0].score_is_bound()); if (!threads.stop) { - if (lastBestMove != rootMoves[0].pv[0]) + if (lastBestMovePV.empty() || lastBestMovePV[0] != rootMoves[0].pv[0]) lastBestMoveDepth = rootDepth; // Do not replace (shorter) mate scores from a previous iteration. if (!forgottenMate) { - lastBestMove = rootMoves[0].pv[0]; - lastIterationScore = rootMoves[0].score; + lastBestMovePV = rootMoves[0].pv; + lastBestMoveScore = rootMoves[0].score; } } @@ -518,12 +527,12 @@ bool Search::Worker::iterative_deepening() { if (abortedLossSearch || (rootMoves[0].score != -VALUE_INFINITE && forgottenMate)) { // Bring the last best move to the front for best thread selection. - if (lastBestMove != Move::none()) + if (!lastBestMovePV.empty()) { - Utility::move_to_front( - rootMoves, [lastBestMove](const auto& rm) { return rm == lastBestMove; }); - rootMoves[0].score = rootMoves[0].uciScore = rootMoves[0].previousScore; - rootMoves[0].pv = rootMoves[0].previousPV; + Utility::move_to_front(rootMoves, [&lastPV = std::as_const(lastBestMovePV)]( + const auto& rm) { return rm == lastPV[0]; }); + rootMoves[0].score = rootMoves[0].uciScore = lastBestMoveScore; + rootMoves[0].pv = lastBestMovePV; rootMoves[0].unset_bound_flags(); if (mainThread) diff --git a/src/search.h b/src/search.h index 68fb99111..428ff8663 100644 --- a/src/search.h +++ b/src/search.h @@ -138,16 +138,17 @@ struct RootMove { return m.score != score ? m.score < score : m.previousScore < previousScore; } - u64 effort = 0; - Value score = -VALUE_INFINITE; - Value previousScore = -VALUE_INFINITE; - Value averageScore = -VALUE_INFINITE; - Value meanSquaredScore = -VALUE_INFINITE * VALUE_INFINITE; - Value uciScore = -VALUE_INFINITE; - bool scoreLowerbound = false; - bool scoreUpperbound = false; - int selDepth = 0; - int tbRank = 0; + u64 effort = 0; + Value score = -VALUE_INFINITE; + Value previousScore = -VALUE_INFINITE; + Value averageScore = -VALUE_INFINITE; + Value meanSquaredScore = -VALUE_INFINITE * VALUE_INFINITE; + Value uciScore = -VALUE_INFINITE; + bool scoreLowerbound = false; + bool scoreUpperbound = false; + bool previousScoreExact = false; + int selDepth = 0; + int tbRank = 0; Value tbScore; PVMoves pv, previousPV; };