Fix incremental zobrist hashing, fix repetition detection, add ttmove, wait for search thread to complete

Chess/nimfish/nimfishpkg/board.nim

@@ -87,17 +87,16 @@ proc toFEN*(self: Chessboard): string =
     return self.position.toFEN()
 
 
-proc drawByRepetition*(self: var Chessboard): bool =
+proc drawnByRepetition*(self: Chessboard): bool =
     ## Returns whether the current position is a draw
     ## by repetition
     # TODO: Improve this
     var i = self.positions.high()
     var count = 0
-    while i > 0:
+    while i >= 0:
         if self.position.zobristKey == self.positions[i].zobristKey:
             inc(count)
             if count == 2:
-                self.position.repetitionDraw = true
                 return true
         dec(i)
 

Chess/nimfish/nimfishpkg/movegen.nim

@@ -340,7 +340,10 @@ proc doMove*(self: var Chessboard, move: Move) =
     self.positions.add(self.position)
 
     # Final checks
-    let piece = self.position.getPiece(move.startSquare)
+    let 
+        piece = self.position.getPiece(move.startSquare)
+        sideToMove = piece.color
+        nonSideToMove = sideToMove.opposite()
     when not defined(danger):
         doAssert piece.kind != Empty and piece.color != None, &"{move} {self.toFEN()}"
 
@@ -369,7 +372,7 @@ proc doMove*(self: var Chessboard, move: Move) =
     self.position = Position(plyFromRoot: self.position.plyFromRoot + 1,
                              halfMoveClock: halfMoveClock,
                              fullMoveCount: fullMoveCount,
-                             sideToMove: self.position.sideToMove.opposite(),
+                             sideToMove: nonSideToMove,
                              enPassantSquare: enPassantTarget,
                              pieces: self.position.pieces,
                              castlingAvailability: self.position.castlingAvailability,
@@ -382,14 +385,24 @@ proc doMove*(self: var Chessboard, move: Move) =
 
     if move.isEnPassant():
         # Make the en passant pawn disappear
-        let epPawnSquare = move.targetSquare.toBitboard().backwardRelativeTo(piece.color).toSquare()
+        let epPawnSquare = move.targetSquare.toBitboard().backwardRelativeTo(sideToMove).toSquare()
         self.position.zobristKey = self.position.zobristKey xor self.position.getPiece(epPawnSquare).getKey(epPawnSquare)
         self.position.removePiece(epPawnSquare)
 
     if move.isCastling() or piece.kind == King:
         # If the king has moved, all castling rights for the side to
         # move are revoked
-        self.position.castlingAvailability[piece.color] = (false, false)
+        if self.position.castlingAvailability[sideToMove].king:
+            # XOR is its own inverse, so while setting a boolean to false more than once
+            # is not a problem, XORing the same key twice would give back the castling 
+            # rights to the moving side!
+            self.position.castlingAvailability[sideToMove].king = false
+            self.position.zobristKey = self.position.zobristKey xor getKingSideCastlingKey(sideToMove)
+
+        if self.position.castlingAvailability[sideToMove].queen:
+            self.position.castlingAvailability[sideToMove].queen = false
+            self.position.zobristKey = self.position.zobristKey xor getQueenSideCastlingKey(sideToMove)
+
         if move.isCastling():
             # Move the rook where it belongs
             var 
@@ -398,26 +411,30 @@ proc doMove*(self: var Chessboard, move: Move) =
                 target: Square
             
             if move.targetSquare == piece.kingSideCastling():
-                source = piece.color.kingSideRook()
+                source = sideToMove.kingSideRook()
                 rook = self.position.getPiece(source)
                 target = rook.kingSideCastling()
 
             elif move.targetSquare == piece.queenSideCastling():
-                source = piece.color.queenSideRook()
+                source = sideToMove.queenSideRook()
                 rook = self.position.getPiece(source)
                 target = rook.queenSideCastling()
 
             self.position.movePiece(source, target)
-            self.position.zobristKey = self.position.zobristKey xor piece.getKey(source)
+            self.position.zobristKey = self.position.zobristKey xor rook.getKey(source)
-            self.position.zobristKey = self.position.zobristKey xor piece.getKey(target)
+            self.position.zobristKey = self.position.zobristKey xor rook.getKey(target)
 
     if piece.kind == Rook:
         # If a rook on either side moves, castling rights are permanently revoked
         # on that side
-        if move.startSquare == piece.color.kingSideRook():
+        if move.startSquare == sideToMove.kingSideRook():
-            self.position.castlingAvailability[piece.color].king = false
+            if self.position.castlingAvailability[sideToMove].king:
-        elif move.startSquare == piece.color.queenSideRook():
+                self.position.castlingAvailability[sideToMove].king = false
-            self.position.castlingAvailability[piece.color].queen = false
+                self.position.zobristKey = self.position.zobristKey xor getKingSideCastlingKey(sideToMove)
+        elif move.startSquare == sideToMove.queenSideRook():
+            if self.position.castlingAvailability[sideToMove].queen:
+                self.position.castlingAvailability[sideToMove].queen = false
+                self.position.zobristKey = self.position.zobristKey xor getQueenSideCastlingKey(sideToMove)
 
     if move.isCapture():
         # Get rid of captured pieces
@@ -427,9 +444,13 @@ proc doMove*(self: var Chessboard, move: Move) =
         # If a rook has been captured, castling on that side is prohibited
         if captured.kind == Rook:
             if move.targetSquare == captured.color.kingSideRook():
-                self.position.castlingAvailability[captured.color].king = false
+                if self.position.castlingAvailability[nonSideToMove].king:
+                    self.position.castlingAvailability[nonSideToMove].king = false
+                    self.position.zobristKey = self.position.zobristKey xor getKingSideCastlingKey(nonSideToMove)
             elif move.targetSquare == captured.color.queenSideRook():
-                self.position.castlingAvailability[captured.color].queen = false
+                if self.position.castlingAvailability[nonSideToMove].queen:
+                    self.position.castlingAvailability[nonSideToMove].queen = false
+                    self.position.zobristKey = self.position.zobristKey xor getQueenSideCastlingKey(nonSideToMove)
 
     # Move the piece to its target square
     self.position.movePiece(move)
@@ -457,12 +478,8 @@ proc doMove*(self: var Chessboard, move: Move) =
         self.position.spawnPiece(move.targetSquare, spawnedPiece)
     # Updates checks and pins for the (new) side to move
     self.position.updateChecksAndPins()
-    # Last updates to zobrist key
+    # Swap the side to move
-    if self.position.castlingAvailability[piece.color].king:
+    self.position.zobristKey = self.position.zobristKey xor getBlackToMoveKey()
-        self.position.zobristKey = self.position.zobristKey xor getKingSideCastlingKey(piece.color)
-    if self.position.castlingAvailability[piece.color].queen:
-        self.position.zobristKey = self.position.zobristKey xor getQueenSideCastlingKey(piece.color)
-    discard self.drawByRepetition()
 
 
 proc isLegal*(self: var Chessboard, move: Move): bool {.inline.} =
@@ -514,9 +531,12 @@ const testFens = staticRead("../../tests/all.txt").splitLines()
 
 proc basicTests* =
 
+
+    # Test the FEN parser
     for fen in testFens:
-        doAssert fen == newChessboardFromFEN(fen).toFEN()
+        doAssert fen == loadFEN(fen).toFEN()
     
+    # Test zobrist hashing
     for fen in testFens:
         var
             board = newChessboardFromFEN(fen)
@@ -532,7 +552,6 @@ proc basicTests* =
             doAssert not hashes.contains(key), &"{fen} has zobrist collisions {move} -> {hashes[key]} (key is {key.uint64})"
             hashes[key] = move
 
-
     var board = newDefaultChessboard()
     # Ensure correct number of pieces
     testPieceCount(board, Pawn, White, 8)
@@ -619,6 +638,11 @@ proc basicTests* =
     testPieceBitboard(blackQueens, blackQueenSquares)
     testPieceBitboard(blackKing, blackKingSquares)
 
+    # Test repetition
+    for move in ["b1c3", "g8f6", "c3b1", "f6g8", "b1c3", "g8f6", "c3b1", "f6g8"]:
+        board.makeMove(createMove(move[0..1].toSquare(), move[2..3].toSquare()))
+    doAssert board.drawnByRepetition()
+
 
 when isMainModule:
     basicTests()

Chess/nimfish/nimfishpkg/position.nim

@@ -58,8 +58,6 @@ type
         checkers*: Bitboard
         # Zobrist hash of this position
         zobristKey*: ZobristKey
-        # Cached result of drawByRepetition()
-        repetitionDraw*: bool
         # A mailbox for fast piece lookup by
         # location
         mailbox*: array[Square(0)..Square(63), Piece]

Chess/nimfish/nimfishpkg/search.nim

@@ -45,14 +45,14 @@ type
         currentExtensionCount: uint8
 
 
-proc newSearchManager*(position: Position, transpositions: TTable): SearchManager =
+proc newSearchManager*(board: Chessboard, transpositions: TTable): SearchManager =
     new(result)
-    result.board = newChessboard()
+    result.board = board
-    result.board.position = position
     result.bestMoveRoot = nullMove()
     result.transpositionTable = transpositions
 
 
+
 proc isSearching*(self: SearchManager): bool =
     ## Returns whether a search for the best
     ## move is in progress
@@ -74,6 +74,10 @@ proc getEstimatedMoveScore(self: SearchManager, move: Move): Score =
         nonSideToMove = sideToMove.opposite()
     if self.previousBestMove != nullMove() and move == self.previousBestMove:
         return highestEval() + 1
+    when defined(useTT):
+        let query = self.transpositionTable.get(self.board.position.zobristKey)
+        if query.success and query.entry.bestMove != nullMove() and query.entry.bestMove == move:
+            return highestEval() + 1
     if move.isCapture():
         # Implementation of MVVLVA: Most Valuable Victim Least Valuable Attacker
         # We prioritize moves that capture the most valuable pieces, and as a
@@ -125,7 +129,7 @@ proc log(self: SearchManager, depth: int) =
         nps = 1000 * (self.nodeCount div max(elapsedMsec, 1))
     var logMsg = &"info depth {depth} time {elapsedMsec} nodes {self.nodeCount} nps {nps}"
     if self.bestMoveRoot != nullMove():
-        logMsg &= &" bestmove {self.bestMoveRoot.toAlgebraic()} score {self.bestRootScore}"
+        logMsg &= &" score cp {self.bestRootScore} pv {self.bestMoveRoot.toAlgebraic()}"
     echo logMsg
 
 
@@ -185,6 +189,8 @@ proc qsearch(self: SearchManager, ply: uint8, alpha, beta: Score): Score =
     if score >= beta:
         # Same as with the regular alpha-beta search
         return score
+    if self.board.position.halfMoveClock >= 100 or self.board.drawnByRepetition():
+        return Score(0)
     var moves = newMoveList()
     self.board.generateMoves(moves, capturesOnly=true)
     self.reorderMoves(moves)
@@ -193,14 +199,7 @@ proc qsearch(self: SearchManager, ply: uint8, alpha, beta: Score): Score =
     for move in moves:
         self.board.doMove(move)
         inc(self.nodeCount)
-        var score: Score
+        let score = -self.qsearch(ply + 1, -beta, -alpha)
-        if self.board.position.halfMoveClock >= 100 or self.board.position.repetitionDraw:
-            # Drawing by repetition is *bad*
-            score = Score(0)
-        else:
-            # Find the best move for us (worst move
-            # for our opponent, hence the negative sign)
-            score = -self.qsearch(ply + 1, -beta, -alpha)
         self.board.unmakeMove()
         bestScore = max(score, bestScore)
         if score >= beta:
@@ -223,18 +222,21 @@ proc search(self: SearchManager, depth, ply: int, alpha, beta: Score): Score {.d
         # one because then we wouldn't have a move to return.
         # In practice this should not be a problem
         return
-    when defined(useTT):
+    # when defined(useTT):
-        let query = self.transpositionTable.get(self.board.position.zobristKey, depth.uint8)
+    #     if ply > 0:
-        if query.success:
+    #         let query = self.transpositionTable.get(self.board.position.zobristKey, depth.uint8)
-            case query.entry.flag:
+    #         if query.success:
-                of Exact:
+    #             case query.entry.flag:
-                    return query.entry.score
+    #                 of Exact:
-                of LowerBound:
+    #                     return query.entry.score
-                    if query.entry.score >= beta:
+    #                 of LowerBound:
-                        return query.entry.score
+    #                     if query.entry.score >= beta:
-                of UpperBound:
+    #                         return query.entry.score
-                    if query.entry.score <= alpha:
+    #                 of UpperBound:
-                        return query.entry.score
+    #                     if query.entry.score <= alpha:
+    #                         return query.entry.score
+    if self.board.drawnByRepetition():
+        return Score(0)
     if depth == 0:
         # Quiescent search gain: 264.8 +/- 71.6
         return self.qsearch(0, alpha, beta)
@@ -264,23 +266,20 @@ proc search(self: SearchManager, depth, ply: int, alpha, beta: Score): Score {.d
         when defined(searchExtensions):
             extension = self.getSearchExtension(move)
         inc(self.nodeCount)
-        # Find the best move for us (worst move
-        # for our opponent, hence the negative sign)
         var score: Score
         var fullDepth = true
         when defined(searchLMR):
             if extension == 0 and i >= 3 and not move.isCapture():
                 # Late Move Reduction: assume our move orderer did a good job,
-                # so it is not worth to look at all moves at the same depth equally.
+                # so it is not worth it to look at all moves at the same depth equally.
                 # If this move turns out to be better than we expected, we'll re-search
                 # it at full depth
                 const reduction = 1
                 score = -self.search(depth - 1 - reduction, ply + 1, -beta, -alpha)
                 fullDepth = score > alpha
-        if self.board.position.halfMoveClock >= 100 or self.board.position.repetitionDraw:
+        if fullDepth:
-            # Drawing by repetition is *bad*
+            # Find the best move for us (worst move
-            score = Score(0)
+            # for our opponent, hence the negative sign)
-        elif fullDepth:
             score = -self.search(depth - 1 + extension, ply + 1, -beta, -alpha)
         self.board.unmakeMove()
         # When a search is cancelled or times out, we need
@@ -289,10 +288,7 @@ proc search(self: SearchManager, depth, ply: int, alpha, beta: Score): Score {.d
         if depth > 1 and self.shouldStop():
             return
         bestScore = max(score, bestScore)
-        let nodeType = if score >= beta: LowerBound elif score <= alpha: UpperBound else: Exact
+        if score >= beta:
-        when defined(useTT):
-            self.transpositionTable.store(depth.uint8, score, self.board.position.zobristKey, nodeType)
-        if nodeType == LowerBound:
             # score >= beta
             # This move was too good for us, opponent will not search it
             break
@@ -302,6 +298,10 @@ proc search(self: SearchManager, depth, ply: int, alpha, beta: Score): Score {.d
             if ply == 0:
                 self.bestMoveRoot = move
                 self.bestRootScore = bestScore
+    when defined(useTT):
+        let nodeType = if bestScore >= beta: LowerBound elif bestScore <= alpha: UpperBound else: Exact
+        self.transpositionTable.store(depth.uint8, bestScore, self.board.position.zobristKey, bestMove, nodeType)
+
     return bestScore
 
 

Chess/nimfish/nimfishpkg/transpositions.nim

@@ -16,6 +16,7 @@
 
 import zobrist
 import eval
+import moves
 
 
 import nint128
@@ -37,7 +38,10 @@ type
         # fit into an int16
         score*: int16
         hash*: ZobristKey
-        depth: uint8
+        depth*: uint8
+        # The best move that was found at the
+        # depth this entry was created at. Could
+        bestMove*: Move
 
     TTable* = ref object
         data: seq[TTEntry]
@@ -65,18 +69,27 @@ func getIndex(self: TTable, key: ZobristKey): uint64 =
     result = (u128(key.uint64) * u128(self.size)).hi
 
 
-func store*(self: TTable, depth: uint8, score: Score, hash: ZobristKey, flag: TTentryFlag) =
+func store*(self: TTable, depth: uint8, score: Score, hash: ZobristKey, bestMove: Move, flag: TTentryFlag) =
     ## Stores an entry in the transposition table
-    self.data[self.getIndex(hash)] = TTEntry(flag: flag, score: int16(score), hash: hash, depth: depth)
+    self.data[self.getIndex(hash)] = TTEntry(flag: flag, score: int16(score), hash: hash, depth: depth, bestMove: bestMove)
 
 
 proc get*(self: TTable, hash: ZobristKey, depth: uint8): tuple[success: bool, entry: TTEntry] =
     ## Attempts to get the entry with the given
-    ## zobrist key at the given depth in the table. 
+    ## zobrist key and the given depth in the table.
-    ## The success parameter is set to false upon detection 
+    ## The success parameter is set to false upon 
-    ## of a hash collision or if the provided depth is greater
+    ## detection of a hash collision or other anomaly:
-    ## than the one stored in the table: the result should be
+    ## the result should be considered invalid unless
-    ## considered invalid unless it's true
+    ## it's true
     result.entry = self.data[self.getIndex(hash)]
     result.success = result.entry.hash == hash and result.entry.depth >= depth
 
+
+proc get*(self: TTable, hash: ZobristKey): tuple[success: bool, entry: TTEntry] =
+    ## Attempts to get the entry with the given
+    ## zobrist key in the table. The success parameter
+    ## is set to false upon detection of a hash collision
+    ## or other anomaly: the result should be considered 
+    ## invalid unless it's true
+    result.entry = self.data[self.getIndex(hash)]
+    result.success = result.entry.hash == hash

Chess/nimfish/nimfishpkg/tui.nim

@@ -68,6 +68,10 @@ proc perft*(board: var Chessboard, ply: int, verbose = false, divide = false, bu
                 echo "None"
             echo "\n", board.pretty()
         board.doMove(move)
+        when defined(debug):
+            let incHash = board.position.zobristKey
+            board.position.hash()
+            doAssert board.position.zobristKey == incHash, &"{board.position.zobristKey} != {incHash} at {move} ({board.positions[^1].toFEN()})"
         if ply == 1:
             if move.isCapture():
                 inc(result.captures)
@@ -452,7 +456,7 @@ proc commandLoop*: int =
                 of "zobrist":
                     echo board.position.zobristKey.uint64
                 of "rep":
-                    echo "Position is drawn by repetition: ", if board.position.repetitionDraw: "yes" else: "no"
+                    echo "Position is drawn by repetition: ", if board.drawnByRepetition(): "yes" else: "no"
                 of "eval":
                     echo &"Eval: {board.evaluate()}"
                 else:

Chess/nimfish/nimfishpkg/uci.nim

@@ -15,8 +15,6 @@
 ## Implementation of a UCI compatible server
 import std/strutils
 import std/strformat
-import std/atomics
-
 
 
 import board
@@ -29,7 +27,8 @@ type
     UCISession = ref object
         debug: bool
         board: Chessboard
-        currentSearch: Atomic[SearchManager]
+        searchManager: SearchManager
+        searchThread: ref Thread[tuple[session: UCISession, command: UCICommand]]
         hashTableSize: uint64
         transpositionTable: TTable
     
@@ -297,14 +296,11 @@ proc bestMove(args: tuple[session: UCISession, command: UCICommand]) {.thread.}
     {.cast(gcsafe).}:
         # Yes yes nim sure this isn't gcsafe. Now stfu and spawn a thread
         var session = args.session
-        when defined(useTT):
-            if session.transpositionTable.isNil():
-                if session.debug:
-                    echo &"info string created {session.hashTableSize} MiB TT"
-                session.transpositionTable = newTranspositionTable(session.hashTableSize * 1024 * 1024)
         var command = args.command
-        var searcher = newSearchManager(session.board.position, session.transpositionTable)
+        if session.debug:
-        session.currentSearch.store(searcher)
+            echo "info string search worker started"
+        var searcher = newSearchManager(session.board, session.transpositionTable)
+        session.searchManager = searcher
         var 
             timeRemaining = (if session.board.position.sideToMove == White: command.wtime else: command.btime)
             increment = (if session.board.position.sideToMove == White: command.winc else: command.binc)
@@ -357,11 +353,22 @@ proc startUCISession* =
                 of NewGame:
                     session.board = newDefaultChessboard()
                 of Go:
-                    var thread = new Thread[tuple[session: UCISession, command: UCICommand]]
+                    when defined(useTT):
-                    createThread(thread[], bestMove, (session, cmd))
+                        if session.transpositionTable.isNil():
-                    GcRef(thread)
+                            if session.debug:
+                                echo &"info string created {session.hashTableSize} MiB TT"
+                            session.transpositionTable = newTranspositionTable(session.hashTableSize * 1024 * 1024)
+                    session.searchThread = new Thread[tuple[session: UCISession, command: UCICommand]]
+                    createThread(session.searchThread[], bestMove, (session, cmd))
+                    if session.debug:
+                        echo "info string search started"
                 of Stop:
-                    session.currentSearch.load().stop()
+                    # TODO: Figure this out. Might be move semantics
+                    GcRef(session.searchManager)
+                    session.searchManager.stop()
+                    joinThread(session.searchThread[])
+                    if session.debug:
+                        echo "info string search stopped"
                 of SetOption:
                     case cmd.name:
                         of "Hash":

Chess/nimfish/nimfishpkg/zobrist.nim

@@ -27,7 +27,7 @@ type
 
 func `xor`*(a, b: ZobristKey): ZobristKey = ZobristKey(a.uint64 xor b.uint64)
 func `==`*(a, b: ZobristKey): bool = a.uint64 == b.uint64
-
+func `$`*(a: ZobristKey): string = $a.uint64
 
 proc computeZobristKeys: array[781, ZobristKey] =
     ## Precomputes our zobrist keys