diff options
| author | Scott Gasch <[email protected]> | 2026-10-15 15:35:54 -0700 |
|---|---|---|
| committer | Scott Gasch <[email protected]> | 2026-10-15 15:35:54 -0700 |
| commit | 3d55dc94497661b1577109b42a94cc3dba1860fb (patch) | |
| tree | d0aaefdcc9a5607c7415d21d162e5c3d2216ff9e /src/move.c | |
| parent | 2bf4eb27a473089b9b30726a678fbac27d0f8a69 (diff) | |
NNUE accumulator correctness test; fix king-capture backup-ordering bug
testnnue.c (new, TEST=1): TestNnueAccumulator proves the one property
nothing else checked -- that SEARCHER_THREAD_CONTEXT.sNnueAccum stays
equal to a from-scratch NnueAccumRecompute through real move
sequences. Fills the (otherwise all-zero placeholder) embedding table
with distinctive, deterministic values first: an all-zero table would
let a broken feature-index formula "pass" trivially, since every row
sums to zero regardless of which squares/pieces are involved. Plays
real games 2 plies deep from curated FENs chosen to reach captures,
promotions, en passant, castling, and plain king moves quickly, and
compares after every MakeMove and every UnmakeMove -- 2+ plies of
recursion is what exercises PLY_INFO.sNnueAccumBackup's per-ply
indexing (two king moves at different, not-yet-unmade plies must not
collide), which a single-ply test could not have caught.
It caught a real bug on its very first run: move.c's MakeMove backed
up the king's own-perspective accumulator *after* a capture's
LiftPiece call instead of before it. For a king capturing a piece
(e.g. Kxd7), LiftPiece already removes the captured piece's feature
from both perspectives -- including the king's own -- before the
backup snapshot was taken, so the backup captured a state already
missing the captured piece. UnmakeMove correctly resurrects the
captured piece via PlacePieceWithoutSigs, but then immediately
overwrote that correct state with the stale backup. Fixed by moving
the backup to the top of the ordinary (non-castling) king-move
handling, before any mutation for that move happens at all -- the
castling branch was already backing up before its own moves and
needed no change.
Verified via the full precommit_check.sh (all 4 gates, including the
combined DEBUG=1 TEST=1 self-test, which now runs this test too).
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Diffstat (limited to 'src/move.c')
| -rw-r--r-- | src/move.c | 22 |
1 files changed, 16 insertions, 6 deletions
@@ -980,6 +980,22 @@ Return value: // Normal move or normal capture // pos->uFifty++; + if (IS_KING(pPiece)) + { + // Back up BEFORE *anything* about this move touches the + // board/accumulator -- including the capture lift just + // below, which (for a king capture) would otherwise already + // have removed the captured piece's feature from uColor's + // own perspective before this snapshot is taken, leaving the + // backup missing it. Found exactly this way: testnnue.c's + // TestNnueAccumulator caught a king-captures-knight sequence + // where the restored accumulator came back short by one + // feature row because of this ordering. + ctx->sPlyInfo[ctx->uPly].sNnueAccumBackup = ctx->sNnueAccum[uColor]; +#ifdef DEBUG + ctx->sPlyInfo[ctx->uPly].uNnueAccumBackupColor = uColor; +#endif + } if (pCaptured) { pos->uFifty = 0; @@ -995,12 +1011,6 @@ Return value: } else if (IS_KING(pPiece)) { - // Ordinary (non-castling) king step -- same backup/recompute - // wrapper as the castling branch above, see its comment. - ctx->sPlyInfo[ctx->uPly].sNnueAccumBackup = ctx->sNnueAccum[uColor]; -#ifdef DEBUG - ctx->sPlyInfo[ctx->uPly].uNnueAccumBackupColor = uColor; -#endif SlidePiece(pos, mv.cFrom, mv.cTo, ctx->sNnueAccum); NnueAccumRecompute(&ctx->sNnueAccum[uColor], pos, uColor); } |
