Overhaul search and add hand-crafted evaluation terms - #13
Conversation
Search (negamax.py): - Null-move pruning (R=2..3, zugzwang/check/low-depth guards, verification search at depth >= 10) - Late move reductions (late quiet non-checking non-TT moves searched 1..3 plies shallower, full-depth re-search on a fail-high) - Principal variation search (full window on move 0, null-window scout + re-search on the rest; the LMR scout shares the null window) - Mate-distance pruning Draw and mate scoring: - Threefold repetition and the fifty-move rule are now scored 0 inside the tree (negamax.claims_draw); a CONTEMPT constant is the hook for a non-zero draw score - Mate scores are distance-to-mate from the root (MATE - ply); the TT rebases them on store/probe (constants.tt_store_score / tt_probe_score) so mate bounds propagate. MATE_GUARD removed. TT probe/store take a ply argument. - UCI info reports `score mate N` for mate scores Evaluation (eval_terms.py): - Passed / isolated / doubled pawns, bishop pair, rook on open/half-open file, knight outposts, pawn-shield king safety, tempo - Pure-pawn terms memoised on the pawn bitboards by PestoEvaluator Docs: CHANGELOG, design.md, performance.md (re-benchmarked), engine-strength.md (estimate re-derived to ~2050), tasks.md (next five items researched and written), README, and the /bench skill. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🟡 Changes recommended
There is a confirmed search correctness issue where claimable-draw detection can override checkmate scoring, and a documentation mismatch about what “nodes” measures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR substantially upgrades pychess’s playing strength by modernizing the core alpha-beta search (Negamax) and layering in additional hand-crafted evaluation terms on top of PeSTO, along with corresponding TT/UCI plumbing and expanded unit coverage.
Changes:
- Implemented major search enhancements (null-move pruning, PVS, LMR, mate-distance pruning) and in-tree claimable draw scoring.
- Switched mate scoring to distance-from-root and updated TT store/probe to rebase mate scores by ply; updated UCI
infoformatting to emitscore mate N. - Added a new positional evaluation layer (
eval_terms) with pawn-structure caching inPestoEvaluator, plus extensive new/updated tests and refreshed docs/changelog.
File summaries
| File | Description |
|---|---|
| tests/test_uci.py | Adds coverage for UCI mate/centipawn score formatting and go output. |
| tests/test_transposition.py | Updates TT tests for ply-rebased mate scores and non-mate invariance. |
| tests/test_negamax.py | Adds tests for null-move pruning, LMR, PVS, draw claiming, and mate distance scoring stability. |
| tests/test_move_ordering.py | Adds tests for new MoveOrderer.is_killer and history_score accessors. |
| tests/test_eval_terms.py | New unit tests for eval_terms symmetry and individual term behaviors plus pawn-cache consistency. |
| src/pychess/transposition.py | Adds ply parameter and mate-score rebasing on TT store/probe. |
| src/pychess/shared_tt.py | Mirrors TT mate-score rebasing changes for the shared-memory TT. |
| src/pychess/negamax.py | Implements the new pruning/reduction/search techniques, in-tree draw claiming, and mate-distance scoring/pruning. |
| src/pychess/move_ordering.py | Exposes killer/history query helpers needed by LMR logic. |
| src/pychess/evaluation.py | Integrates eval_terms, adds pawn-structure memoization, and applies tempo. |
| src/pychess/eval_terms.py | New positional evaluation term implementations (pawn structure, bishop pair, rook files, outposts, pawn-shield king safety). |
| src/pychess/constants.py | Defines CONTEMPT and mate TT rebase helpers (tt_store_score/tt_probe_score); updates mate conventions. |
| src/pychess/main.py | Updates UCI info score formatting to emit mate when appropriate. |
| README.md | Refreshes feature list and strength estimate to reflect the new search/eval. |
| docs/tasks.md | Advances roadmap to next items now that the prior search/eval work landed. |
| docs/performance.md | Rewrites benchmark narrative/tables to reflect the new engine and measurements. |
| docs/engine-strength.md | Re-derives the (still unmeasured) strength estimate based on the new feature set. |
| docs/design.md | Updates design overview to include the new search stack and eval terms. |
| CHANGELOG.md | Documents new features and behavioral changes under Unreleased. |
| .claude/skills/bench/SKILL.md | Updates the bench skill instructions to match the new performance doc structure. |
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: cjunius <41166228+cjunius@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Null-move pruning currently doesn’t exclude negative mate-score windows (beta), which can allow NMP in mate-scoring regions and undermine mate-distance correctness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core search/evaluation/TT semantics across multiple critical modules, so it needs careful human validation despite the strong added test coverage.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/pychess/constants.py:20
- The inline comment on
MATEis misleading: mate-ness is determined byMATE_IN_MAX, whileMATEis the base mate score used forMATE - pliesdistance-to-mate values. Clarifying this avoids confusion when reading or extending the scoring/TT rebase logic.
INF = 99999 # alpha-beta window sentinel
MATE = 9999 # abs(score) at or above MATE_IN_MAX is mate-related
MATE_IN_MAX = 9899 # MATE - 100: abs(score) >= this is a mate (max tree ply << 100)
docs/performance.md:52
- This paragraph describes "nodes" as counting
board.push, but in this codebasenodesis the engine node counter (number ofNegamax.search/Negamax.quiescecalls). The current wording contradicts the definition earlier in this doc and can mislead readers comparing the figures.
The `pruned` column is an *estimate*: the two counts aren't the same unit.
"nodes" is every `board.push` in the pruned tree (internal nodes and quiescence
included, and quiescence looks past `depth` in forcing lines); `perft` is only
the leaves at exactly `depth`. So it overstates pruning a little at shallow
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
Adds tools/self_play_gif.py (the original GIF was a checked-in binary with no generator) and re-runs it: 30-move game, 5 s/move, Lazy SMP, no opening book. The engine plays a Nimzo-style line, drops material around move 15, and Black wins - "Black is winning (-9.3, up a rook)" at the 30-move cap. Same frame format as before (566x514, one frame per ply plus a summary), now with mate scores shown as `#N`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Companion to 8d23bfe - the `pruned`-column paragraph had the same wrong description of the node counter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It makes large, cross-cutting changes to core search/evaluation/TT semantics where correctness and strength regressions are possible despite strong unit test additions.
Review details
- Files reviewed: 21/22 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It significantly changes core search, TT semantics, and evaluation logic (including multiprocessing/shared-memory interactions), warranting careful human review despite strong test coverage.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/pychess/constants.py:20
- The inline comment for
MATEis misleading: mate-ness is determined byabs(score) >= MATE_IN_MAX(as described in the module docstring), but theMATEcomment currently mentionsMATE_IN_MAXrather than describingMATEas the sentinel maximum mate score. Clarify the comments so each constant documents its own role.
INF = 99999 # alpha-beta window sentinel
MATE = 9999 # abs(score) at or above MATE_IN_MAX is mate-related
MATE_IN_MAX = 9899 # MATE - 100: abs(score) >= this is a mate (max tree ply << 100)
src/pychess/eval_terms.py:10
- The module docstring says
positional(board)returns a score pair, but the actual public function signature ispositional(board, pawn_mg, pawn_eg). Updating the docstring avoids sending readers to a non-existent API.
"""Positional evaluation terms layered on top of the PeSTO piece-square tables.
``positional(board)`` returns a ``(mg, eg)`` pair from White's point of view, to
be added to the tapered PeSTO score before the side-to-move flip. Everything
here is recomputed from scratch per call; the pure-pawn terms (passed /
isolated / doubled) are the expensive part and are memoised by
``PestoEvaluator`` on the pawn bitboards.
- Files reviewed: 21/22 changed files
- Comments generated: 0 new
- Review effort level: Lite
- I001: blank line between the `import chess` group and the guarded Pillow import (from 7ce9d51), and restore the two blank lines before `_render` (3131686 dropped one). - Revert 3131686's re-addition of "White"/"Black" to `_material_note`: the verdict line it feeds already names the winning side, and the longer string overflowed the frame. The committed GIF was rendered with the short form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The new tools/self_play_gif.py has a fallback font-load path that will raise at runtime (ImageFont.load_default(size)), so the tool can crash on systems without the candidate font files.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tools/self_play_gif.py:48
ImageFont.load_default()does not accept asizeargument (it will raiseTypeError), and it also returns a different font type thantruetype(). This fallback path will currently crash on systems where none of the candidate font files exist.
def _font(size: int) -> ImageFont.FreeTypeFont:
for path in _FONT_CANDIDATES:
if Path(path).exists():
return ImageFont.truetype(path, size)
return ImageFont.load_default(size)
- Files reviewed: 21/22 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
tools/self_play_gif.py uses ImageFont.load_default(size) which will raise at runtime on the fallback path because Pillow’s load_default() does not take a size argument.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
tools/self_play_gif.py:48
ImageFont.load_default()does not accept asizeargument in Pillow, so this fallback path will raise aTypeErroron systems without one of the candidate fonts installed.
return ImageFont.load_default(size)
src/pychess/eval_terms.py:7
- The module docstring says
positional(board)but the actual API requirespawn_mg/pawn_egarguments; this can mislead callers and readers.
``positional(board)`` returns a ``(mg, eg)`` pair from White's point of view, to
be added to the tapered PeSTO score before the side-to-move flip. Everything
here is recomputed from scratch per call; the pure-pawn terms (passed /
isolated / doubled) are the expensive part and are memoised by
``PestoEvaluator`` on the pawn bitboards.
- Files reviewed: 21/22 changed files
- Comments generated: 0 new
- Review effort level: Lite
Implements the remaining
docs/tasks.mdroadmap items and re-derives the strength estimate.Search (
negamax.py)R = 2..3, skipped at the root / in check / in likely zugzwang / below depth 3 / when already tried; verification search at depth ≥ 10.1..3plies shallower (grows with move index & depth, −1 for killers / good history), full-depth re-search on a fail-high. NewMoveOrderer.is_killer/history_scoreaccessors.Draw & mate scoring
0inside the tree (negamax.claims_draw), not just the automatic five-fold / seventy-five-move draws.CONTEMPTconstant is the hook for a non-zero draw score.MATE - ply). The TT rebases them on store/probe (constants.tt_store_score/tt_probe_score) so mate bounds propagate;MATE_GUARDremoved.TranspositionTable/SharedTTprobe/storegain aplyargument.inforeportsscore mate N(signed, in moves) for mate scores.Evaluation (
eval_terms.py, new)Positional terms layered on PeSTO: passed / isolated / doubled pawns, bishop pair, rook on open / half-open file, knight outposts, pawn-shield king safety, tempo. The pure-pawn terms are memoised on the pawn bitboards by
PestoEvaluator. Verified colour-symmetric; PeSTO stays incremental.Docs
performance.mdre-benchmarked (node counts dropped ~20% at depth 8; Lazy SMP now reaches depth 8 in ~2s).engine-strength.mdestimate re-derived: ~1900 → ~2050 (range 1950–2150, still unmeasured).tasks.md— the next five items researched and written (aspiration windows, shallow-depth pruning, SEE, extensions, eval-tuning harness), plus Syzygy /setoptionnotes.CHANGELOG.md,design.md, README,/benchskill updated.Testing
ruff+ruff format+mypyclean; 112 tests pass (85% gate, 94% coverage). New coverage intest_negamax.py,test_eval_terms.py,test_transposition.py,test_uci.py,test_move_ordering.py. Mate-in-2 scoresMATE - 3stably across depths 4–7; 60-ply Lazy SMP self-play runs clean; tactical regression tests intact.Weights are untuned — a Texel pass (roadmap item 5) is the natural follow-up.
🤖 Generated with Claude Code