From d3a4587fe881a531d08b17d45d9c12ded9680ee4 Mon Sep 17 00:00:00 2001 From: "Robert Nurnberg @ elitebook" <28635489+robertnurnberg@users.noreply.github.com> Date: Thu, 9 Apr 2026 21:45:48 +0200 Subject: [PATCH] Simplify uci pv output logic The uci pv suppression and PV roll-back logic in master is a bit convoluted, which makes it hard to reason about the code. In fact, subtle bugs that led to wrong mated-in scores in game play or a mismatch between bestmove and first PV move were only recently fixed. Moreover, in master the uci pv output through `pv()` is called in four different places. This PR proposes to simplify this logic. In this patch, the PV is sent to the GUI from within `iterative_deepening()` only (a) for fail highs/lows or (b) when an iteration for a root move is completed. All other PV outputs are now handled by `start_searching()`, just before the bestmove extraction. This easily ensures that bestmove (and ponder move) will always be in sync with the final PV output. The only noticeable change to master is for multi-threaded searches that do not involve `limits.depth`. Here master would show both the PV from the aborted search in main thread, as well as the new PV from the selected best thread. This patch would only show the latter. While at it, we also remove the requirement to finish at least a depth 1 search and simplify the logic around stalemates/checkmates at root and in particular avoid any PVs that start with `Move::none()`. Passed STC non-reg: LLR: 2.96 (-2.94,2.94) <-1.75,0.25> Total: 72416 W: 18784 L: 18609 D: 35023 Ptnml(0-2): 200, 7762, 20127, 7901, 218 https://tests.stockfishchess.org/tests/view/69d3ef0c33584dad27b3c85a closes https://github.com/official-stockfish/Stockfish/pull/6704 No functional change --- src/search.cpp | 98 +++++++++++++++++++++++--------------------------- src/search.h | 2 +- src/uci.h | 2 +- 3 files changed, 47 insertions(+), 55 deletions(-) diff --git a/src/search.cpp b/src/search.cpp index 47dc777a6..161767d7c 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -64,6 +64,8 @@ using namespace Search; namespace { +constexpr uint64_t NODES_LIMIT_OUTPUT = 10'000'000; + constexpr int SEARCHEDLIST_CAPACITY = 32; using SearchedList = ValueList; @@ -197,15 +199,15 @@ void Search::Worker::start_searching() { if (rootMoves.empty()) { - rootMoves.emplace_back(Move::none()); main_manager()->updates.onUpdateNoMoves( {0, {rootPos.checkers() ? -VALUE_MATE : VALUE_DRAW, rootPos}}); + main_manager()->updates.onBestmove(UCIEngine::move(Move::none()), ""); + return; } - else - { - threads.start_searching(); // start non-main threads - iterative_deepening(); // main thread start searching - } + + // Main thread starts non-main threads, and begins own search. + threads.start_searching(); + bool uciPvSent = iterative_deepening(); // When we reach the maximum depth, we can arrive here without a raise of // threads.stop. However, if we are pondering or in an infinite search, @@ -232,24 +234,22 @@ void Search::Worker::start_searching() { Skill skill = Skill(options["Skill Level"], options["UCI_LimitStrength"] ? int(options["UCI_Elo"]) : 0); - if (int(options["MultiPV"]) == 1 && !limits.depth && !skill.enabled() - && rootMoves[0].pv[0] != Move::none()) + if (int(options["MultiPV"]) == 1 && !limits.depth && !skill.enabled()) bestThread = threads.get_best_thread()->worker.get(); main_manager()->bestPreviousScore = bestThread->rootMoves[0].score; main_manager()->bestPreviousAverageScore = bestThread->rootMoves[0].averageScore; - std::string ponder; - bool extractedPonder = false; + if (bestThread->rootMoves[0].pv.size() == 1 + && bestThread->rootMoves[0].extract_ponder_from_tt(tt, rootPos)) + uciPvSent = false; - if (bestThread->rootMoves[0].pv.size() == 1) - extractedPonder = bestThread->rootMoves[0].extract_ponder_from_tt(tt, rootPos); - - // Send again PV info if we have a new best thread or extracted a ponder move. - if (bestThread != this || extractedPonder) + // Send PV info if it has changed since last output in iterative_deepening(). + if (!uciPvSent || bestThread != this) main_manager()->pv(*bestThread, threads, tt, bestThread->completedDepth); // In rare cases, pv() may change the ponder move through syzygy_extend_pv(). + std::string ponder; if (bestThread->rootMoves[0].pv.size() > 1) ponder = UCIEngine::move(bestThread->rootMoves[0].pv[1], rootPos.is_chess960()); @@ -260,7 +260,7 @@ void Search::Worker::start_searching() { // Main iterative deepening loop. It calls search() // repeatedly with increasing depth until the allocated thinking time has been // consumed, the user stops the search, or the maximum search depth is reached. -void Search::Worker::iterative_deepening() { +bool Search::Worker::iterative_deepening() { SearchManager* mainThread = (is_mainthread() ? main_manager() : nullptr); @@ -311,7 +311,8 @@ void Search::Worker::iterative_deepening() { multiPV = std::min(multiPV, rootMoves.size()); - int searchAgainCounter = 0; + int searchAgainCounter = 0; + bool uciPvSent = false; lowPlyHistory.fill(98); @@ -323,9 +324,12 @@ void Search::Worker::iterative_deepening() { while (++rootDepth < MAX_PLY && !threads.stop && !(limits.depth && mainThread && rootDepth > limits.depth)) { - // Age out PV variability metric + // Age out PV variability metric and signal the start of a new iteration. if (mainThread) + { totBestMoveChanges /= 2; + uciPvSent = false; + } // 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. @@ -393,7 +397,7 @@ void Search::Worker::iterative_deepening() { // excessive output that could hang GUIs like Fritz 19, only start // at nodes > 10M (rather than depth N, which can be reached quickly) if (mainThread && multiPV == 1 && (bestValue <= alpha || bestValue >= beta) - && nodes > 10000000) + && nodes > NODES_LIMIT_OUTPUT) main_manager()->pv(*this, threads, tt, rootDepth); // In case of failing low/high increase aspiration window and re-search, @@ -424,16 +428,11 @@ void Search::Worker::iterative_deepening() { // Sort the PV lines searched so far and update the GUI std::stable_sort(rootMoves.begin() + pvFirst, rootMoves.begin() + pvIdx + 1); - if (mainThread - && (threads.stop || pvIdx + 1 == multiPV || nodes > 10000000) - // A thread that aborted search can have a mated-in/TB-loss score and - // PV that cannot be trusted, i.e. it can be delayed or refuted if we - // would have had time to fully search other root-moves. Thus here we - // suppress any exact mated-in/TB loss output and, if we do, below pick - // the score/PV from the previous iteration. - && !(threads.stop && is_loss(rootMoves[0].uciScore) - && rootMoves[0].score == rootMoves[0].uciScore)) + if (mainThread && !threads.stop && (pvIdx + 1 == multiPV || nodes > NODES_LIMIT_OUTPUT)) + { main_manager()->pv(*this, threads, tt, rootDepth); + uciPvSent = (pvIdx + 1 == multiPV); + } if (threads.stop) break; @@ -449,27 +448,25 @@ void Search::Worker::iterative_deepening() { lastIterationPV = rootMoves[0].pv; } - // We make sure not to pick an unproven mated-in score, - // in case this thread prematurely stopped search (aborted-search). - if (completedDepth != rootDepth && rootMoves[0].score != -VALUE_INFINITE - && is_loss(rootMoves[0].score)) + // A mated-in/TB-loss score from an aborted search cannot be trusted: the loss + // could be delayed or refuted upon exploring the remaining root-moves. + // Thus here we roll back to the score from the previous iteration. + else if (rootMoves[0].score != -VALUE_INFINITE && is_loss(rootMoves[0].score)) { // Bring the last best move to the front for best thread selection. - // For an aborted d1 search we label the loss score as inexact. if (!lastIterationPV.empty()) { Utility::move_to_front(rootMoves, [&lastPV = std::as_const(lastIterationPV)]( const auto& rm) { return rm == lastPV[0]; }); rootMoves[0].pv = lastIterationPV; rootMoves[0].score = rootMoves[0].uciScore = rootMoves[0].previousScore; - } - else - { - if (!rootMoves[0].scoreLowerbound) - rootMoves[0].scoreUpperbound = true; + if (mainThread) - main_manager()->pv(*this, threads, tt, rootDepth); + uciPvSent = true; } + // For an aborted d1 search we label the loss score as inexact. + else if (!rootMoves[0].scoreLowerbound) + rootMoves[0].scoreUpperbound = true; } // Have we found a "mate in x" after a completed iteration? @@ -545,7 +542,7 @@ void Search::Worker::iterative_deepening() { } if (!mainThread) - return; + return false; mainThread->previousTimeReduction = timeReduction; @@ -554,6 +551,8 @@ void Search::Worker::iterative_deepening() { std::swap(rootMoves[0], *std::find(rootMoves.begin(), rootMoves.end(), skill.best ? skill.best : skill.pick_best(rootMoves, multiPV))); + + return uciPvSent; } @@ -1038,7 +1037,7 @@ moves_loop: // When in check, search starts here ss->moveCount = ++moveCount; - if (rootNode && is_mainthread() && nodes > 10000000) + if (rootNode && is_mainthread() && nodes > NODES_LIMIT_OUTPUT) { main_manager()->updates.onIter( {depth, UCIEngine::move(move, pos.is_chess960()), moveCount + pvIdx}); @@ -1986,13 +1985,9 @@ void SearchManager::check_time(Search::Worker& worker) { if (ponder) return; - if ( - // Later we rely on the fact that we can at least use the mainthread previous - // root-search score and PV in a multithreaded environment to prove mated-in scores. - worker.completedDepth >= 1 - && ((worker.limits.use_time_management() && (elapsed > tm.maximum() || stopOnPonderhit)) - || (worker.limits.movetime && elapsed >= worker.limits.movetime) - || (worker.limits.nodes && worker.threads.nodes_searched() >= worker.limits.nodes))) + if ((worker.limits.use_time_management() && (elapsed > tm.maximum() || stopOnPonderhit)) + || (worker.limits.movetime && elapsed >= worker.limits.movetime) + || (worker.limits.nodes && worker.threads.nodes_searched() >= worker.limits.nodes)) worker.threads.stop = true; } @@ -2209,12 +2204,9 @@ void SearchManager::pv(Search::Worker& worker, // otherwise in case of 'ponder on' we have nothing to think about. bool RootMove::extract_ponder_from_tt(const TranspositionTable& tt, Position& pos) { + assert(pv.size() == 1 && pv[0] != Move::none()); + StateInfo st; - - assert(pv.size() == 1); - if (pv[0] == Move::none()) - return false; - pos.do_move(pv[0], st, &tt); if (!pos.is_draw(1)) diff --git a/src/search.h b/src/search.h index 1d22b01d8..c70b1f885 100644 --- a/src/search.h +++ b/src/search.h @@ -299,7 +299,7 @@ class Worker { SharedHistories& sharedHistory; private: - void iterative_deepening(); + bool iterative_deepening(); void do_move(Position& pos, const Move move, StateInfo& st, Stack* const ss); void diff --git a/src/uci.h b/src/uci.h index 7ab568bac..cfebc8af0 100644 --- a/src/uci.h +++ b/src/uci.h @@ -47,7 +47,7 @@ class UCIEngine { static int to_cp(Value v, const Position& pos); static std::string format_score(const Score& s); static std::string square(Square s); - static std::string move(Move m, bool chess960); + static std::string move(Move m, bool chess960 = false); static std::string wdl(Value v, const Position& pos); static std::string to_lower(std::string str); static Move to_move(const Position& pos, std::string str);