From 5c75c1c2fbb7bb4f0bf7c44fb855c415b788cbf7 Mon Sep 17 00:00:00 2001 From: Michael Chaly Date: Fri, 24 Feb 2023 12:09:45 +0300 Subject: [PATCH] Fix duplicated moves generation in movepicker in a some of cases movepicker returned some moves more than once which lead to them being searched more than once. This bug was possible because of how we use queen promotions - they are generated as a captures but are not included in position function which checks if move is a capture. Thus if any refutation (killer or countermove) was a queen promotion it was searched twice - once as a capture and one as a refutation. This patch affects various things, namely stats assignments for queen promotions and other moves if best move is queen promotion, also some heuristics in search and qsearch. With this patch every queen promotion is now considered a capture. After this patch number of found duplicated moves is 0 during normal 13 depth bench run. Passed STC: https://tests.stockfishchess.org/tests/view/63f77e01e74a12625bcd87d7 LLR: 2.95 (-2.94,2.94) <-1.75,0.25> Total: 80920 W: 21455 L: 21289 D: 38176 Ptnml(0-2): 198, 8839, 22241, 8963, 219 Passed LTC: https://tests.stockfishchess.org/tests/view/63f7e020e74a12625bcd9a76 LLR: 2.94 (-2.94,2.94) <-1.75,0.25> Total: 89712 W: 23674 L: 23533 D: 42505 Ptnml(0-2): 24, 8737, 27202, 8860, 33 closes https://github.com/official-stockfish/Stockfish/pull/4405 bench 4681731 --- src/movepick.cpp | 6 +++--- src/position.h | 12 ++++++++---- src/search.cpp | 12 ++++++------ src/syzygy/tbprobe.cpp | 4 ++-- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/movepick.cpp b/src/movepick.cpp index 65155a73..36ee46b5 100644 --- a/src/movepick.cpp +++ b/src/movepick.cpp @@ -93,7 +93,7 @@ MovePicker::MovePicker(const Position& p, Move ttm, Value th, const CapturePiece { assert(!pos.checkers()); - stage = PROBCUT_TT + !(ttm && pos.capture(ttm) + stage = PROBCUT_TT + !(ttm && pos.capture_stage(ttm) && pos.pseudo_legal(ttm) && pos.see_ge(ttm, threshold)); } @@ -141,7 +141,7 @@ void MovePicker::score() { + bool(pos.check_squares(type_of(pos.moved_piece(m))) & to_sq(m)) * 16384; else // Type == EVASIONS { - if (pos.capture(m)) + if (pos.capture_stage(m)) m.value = PieceValue[MG][pos.piece_on(to_sq(m))] - Value(type_of(pos.moved_piece(m))) + (1 << 28); @@ -216,7 +216,7 @@ top: case REFUTATION: if (select([&](){ return *cur != MOVE_NONE - && !pos.capture(*cur) + && !pos.capture_stage(*cur) && pos.pseudo_legal(*cur); })) return *(cur - 1); ++stage; diff --git a/src/position.h b/src/position.h index c82c7a8b..485540ef 100644 --- a/src/position.h +++ b/src/position.h @@ -125,7 +125,7 @@ public: // Properties of moves bool legal(Move m) const; bool pseudo_legal(const Move m) const; - bool capture(Move m) const; + bool capture_stage(Move m) const; bool gives_check(Move m) const; Piece moved_piece(Move m) const; Piece captured_piece() const; @@ -381,10 +381,14 @@ inline bool Position::is_chess960() const { return chess960; } -inline bool Position::capture(Move m) const { +// returns true if a move is generated from the capture stage +// having also queen promotions covered, i.e. consistency with the capture stage move generation +// is needed to avoid the generation of duplicate moves. +inline bool Position::capture_stage(Move m) const { assert(is_ok(m)); - // Castling is encoded as "king captures rook" - return (!empty(to_sq(m)) && type_of(m) != CASTLING) || type_of(m) == EN_PASSANT; + return (!empty(to_sq(m)) && type_of(m) != CASTLING) + || (type_of(m) == PROMOTION && promotion_type(m) == QUEEN) + || type_of(m) == EN_PASSANT; } inline Piece Position::captured_piece() const { diff --git a/src/search.cpp b/src/search.cpp index 6585f7bc..fcdb8d67 100644 --- a/src/search.cpp +++ b/src/search.cpp @@ -623,7 +623,7 @@ namespace { ttValue = ss->ttHit ? value_from_tt(tte->value(), ss->ply, pos.rule50_count()) : VALUE_NONE; ttMove = rootNode ? thisThread->rootMoves[thisThread->pvIdx].pv[0] : ss->ttHit ? tte->move() : MOVE_NONE; - ttCapture = ttMove && pos.capture(ttMove); + ttCapture = ttMove && pos.capture_stage(ttMove); // At this point, if excluded, skip straight to step 6, static eval. However, // to save indentation, we list the condition in all code between here and there. @@ -852,7 +852,7 @@ namespace { probCutBeta = beta + 186 - 54 * improving; // Step 10. ProbCut (~10 Elo) - // If we have a good enough capture and a reduced search returns a value + // If we have a good enough capture (or queen promotion) and a reduced search returns a value // much above beta, we can (almost) safely prune the previous move. if ( !PvNode && depth > 4 @@ -873,7 +873,7 @@ namespace { while ((move = mp.next_move()) != MOVE_NONE) if (move != excludedMove && pos.legal(move)) { - assert(pos.capture(move) || promotion_type(move) == QUEEN); + assert(pos.capture_stage(move)); ss->currentMove = move; ss->continuationHistory = &thisThread->continuationHistory[ss->inCheck] @@ -985,7 +985,7 @@ moves_loop: // When in check, search starts here (ss+1)->pv = nullptr; extension = 0; - capture = pos.capture(move); + capture = pos.capture_stage(move); movedPiece = pos.moved_piece(move); givesCheck = pos.gives_check(move); @@ -1542,7 +1542,7 @@ moves_loop: // When in check, search starts here continue; givesCheck = pos.gives_check(move); - capture = pos.capture(move); + capture = pos.capture_stage(move); moveCount++; @@ -1715,7 +1715,7 @@ moves_loop: // When in check, search starts here PieceType captured = type_of(pos.piece_on(to_sq(bestMove))); int bonus1 = stat_bonus(depth + 1); - if (!pos.capture(bestMove)) + if (!pos.capture_stage(bestMove)) { int bonus2 = bestValue > beta + 153 ? bonus1 // larger bonus : stat_bonus(depth); // smaller bonus diff --git a/src/syzygy/tbprobe.cpp b/src/syzygy/tbprobe.cpp index b594ac37..2a9e1b68 100644 --- a/src/syzygy/tbprobe.cpp +++ b/src/syzygy/tbprobe.cpp @@ -1203,7 +1203,7 @@ WDLScore search(Position& pos, ProbeState* result) { for (const Move move : moveList) { - if ( !pos.capture(move) + if ( !pos.capture_stage(move) && (!CheckZeroingMoves || type_of(pos.moved_piece(move)) != PAWN)) continue; @@ -1472,7 +1472,7 @@ int Tablebases::probe_dtz(Position& pos, ProbeState* result) { for (const Move move : MoveList(pos)) { - bool zeroing = pos.capture(move) || type_of(pos.moved_piece(move)) == PAWN; + bool zeroing = pos.capture_stage(move) || type_of(pos.moved_piece(move)) == PAWN; pos.do_move(move, st); -- 2.39.2