From 538df17868afff51d49927fb5c8a91724752f26c Mon Sep 17 00:00:00 2001 From: Scott Gasch Date: Fri, 28 Aug 2026 08:29:09 -0700 Subject: 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 marker. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01YGSMkwjqiCk4XhbfN7ugD2 --- src/searchsup.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) (limited to 'src/searchsup.c') 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); } } -- cgit v1.3