summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-10-15 15:35:54 -0700
committerScott Gasch <[email protected]>2026-10-15 15:35:54 -0700
commit3d55dc94497661b1577109b42a94cc3dba1860fb (patch)
treed0aaefdcc9a5607c7415d21d162e5c3d2216ff9e /src
parent2bf4eb27a473089b9b30726a678fbac27d0f8a69 (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')
-rw-r--r--src/GNUmakefile2
-rwxr-xr-xsrc/chess.h3
-rw-r--r--src/main.c1
-rw-r--r--src/move.c22
-rw-r--r--src/testnnue.c273
5 files changed, 294 insertions, 7 deletions
diff --git a/src/GNUmakefile b/src/GNUmakefile
index 3cfee6c..a66d059 100644
--- a/src/GNUmakefile
+++ b/src/GNUmakefile
@@ -353,7 +353,7 @@ ifdef TEST
OBJS += testdraw.o testmove.o testfen.o testgenerate.o \
testsan.o testics.o testeval.o testbitboard.o \
testhash.o testsee.o testsearch.o testsup.o \
- testrecogn.o
+ testrecogn.o testnnue.o
PROFILE += -DTEST
else
ifdef EVAL_DUMP
diff --git a/src/chess.h b/src/chess.h
index d0cbd34..1f67df1 100755
--- a/src/chess.h
+++ b/src/chess.h
@@ -3363,6 +3363,9 @@ void
TestLegalMoveGenerator(void);
void
+TestNnueAccumulator(void);
+
+void
TestGenerateKnightSpeed(void);
void
diff --git a/src/main.c b/src/main.c
index ad954ad..ac2ebf6 100644
--- a/src/main.c
+++ b/src/main.c
@@ -659,6 +659,7 @@ Return value:
TestWhoAttacksSquareMagic();
TestMoveGenerator();
TestLegalMoveGenerator();
+ TestNnueAccumulator();
TestGenerateKnightSpeed();
TestGenerateKingSpeed();
TestGenerateRookSpeed();
diff --git a/src/move.c b/src/move.c
index c7b659a..0b6f578 100644
--- a/src/move.c
+++ b/src/move.c
@@ -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);
}
diff --git a/src/testnnue.c b/src/testnnue.c
new file mode 100644
index 0000000..9715747
--- /dev/null
+++ b/src/testnnue.c
@@ -0,0 +1,273 @@
+/**
+
+Copyright (c) Scott Gasch
+
+Module Name:
+
+ testnnue.c
+
+Abstract:
+
+ Correctness test for the NNUE accumulator's incremental
+ maintenance (move.c's Lift/Place/Slide(WithoutSigs) hooks, plus
+ MakeMove/UnmakeMove's king-move backup/recompute wrapper). Proves
+ the one property nothing else checks: that the incrementally-
+ maintained SEARCHER_THREAD_CONTEXT.sNnueAccum[2] always equals a
+ fresh NnueAccumRecompute from scratch, through real move
+ sequences -- captures, promotions, en passant, castling, plain
+ king moves, and nested nesting of all of the above two plies deep
+ (specifically to exercise PLY_INFO.sNnueAccumBackup's per-ply
+ indexing: two king moves at different plies, not yet unmade,
+ must not collide).
+
+ Uses a deliberately NON-zero, distinctive weight table for the
+ duration of this test -- the real embedding table
+ (g_rgfNnueWeight/g_rgfNnueBias) is still an all-zero placeholder
+ (no trained weights loaded yet), and an all-zero table would let
+ a broken feature-index formula "pass" trivially (every row sums
+ to zero regardless of which squares/pieces are involved, so a
+ wrong index looks identical to a right one). Restored to zero
+ afterward so this test has no side effect on anything that runs
+ after it in the same process.
+
+Author:
+
+ Scott Gasch ([email protected]) 15 Oct 2026
+
+**/
+
+#ifdef TEST
+#include "chess.h"
+
+static void
+_FillTestNnueWeights(void)
+/**
+
+Routine description:
+
+ Overwrites g_rgfNnueWeight/g_rgfNnueBias with distinctive,
+ deterministic (not all-equal, not all-zero) values so a wrong
+ feature index produces a detectably different accumulator sum
+ rather than trivially matching.
+
+**/
+{
+ ULONG i, j;
+
+ for (i = 0; i < NNUE_DIMENSIONS; i++)
+ {
+ for (j = 0; j < NNUE_ACCUMULATOR_SIZE; j++)
+ {
+ g_rgfNnueWeight[i][j] =
+ ((float)(i % 97) * 0.1f) + ((float)(j % 13) * 0.01f) + 1.0f;
+ }
+ }
+ for (j = 0; j < NNUE_ACCUMULATOR_SIZE; j++)
+ {
+ g_rgfNnueBias[j] = (float)(j % 7) * 0.5f;
+ }
+}
+
+
+static void
+_ZeroTestNnueWeights(void)
+/**
+
+Routine description:
+
+ Restores the placeholder all-zero state so this test has no
+ lingering effect on anything run afterward in the same process.
+
+**/
+{
+ memset(g_rgfNnueWeight, 0, sizeof(g_rgfNnueWeight));
+ memset(g_rgfNnueBias, 0, sizeof(g_rgfNnueBias));
+}
+
+
+static void
+_CompareAccumToRecompute(SEARCHER_THREAD_CONTEXT *ctx, MOVE mv, CHAR *szLabel)
+/**
+
+Routine description:
+
+ For both colors, recomputes a fresh accumulator from scratch
+ against ctx->sPosition and compares it element-by-element against
+ the incrementally-maintained ctx->sNnueAccum. Panics (via
+ UtilPanic, same convention as every other self-test) on any
+ mismatch outside a small float-rounding tolerance -- real
+ indexing/logic bugs produce large deltas (a whole missing or
+ extra ~1-100-magnitude weight row), not rounding noise, so the
+ tolerance only needs to be loose enough to absorb summation-order
+ differences between the two paths, not loose enough to hide a
+ real bug.
+
+Parameters:
+
+ SEARCHER_THREAD_CONTEXT *ctx
+ CHAR *szLabel : identifies which step of the test this is, for
+ the panic message if one fires
+
+Return value:
+
+ void
+
+**/
+{
+ static const float EPSILON = 0.01f;
+ NNUE_ACCUMULATOR sFresh;
+ ULONG uColor;
+ ULONG i;
+
+ for (uColor = 0; uColor < 2; uColor++)
+ {
+ NnueAccumRecompute(&sFresh, &ctx->sPosition, uColor);
+ for (i = 0; i < NNUE_ACCUMULATOR_SIZE; i++)
+ {
+ float fDelta = sFresh.rgfValue[i] - ctx->sNnueAccum[uColor].rgfValue[i];
+ if ((fDelta > EPSILON) || (fDelta < -EPSILON))
+ {
+ Trace("TestNnueAccumulator: MISMATCH at %s, color=%lu, "
+ "index=%lu: incremental=%f recompute=%f "
+ "mv.uMove=0x%lx cFrom=0x%x cTo=0x%x pMoved=%u "
+ "pCaptured=%u pPromoted=%u bvFlags=0x%x FEN=%s\n",
+ szLabel, uColor, i,
+ ctx->sNnueAccum[uColor].rgfValue[i],
+ sFresh.rgfValue[i],
+ mv.uMove, mv.cFrom, mv.cTo, mv.pMoved,
+ mv.pCaptured, mv.pPromoted, mv.bvFlags,
+ PositionToFen(&ctx->sPosition));
+ UtilPanic(TESTCASE_FAILURE, NULL, "TestNnueAccumulator",
+ NULL, NULL, __FILE__, __LINE__);
+ }
+ }
+ }
+}
+
+
+static void
+_PlyTestNnue(SEARCHER_THREAD_CONTEXT *ctx, ULONG uDepth)
+/**
+
+Routine description:
+
+ Recursively tries every legal move at the current position to
+ uDepth plies, comparing the accumulator against a fresh recompute
+ after every MakeMove and every UnmakeMove. Two-plus plies of
+ recursion is specifically what exercises PLY_INFO.sNnueAccumBackup's
+ per-ply indexing (two king moves at different, not-yet-unmade
+ plies must not collide) -- a single-ply test could not catch that
+ class of bug.
+
+Parameters:
+
+ SEARCHER_THREAD_CONTEXT *ctx
+ ULONG uDepth : plies left to recurse
+
+Return value:
+
+ void
+
+**/
+{
+ ULONG uPly = ctx->uPly;
+ MOVE mv;
+ ULONG u;
+ FLAG fInCheck = InCheck(&ctx->sPosition, ctx->sPosition.uToMove);
+
+ if (uDepth == 0)
+ {
+ return;
+ }
+
+ mv.uMove = 0;
+ GenerateMoves(ctx, mv, (fInCheck ? GENERATE_ESCAPES : GENERATE_ALL_MOVES));
+ for (u = ctx->sMoveStack.uBegin[uPly];
+ u < ctx->sMoveStack.uEnd[uPly];
+ u++)
+ {
+ mv = ctx->sMoveStack.mvf[u].mv;
+ if (MakeMove(ctx, mv))
+ {
+ _CompareAccumToRecompute(ctx, mv, "after MakeMove");
+ _PlyTestNnue(ctx, uDepth - 1);
+ UnmakeMove(ctx, mv);
+ _CompareAccumToRecompute(ctx, mv, "after UnmakeMove");
+ }
+ }
+}
+
+
+void
+TestNnueAccumulator(void)
+/**
+
+Routine description:
+
+ Entry point -- see this file's module comment.
+
+Parameters:
+
+ void
+
+Return value:
+
+ void
+
+**/
+{
+ // Curated to reach captures, promotions, en passant, castling
+ // (both sides, both directions), and plain king moves within the
+ // first couple of plies -- not just quiet middlegame shuffling,
+ // which would never exercise the king-move backup/recompute path
+ // this test exists to check.
+ static const char *rgszFen[] =
+ {
+ "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1",
+ "r3k2r/p1ppqpb1/bn2pnp1/3PN3/1p2P3/2N2Q1p/PPPBBPPP/R3K2R w KQkq - 0 1",
+ "r3k2r/p1ppqpb1/bn2pnp1/3PN3/1p2P3/2N2Q1p/PPPBBPPP/R3K2R b KQkq - 0 1",
+ "rnbqkbnr/ppp1p1pp/8/3pPp2/8/8/PPPP1PPP/RNBQKBNR w KQkq f6 0 3",
+ "8/PPP4k/8/8/8/8/4Kppp/8 w - - 0 1",
+ "8/PPP4k/8/8/8/8/4Kppp/8 b - - 0 1",
+ "4k3/8/8/8/8/8/8/4K3 w - - 0 1",
+ };
+ SEARCHER_THREAD_CONTEXT *ctx;
+ POSITION pos;
+ ULONG u;
+
+ Trace("Testing NNUE accumulator incremental maintenance...\n");
+ _FillTestNnueWeights();
+
+ ctx = SystemAllocateMemory(sizeof(SEARCHER_THREAD_CONTEXT));
+ ASSERT(ctx);
+
+ for (u = 0; u < ARRAY_LENGTH(rgszFen); u++)
+ {
+ if (FALSE == FenToPosition(&pos, (char *)rgszFen[u]))
+ {
+ UtilPanic(TESTCASE_FAILURE, NULL, "FenToPosition", NULL, NULL,
+ __FILE__, __LINE__);
+ }
+ InitializeSearcherContext(&pos, ctx);
+
+ // This test's own responsibility, not anything move.c does
+ // automatically: establish the correct starting accumulator
+ // for this fresh position before any incremental maintenance
+ // can be trusted to keep it that way.
+ NnueAccumRecompute(&ctx->sNnueAccum[WHITE], &ctx->sPosition, WHITE);
+ NnueAccumRecompute(&ctx->sNnueAccum[BLACK], &ctx->sPosition, BLACK);
+ {
+ MOVE mvNone;
+ mvNone.uMove = 0;
+ _CompareAccumToRecompute(ctx, mvNone, "initial setup");
+ }
+
+ _PlyTestNnue(ctx, 2);
+ }
+
+ SystemFreeMemory(ctx);
+ _ZeroTestNnueWeights();
+ Trace(" NNUE accumulator test passed.\n");
+}
+
+#endif // TEST