Skip to content

chore: minor improvements and correction #402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ptsouchlos merged 8 commits into from
Jul 12, 2026
Merged

chore: minor improvements and correction #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3dc8a6d
chore: don't clone board for razoring or nmp
ptsouchlos Jul 9, 2026
b13118e
chore: only check hard timeout after certain number of nodes
ptsouchlos Jul 11, 2026
969dd2e
chore: compute check/pin metadata once per node
ptsouchlos Jul 11, 2026
4fb72e8
chore: avoid double see check in qsearch
ptsouchlos Jul 11, 2026
7a6e54a
fix: minor typo
ptsouchlos Jul 11, 2026
47f2bc4
chore: add debug assert to validate pre-condition
ptsouchlos Jul 11, 2026
4daf2d8
chore: add is_stopped helper in thread data
ptsouchlos Jul 11, 2026
250bbce
fix: don't store scores and moves from aborted iterations
ptsouchlos Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 25 additions & 39 deletions crates/engine/src/move_picker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<KillerEntry>; 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<CheckPinMetadata>,
/// Monotonically increasing selection-sort cursor (absolute index).
pick_index: usize,
Expand All @@ -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<Move>, 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<Move>,
killers_table: &KillerMovesTable,
ply: usize,
metadata: CheckPinMetadata,
) -> Self {
let killer_slice = killers_table.get(ply);
let killers = [
killer_slice.first().copied().unwrap_or(None),
Expand All @@ -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(),
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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<chess::moves::Move> = Vec::new();
while let Some(mv) = picker.next(&board, &history) {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Expand All @@ -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();
Expand Down
Loading