Skip to content

Commit 74a0a73

Browse files
robertnurnbergDisservin
robertnurnberg
authored and
Disservin
committed
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 official-stockfish#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 official-stockfish#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 official-stockfish#6906 No functional change
1 parent f9b002c commit 74a0a73

2 files changed

Lines changed: 69 additions & 59 deletions

File tree

‎src/search.cpp‎

Lines changed: 58 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -269,13 +269,13 @@ bool Search::Worker::iterative_deepening() {
269269

270270
PVMoves pv;
271271

272-
Move lastBestMove = Move::none();
273-
Depth lastBestMoveDepth = 0;
272+
PVMoves lastBestMovePV;
273+
Depth lastBestMoveDepth = 0;
274+
Value lastBestMoveScore = -VALUE_INFINITE;
274275

275276
Value alpha, beta;
276-
Value bestValue = -VALUE_INFINITE;
277-
Value lastIterationScore = -VALUE_INFINITE;
278-
Color us = rootPos.side_to_move();
277+
Value bestValue = -VALUE_INFINITE;
278+
Color us = rootPos.side_to_move();
279279
double timeReduction = 1, totBestMoveChanges = 0;
280280
int delta, iterIdx = 0;
281281

@@ -340,10 +340,11 @@ bool Search::Worker::iterative_deepening() {
340340

341341
// Save the last iteration's scores before the first PV line is searched and
342342
// all the move scores except the (new) PV are set to -VALUE_INFINITE.
343-
for (RootMove& rm : rootMoves)
343+
for (usize i = 0; i < rootMoves.size(); ++i)
344344
{
345-
rm.previousScore = rm.score;
346-
rm.previousPV = rm.pv;
345+
rootMoves[i].previousScore = rootMoves[i].score;
346+
rootMoves[i].previousPV = rootMoves[i].pv;
347+
rootMoves[i].previousScoreExact = i < multiPV;
347348
}
348349

349350
usize pvFirst = 0;
@@ -437,44 +438,52 @@ bool Search::Worker::iterative_deepening() {
437438
assert(alpha >= -VALUE_INFINITE && beta <= VALUE_INFINITE);
438439
}
439440

440-
// In multiPV analysis we do not let aborted searches spoil mated-in/
441-
// TB loss scores from a completed search in an earlier PV line.
442-
// Hence we guard against an aborted pvIdx line overtaking pvIdx - 1
443-
// when pvIdx - 1 is a proven loss.
444-
// Moreover, we do not trust an exact loss score from an aborted search.
445-
if (threads.stop && pvIdx
446-
&& ((is_loss(rootMoves[pvIdx - 1].score) && rootMoves[pvIdx] < rootMoves[pvIdx - 1])
447-
|| rootMoves[pvIdx].score_is_exact_loss()))
441+
if (threads.stop && pvIdx)
448442
{
449-
// If the previous score is worse than pvIdx - 1, we can safely use it.
450-
// If it is equal, we make sure it cannot overtake pvIdx - 1.
451-
if (rootMoves[pvIdx].previousScore != -VALUE_INFINITE
452-
&& rootMoves[pvIdx].previousScore <= rootMoves[pvIdx - 1].score)
443+
// In multiPV analysis we do not let aborted searches spoil mated-in/
444+
// TB loss scores from a completed search in an earlier PV line.
445+
// Hence we guard against an aborted pvIdx line overtaking pvIdx - 1
446+
// when pvIdx - 1 is a proven loss.
447+
// Moreover, we do not trust an exact loss score from an aborted search.
448+
if ((is_loss(rootMoves[pvIdx - 1].score) && rootMoves[pvIdx] < rootMoves[pvIdx - 1])
449+
|| rootMoves[pvIdx].score_is_exact_loss())
453450
{
454-
rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore =
455-
rootMoves[pvIdx].previousScore;
456-
rootMoves[pvIdx].previousScore = -VALUE_INFINITE;
457-
rootMoves[pvIdx].pv = rootMoves[pvIdx].previousPV;
458-
rootMoves[pvIdx].unset_bound_flags();
459-
}
460-
461-
// Otherwise, if we can, we cap the score to the best possible, and mark
462-
// the score as a bound (also a valid excuse for the incomplete PV.)
463-
else
464-
{
465-
if (is_loss(rootMoves[pvIdx - 1].score))
451+
// If previousScore is exact and worse than pvIdx - 1, we can safely use it.
452+
// If it is equal, we make sure it cannot overtake pvIdx - 1.
453+
if (rootMoves[pvIdx].previousScore != -VALUE_INFINITE
454+
&& rootMoves[pvIdx].previousScoreExact
455+
&& rootMoves[pvIdx].previousScore <= rootMoves[pvIdx - 1].score)
466456
{
467457
rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore =
468-
rootMoves[pvIdx - 1].score;
458+
rootMoves[pvIdx].previousScore;
469459
rootMoves[pvIdx].previousScore = -VALUE_INFINITE;
470-
rootMoves[pvIdx].pv.resize(1);
471-
rootMoves[pvIdx].scoreUpperbound = true;
460+
rootMoves[pvIdx].pv = rootMoves[pvIdx].previousPV;
461+
rootMoves[pvIdx].unset_bound_flags();
472462
}
473-
else
474-
rootMoves[pvIdx].scoreUpperbound = false;
475463

476-
rootMoves[pvIdx].scoreLowerbound = !rootMoves[pvIdx].scoreUpperbound;
464+
// Otherwise, if we can, we cap the score to the best possible, and mark
465+
// the score as a bound (also a valid excuse for the incomplete PV.)
466+
else
467+
{
468+
if (is_loss(rootMoves[pvIdx - 1].score))
469+
{
470+
rootMoves[pvIdx].score = rootMoves[pvIdx].uciScore =
471+
rootMoves[pvIdx - 1].score;
472+
rootMoves[pvIdx].previousScore = -VALUE_INFINITE;
473+
rootMoves[pvIdx].pv.resize(1);
474+
rootMoves[pvIdx].scoreUpperbound = true;
475+
}
476+
else
477+
rootMoves[pvIdx].scoreUpperbound = false;
478+
479+
rootMoves[pvIdx].scoreLowerbound = !rootMoves[pvIdx].scoreUpperbound;
480+
}
477481
}
482+
483+
// Finally, we mark all loss scores from partially searched moves as a bound.
484+
for (usize i = pvIdx + 1; i < multiPV; ++i)
485+
if (rootMoves[i].score_is_exact_loss())
486+
rootMoves[i].scoreLowerbound = true;
478487
}
479488

480489
// Sort the PV lines searched so far and update the GUI
@@ -490,21 +499,21 @@ bool Search::Worker::iterative_deepening() {
490499
break;
491500
}
492501

493-
const bool forgottenMate = lastIterationScore != -VALUE_INFINITE
494-
&& is_mate_or_mated(lastIterationScore)
495-
&& (std::abs(rootMoves[0].score) < std::abs(lastIterationScore)
502+
const bool forgottenMate = lastBestMoveScore != -VALUE_INFINITE
503+
&& is_mate_or_mated(lastBestMoveScore)
504+
&& (std::abs(rootMoves[0].score) < std::abs(lastBestMoveScore)
496505
|| rootMoves[0].score_is_bound());
497506

498507
if (!threads.stop)
499508
{
500-
if (lastBestMove != rootMoves[0].pv[0])
509+
if (lastBestMovePV.empty() || lastBestMovePV[0] != rootMoves[0].pv[0])
501510
lastBestMoveDepth = rootDepth;
502511

503512
// Do not replace (shorter) mate scores from a previous iteration.
504513
if (!forgottenMate)
505514
{
506-
lastBestMove = rootMoves[0].pv[0];
507-
lastIterationScore = rootMoves[0].score;
515+
lastBestMovePV = rootMoves[0].pv;
516+
lastBestMoveScore = rootMoves[0].score;
508517
}
509518
}
510519

@@ -518,12 +527,12 @@ bool Search::Worker::iterative_deepening() {
518527
if (abortedLossSearch || (rootMoves[0].score != -VALUE_INFINITE && forgottenMate))
519528
{
520529
// Bring the last best move to the front for best thread selection.
521-
if (lastBestMove != Move::none())
530+
if (!lastBestMovePV.empty())
522531
{
523-
Utility::move_to_front(
524-
rootMoves, [lastBestMove](const auto& rm) { return rm == lastBestMove; });
525-
rootMoves[0].score = rootMoves[0].uciScore = rootMoves[0].previousScore;
526-
rootMoves[0].pv = rootMoves[0].previousPV;
532+
Utility::move_to_front(rootMoves, [&lastPV = std::as_const(lastBestMovePV)](
533+
const auto& rm) { return rm == lastPV[0]; });
534+
rootMoves[0].score = rootMoves[0].uciScore = lastBestMoveScore;
535+
rootMoves[0].pv = lastBestMovePV;
527536
rootMoves[0].unset_bound_flags();
528537

529538
if (mainThread)

‎src/search.h‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,16 +138,17 @@ struct RootMove {
138138
return m.score != score ? m.score < score : m.previousScore < previousScore;
139139
}
140140

141-
u64 effort = 0;
142-
Value score = -VALUE_INFINITE;
143-
Value previousScore = -VALUE_INFINITE;
144-
Value averageScore = -VALUE_INFINITE;
145-
Value meanSquaredScore = -VALUE_INFINITE * VALUE_INFINITE;
146-
Value uciScore = -VALUE_INFINITE;
147-
bool scoreLowerbound = false;
148-
bool scoreUpperbound = false;
149-
int selDepth = 0;
150-
int tbRank = 0;
141+
u64 effort = 0;
142+
Value score = -VALUE_INFINITE;
143+
Value previousScore = -VALUE_INFINITE;
144+
Value averageScore = -VALUE_INFINITE;
145+
Value meanSquaredScore = -VALUE_INFINITE * VALUE_INFINITE;
146+
Value uciScore = -VALUE_INFINITE;
147+
bool scoreLowerbound = false;
148+
bool scoreUpperbound = false;
149+
bool previousScoreExact = false;
150+
int selDepth = 0;
151+
int tbRank = 0;
151152
Value tbScore;
152153
PVMoves pv, previousPV;
153154
};

0 commit comments

Comments
 (0)