refactor(search): Hold a node's facts and its answer in one value and ask for a child search with the loop's decision - #369
Merged
Merged
Conversation
… reductions read A full width node's state was held three times. `NodeFacts` had the seven facts the census and the effort instrument record. `NodeAnswer` had the bounds as they stood, the opening alpha, the root bounds, the fail soft best, the taint and the searched count. `late_move::Node` repeated six of the seven facts beside its three memos and the two rule halves. The loop kept the copies in step by handing the late move node alpha after every rise, and only debug assertions checked that it had. Now one `Node` in engine.rs holds the facts and the answer, and alpha lives there alone. `open`, `absorb` and `raised_alpha` are its methods. The late move struct keeps only the memos and the rule halves, and is renamed `Rules`. Its calls take the node, so each rule half reads alpha and the searched count off it as the move is reached: the mate test on alpha is asked per move rather than per rise, and the futility memo records the alpha it was found short at and asks again at any other. `raised` goes, with the assertions that kept the copies in step and the `search` parameter only one of them read. `record_node`, `census_event`, `ledger_skip` and `search_table_move` read the node. The census still records the window the node opened with, and the ledger the bounds as the move is reached. New tests read the node with no search behind it: a rise moves alpha and the root bounds together and nothing lowers either, the answer is a ceiling until a rise, and the searched count takes a legal table move and not a pinned piece's. The tree is unchanged. The bench counts 5,965,973, and every instrument's rows at depth 5 print as on the parent: cutoffs, reductions, effort with no switch off and with null_move off, and residuals, both at their default sampling and at every 1. Callgrind over the depth 5 bench counts 201,360,782 instructions on the parent and 202,312,034 here, +951,252 (+0.47%). All but 12,103 of it is in the body of alpha_beta, where every node path is inlined. There the rules' own lines are 155,063 fewer and the mate test on alpha is 175,428 more (is_mate and the abs it calls). The rest is spread over engine.rs, board.rs and value.rs lines that did not change, which reads as register allocation: a variant that changed only how the cold recorders take the node moved the same body by a further 364,571. Bench: 5965973 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A child search was asked for with three parameters, `first`, `reduction`
and `staged`, through `search_child` and `windowed`, and an assertion held
that a first move carried no reduction. The move loop already had the
later move's case as a value, `Decision::Search { reduction, staged }`.
`Decision` gains `First` beside `Skip` and `Search`, and both functions
take the decision by reference in place of the three. A first move cannot
carry a reduction, so the assertion goes. `late_move_decision` answers
`First` for the node's first searched move without asking the late move
rules, which could not act on it (neither shallow rule reaches a node that
has searched nothing, and the reduction starts at the fifth). The table's
move is asked for as `First`, and the root builds the same two cases from
its own locals. A `Skip` reaching `search_child` is a programming error: a
debug assertion says so there, and `windowed` treats it as unreachable.
The `too_many_arguments` allowances on the two functions go.
The tree is unchanged. The bench counts 5,965,973, and every instrument's
rows at depth 5 print as on the parent: cutoffs, reductions, effort with
no switch off and with null_move off, and residuals, both at their default
sampling and at every 1.
Callgrind over the depth 5 bench counts 202,312,034 instructions on the
parent and 201,147,133 here, -1,164,901 (-0.58%). The body of alpha_beta
is 1,658,535 fewer across its two copies, and windowed, which now matches
on the decision, is 481,276 more. Of the fall in alpha_beta, 502,559 is on
the lines of std's option.rs and 385,287 on the late move rules' lines,
which the first move no longer asks.
Bench: 5965973
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Speed against
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits, meant to land as two (rebase merge), because each carries its
own measurement against its own parent.
55d19a3refactor(search): Hold a node's facts and its answer in one valuethe reductions read. A full width node's state was held three times:
NodeFactsfor the recorders,NodeAnswerfor the bounds and the fail softbest, and
late_move::Node, which repeated six of the seven facts beside itsmemos and the two rule halves. The loop kept them in step by handing the late
move node alpha after every rise, checked only by debug assertions. Now one
Nodeholds the facts and the answer, alpha lives there alone, and the latemove struct keeps only its memos and rule halves as
Rules, reading the nodeper call.
raisedand the assertions go. New tests read the node with nosearch behind it, and two pin what the census and the ledger record after a
rise.
fb42c41refactor(search): Ask for a child search with the loop'sdecision.
search_childandwindowedtookfirst: bool, reduction: u8, staged: Option<&Staged>and an assertion that a first move carried noreduction.
DecisiongainsFirstbesideSkipandSearch { reduction, staged }, and both functions take it. The first move no longer asks the latemove rules, which could not act on it.
What a reviewer should know
arche benchcounts 5,965,973 on the base and onboth commits, and every instrument's rows at depth 5 (cutoffs, reductions,
effort with and without
null_moveoff, residuals) print identically, atthe default sampling and at
every 1. A second review's own builds alsoheld the reference configuration, each switch off, each shallow rule alone
and
bench 7identical.bench 5, each commit against its parent: the node +0.47%,the child value -0.58%, together -0.11%. Of the node's rise, 175,428 is the
mate test on alpha, asked per admitted move where it was per rise; the rules'
own lines fell 155,063 and the rest is register allocation in lines the
change does not touch. A one-bit memo of a mated alpha on the node, to make
the test per rise again, was measured and read 67,365 worse, so it is not
taken. A speed round on the pair read -0.8% with a 95% interval from -2.3%
to +0.9%, not resolved, as a change this size should read.
instrument rows yet passed every test: with the opening and the risen alpha
now in one struct, the census or the ledger reading the wrong one is a one
word slip, and the recorder tests only built nodes that had never risen.
Both have tests now, as does the admission's mate guard.
filled as before. Both are deliberately out of scope.
Tests go from 770 release and 772 debug in the workspace to 776 and 778.
🤖 Generated with Claude Code