diff options
| author | Scott Gasch <[email protected]> | 2026-08-28 08:29:09 -0700 |
|---|---|---|
| committer | Scott Gasch <[email protected]> | 2026-08-28 08:29:09 -0700 |
| commit | 538df17868afff51d49927fb5c8a91724752f26c (patch) | |
| tree | a2f945d846fe9978ad64c487f6925df317b9e3b3 /src | |
| parent | eaed74ec737e5ac38a6c47d3f3d857d13a3153b3 (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-x | src/chess.h | 12 | ||||
| -rwxr-xr-x | src/search.c | 7 | ||||
| -rw-r--r-- | src/searchsup.c | 26 | ||||
| -rwxr-xr-x | src/util.c | 28 |
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); } } @@ -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 // |
