Exact go nodes

[xu-shawn]

A complete version of PR #5612. This patch guards against all possible misuse of information from an aborted search. As a by-product of this modification, Stockfish can now search exactly m nodes when given the command go nodes m, except for two edge cases:

• When ID reaches maximum depth before reaching target node count.
• When the fixed nodes search ends exactly before a depth is completed

Supersedes #5612

Passed Non-regression STC:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 109632 W: 28340 L: 28206 D: 53086
Ptnml(0-2): 261, 11926, 30317, 12042, 270
https://tests.stockfishchess.org/tests/view/6733aabf86d5ee47d953e918

Passed Non-regression LTC:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 222474 W: 56364 L: 56350 D: 109760
Ptnml(0-2): 120, 21983, 67031, 21969, 134
https://tests.stockfishchess.org/tests/view/6734c4d286d5ee47d953ea2d

no bench change

[github-actions]

clang-format 18 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/noble/clang-format-18.

(execution 11873702841 / attempt 1)

[Disservin]

this would also need a non regression test but i'll wait for what @vondele has to say

[xu-shawn]

this would also need a non regression test but i'll wait for what @vondele has to say

Sorry forgot to include the test results. It's in the description now.

[peregrineshahin]

I assume there are two problems the PR is trying to solve that I don't see are necessarily related because you can approach one without the other:
1- The problem of using untrusted scores when unwinding aborted branches and possibly writing those scores to TT
2- Making go nodes exact

Now this PR has new changes that are non-functional to fix both, because to solve the first problem you need to check for a stop only at the beginning of search or in a moves-loop, but this PR checks for a stop to return 0 instead of a value which already means nothing if the parent will handle it.

[xu-shawn]

1- The problem of using untrusted scores when unwinding aborted branches and possibly writing those scores to TT

I feel that this is more of a secondary goal of the PR. The primary goal is always to make go nodes an exact bound, and prevents bad TT/History updates is always a secondary concern. Parts of the code such as Post-LMR conthist updates will still be wrongly updated by garbage results, but since guarding them do not contribute to exact go nodes bound, the fix is not included.

[peregrineshahin]

Parts of the code such as Post-LMR conthist updates will still be wrongly updated by garbage results, but since guarding them do not contribute to exact go nodes bound, the fix is not included.

Yes, but I am failing to understand how can checking for the stop flag in those specifically three places help make go nodes exact, i.e. what are the checks exactly fixing:
1- after the search call n Razoring
2- after the search call in Probcut
3- after the search call in NMP

[xu-shawn]

Yes, but I am failing to understand how can checking for the stop flag in those specifically three places help make go nodes exact, i.e. what are the checks exactly fixing: 1- after the search call n Razoring 2- after the search call in Probcut 3- after the search call in NMP

If time runs out during the Probcut search, not only will an invalid tt be stored, but the following line inside the parent node will still be run:

Stockfish/src/search.cpp

Line 1150 in 82b092c

thisThread->nodes.fetch_add(1, std::memory_order_relaxed);

Causing the node counter to overshoot. Same applies to the two other cases.

[peregrineshahin]

Okay so the problem is exactly here, why can't we run the check before doing the move and after undoing it, instead of putting the check at every early prunning heuristic?

[xu-shawn]

Okay so the problem is exactly here, why can't we run the check before doing the move and after undoing it, instead of putting the check at every early prunning heuristic?

This can probably work as well, but I believe that catching timeouts right when they happen has two advantages:

• It fixes certain heuristics updating TT/history with garbage information on timeout
• It is easier to maintain since there is 100% guarantee that nothing could happen from when abort flag is raised, to when the abort flag is checked

[peregrineshahin]

I see your point regarding timeouts, but it's the opposite of 'easier to maintain', since this requires everyone who wants to introduce any new pre-moves heuristic to add this check after their search calls, and it's very easy to miss, let alone the amount of duplication there, whereas in the other option one can refactor those checks as parts of the do/undo functions including the addition of a node, forgetting about this issue for good.