diff options
| author | Scott Gasch <[email protected]> | 2026-09-18 09:43:17 -0700 |
|---|---|---|
| committer | Scott Gasch <[email protected]> | 2026-09-18 09:43:17 -0700 |
| commit | f698c02de213d0ed8968fb7cca5f70df51fa0763 (patch) | |
| tree | 080d6443fbe62d71de092f9acde61da387dff408 /src/root.c | |
| parent | 4a7ec9ac91939957fb3d3e5564fc24a385b422ff (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.c | 56 |
1 files changed, 56 insertions, 0 deletions
@@ -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; |
