summaryrefslogtreecommitdiff
path: root/src/searchsup.c
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-08-28 11:10:14 -0700
committerScott Gasch <[email protected]>2026-08-28 11:10:14 -0700
commit1a0712fee7e3fa9bcf124a942d8ef15efe4578e7 (patch)
tree29973e7deb28d8ddca12d0aff5dff5381be489f5 /src/searchsup.c
parent538df17868afff51d49927fb5c8a91724752f26c (diff)
Fix IID's killer-blind gate and its ordering-flag/history contamination
of iValue; harden against a latent ComputeMoveExtension bug. DO_IID's "is the top move crappy" gate compared raw iValue against SORT_THESE_FIRST only, missing that ordinary killer moves (FIRST_KILLER through FOURTH_KILLER) sit below that threshold too -- a killer that already proved itself elsewhere in the tree was being treated as "crappy" and triggering an unnecessary shallow rescore. Fixed by also excluding killer-flagged moves from the gate. RescoreMovesViaSearch corrupted the winning move's real search score by OR-ing in SORT_THESE_FIRST to force it to sort first (`mvf[uBest].iValue |= SORT_THESE_FIRST`) -- unnecessary (SelectBest{With,No}History already find the true max by plain magnitude comparison, no flag needed) and actively dangerous: a later ComputeMoveScore() call on that same move, if it's a capture, would see the corrupted value, mistake it for generate.c's biased-capture-ordering format, and subtract the wrong bias entirely. Removed the OR; added an explicit PLY_INFO.fMovesRescoredByIID flag so ComputeMoveScore and the main search-loop's move-selection call can both recognize "this ply's iValue holds a real eval-axis score" without relying on bit-pattern inference. Consequently, ComputeMoveScore now trusts an IID-rescored move's score outright instead of running it through the capture-bias-subtraction or quiet-move-collapse-to-0 logic (both of which assume generate.c's ordering encoding, which a rescored ply no longer holds). Separately hardened it against quiet killer-mate moves, which can reach SORT_THESE_FIRST via a different, capture-unrelated path and were incorrectly getting the capture bias subtracted from them; they now correctly collapse to 0 like other quiet moves. Two follow-on ideas -- blending history into the real IID score (scaled or capped) and a exact-tie-only history tiebreak -- were implemented, measured, and rejected: blending invents a new, leak-prone move-scoring axis for no measured benefit, and the tiebreak-only compromise still cost solves relative to just trusting the real score outright. Main search's move-selection call now branches once per selection (not once per candidate move) between SelectBestNoHistory (IID-rescored plies) and SelectBestWithHistory (everyone else), keeping the overwhelmingly common non-rescored path at zero added cost. Net measured effect (ecm_ringers.ep_/ecm_confident_quick.ep_/ ecm_hard_quick.ep_, sn=5M): 10/90/9, down from a pre-existing 11/88/10 on ringers and hard specifically -- see lmr_testing/RESULTS.md for the full sweep of rejected alternatives and why the regression was accepted as the cost of removing a latent, leak-prone bug class rather than chasing the exact prior numbers. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YGSMkwjqiCk4XhbfN7ugD2
Diffstat (limited to 'src/searchsup.c')
-rw-r--r--src/searchsup.c31
1 files changed, 28 insertions, 3 deletions
diff --git a/src/searchsup.c b/src/searchsup.c
index 585bcc6..efdde67 100644
--- a/src/searchsup.c
+++ b/src/searchsup.c
@@ -239,7 +239,7 @@ Return value:
SCORE
ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
IN MOVE mv,
- IN ULONG uMoveNum)
+ IN ULONG uMoveNum)
{
SCORE iMoveScore = (PIECE_VALUE(mv.pCaptured) +
PIECE_VALUE(mv.pPromoted));
@@ -247,8 +247,25 @@ ComputeMoveScore(IN SEARCHER_THREAD_CONTEXT *ctx,
{
ASSERT(uMoveNum < MAX_MOVE_STACK);
ASSERT(IS_SAME_MOVE(mv, ctx->sMoveStack.mvf[uMoveNum].mv));
+ ASSERT(ctx->uPly > 0);
iMoveScore = ctx->sMoveStack.mvf[uMoveNum].iValue;
- if ((iMoveScore >= SORT_THESE_FIRST) && IS_CAPTURE_OR_PROMOTION(mv))
+ // Every current caller (search.c's EFP check, and
+ // ComputeMoveExtension's two ComputeMoveScore() call sites) only
+ // reaches here after MakeMove(ctx, mv) has already succeeded, so
+ // ctx->uPly is always the *child's* ply here -- uMoveNum indexes
+ // the parent's move list, i.e. ctx->uPly - 1, not ctx->uPly. That's
+ // the ply RescoreMovesViaSearch (if it ran) would have rescored,
+ // so that's the flag to check.
+ if (TRUE == ctx->sPlyInfo[ctx->uPly - 1].fMovesRescoredByIID)
+ {
+ // RescoreMovesViaSearch already put a real, searched eval-axis
+ // score here -- better than SEE/MVV-LVA, since it reflects an
+ // entire subtree, not just the immediate exchange. Trust it
+ // exactly as-is; don't run it through the capture-bias
+ // subtraction or collapse it via MIN0, both of which assume
+ // generate.c's ordering-encoded format, which this isn't.
+ }
+ else if ((iMoveScore >= SORT_THESE_FIRST) && IS_CAPTURE_OR_PROMOTION(mv))
{
ASSERT(iMoveScore > 0);
iMoveScore &= STRIP_OFF_FLAGS;
@@ -668,7 +685,15 @@ Return value:
ctx->sMoveStack.mvf[x].iValue = -INFINITY;
x++;
}
- ctx->sMoveStack.mvf[uBest].iValue |= SORT_THESE_FIRST;
+ // uBest already holds the largest real score in the list (iBestScore
+ // tracked the running max as we went) -- SelectBest{With,No}History
+ // just compare raw magnitude, so it naturally sorts first without
+ // needing a flag. OR-ing in SORT_THESE_FIRST here used to corrupt
+ // that real score into looking like generate.c's biased-capture-
+ // ordering format to any later ComputeMoveScore() caller; removed.
+ // fMovesRescoredByIID (below) is the correct, non-destructive way to
+ // signal "trust this ply's iValue as a real eval-axis score."
+ ctx->sPlyInfo[ctx->uPly].fMovesRescoredByIID = TRUE;
ASSERT(IS_VALID_SCORE(iBestScore));
return(iBestScore);
}