summaryrefslogtreecommitdiff
path: root/src/root.c
diff options
context:
space:
mode:
authorScott Gasch <[email protected]>2026-09-18 09:43:17 -0700
committerScott Gasch <[email protected]>2026-09-18 09:43:17 -0700
commitf698c02de213d0ed8968fb7cca5f70df51fa0763 (patch)
tree080d6443fbe62d71de092f9acde61da387dff408 /src/root.c
parent4a7ec9ac91939957fb3d3e5564fc24a385b422ff (diff)
Redesign lazy/super-lazy eval margins with position-specific percentile
tables, and fix four real search/recognizer bugs found while validating it Main theme: replace the flat, heavily-padded LAZY_EVAL_BASE_MARGIN / SUPER_LAZY_MARGIN_BY_ARMY constants with per-material-bucket percentile tables (king safety by attacker army, residual by own army, separate tables for the lazy vs super-lazy baselines) derived from a 30k-position calibration pool, targeting a measured ~1-1.5% exceeded-margin rate instead of an unmeasured "add lots of headroom" floor. QSearch's FUTILITY_BASE_MARGIN_LAZY/SUPERLAZY were re-derived to match the new, now-meaningful (p50-based) iPositional this produces. Self-play confirms the working tree beats both HEAD, a more liberal (~5% miss target) variant, and a more conservative (near-zero miss) variant of the same margin mechanism. Along the way, an initial self-play regression (this margin work bundled with several changes in one pass, one of which broke things) led to a day of ZFS-snapshot bisection and then targeted DEBUG-build fuzzing against a stratified 200k-position corpus (tests/cross_sectional_200k.ep_, new), which is what actually found and fixed four distinct, real, pre-existing engine bugs -- none of them related to the margin redesign itself: - searchsup.c (CommonSearchInit): three early-return paths (timer abort, draw, too-deep bailout) left a node's own PV[] row stale instead of clearing it, letting an ancestor's UpdatePV splice a leftover move from a completely unrelated node into the displayed PV -- root cause of a WalkPV/MakeMove assert ("moving a piece to capture, but nothing was on the target square"). - dynamic.c (RecordEnprisePieceAtPly): the two-slot en-prise dedup compared (square, piece) instead of square alone, so a same-square re-record with a different piece (legitimate -- a fail-high capture's victim is restored by UnmakeMove and can differ across calls at the same ply) shifted a duplicate square into both slots, tripping ValueOfMaterialInTroubleDespiteMove's two-distinct-squares assert. - recogn.c (_RecognizeKBKP): the wrong-colored-bishop corner-fortress draw shortcut fired even when the weak side has its own pawn(s), which can hand the strong side real zugzwang resources the fortress logic doesn't account for -- produced a false hard draw bound where EGTB shows a near-forced loss. - eval.c (_EvalPasserRacesAgainstLoneKings): a rook pawn's race was credited as winning even when the strong side's own king sits exactly on its own pawn's queening square (distance 0 specifically -- adjacent is fine, the square is empty) -- a classic stalemate resource the race heuristic can't see. Only reachable through _RecognizeKPK's pawn-hash-cache-hit path, which is why testrecogn.c's TestRecognExhaustiveKPK reported "0 wrong" the whole time; extended it to cover both color assignments and the full board (no mirror-shortcut) and documented the remaining cache-hit coverage gap for a future session to close. Also: match_play.py gains a local_ebf metric (geometric-mean of depth-normalized node-growth ratios between consecutive completed iterations) alongside the existing cumulative nodes**(1/depth) ebf, which turned out to be misleading when comparing engines with different lazy-exit aggressiveness -- and probe.c/hash.c pick up two small, independently-found correctness fixes (TB root-probe promotion piece missing its color bit; ASSERT(g_pHashTable) crashing on the legitimate "--hash none" config) noticed along the way. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Diffstat (limited to 'src/root.c')
-rw-r--r--src/root.c56
1 files changed, 56 insertions, 0 deletions
diff --git a/src/root.c b/src/root.c
index 314395a..74d2327 100644
--- a/src/root.c
+++ b/src/root.c
@@ -1768,6 +1768,62 @@ Return value:
MOVE mvBook;
CHAR *p;
+ //
+ // 2026-09-16: barrier against a real, confirmed race (split.c's
+ // per-split rendezvous only guarantees that *that split's own*
+ // recruited helpers have finished before its slot is recycled --
+ // nothing previously stitched that into a *global* guarantee that
+ // every helper thread anywhere is idle before a brand-new search
+ // begins reusing g_SplitInfo[] slots). Concretely reproduced: a
+ // helper thread recruited by one Think() call's search (e.g. an
+ // engine-startup analysis on the default position) can still be
+ // mid-flight when a *later*, unrelated Think() call starts,
+ // recycles the same slot index, and overwrites its mvPathToHere/
+ // sRootPosition -- the straggler helper then replays a stale move
+ // from the earlier search against the new search's root position
+ // (observed directly: split.c's helper-side SanityCheckMove failed
+ // replaying "Ng1-f3" -- an opening-book-shaped move from the
+ // default starting position -- against an unrelated mid-game FEN).
+ // g_uNumHelpersAvailable/g_uNumHelperThreads are always valid (0/0
+ // in non-MP or single-cpu builds), so this is unconditional and a
+ // no-op there. Deliberately unbounded, not a bounded best-effort
+ // retry like QuitCommand's shutdown wait (command.c) -- silently
+ // proceeding without this invariant is exactly the bug being
+ // fixed, so giving up early here would just reintroduce it; a
+ // warning fires if it's taking unexpectedly long so a genuine
+ // deadlock elsewhere is still observable rather than a silent hang.
+ // g_uNumHelpersAvailable/g_uNumHelperThreads only exist when split.c
+ // is actually compiled in (its whole body is #ifdef MP) -- guard
+ // this the same way, or a non-MP profile (e.g. TEST=1) fails to
+ // link against symbols that were never defined.
+#ifdef MP
+ if (g_uNumHelpersAvailable != g_uNumHelperThreads)
+ {
+ double dWaitStart = SystemTimeStamp();
+ FLAG fWarned = FALSE;
+
+ // Tight busy-spin, no sleep -- this should normally resolve in
+ // microseconds (same idiom as split.c's own per-split
+ // "wait for uNumThreadsHelping" loop), so sleeping even 1ms per
+ // iteration here would add real, gratuitous latency to every
+ // single move on the ordinary (non-racing) path.
+ while (g_uNumHelpersAvailable != g_uNumHelperThreads)
+ {
+ if ((FALSE == fWarned) &&
+ ((SystemTimeStamp() - dWaitStart) > 1.0))
+ {
+ Trace("Think(): still waiting for %lu/%lu helper "
+ "thread(s) to go idle before starting a new "
+ "search (%.1fs so far) -- possible stuck helper?\n",
+ (unsigned long)(g_uNumHelperThreads - g_uNumHelpersAvailable),
+ (unsigned long)g_uNumHelperThreads,
+ SystemTimeStamp() - dWaitStart);
+ fWarned = TRUE;
+ }
+ }
+ }
+#endif
+
InitializeSearcherContext(pos, &ctx);
g_MoveTimer.bvFlags = 0;
g_Options.fPondering = FALSE;