Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
45 changes: 39 additions & 6 deletions moonfish/engines/alpha_beta.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
NEG_INF = float("-inf")
NULL_MOVE = Move.null()

# Number of killer moves to store per ply
NUM_KILLERS = 2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused constant NUM_KILLERS

NUM_KILLERS is defined here but never referenced. The killer table initialization on line 347 hardcodes [None, None], and _store_killer uses literal indices [0] and [1]. Consider either using this constant in both places or removing it to avoid confusion.

Suggested change
NUM_KILLERS = 2



class AlphaBeta:
"""
Expand Down Expand Up @@ -168,6 +171,19 @@ def quiescence_search(

return best_score

def _store_killer(
self, killers: list[list[Move | None]], ply: int, move: Move
) -> None:
"""Store a killer move at the given ply, shifting the existing one."""
if ply >= len(killers):
return
# Don't store duplicates
if killers[ply][0] == move:
return
# Shift: slot 1 gets old slot 0, slot 0 gets new move
killers[ply][1] = killers[ply][0]
killers[ply][0] = move

def negamax(
self,
board: Board,
Expand All @@ -176,6 +192,8 @@ def negamax(
cache: DictProxy | CACHE_KEY,
alpha: float = NEG_INF,
beta: float = INF,
ply: int = 0,
killers: list[list[Move | None]] | None = None,
) -> tuple[float | int, Move | None]:
"""
This functions receives a board, depth and a player; and it returns
Expand All @@ -198,10 +216,10 @@ def negamax(
- null_move: if we want to use null move pruning
- cache: a shared hash table to store the best
move for each board state and depth.
- alpha: best score for the maximizing player (best choice
(highest value) we've found along the path for max)
- beta: best score for the minimizing player (best choice
(lowest value) we've found along the path for min).
- alpha: best score for the maximizing player
- beta: best score for the minimizing player
- ply: current ply from root (for killer move indexing)
- killers: killer move table [ply][slot] -> Move

Returns:
- best_score, best_move: returns best move that it found and its value.
Expand Down Expand Up @@ -250,6 +268,8 @@ def negamax(
cache,
-beta,
-beta + 1,
ply + 1,
killers,
)[0]
board.pop()
if board_score >= beta:
Expand All @@ -260,7 +280,10 @@ def negamax(

# initializing best_score
best_score = NEG_INF
moves = organize_moves(board)

# Get killer moves for this ply
ply_killers = killers[ply] if killers is not None and ply < len(killers) else None
moves = organize_moves(board, killers=ply_killers)

for move in moves:
# make the move
Expand All @@ -273,6 +296,8 @@ def negamax(
cache,
-beta,
-alpha,
ply + 1,
killers,
)[0]
if board_score > self.config.checkmate_threshold:
board_score -= 1
Expand All @@ -284,6 +309,9 @@ def negamax(

# beta-cutoff
if board_score >= beta:
# Store killer move for quiet moves that cause cutoff
if killers is not None and not board.is_capture(move):
self._store_killer(killers, ply, move)
cache[cache_key] = (board_score, move)
return board_score, move

Expand Down Expand Up @@ -314,8 +342,13 @@ def search_move(self, board: Board) -> Move:
# create shared cache
cache: CACHE_KEY = {}

# Initialize killer move table: max_depth + some margin for extensions
max_ply = self.config.negamax_depth + 20
killers: list[list[Move | None]] = [[None, None] for _ in range(max_ply)]

best_move = self.negamax(
board, self.config.negamax_depth, self.config.null_move, cache
board, self.config.negamax_depth, self.config.null_move, cache,
killers=killers,
)[1]
assert best_move is not None, "Best move from root should not be None"
return best_move
30 changes: 26 additions & 4 deletions moonfish/move_ordering.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,53 @@
from moonfish.psqt import evaluate_capture, evaluate_piece, get_phase


def organize_moves(board: Board):
def organize_moves(
board: Board,
killers: "list[Move | None] | None" = None,
):
"""
This function receives a board and it returns a list of all the
possible moves for the current player, sorted by importance.
It sends capturing moves at the starting positions in
the array (to try to increase pruning and do so earlier).
Order: captures first, then killer moves, then remaining quiet moves.

Arguments:
- board: chess board state
- killers: list of killer moves for the current ply (tried after captures)

Returns:
- legal_moves: list of all the possible moves for the current player.
"""
non_captures = []
captures = []
killer_set = set()

# Build set of killer moves for fast lookup
if killers is not None:
for k in killers:
if k is not None:
killer_set.add(k)

for move in board.legal_moves:
if board.is_capture(move):
captures.append(move)
elif move in killer_set:
# Skip killers from non_captures; we'll insert them after captures
continue
else:
non_captures.append(move)

random.shuffle(captures)
random.shuffle(non_captures)
return captures + non_captures

# Insert legal killer moves between captures and quiet moves
killer_moves = []
if killers is not None:
legal_moves_set = set(board.legal_moves)
for k in killers:
if k is not None and k in legal_moves_set and not board.is_capture(k):
killer_moves.append(k)

Comment on lines 34 to +53
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant legality check for killer moves

Killer moves are filtered from non_captures at line 37-39 using killer_set, but then a second pass at lines 48-52 re-validates them against legal_moves_set. Since the first loop already iterates all legal moves, any killer move that wasn't seen in the loop is guaranteed to be illegal (or a capture). You could track "seen legal killers" in the first loop and avoid constructing legal_moves_set entirely:

Suggested change
for move in board.legal_moves:
if board.is_capture(move):
captures.append(move)
elif move in killer_set:
# Skip killers from non_captures; we'll insert them after captures
continue
else:
non_captures.append(move)
random.shuffle(captures)
random.shuffle(non_captures)
return captures + non_captures
# Insert legal killer moves between captures and quiet moves
killer_moves = []
if killers is not None:
legal_moves_set = set(board.legal_moves)
for k in killers:
if k is not None and k in legal_moves_set and not board.is_capture(k):
killer_moves.append(k)
for move in board.legal_moves:
if board.is_capture(move):
captures.append(move)
elif move in killer_set:
# Skip killers from non_captures; we'll insert them after captures
killer_set.discard(move) # mark as legal
continue
else:
non_captures.append(move)
random.shuffle(captures)
random.shuffle(non_captures)
# Insert legal killer moves between captures and quiet moves
# Only include killers that were discarded above (i.e., legal and non-capture)
killer_moves = []
if killers is not None:
for k in killers:
if k is not None and k not in killer_set and not board.is_capture(k):
killer_moves.append(k)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

return captures + killer_moves + non_captures


def is_tactical_move(board: Board, move: Move) -> bool:
Expand Down
Loading