jirayu_sri/stockfish
1
0
Fork 0
mirror of https://github.com/official-stockfish/Stockfish.git synced 2026-07-22 12:47:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Guarantee complete mate PVs in multiPV analysis

Browse Source 
This PR introduces the additional `RootMove` attribute `previousPV` so that scores and PVs we send to the GUI in MultiPV analysis always match. This allows us in particular to extend our guarantee of exact mate (and TB win/loss) scores having a complete PV (leading to checkmate in the correct number of plies) to all PV lines. Recall that master fails here, since partially searched root moves may send to the GUI the previous score with the current/modified PV. See #6784.

The PR also uses the new attribute to extend the followPV logic to the analysis of sidelines, building on the idea in #6813 by @joergoster.

Passed non-reg STC:
LLR: 2.95 (-2.94,2.94) <-1.75,0.25>
Total: 166880 W: 42357 L: 42282 D: 82241
Ptnml(0-2): 394, 18685, 45177, 18820, 364
https://tests.stockfishchess.org/tests/view/6a0dea55818cacc1db0abb6a

Failed non-reg LTC:
LLR: -2.97 (-2.94,2.94) <-1.75,0.25>
Total: 890520 W: 224168 L: 225282 D: 441070
Ptnml(0-2): 390, 91902, 261789, 90790, 389
https://tests.stockfishchess.org/tests/view/6a1143ad818cacc1db0ac14c

Opening as draft for discussion on how to proceed. In SinglePV analysis, the patch is completely nonfunctional. But it is maybe a (small?) slowdown because of the increased size of `RootMove`. I am not sure if there as an elegant way to enrich the class only for MultiPV analysis (but the switch can happen at any time through the UCI interface), or to mitigate the speed penalty in some other way.

A local speedup test shows only a small slowdown on my system (but still high error bars):
```
sf_base =  1156928 +/-   1459 (95%)
sf_test =  1155885 +/-   1283 (95%)
diff    =    -1043 +/-   1777 (95%)
speedup = -0.09021% +/- 0.154% (95%)
```

The PR also adds the new MultiPV mate PV correctness check to the CI.

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

No functional change
This commit is contained in:
Robert Nurnberg @ elitebook
2026-06-09 19:36:14 +02:00
committed by Joost VandeVondele
parent 8e711c29fe
commit 1eff8b0389
3 changed files with 66 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/search.cpp
+57 -29
View File
@@ -187,7 +187,6 @@ void Search::Worker::ensure_network_replicated() {
void Search::Worker::start_searching() {
accumulatorStack.reset();
lastIterationPV.clear();
// Non-main threads go directly to iterative_deepening()
if (!is_mainthread())
@@ -269,6 +268,7 @@ bool Search::Worker::iterative_deepening() {
PVMoves pv;
Move lastBestMove = Move::none();
Depth lastBestMoveDepth = 0;
Value alpha, beta;
@@ -340,7 +340,10 @@ 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)
{
rm.previousScore = rm.score;
rm.previousPV = rm.pv;
}
usize pvFirst = 0;
pvLast = 0;
@@ -359,6 +362,8 @@ bool Search::Worker::iterative_deepening() {
break;
}
lastIterationIdxPV = rootMoves[pvIdx].previousPV;
// Reset UCI info selDepth for each depth and each PV line
selDepth = 0;
@@ -433,20 +438,42 @@ bool Search::Worker::iterative_deepening() {
// In multiPV analysis we do not let aborted searches spoil mated-in/
// TB loss scores from a completed search in an earlier PV line.
// A mated-in/TB loss from an aborted search for pvIdx > 0 can only become
// bestmove in the sorting below, if the current bestmove (and hence also
// the previously searched pvIdx - 1 line) is already a proven loss.
if (threads.stop && pvIdx && is_loss(rootMoves[pvIdx - 1].score)
&& rootMoves[pvIdx] < rootMoves[pvIdx - 1])
// 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()))
{
rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore =
(rootMoves[pvIdx].previousScore != -VALUE_INFINITE
&& rootMoves[pvIdx].previousScore < rootMoves[pvIdx - 1].score)
? rootMoves[pvIdx].previousScore
: rootMoves[pvIdx - 1].score;
rootMoves[pvIdx].previousScore = -VALUE_INFINITE;
rootMoves[pvIdx].unset_bound_flags();
rootMoves[pvIdx].pv.resize(1);
// 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)
{
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))
{
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;
}
}
// Sort the PV lines searched so far and update the GUI
@@ -469,20 +496,18 @@ bool Search::Worker::iterative_deepening() {
if (!threads.stop)
{
if (lastIterationPV.empty() || rootMoves[0].pv[0] != lastIterationPV[0])
if (lastBestMove != rootMoves[0].pv[0])
lastBestMoveDepth = rootDepth;
// Do not replace (shorter) mate scores from a previous iteration.
if (!forgottenMate)
{
lastIterationPV = rootMoves[0].pv;
lastBestMove = rootMoves[0].pv[0];
lastIterationScore = rootMoves[0].score;
}
}
const bool abortedLossSearch =
threads.stop && !pvIdx && rootMoves[0].score != -VALUE_INFINITE
&& is_loss(rootMoves[0].score) && !rootMoves[0].score_is_bound();
const bool abortedLossSearch = threads.stop && !pvIdx && rootMoves[0].score_is_exact_loss();
// An exact 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.
@@ -491,12 +516,13 @@ bool Search::Worker::iterative_deepening() {
// in a previous iteration.
if (abortedLossSearch || (rootMoves[0].score != -VALUE_INFINITE && forgottenMate))
{
if (!lastIterationPV.empty())
// Bring the last best move to the front for best thread selection.
if (lastBestMove != Move::none())
{
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 = lastIterationScore;
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;
rootMoves[0].unset_bound_flags();
if (mainThread)
@@ -709,8 +735,9 @@ Value Search::Worker::search(
maxValue = VALUE_INFINITE;
ss->followPV = rootNode
|| ((ss - 1)->followPV && static_cast<usize>(ss->ply - 1) < lastIterationPV.size()
&& (ss - 1)->currentMove == lastIterationPV[ss->ply - 1]);
|| ((ss - 1)->followPV
&& (static_cast<usize>(ss->ply - 1) < lastIterationIdxPV.size()
&& (ss - 1)->currentMove == lastIterationIdxPV[ss->ply - 1]));
// Check for the available remaining time
if (is_mainthread())
@@ -2189,12 +2216,13 @@ void SearchManager::pv(Search::Worker& worker,
v = isTBScore ? rootMoves[i].tbScore : v;
// Potentially correct and extend the PV, and in exceptional cases v.
// Bound flags indicate an unreliable PV, also when we usePreviousScore.
if (is_decisive(v) && !is_mate_or_mated(v) && (!rootMoves[i].score_is_bound() || isTBScore))
// Previous PVs have already been extended. Bound flags indicate an unreliable PV.
if (is_decisive(v) && !is_mate_or_mated(v) && !usePreviousScore
&& (!rootMoves[i].score_is_bound() || isTBScore))
syzygy_extend_pv(worker.options, worker.limits, pos, rootMoves[i], v);
std::string pv;
for (Move m : rootMoves[i].pv)
for (Move m : usePreviousScore ? rootMoves[i].previousPV : rootMoves[i].pv)
pv += UCIEngine::move(m, pos.is_chess960()) + " ";
// Remove last whitespace
src/search.h
+5 -2
View File
@@ -128,6 +128,9 @@ struct RootMove {
explicit RootMove(Move m) { pv.push_back(m); }
bool extract_ponder_from_tt(const TranspositionTable& tt, Position& pos);
bool score_is_bound() const { return scoreLowerbound || scoreUpperbound; }
bool score_is_exact_loss() const {
return score != -VALUE_INFINITE && is_loss(score) && !score_is_bound();
}
void unset_bound_flags() { scoreLowerbound = scoreUpperbound = false; }
bool operator==(const Move& m) const { return pv[0] == m; }
// Sort in descending order
@@ -146,7 +149,7 @@ struct RootMove {
int selDepth = 0;
int tbRank = 0;
Value tbScore;
PVMoves pv;
PVMoves pv, previousPV;
};
using RootMoves = std::vector<RootMove>;
@@ -384,7 +387,7 @@ class Worker {
Depth rootDepth;
Value rootDelta;
PVMoves lastIterationPV;
PVMoves lastIterationIdxPV;
usize threadIdx, numaThreadIdx, numaTotal;
NumaReplicatedAccessToken numaAccessToken;