refactor: removed duplicate code from search/negamax

brogle/src/config.rs

@@ -1,14 +1,11 @@
-/// Wrapper over `i32` for scoring positions
-pub type Score = i32;
-
 /// Largest possible score ever achievable
-pub const INF: Score = i16::MAX as Score;
+pub const INF: i32 = i16::MAX as i32;
 
 /// Score of mate in 1 move
-pub const MATE: Score = INF - 1;
+pub const MATE: i32 = INF - 1;
 
 /// Maximum depth that can be searched
 pub const MAX_DEPTH: u32 = 255;
 
 /// Maximum possible score for mate
-pub const MAX_MATE: Score = MATE - MAX_DEPTH as Score;
+pub const MAX_MATE: i32 = MATE - MAX_DEPTH as i32;

brogle/src/engine.rs

@@ -632,9 +632,9 @@ impl UciEngine for Engine {
                 // Create a search instance with the appropriate thread data
                 let search = Searcher::new(
                     &game,
+                    &mut ttable,
                     starttime,
                     timeout,
-                    &mut ttable,
                     Arc::clone(&is_searching),
                 );
 
@@ -673,10 +673,7 @@ impl UciEngine for Engine {
                     }
 
                     // Search was stopped abruptly; exit iterative deepening loop
-                    Err(_err) => {
-                        // eprintln!("{_err}");
-                        break;
-                    }
+                    Err(_) => break,
                 }
             }
             // If this line of code is reached, it means the search has stopped.

brogle/src/search/searcher.rs

@@ -6,14 +6,22 @@ use std::time::Instant;
 use anyhow::{bail, Result};
 use brogle_core::{Game, Move, PieceKind, ZobristKey};
 
-use crate::{value_of, Evaluator, Score, INF, MATE};
-
 use super::{NodeType, TTable, TTableEntry};
+use crate::{value_of, Evaluator, INF, MATE};
 
 pub struct SearchData {
     pub nodes_searched: usize,
-    pub score: Score,
+    pub score: i32,
     pub bestmove: Option<Move>,
+    /*
+    /// Principle Variation of the search.
+    ///
+    /// pv[i][j] is the PV at the i'th ply (0 is root), which has j descendants
+    ///
+    /// For example, if you searched at depth 4, then 'i' would be at most 3.
+    /// pv[0] would have depth+q entries, where 'q' is the number of plies searched in qsearch.
+    pub(crate) pv: Vec<Vec<Move>>,
+     */
 }
 
 impl Default for SearchData {
@@ -29,105 +37,40 @@ impl Default for SearchData {
 /// A struct to encapsulate the logic of searching through moves for a given a chess position.
 pub struct Searcher<'a> {
     game: &'a Game,
-    timeout: Duration,
     ttable: &'a mut TTable,
-    stopper: Arc<AtomicBool>,
     starttime: Instant,
+    timeout: Duration,
+    stopper: Arc<AtomicBool>,
 
     // Search data
-    pub(crate) data: SearchData,
-    /*
-    /// Principle Variation of the search
-    /// pv[i][j] is the PV at the i'th ply (0 is root), which has j descendants
-    pub(crate) pv: Vec<Vec<Move>>,
-     */
+    data: SearchData,
 }
 
 impl<'a> Searcher<'a> {
-    /// Create a new search that will search the provided position at a depth of 1.
+    /// Create a new search that will search the provided position.
     pub fn new(
         game: &'a Game,
+        ttable: &'a mut TTable,
         starttime: Instant,
         timeout: Duration,
-        ttable: &'a mut TTable,
         stopper: Arc<AtomicBool>,
     ) -> Self {
         Self {
             game,
+            ttable,
             starttime,
             timeout,
             stopper,
-            ttable,
             data: SearchData::default(),
         }
     }
 
-    /// Starts a search from the supplied depth.
+    /// Start a search of depth `depth` at the root node.
     pub fn start(mut self, depth: u32) -> Result<SearchData> {
-        // eprintln!("\nStarting search of depth {depth} on {}", self.game.fen());
-
-        // No need to check depth 0 here because this cannot be called with `depth < 1`
-        let mut moves = self.game.legal_moves();
-
-        if moves.is_empty() {
-            self.data.score = if self.game.is_in_check() { -MATE } else { 0 };
-            return Ok(self.data);
-        }
-
-        self.data.bestmove = moves.first().cloned();
-
-        let tt_bestmove = self.get_tt_bestmove(self.game.key());
-        moves.sort_by_cached_key(|mv| score_move(self.game, mv, tt_bestmove));
-
-        // Start with a default (very bad) result.
-        let mut alpha = -INF;
-        let original_alpha = alpha;
-        let beta = INF;
-        let ply = 0;
-
-        for i in 0..moves.len() {
-            let mv = moves[i];
-
-            // Make the score move on the position, getting a new position in return
-            let new_pos = self.game.clone().with_move_made(mv);
+        let key = self.game.key();
+        self.data.score = self.negamax(self.game, depth, 0, -INF, INF)?;
+        self.data.bestmove = self.ttable.get(&key).map(|entry| entry.bestmove);
 
-            if new_pos.is_repetition() || new_pos.can_draw_by_fifty() {
-                continue;
-            }
-
-            // Recursively search our opponent's responses
-            let score = -self.negamax(&new_pos, depth - 1, ply + 1, -beta, -alpha)?;
-            self.data.nodes_searched += 1;
-
-            // Check if we've run out of time or if we've been told to stop searching
-            if self.starttime.elapsed() >= self.timeout || !self.stopper.load(Ordering::Relaxed) {
-                // If we must cancel this search, we need to return the result from the previous iteration
-                bail!(
-                    "Search was stopped while evaluating {mv}. Elapsed: {:?}. Current bestmove: {:?}",
-                    self.starttime.elapsed(),
-                    self.data.bestmove
-                );
-            }
-
-            if score > self.data.score {
-                self.data.score = score;
-
-                if self.data.score > alpha {
-                    alpha = score;
-                    self.data.bestmove = Some(mv);
-                }
-
-                // Fail soft beta-cutoff.
-                // Could also `return Ok(self.data.score)` here, but we need to save bestmove to TT
-                if self.data.score >= beta {
-                    break;
-                }
-            }
-        }
-
-        let bestmove = self.data.bestmove.unwrap(); // safe unwrap because if `moves` was empty, we would have returned earlier. So `bestmove` is guaranteed to be *something*
-        let flag = NodeType::new(self.data.score, original_alpha, beta);
-        self.save_to_ttable(self.game.key(), bestmove, self.data.score, 0, flag);
         Ok(self.data)
     }
 
@@ -137,13 +80,9 @@ impl<'a> Searcher<'a> {
         game: &Game,
         depth: u32,
         ply: i32,
-        mut alpha: Score,
-        beta: Score,
-    ) -> Result<Score> {
-        // Clear the PV for score node
-        // self.pv.push(Vec::with_capacity(depth));
-        // self.pv[ply] = Vec::with_capacity(depth);
-
+        mut alpha: i32,
+        beta: i32,
+    ) -> Result<i32> {
         // Reached the end of the depth; start a qsearch for captures only
         if depth == 0 {
             return self.quiescence(game, ply + 1, alpha, beta);
@@ -152,19 +91,17 @@ impl<'a> Searcher<'a> {
 
         let mut moves = game.legal_moves();
         if moves.is_empty() {
-            return Ok(if game.is_in_check() {
-                -MATE + ply // Prefer earlier mates
-            } else {
-                0 // A draw is better than losing
-            });
+            // Prefer earlier mates, and drawing is better than being mated.
+            let score = if game.is_in_check() { -MATE + ply } else { 0 };
+            return Ok(score);
         }
 
         let tt_bestmove = self.get_tt_bestmove(game.key());
         moves.sort_by_cached_key(|mv| score_move(game, mv, tt_bestmove));
 
         // Start with a default (very bad) result.
         let mut best = -INF;
-        let mut bestmove = moves[0]; // Safe because we already checked that moves isn't empty
+        let mut bestmove = moves[0]; // Safe because we already checked that `moves` isn't empty
         let original_alpha = alpha;
 
         for i in 0..moves.len() {
@@ -183,11 +120,7 @@ impl<'a> Searcher<'a> {
 
             // Check if we've run out of time or if we've been told to stop searching
             if self.starttime.elapsed() >= self.timeout || !self.stopper.load(Ordering::Relaxed) {
-                // If we must cancel this search, we need to return the result from the previous iteration
-                bail!(
-                    "Negamax was stopped while evaluating {mv}. Elapsed: {:?}. Current bestmove: {bestmove:?}",
-                    self.starttime.elapsed(),
-                );
+                bail!("Negamax was cancelled while evaluating {mv}");
             }
 
             // If we've found a better move than our current best, update our result
@@ -210,16 +143,11 @@ impl<'a> Searcher<'a> {
 
         let flag = NodeType::new(best, original_alpha, beta);
         self.save_to_ttable(game.key(), bestmove, best, depth, flag);
-        Ok(best)
+
+        Ok(best) // fail-soft
     }
 
-    fn quiescence(
-        &mut self,
-        game: &Game,
-        ply: i32,
-        mut alpha: Score,
-        beta: Score,
-    ) -> Result<Score> {
+    fn quiescence(&mut self, game: &Game, ply: i32, mut alpha: i32, beta: i32) -> Result<i32> {
         let stand_pat = Evaluator::new(game).eval_current_player();
         if stand_pat >= beta {
             return Ok(beta);
@@ -239,7 +167,7 @@ impl<'a> Searcher<'a> {
 
         // let original_alpha = alpha;
         let mut best = stand_pat;
-        let mut bestmove = captures[0]; // Safe because we already checked that moves isn't empty
+        let mut bestmove = captures[0]; // Safe because we already checked that `captures` isn't empty
         let original_alpha = alpha;
 
         // Only search captures
@@ -255,11 +183,7 @@ impl<'a> Searcher<'a> {
 
             // Check if we've run out of time or if we've been told to stop searching
             if self.starttime.elapsed() >= self.timeout || !self.stopper.load(Ordering::Relaxed) {
-                // If we must cancel this search, we need to return the result from the previous iteration
-                bail!(
-                    "QSearch was stopped while evaluating {mv}. Elapsed: {:?}. Current bestmove: {bestmove:?}",
-                    self.starttime.elapsed(),
-                );
+                bail!("QSearch was cancelled while evaluating {mv}");
             }
 
             // If we've found a better move than our current best, update our result
@@ -282,19 +206,25 @@ impl<'a> Searcher<'a> {
 
         let flag = NodeType::new(best, original_alpha, beta);
         self.save_to_ttable(game.key(), bestmove, best, 0, flag);
-        Ok(best)
+
+        Ok(best) // fail-soft
     }
 
     fn save_to_ttable(
         &mut self,
         key: ZobristKey,
         bestmove: Move,
-        score: Score,
+        score: i32,
         depth: u32,
         flag: NodeType,
     ) {
-        let entry = TTableEntry::new(key, bestmove, score, depth, flag);
-        self.ttable.store(entry);
+        self.ttable.store(TTableEntry {
+            key,
+            bestmove,
+            score,
+            depth,
+            flag,
+        });
     }
 
     fn get_tt_bestmove(&self, key: ZobristKey) -> Option<Move> {
@@ -309,11 +239,12 @@ fn mvv_lva(kind: PieceKind, captured: PieceKind) -> i32 {
 
 fn score_move(game: &Game, mv: &Move, tt_bestmove: Option<Move>) -> i32 {
     if tt_bestmove.is_some_and(|tt_mv| tt_mv == *mv) {
-        return Score::MIN;
+        return i32::MIN;
     }
 
-    let mut score = 0;
+    // Safe unwrap because we can't move unless there's a piece at `from`
     let kind = game.kind_at(mv.from()).unwrap();
+    let mut score = 0;
 
     // Capturing a high-value piece with a low-value piece is a good idea
     if let Some(captured) = game.kind_at(mv.to()) {