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

diff --git a/src/movepick.cpp b/src/movepick.cpp
index 65155a73f7936a308b13d099cb9071e704ce4298..36ee46b50f08302a52e2fc22b6aff8a03c887cd8 100644 (file)
--- 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<Next>([&](){ 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 c82c7a8bf87acbfc6c4c683599ce8c57a4dace99..485540ef866f9610ae9c1978701764816d90cbbc 100644 (file)
--- 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 6585f7bcabe965f85f06dde4db7fa842e18795a8..fcdb8d67545fd302ed782f743bc41b9cc6d53912 100644 (file)
--- 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 b594ac3714e5cf3e853289ebd09f0be03f1bd22b..2a9e1b68d378661dff78e3efe3428ec202e2668e 100644 (file)
--- 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<LEGAL>(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);