Restore 5D-alpha + 4D-beta FH% collection (improve axis + whole-node bucket), fix two bugs found via validation

Restores the full dual-table FH% collection instrumentation under
FH_TABLE_COLLECTION, which had regressed to a 4D-only alpha joint with
no beta side at all (see the prior commit's note): adds back the
`improve` axis to the alpha joint and a new 4D beta joint (depth,
gap-to-beta, own-improve, enemy-improve), both with real INC() call
sites and ALPHARAW/BETARAW/ESTRAW dump lines matching dual_table_run/'s
existing (unmodified) aggregate.py/dashboard.py/marginal_crosscheck.py
parsers exactly.

Also adds a genuine NODE-level sample into the alpha table's previously
unused late==0 bucket: "will something at this node fail high" when
iEval <= iAlpha, ground truth = the real move loop's outcome. This is
the alpha-side analog of the beta table (which only covers iEval >
iBeta) -- together they cover every null-window child's self-estimate
with no gap, motivating a follow-up (not part of this change) on
combining a parent's per-move FH% prediction with a child's own
node-level self-estimate for a sharper NODE_CUT/NODE_ALL signal than
CommonSearchInit's current purely-structural PVS alternation.

Collecting real ground truth for both new node-level samples requires
not taking the beta-side static-null-move shortcut's early `goto end`
during collection builds, same reasoning as the existing LMP disable.

Two bugs found and fixed via validation before trusting this for a
multi-day run:

1. goto-skips-initializer: Search()'s early QSearch-depth `goto end`
   jumped over the new collection variables' declarations, leaving
   them uninitialized (indeterminate stack garbage) on that path. Only
   manifested in the release (-O3) build -- a DEBUG (-O0) build worked
   by accident (the skipped stack slots happened to read as zero).
   Caught via lldb on a real segfault: garbage "eligible" flags feeding
   wildly out-of-range array indices. Fixed by moving the declarations
   before the early goto.

2. MAX_SPLITS amplification: the new arrays were nested inside
   COUNTERS.tree, which is embedded both in SEARCHER_THREAD_CONTEXT and
   in split.c's g_SplitInfo[MAX_SPLITS] array -- so the ~19MB growth
   got replicated 64x, inflating the binary's BSS from ~220MB to
   ~1.47GB and breaking valgrind's fixed-address mmap outright (harmless
   at runtime for a --cpus 1 collection shard, since g_SplitInfo's
   copies are never touched, but it also meant any real MP split would
   have silently lost a helper thread's collected data, since no
   merge-back code exists for these fields). Fixed by moving them into
   a new standalone FH_COLLECTION_COUNTERS struct referenced only from
   SEARCHER_THREAD_CONTEXT, not from SPLIT_INFO. BSS dropped to ~281MB;
   valgrind (--track-origins=yes) now runs clean: 0 errors, 0 bytes
   definitely/indirectly/possibly lost.

Validated end-to-end against real NNUE weights (--nnueweights
eval.nnue), release build, full tests/ecm_ringers.ep_ suite at sd 10:
ALPHARAW/BETARAW/ESTRAW all populated (including real late==0 "whole
node" rows), dual_table_run/'s existing aggregate.py and
marginal_crosscheck.py parse it unmodified -- depth/gap/improve axes
match exactly; the only two "mismatches" (dumb and late bucket 0) are
exactly and fully explained by the new whole-node samples, which
deliberately don't feed those two per-move marginals (hang=0/late=0
are placeholders there, not real measurements) -- not a real bug, and
reproduced byte-for-byte identical after the struct relocation above.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

1 files changed, 110 insertions, 2 deletions

diff --git a/src/search.c b/src/search.c
index 7d8d686..6f5e90c 100644
--- a/src/search.c
+++ b/src/search.c
@@ -374,6 +374,24 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
     // same QUARTER_PLY off the per-iteration depth budget so this doesn't
     // just add a blanket 1/4 ply to every search.
     SCORE iBestScore;
+#ifdef FH_TABLE_COLLECTION
+    // Declared here, BEFORE the QSearch-jump `goto end;` just below --
+    // these must be initialized on every path reaching end:, including
+    // the early QSearch exit, or the commit code there reads
+    // indeterminate (garbage) values. Declaring them further down
+    // (alongside the other per-node locals) put their initializers
+    // after this goto, which skips them entirely on the QSearch-exit
+    // path -- caught live via a release-build (-O3) segfault that
+    // didn't reproduce in DEBUG (-O0, where the skipped stack slots
+    // happened to already read as zero): fBetaCollectEligible/
+    // fAlpha0CollectEligible read back as nonzero garbage, feeding
+    // equally-garbage bucket indices into the array indexing at end:.
+    FLAG fBetaCollectEligible = FALSE;
+    ULONG uBetaCollectDepthB = 0, uBetaCollectGapB = 0,
+          uBetaCollectOwnImpB = 0, uBetaCollectEnemyImpB = 0;
+    FLAG fAlpha0CollectEligible = FALSE;
+    ULONG uAlpha0CollectDepthB = 0, uAlpha0CollectGapB = 0;
+#endif
     if (uDepth < THREE_QUARTERS_PLY)
     {
         pf->fCouldStandPat[BLACK] = pf->fCouldStandPat[WHITE] = FALSE;
@@ -708,6 +726,27 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
             ULONG uRawEnemyImproveB = ImproveBucket(iEnemyImprovement);
 
             INC(ctx->sCounters.tree.u64QuickNullMoves);
+#ifdef FH_TABLE_COLLECTION
+            // Collection builds record a genuine whole-node try here
+            // (ground truth committed at end:) and must NOT take the
+            // shortcut below -- it would skip the real move loop
+            // based on the CURRENTLY LOADED (soon-to-be-replaced)
+            // data.c table, censoring exactly the population this
+            // collection is trying to measure. Same reasoning as the
+            // LMP disable in searchsup.c.
+            fBetaCollectEligible = TRUE;
+            uBetaCollectDepthB = uStaticNMDepthB;
+            uBetaCollectGapB = GapBetaBucket((SCORE)(iEval - iBeta));
+            uBetaCollectOwnImpB = uRawImproveB;
+            uBetaCollectEnemyImpB = uRawEnemyImproveB;
+            INC(ctx->sFhCollection.u64BetaDepthEstimateTries[uBetaCollectDepthB]);
+            INC(ctx->sFhCollection.u64BetaGapEstimateTries[uBetaCollectGapB]);
+            INC(ctx->sFhCollection.u64BetaOwnImpEstimateTries[uBetaCollectOwnImpB]);
+            INC(ctx->sFhCollection.u64BetaEnemyImpEstimateTries[uBetaCollectEnemyImpB]);
+            INC(ctx->sFhCollection.u64BetaTries
+                [uBetaCollectDepthB][uBetaCollectGapB]
+                [uBetaCollectOwnImpB][uBetaCollectEnemyImpB]);
+#else
             ULONG uBetaPredictedFHThousandths = _BetaTablePredictedFHThousandths(
                 uStaticNMDepthB,
             GapBetaBucket((SCORE)(iEval - iBeta)),
@@ -723,12 +762,37 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                 iBestScore = iBeta;
                 goto end;
             }
+#endif
         }
-#if (NULLMOVE_SKIP_FH_CEILING_THOUSANDTHS > 0)
         else if (iEval <= iAlpha)
         {
             ULONG uNullSkipDepthB = DepthBucket(uDepth / ONE_PLY);
             ULONG uRawGapAlphaB = GapCpBucket((SCORE)(iAlpha - iEval));
+#ifdef FH_TABLE_COLLECTION
+            // Whole-node sample for the alpha table's reserved
+            // late==0 bucket (see chess.h) -- the alpha-side analog
+            // of the beta-side whole-node try just above, covering
+            // the iEval <= iAlpha regime the beta table doesn't.
+            // hang is pinned to 0 (no specific move; "nothing hung"
+            // is the only sensible placeholder) and late is pinned to
+            // 0 (the reserved "whole node" sentinel, never produced
+            // by a real per-move sample) -- deliberately NOT fed into
+            // the shared per-move hang/late marginals below (those
+            // values are placeholders here, not real measurements),
+            // but depth/gap/improve ARE real, equally-meaningful
+            // quantities for a whole-node sample, so they DO feed the
+            // shared marginals.
+            fAlpha0CollectEligible = TRUE;
+            uAlpha0CollectDepthB = uNullSkipDepthB;
+            uAlpha0CollectGapB = uRawGapAlphaB;
+            INC(ctx->sCounters.tree.u64DepthEstimateTries[uAlpha0CollectDepthB]);
+            INC(ctx->sCounters.tree.u64GapEstimateTries[uAlpha0CollectGapB]);
+            INC(ctx->sFhCollection.u64ImproveEstimateTries[uRawImproveB]);
+            INC(ctx->sFhCollection.u64Alpha5Tries
+                [uAlpha0CollectDepthB][0][uAlpha0CollectGapB][0][uRawImproveB]);
+#endif
+#if (NULLMOVE_SKIP_FH_CEILING_THOUSANDTHS > 0)
+            {
             USHORT uAlphaPredictedFH = _JointTablePredictedFHThousandths(
                 uNullSkipDepthB,
             0,                  // hang bucket: assume nothing hung
@@ -741,8 +805,9 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                 fSkipNullForLowFH = TRUE;
                 INC(ctx->sCounters.tree.u64NullMoveSkippedLowFH);
             }
-        }
+            }
 #endif
+        }
         if ((FALSE == fSkipNullForLowFH) &&
             WeShouldTryNullmovePruning(ctx,
                                        iAlpha,
@@ -1138,6 +1203,12 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
                             INC(ctx->sCounters.tree.u64JointFH
                                 [uThisMoveDepthBucket][uThisMoveHangBucket]
                                 [uThisMoveGapBucket][uThisMoveNumBucket]);
+#ifdef FH_TABLE_COLLECTION
+                            INC(ctx->sFhCollection.u64ImproveEstimateFH[uRawImproveB]);
+                            INC(ctx->sFhCollection.u64Alpha5FH
+                                [uThisMoveDepthBucket][uThisMoveHangBucket]
+                                [uThisMoveGapBucket][uThisMoveNumBucket][uRawImproveB]);
+#endif
                         }
                     }
 #endif // PERF_COUNTERS
@@ -1271,6 +1342,43 @@ Search(IN SEARCHER_THREAD_CONTEXT *ctx,
     ASSERT(IS_VALID_SCORE(iAlpha));
     ASSERT(IS_VALID_SCORE(iBestScore) || WE_SHOULD_STOP_SEARCHING);
     ASSERT(PositionsAreEquivalent(pos, &pi->sPosition));
+#ifdef FH_TABLE_COLLECTION
+    // Commit the whole-node ground truth stashed above, now that
+    // iBestScore's real fate against iBeta is known -- same two-phase
+    // pattern as TEST_NULL's commit just below. Skipped on an
+    // aborted/incomplete search (WE_SHOULD_STOP_SEARCHING): iBestScore
+    // may not reflect a fully-searched node in that case, and counting
+    // it as ground truth would bias the table toward whatever got
+    // searched before the clock/node-limit fired.
+    if ((!WE_SHOULD_STOP_SEARCHING) && IS_VALID_SCORE(iBestScore))
+    {
+        FLAG fRealFH = (iBestScore >= iBeta);
+        if (fBetaCollectEligible)
+        {
+            if (fRealFH)
+            {
+                INC(ctx->sFhCollection.u64BetaDepthEstimateFH[uBetaCollectDepthB]);
+                INC(ctx->sFhCollection.u64BetaGapEstimateFH[uBetaCollectGapB]);
+                INC(ctx->sFhCollection.u64BetaOwnImpEstimateFH[uBetaCollectOwnImpB]);
+                INC(ctx->sFhCollection.u64BetaEnemyImpEstimateFH[uBetaCollectEnemyImpB]);
+                INC(ctx->sFhCollection.u64BetaFH
+                    [uBetaCollectDepthB][uBetaCollectGapB]
+                    [uBetaCollectOwnImpB][uBetaCollectEnemyImpB]);
+            }
+        }
+        if (fAlpha0CollectEligible)
+        {
+            if (fRealFH)
+            {
+                INC(ctx->sCounters.tree.u64DepthEstimateFH[uAlpha0CollectDepthB]);
+                INC(ctx->sCounters.tree.u64GapEstimateFH[uAlpha0CollectGapB]);
+                INC(ctx->sFhCollection.u64ImproveEstimateFH[uRawImproveB]);
+                INC(ctx->sFhCollection.u64Alpha5FH
+                    [uAlpha0CollectDepthB][0][uAlpha0CollectGapB][0][uRawImproveB]);
+            }
+        }
+    }
+#endif // FH_TABLE_COLLECTION
 #ifdef TEST_NULL
     // Commit the calibration record stashed at the diagnostic probe
     // site above, now that iBestScore's fate against iBeta (this