summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-08-28 08:29:09 -0700
committerScott Gasch <[email protected]>2026-08-28 08:29:09 -0700
commit538df17868afff51d49927fb5c8a91724752f26c (patch)
treea2f945d846fe9978ad64c487f6925df317b9e3b3 /src
parenteaed74ec737e5ac38a6c47d3f3d857d13a3153b3 (diff)
Fix _ShouldWeConsiderThisMove's SEE-purity bug and ComputeMoveScore's
killer-mate edge case; fix PV-display cycle hang. _ShouldWeConsiderThisMove (QSearch's move-consider gate) read the raw, move-ordering-biased mvf[].iValue directly instead of going through ComputeMoveScore, so it inherited the same +120-ish flat bias (plus small MVV-LVA nudges) on winning/even captures that ComputeMoveScore was already fixed to strip out. Fixed via the same MOVE_SCORE_ORDERING_BIAS subtraction, now factored into a shared chess.h macro. Restoring the old effective leniency required an explicit QSEARCH_CONSIDER_MARGIN (120, A/B'd against 0/60/120 on ecm_ringers/confident_quick/hard_quick) rather than assuming the bug's magnitude was itself a meaningful margin -- net effect vs the pre-fix baseline is -2 solves on hard_quick, accepted as the cost of correctness (see lmr_testing/RESULTS.md for the full sweep). ComputeMoveScore separately mishandled quiet killer-mate moves: they can reach SORT_THESE_FIRST via generate.c's killer-mate bonus (unrelated to the capture-bias path), so the bias-subtraction was wrongly applied to a move that never had that bias. Gated the subtraction on IS_CAPTURE_OR_PROMOTION(mv); quiet moves (including killer-mate ones) now correctly collapse to 0, per the function's contract of estimating a move's value on the 100=1-pawn axis. Measured as a no-op on all three suites -- rare in practice, but a real correctness fix. Left a comment documenting two candidate refinements for scoring quiet moves as non-uniform future work, deliberately not implemented (each needs its own isolated test). FinishPVTailFromHash (cosmetic PV-display hash-walk, used only for printing) had no cycle detection, so a drawish/repeating position could spin until the output buffer filled instead of terminating naturally. Added visited-position-signature tracking and a <REP> marker. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YGSMkwjqiCk4XhbfN7ugD2
Diffstat (limited to 'src')
-rwxr-xr-xsrc/chess.h12
-rwxr-xr-xsrc/search.c7
-rw-r--r--src/searchsup.c26
-rwxr-xr-xsrc/util.c28
4 files changed, 70 insertions, 3 deletions
diff --git a/src/chess.h b/src/chess.h
index 630f090..9b6303d 100755
--- a/src/chess.h
+++ b/src/chess.h
@@ -1652,6 +1652,18 @@ typedef struct _PIECE_DATA
extern PIECE_DATA g_PieceData[8];
#define PIECE_VALUE_OVER_100(p) (g_PieceData[PIECE_TYPE(p)].uValueOver100)
+
+// _ScoreAllMoves / _ScoreQSearchMoves{InclChecks,NoChecks} (generate.c) bake
+// a flat +120 move-ordering bias plus small MVV-LVA tie-break nudges into
+// winning/even captures'/promotions' iValue -- harmless for sorting, but
+// wrong once the value is used on the eval axis (compared against
+// material-scale margins). Back it out to recover the pure SEE/
+// material-diff value. ComputeMoveScore() (searchsup.c) already does this;
+// _ShouldWeConsiderThisMove() (search.c) reads iValue raw and needs it too.
+#define MOVE_SCORE_ORDERING_BIAS(mv) \
+ (PIECE_VALUE_OVER_100((mv).pCaptured) + 120 + \
+ PIECE_VALUE_OVER_100((mv).pPromoted) - \
+ PIECE_VALUE_OVER_100((mv).pMoved))
extern ULONG
PieceValueOver100(PIECE p);
diff --git a/src/search.c b/src/search.c
index e0339cf..eb931bc 100755
--- a/src/search.c
+++ b/src/search.c
@@ -792,6 +792,8 @@ Return value:
FALSE if it can be skipped
**/
+#define QSEARCH_CONSIDER_MARGIN (120)
+
static FLAG INLINE
_ShouldWeConsiderThisMove(IN SEARCHER_THREAD_CONTEXT *ctx,
IN ULONG uMoveNum,
@@ -827,6 +829,7 @@ _ShouldWeConsiderThisMove(IN SEARCHER_THREAD_CONTEXT *ctx,
{
i &= STRIP_OFF_FLAGS;
ASSERT(i >= 0);
+ i -= MOVE_SCORE_ORDERING_BIAS(mv);
if (mv.pCaptured)
{
// If there are very few pieces left on the board,
@@ -859,7 +862,7 @@ _ShouldWeConsiderThisMove(IN SEARCHER_THREAD_CONTEXT *ctx,
// penalty can make them look "futile" sometimes.
if ((PIECE_VALUE(mv.pCaptured) ==
PIECE_VALUE(mvLast.pCaptured)) &&
- (i + 200 > iFutility))
+ (i + 200 + QSEARCH_CONSIDER_MARGIN > iFutility))
{
return(TRUE);
}
@@ -867,7 +870,7 @@ _ShouldWeConsiderThisMove(IN SEARCHER_THREAD_CONTEXT *ctx,
// Otherwise, even if a move is even/winning, make sure it
// brings the score up to at least somewhere near alpha.
- if (i > iFutility)
+ if (i + QSEARCH_CONSIDER_MARGIN > iFutility)
{
return(TRUE);
}
diff --git a/src/searchsup.c b/src/searchsup.c
index 8d15e57..585bcc6 100644
--- a/src/searchsup.c
+++ b/src/searchsup.c
@@ -248,7 +248,7 @@ ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
ASSERT(uMoveNum < MAX_MOVE_STACK);
ASSERT(IS_SAME_MOVE(mv, ctx->sMoveStack.mvf[uMoveNum].mv));
iMoveScore = ctx->sMoveStack.mvf[uMoveNum].iValue;
- if (iMoveScore >= SORT_THESE_FIRST)
+ if ((iMoveScore >= SORT_THESE_FIRST) && IS_CAPTURE_OR_PROMOTION(mv))
{
ASSERT(iMoveScore > 0);
iMoveScore &= STRIP_OFF_FLAGS;
@@ -270,6 +270,30 @@ ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
}
else
{
+ // All quiet moves (including killer/killer-mate quiet moves,
+ // which can reach SORT_THESE_FIRST or the killer-tier bits via
+ // generate.c's ordering bonuses with no relation to a move's
+ // real eval-axis value) collapse to 0 here -- correct, since
+ // this function's contract is "estimate the move's value on
+ // the 100=1-pawn axis," and a quiet move's ordering bonus
+ // carries no such estimate.
+ //
+ // FUTURE WORK: 0 is uniform across every quiet move, which
+ // throws away signal we plausibly have. Two candidate
+ // refinements, deliberately not implemented yet (each needs
+ // its own isolated A/B, not bundled together):
+ // 1. A small flat bonus (tested at +10: net wash, moved
+ // solves from confident_quick to hard_quick rather than
+ // a clear win/loss -- see RESULTS.md) for killer/
+ // killer-mate moves, on the theory that a move which
+ // already caused a cutoff elsewhere in the tree is more
+ // likely to be good than an untested quiet shuffle.
+ // 2. eval.c square-delta scoring (e.g. PAWN_CENTRALITY_BONUS/
+ // KNIGHT_CENTRALITY_BONUS[cTo]-[cFrom]) for a directional,
+ // if not strictly accurate, signal on ordinary quiet
+ // moves -- eval.c's real value is far more than per-square
+ // tables (mobility, king safety, pawn structure), so this
+ // would only ever be directionally suggestive, not exact.
iMoveScore = MIN0(iMoveScore);
}
}
diff --git a/src/util.c b/src/util.c
index b69a166..1bcfca9 100755
--- a/src/util.c
+++ b/src/util.c
@@ -991,6 +991,9 @@ FinishPVTailFromHash(SEARCHER_THREAD_CONTEXT *ctx,
MOVE mv;
ULONG uPly = ctx->uPly;
ULONG uLen;
+ ULONG u, uVisited = 0;
+ UINT64 u64Sig;
+ UINT64 u64VisitedSigs[MAX_PLY_PER_SEARCH];
#ifdef DEBUG
POSITION board;
memcpy(&board, &(ctx->sPosition), sizeof(POSITION));
@@ -999,6 +1002,29 @@ FinishPVTailFromHash(SEARCHER_THREAD_CONTEXT *ctx,
if (NULL == g_pHashTable) return;
do
{
+ // This tail is display-only (cosmetic) -- it walks hash-table
+ // best-moves one at a time with no relation to the real,
+ // backed-up search score printed alongside it (that score
+ // covers only the PV up to the <TT> marker). Unlike the real
+ // search, there's no natural depth bound here, so a drawish
+ // position whose hash entries chain back into each other (very
+ // common in king-shuffle endgames) can loop forever. Break on
+ // the first repeated position instead of relying on the output
+ // buffer filling up, which just produces a wall of repeated
+ // moves that looks like -- but has no actual bearing on --
+ // the printed score.
+ u64Sig = (ctx->sPosition.u64NonPawnSig ^ ctx->sPosition.u64PawnSig);
+ for (u = 0; u < uVisited; u++)
+ {
+ if (u64VisitedSigs[u] == u64Sig)
+ {
+ if (uLenRemain > 6) strcat(buf, "<REP>");
+ goto unmake;
+ }
+ }
+ ASSERT(uVisited < MAX_PLY_PER_SEARCH);
+ u64VisitedSigs[uVisited++] = u64Sig;
+
mv = GetPonderMove(&ctx->sPosition);
if (mv.uMove == 0) break;
PV[ctx->uPly] = mv;
@@ -1014,6 +1040,8 @@ FinishPVTailFromHash(SEARCHER_THREAD_CONTEXT *ctx,
}
while(1);
+ unmake:
+
//
// Unmake the moves
//