diff --git a/crates/engine/src/move_picker.rs b/crates/engine/src/move_picker.rs index 578a9929..f3c11d6a 100644 --- a/crates/engine/src/move_picker.rs +++ b/crates/engine/src/move_picker.rs @@ -77,7 +77,8 @@ pub(crate) struct MovePicker { /// True after the TT move has been yielded, so yield stages can skip it. tt_move_yielded: bool, killers: [Option; 2], - /// Cached check/pin metadata computed in GenerateTacticals; reused in GenerateQuiets. + /// Check/pin metadata shared across staged move generation. Provided at + /// construction by the main search; computed lazily on first generation in qsearch. metadata: Option, /// Monotonically increasing selection-sort cursor (absolute index). pick_index: usize, @@ -100,7 +101,14 @@ impl MovePicker { /// Creates a `MovePicker` for the main negamax search. /// /// No moves are generated upfront; generation is deferred to the stage machine. - pub(crate) fn new(tt_move: Option, killers_table: &KillerMovesTable, ply: usize) -> Self { + /// The caller provides check/pin metadata it already computed for this node so + /// move generation doesn't recompute it. + pub(crate) fn new( + tt_move: Option, + killers_table: &KillerMovesTable, + ply: usize, + metadata: CheckPinMetadata, + ) -> Self { let killer_slice = killers_table.get(ply); let killers = [ killer_slice.first().copied().unwrap_or(None), @@ -118,7 +126,7 @@ impl MovePicker { tt_move, tt_move_yielded: false, killers, - metadata: None, + metadata: Some(metadata), pick_index: 0, moves_yielded: 0, searched_quiets: ArrayVec::new(), @@ -156,14 +164,6 @@ impl MovePicker { } } - /// Returns true if the side to move is in check. - /// - /// Only valid after `GenerateTacticals` has run (i.e., after `next()` has been - /// called at least once past the TT move stage). - pub(crate) fn in_check(&self) -> bool { - self.metadata.as_ref().is_some_and(|m| m.in_check()) - } - /// Returns the number of moves yielded so far. /// The caller can compute `loop_counter = moves_yielded() - 1` (0-based) after each `next()`. pub(crate) fn moves_yielded(&self) -> usize { @@ -235,7 +235,7 @@ impl MovePicker { /// Helper to generate tactical moves in the move picker. fn generate_tactical_moves(&mut self, board: &Board, history_table: &HistoryTable) { - // Reuse metadata computed in TtMove stage if available. + // Main search provides metadata at construction; qsearch computes it here. let meta = self .metadata .get_or_insert_with(|| move_generation::metadata::compute(board)) @@ -439,6 +439,10 @@ mod tests { .expect("From piece must exist") } + fn meta(board: &Board) -> move_generation::metadata::CheckPinMetadata { + move_generation::metadata::compute(board) + } + fn collect_all( picker: &mut MovePicker, board: &Board, @@ -487,7 +491,7 @@ mod tests { let tt_entry = tt.get_entry(board.zobrist_hash()).unwrap(); let tt_move = Some(tt_entry.board_move); - let mut picker = MovePicker::new(tt_move, &killers, 0); + let mut picker = MovePicker::new(tt_move, &killers, 0, meta(&board)); let first = picker.next(&board, &history).expect("must have a move"); assert_eq!(first, chosen, "TT move must be yielded first"); } @@ -497,7 +501,7 @@ mod tests { let board = Board::from_fen(CAPTURES_FEN).unwrap(); let history = HistoryTable::new(); let killers = KillerMovesTable::new(); - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); let mut seen_quiet = false; while let Some(mv) = picker.next(&board, &history) { @@ -530,7 +534,7 @@ mod tests { let board = Board::from_fen(MULTI_CAPTURE_FEN).unwrap(); let history = HistoryTable::new(); let killers = KillerMovesTable::new(); - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); let mut captures: Vec = Vec::new(); while let Some(mv) = picker.next(&board, &history) { @@ -575,7 +579,7 @@ mod tests { let killer_piece = piece_for_move(&board, &killer_mv); killers.update(0, killer_mv, killer_piece); - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); // Starting position has no captures; all moves are quiet. let moves = collect_all(&mut picker, &board, &history); @@ -603,7 +607,7 @@ mod tests { Score::MAX_HISTORY, ); - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); let moves = collect_all(&mut picker, &board, &history); // The favored move should be yielded first (highest history score). @@ -639,7 +643,7 @@ mod tests { let tt_entry = tt.get_entry(board.zobrist_hash()).unwrap(); let tt_move = Some(tt_entry.board_move); - let mut picker = MovePicker::new(tt_move, &killers, 0); + let mut picker = MovePicker::new(tt_move, &killers, 0, meta(&board)); let moves = collect_all(&mut picker, &board, &history); let count = moves.iter().filter(|&&m| m == capture_mv).count(); @@ -690,7 +694,7 @@ mod tests { let board = Board::from_fen(STARTING_FEN).unwrap(); let history = HistoryTable::new(); let killers = KillerMovesTable::new(); - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); let mut expected_counter = 0usize; while let Some(_mv) = picker.next(&board, &history) { @@ -708,7 +712,7 @@ mod tests { let board = Board::from_fen("8/P3k3/8/8/8/8/8/4K3 w - - 0 1").unwrap(); let history = HistoryTable::new(); let killers = KillerMovesTable::new(); - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); let moves = collect_all(&mut picker, &board, &history); let queen_push_promos: Vec<_> = moves @@ -754,7 +758,7 @@ mod tests { .collect(); // Lazy: collect via picker - let mut picker = MovePicker::new(None, &killers, 0); + let mut picker = MovePicker::new(None, &killers, 0, meta(&board)); let lazy_moves = collect_all(&mut picker, &board, &history); assert_eq!( lazy_moves.len(), @@ -780,24 +784,6 @@ mod tests { } } - #[test] - fn in_check_returns_correct_value() { - // Position where white king is in check - let board_in_check = Board::from_fen("4k3/8/8/8/8/8/4r3/4K3 w - - 0 1").unwrap(); - let history = HistoryTable::new(); - let killers = KillerMovesTable::new(); - let mut picker = MovePicker::new(None, &killers, 0); - // Drive past TtMove stage by calling next() once - let _ = picker.next(&board_in_check, &history); - assert!(picker.in_check(), "in_check() should return true"); - - // Position where white king is NOT in check - let board_safe = Board::from_fen(STARTING_FEN).unwrap(); - let mut picker2 = MovePicker::new(None, &killers, 0); - let _ = picker2.next(&board_safe, &history); - assert!(!picker2.in_check(), "in_check() should return false"); - } - #[test] fn no_bad_tacticals_in_qsearch() { let board = Board::from_fen("8/P3k3/8/8/8/8/8/4K3 w - - 0 1").unwrap(); diff --git a/crates/engine/src/search.rs b/crates/engine/src/search.rs index 8b3d4677..b31b29d0 100644 --- a/crates/engine/src/search.rs +++ b/crates/engine/src/search.rs @@ -226,6 +226,13 @@ impl<'a, Log: LogLevel> Search<'a, Log> { let _unused = writeln!(self.output, "{message}"); } + /// Returns true if an external stop was requested via the stop flag. + fn stop_requested(&self) -> bool { + self.stop_flag + .as_ref() + .is_some_and(|f| f.load(Ordering::Relaxed)) + } + /// Verify that a given [PrincipleVariation] is valid. This is expensive and should only be used for debugging. #[allow(clippy::expect_used)] fn verify_pv_moves(&self, pv: &PrincipleVariation, board: &Board) -> Result<()> { @@ -256,12 +263,7 @@ impl<'a, Log: LogLevel> Search<'a, Log> { } 'deepening: loop { - if td.should_stop(LimitType::Soft) - || self - .stop_flag - .as_ref() - .is_some_and(|f| f.load(Ordering::Relaxed)) - { + if td.should_stop(LimitType::Soft) || self.stop_requested() { break 'deepening; } @@ -286,6 +288,12 @@ impl<'a, Log: LogLevel> Search<'a, Log> { &mut pv, ); + // If the search aborted mid-tree, `score` is truncated — discard + // the whole iteration and keep the last completed result. + if td.is_stopped() || self.stop_requested() { + break 'deepening; + } + if aspiration_window.failed_low(score) { // fail low, widen the window aspiration_window.widen_down(score, td.depth as ScoreType); @@ -296,18 +304,6 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // we have a valid score, break the loop break 'aspiration_window; } - - // check stop conditions - if td.should_stop(LimitType::Hard) - || self - .stop_flag - .as_ref() - .is_some_and(|f| f.load(Ordering::Relaxed)) - { - // we have to stop searching now, use the best result we have - // no score update - break 'deepening; - } } // update the best result (commit the completed iteration) @@ -392,7 +388,6 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // increment node count td.nodes += 1; td.seldepth = td.seldepth.max(ply); - let in_check = move_generation::is_in_check(board); // Ply guard: prevent unbounded recursion if ply >= MAX_PLY { @@ -448,6 +443,12 @@ impl<'a, Log: LogLevel> Search<'a, Log> { let tt_move = tt_entry.map(|entry| entry.board_move); + // Compute check/pin metadata once for this node. It provides `in_check` here + // and is reused by the move picker for legal move generation. Computed after + // the TT probe so cutoffs don't pay for it. + let metadata = move_generation::metadata::compute(board); + let in_check = metadata.in_check(); + // Really "bad" initial score let mut best_score = -Score::INF; let mut best_move: Option = None; @@ -483,7 +484,8 @@ impl<'a, Log: LogLevel> Search<'a, Log> { } // Build move picker. Move generation is lazy (deferred to stage machine). - let mut picker = move_picker::MovePicker::new(tt_move, &td.killers_table, ply as usize); + let mut picker = + move_picker::MovePicker::new(tt_move, &td.killers_table, ply as usize, metadata); // How much to extend the depth. let mut extension = 0; @@ -508,7 +510,6 @@ impl<'a, Log: LogLevel> Search<'a, Log> { let lmr_reduction = (1f64 + base_reduction).floor() as i16; let is_mated = best_score.mated(); - let is_in_check = picker.in_check(); let is_root = Node::ROOT; let is_pv = Node::PV; let is_quiet = board.captured(&mv).is_none() && !mv.is_promotion(); @@ -519,7 +520,7 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // SEE prune bad tacticals at shallow depth if !is_root && !is_pv - && !is_in_check + && !in_check && !is_mated && is_bad_tactical && (depth as i32) <= see_tacticals_max_depth() @@ -536,7 +537,7 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // ------------------------------------------------------------------------------------------ if !is_root && !is_pv - && !is_in_check + && !in_check && !is_mated && is_quiet && moves_seen > 0 @@ -556,7 +557,7 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // --------------------------------------------------------------------------------- if !is_root && !is_pv - && !is_in_check + && !in_check && !is_mated && is_quiet && depth <= lmp_max_depth() as i16 @@ -669,6 +670,14 @@ impl<'a, Log: LogLevel> Search<'a, Log> { board.unmake_move().unwrap(); moves_seen += 1; + // If the search aborted while this move was being searched, its score + // is truncated so we return without letting it touch other tables like + // history or TT. Ancestors discard the result the same way, so the + // partial score never propagates. + if td.should_stop(LimitType::Hard) || self.stop_requested() { + return best_score; + } + // check the results if score > best_score { // we improved, so update the score and best move @@ -714,21 +723,11 @@ impl<'a, Log: LogLevel> Search<'a, Log> { break; } } - - // do we need to stop searching? - if td.should_stop(LimitType::Hard) - || self - .stop_flag - .as_ref() - .is_some_and(|f| f.load(Ordering::Relaxed)) - { - break; - } } // No moves were yielded: checkmate or stalemate. if picker.moves_yielded() == 0 { - return if picker.in_check() { + return if in_check { -Score::MATE + ply } else { Score::DRAW @@ -771,7 +770,7 @@ impl<'a, Log: LogLevel> Search<'a, Log> { fn pruned_score( &mut self, tt_entry: Option, - board: &Board, + board: &mut Board, td: &mut ThreadData, depth: ScoreType, ply: ScoreType, @@ -793,16 +792,9 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // ------------------------------------------------------------------------------------------------------------ let razoring_margin = razoring_offset() + razoring_scaling() * depth as i32; if static_eval.as_i32() + razoring_margin < alpha.as_i32() { - let mut brd_cpy = board.clone(); let mut razor_pv = PrincipleVariation::new(); - let score = self.quiescence::( - &mut brd_cpy, - td, - ply, - alpha, - alpha + 1, - &mut razor_pv, - ); + let score = + self.quiescence::(board, td, ply, alpha, alpha + 1, &mut razor_pv); if score < alpha && !score.is_mate() { return Some(score); } @@ -844,12 +836,11 @@ impl<'a, Log: LogLevel> Search<'a, Log> { && tt_entry.is_none_or(|entry| entry.flag() != ttable::EntryFlag::UpperBound) { let null_move_depth = depth - params::nmp_reduction(depth as i32, improving) as i16 - 1; - let mut null_board = board.clone(); - null_board.null_move(); - td.transposition_table.prefetch(null_board.zobrist_hash()); + board.null_move(); + td.transposition_table.prefetch(board.zobrist_hash()); let mut nmp_pv = PrincipleVariation::new(); let null_score = -self.negamax::( - &mut null_board, + board, td, null_move_depth, ply + 1, @@ -857,6 +848,7 @@ impl<'a, Log: LogLevel> Search<'a, Log> { -beta + 1, &mut nmp_pv, ); + board.unmake_move().unwrap(); if null_score >= beta { return if null_score.is_mate() { @@ -969,12 +961,28 @@ impl<'a, Log: LogLevel> Search<'a, Log> { // Quiescence SEE pruning // https://www.chessprogramming.org/Static_Exchange_Evaluation // https://talkchess.com/viewtopic.php?t=41217 - // Skip moves that lose material if we're not in check + // Skip moves that lose material if we're not in check. + // + // Captures yielded past the TT stage already passed the picker's + // `see(mv, 0)` classification, which implies `see(mv, t)` for any + // t <= 0 — rechecking them is redundant. Only the TT move + // (yielded unclassified) and promotions (classified good without + // SEE) still need the check, unless the threshold is tuned + // positive, in which case classification no longer covers it. // ------------------------------------------------------------ - if !in_check && !see::see(board, mv, qs_see_threshold()) { + let see_checked_by_picker = + Some(mv) != tt_move && !mv.is_promotion() && qs_see_threshold() <= 0; + if !in_check && !see_checked_by_picker && !see::see(board, mv, qs_see_threshold()) { continue; } + // When skipping, verify the picker actually guaranteed `see(mv, 0)`. + debug_assert!( + in_check || !see_checked_by_picker || see::see(board, mv, 0), + "qsearch picker yielded a SEE-losing capture: {}", + mv.to_long_algebraic() + ); + // local PV is for each node below this one is different when we call negamax recursively // so we have to clear it local_pv.clear(); @@ -993,6 +1001,13 @@ impl<'a, Log: LogLevel> Search<'a, Log> { board.unmake_move().unwrap(); + // If the search aborted while this move was being searched, its score + // is truncated — return without letting it touch best/alpha or the + // TT store below. + if td.should_stop(LimitType::Hard) || self.stop_requested() { + return best; + } + if score > best { best = score; best_move = Some(mv); @@ -1010,15 +1025,6 @@ impl<'a, Log: LogLevel> Search<'a, Log> { alpha_use = score; } } - - if td.should_stop(LimitType::Hard) - || self - .stop_flag - .as_ref() - .is_some_and(|f| f.load(Ordering::Relaxed)) - { - break; - } } // In check with no legal moves: checkmate. diff --git a/crates/engine/src/thread_data.rs b/crates/engine/src/thread_data.rs index ad1f25ce..608a9619 100644 --- a/crates/engine/src/thread_data.rs +++ b/crates/engine/src/thread_data.rs @@ -16,6 +16,10 @@ use crate::{ score::ScoreType, search::limits::SearchLimits, ttable::TranspositionTable, }; +/// Number of nodes searched between wall-clock polls for the hard time limit. +/// Node and depth limits are still enforced exactly on every check. +const NODES_BETWEEN_TIME_CHECKS: u64 = 2048; + pub struct ThreadData { pub(crate) transposition_table: TranspositionTable, pub(crate) history_table: HistoryTable, @@ -27,6 +31,8 @@ pub struct ThreadData { pub(crate) depth: i32, pub(crate) seldepth: ScoreType, pub(crate) nodes: u64, + nodes_until_time_check: u64, + stopped: bool, pub(crate) stack: NodeStack, } @@ -48,6 +54,8 @@ impl Default for ThreadData { depth: 1, seldepth: 0, nodes: 0, + nodes_until_time_check: 0, + stopped: false, stack: NodeStack::default(), } } @@ -69,6 +77,8 @@ impl ThreadData { pub fn reset(&mut self) { self.depth = 1; self.nodes = 0; + self.nodes_until_time_check = 0; + self.stopped = false; self.seldepth = 0; self.bestmove_stability = 0; self.prev_best_move = None; @@ -104,7 +114,14 @@ impl ThreadData { self.prev_best_move = new_best; } - pub fn should_stop(&self, limit_type: LimitType) -> bool { + /// Returns true if a hard limit already stopped the search. Unlike + /// [`Self::should_stop`], this never polls the clock. + pub fn is_stopped(&self) -> bool { + self.stopped + } + + /// Check if the current search should stop for the given [`LimitType`]. + pub fn should_stop(&mut self, limit_type: LimitType) -> bool { match limit_type { LimitType::Soft => self.soft_limit_reached(), LimitType::Hard => self.hard_limit_reached(), @@ -113,6 +130,11 @@ impl ThreadData { /// Check if the soft limit has been reached. fn soft_limit_reached(&self) -> bool { + // A hard stop already triggered mid-iteration; don't start another one. + if self.stopped { + return true; + } + let best_move_stability = self.bestmove_stability_for_scaling(); if let Some(soft_time) = self.limits.scaled_soft_limit(best_move_stability) && self.start_time.elapsed() >= soft_time @@ -131,12 +153,16 @@ impl ThreadData { /// Check if the hard limit has been reached. /// This includes time and nodes. - fn hard_limit_reached(&self) -> bool { - if self.start_time.elapsed() >= self.limits.hard_timeout { + fn hard_limit_reached(&mut self) -> bool { + // Something previously stopped the search. + // This flag is reset in [`ThreadData::reset`] + if self.stopped { return true; } + // Have we exceeded the max nodes if self.nodes >= self.limits.max_nodes { + self.stopped = true; return true; } @@ -144,6 +170,17 @@ impl ThreadData { return true; } + // Reading the wall clock on every node is expensive, so only poll it + // once the node count crosses the next check threshold. + if self.nodes >= self.nodes_until_time_check { + // Update for the next time check + self.nodes_until_time_check = self.nodes + NODES_BETWEEN_TIME_CHECKS; + if self.start_time.elapsed() >= self.limits.hard_timeout { + self.stopped = true; + return true; + } + } + false }