`Skill::pick_best()` assumes the root moves are sorted by score in descending order:
```cpp
Value topScore = rootMoves[0].score;
int delta = std::min(topScore - rootMoves[multiPV - 1].score, int(PawnValue));
```
That does not have to be so with Syzygy tablebases at the root, where the moves are ordered by `tbRank` instead. `rootMoves[0]` is then not the highest score and `delta` can go negative. Since `delta * (rng.rand<unsigned>() % int(weakness))` is evaluated as unsigned, a negative `delta` wraps to a large value and the `int(...)` cast overflows, so the `score + push >= maxScore` check never passes and `best` stays `Move::none()`. The caller
```cpp
std::swap(rootMoves[0],
*std::find(rootMoves.begin(), rootMoves.end(),
skill.best ? skill.best : skill.pick_best(rootMoves, multiPV)));
```
then dereferences `end()`. The result is a crash, or sometimes `bestmove (none)` / an illegal move, in tablebase endgames when `UCI_LimitStrength` is set (or `Skill Level` is below 20).
The fix computes the score range over the candidate moves directly, so `delta` stays non-negative and a valid move is always returned.
**To reproduce**:
```
setoption name UCI_LimitStrength value true
setoption name UCI_Elo value 2900
setoption name SyzygyPath value <syzygy tablebases path>
position fen 8/8/8/4k3/8/8/3BKN2/8 b - - 0 1
go wtime 250 btime 250 winc 100 binc 100
```
You need to call the go command a couple of times before crash, after which is ends with
```
bestmove a1e5 ponder (none)
```
and on the next `go wtime 250 btime 250 winc 100 binc 100` it crashes printing:
```
info string Available processors: 0-31
info string Using 1 thread
info string NNUE evaluation using nn-af1339a6dea3.nnue (106MiB, (83248, 1024, 31, 32, 1))
info string Network replica 1: Shared memory.
```
Bench is unchanged, since this code only runs under `UCI_LimitStrength` / `Skill Level`.
closes https://github.com/official-stockfish/Stockfish/pull/6957
No functional change
In the spirit of previous such PRs, this PR proposes a collection of nonfunctional changes.
I am happy to incorporate changes from other devs, and revert some of the proposed changes if the maintainers ask me to.
Apart from trivial changes, the proposed changes so far include:
* A requested edit to `AUTHORS` and a resorting of all entries, following DIN 5007 for the treatment of any special characters.
* Exclude the two recent integer type renaming commits from git blame.
* Tightening of some static asserts in `history.h` to avoid overflows. (Note that rounding errors for floating point types could lead to the assert in `operator<<` triggering at run-time.)
* Re-instate the 0.5s maximal thinking time in case of a single legal move and reword the comment to make it clear that it should not be tuned.
* ~~A small refactoring of the network loading code thanks to @dubslow.~~
* A refactoring of the "dtz is dtm" code, also thanks to @dubslow.
closes https://github.com/official-stockfish/Stockfish/pull/6928
No functional change
The following have zero call sites repo-wide:
SearchManager:🆔 never read or written (also never initialized, nor was it ever used).
Search::Worker::elapsed_time(): never called. PV output uses tm.elapsed_time() (TimeManager) directly. (removed callers on 25361e5)
MovePicker::begin()/end(): unused private accessors. (removed callers on 8c2d21f)
closes https://github.com/official-stockfish/Stockfish/pull/6909
No functional change
Worker::do_move computes the successor hash key via the new
Position::key_after(m) and prefetches the TT entry one full do_move
earlier than the existing prefetch in Position::do_move. key_after does
not model castling, en passant or promotion keys exactly; for rare
moves the prefetch lands on an unused line.
`key_after` has been around since 2014 (https://github.com/official-stockfish/Stockfish/commit/82d065b0) and was removed in (https://github.com/official-stockfish/Stockfish/pull/5770). Adding back `prefetch_key` helps in common, normal moves at the cost of extra compute.
Speedup (PGO vs PGO, interleaved paired bench, n=48 pairs, Apple M2
Pro / apple-silicon): +0.69% [0.47, 0.91]
Passed STC:
https://tests.stockfishchess.org/tests/view/6a291f8d7c758d82accea17f
LLR: 4.24 (-2.94,2.94) <0.00,2.00>
Total: 473504 W: 121250 L: 120228 D: 232026
Ptnml(0-2): 1112, 51137, 131251, 52121, 1131
No functional change
closes https://github.com/official-stockfish/Stockfish/pull/6911
No functional change
This PR fixes the remaining corner cases in the treatment of MultiPV
mated-in PVs, as well as an oversight in #6886. See the discussion in
In particular:
1. `previousScore` and `previousPV` can only be trusted, if that
rootmove was indeed fully searched in the previous iteration.
2. A move beyond `pvIdx` (that was hence not fully searched) may have an
exact loss score that cannot be trusted. So if a MultiPV search gets
aborted while searching `pvIdx`, we mark all the following loss
scores as bounds.
3. The forgotten mate logic also got broken in #6886, because the
`previousPV` of the forgotten mate's bestmove can only be trusted if
that move was fully searched in the previous iteration, something
that is not guaranteed. So we now store both `lastBestMoveScore` and
`lastBestMovePV`.
Here some scenarios for MultiPV = 8 that explain how master was broken:
1. Move A with an inexact mated-in-2 score from the previous iteration
(so outside the top8 moves) gets flushed into the top8 moves for the
current iteration, because the previous top8 move B is now scored as
a mated-in-1. Hence we cannot trust `previousScore` or `previousPV`
for move A, if the search gets aborted while it is being searched.
2. In the scenario above, move B has `Score != -VALUE_INFINITE` and a
mated-in-1 score, which cannot be trusted as it was not fully
searched.
3. Iteration N has bestmove A with mated-in-10, which gets recorded in
`lastBestMoveScore` (renamed from `lastIterationScore`). Iteration 11
forgets the mate and has bestmove B with a cp score, move A may have
an incomplete PV, and may even have a non-mate score. Iteration 12
gets aborted, and in trying to remember the forgotten mate, master
recovers the `previousScore` and `previousPV` of move A, which may be
neither mate nor complete.
Passed STC non-reg:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 69728 W: 17748 L: 17573 D: 34407
Ptnml(0-2): 143, 7571, 19274, 7720, 156
https://tests.stockfishchess.org/tests/view/6a2c40c60d5d4b19d08052f2
closes https://github.com/official-stockfish/Stockfish/pull/6906
No functional change
In a recent CCC event, Stockfish (probably through no fault of its own), lost some games on time when it was winning and when it had already found the move that delivers checkmate.
This patch stops the search when TM is active, and when mainthread can be certain that it is impossible to find a better move. That is if (i) it has found mate-in-1, (ii) it has found a mate-in-2 or (iii) it has found a mated-in-1.
patch:
```
position fen 5K2/8/2qk4/2nPp3/3r4/6B1/B7/3R4 w - e6
go wtime 100000000 winc 100000000
info string Available processors: 0-7
info string Using 1 thread
info string NNUE evaluation using nn-71d6d32cb962.nnue (106MiB, (83248, 1024, 31, 32, 1))
info string Network replica 1: Shared memory.
info depth 1 seldepth 3 multipv 1 score mate 1 nodes 30 nps 30000 hashfull 0 tbhits 0 time 1 pv d5e6
bestmove d5e6
```
master:
```
position fen 5K2/8/2qk4/2nPp3/3r4/6B1/B7/3R4 w - e6
go wtime 100000000 winc 100000000
info string Available processors: 0-7
info string Using 1 thread
info string NNUE evaluation using nn-71d6d32cb962.nnue (106MiB, (83248, 1024, 31, 32, 1))
info string Network replica 1: Shared memory.
info depth 1 seldepth 3 multipv 1 score mate 1 nodes 30 nps 15000 hashfull 0 tbhits 0 time 2 pv d5e6
<snip>
info depth 245 seldepth 2 multipv 1 score mate 1 nodes 5886 nps 367875 hashfull 0 tbhits 0 time 16 pv d5e6
bestmove d5e6
```
Note: In MultiPV analysis (extremely rare with TM active), we take the point of view that the user would like to continue to search until none of the PVs can be improved anymore. This means we only stop if the worst searched line is at least a mate-in-2, or if the best searched line is a mated-in-1.
closes https://github.com/official-stockfish/Stockfish/pull/6879
No functional change
Adds a `RelaxedAtomic` wrapper around either `T` or `std::atomic<T>` and `USE_SLOPPY_ATOMICS` preprocessor define. The intent of this flag is to allow easy disabling of atomics on WASM, where even relaxed atomics are expensive because all atomics have `seq_cst` semantics.
Passed non regression STC
LLR: 2.99 (-2.94,2.94) <-1.75,0.25>
Total: 50624 W: 12976 L: 12776 D: 24872
Ptnml(0-2): 112, 5445, 14005, 5631, 119
https://tests.stockfishchess.org/tests/view/6a1f690e818cacc1db0ad2c7
Passed non-regression STC SMP
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 163696 W: 41514 L: 41438 D: 80744
Ptnml(0-2): 162, 18272, 44904, 18348, 162
https://tests.stockfishchess.org/tests/view/6a21fb97351b79f679cc44b3
Using this class for the TT also allows us to remove the TSAN suppressions, since the UB is fixed.
closes https://github.com/official-stockfish/Stockfish/pull/6877
No functional change
This PR introduces the additional `RootMove` attribute `previousPV` so that scores and PVs we send to the GUI in MultiPV analysis always match. This allows us in particular to extend our guarantee of exact mate (and TB win/loss) scores having a complete PV (leading to checkmate in the correct number of plies) to all PV lines. Recall that master fails here, since partially searched root moves may send to the GUI the previous score with the current/modified PV. See #6784.
The PR also uses the new attribute to extend the followPV logic to the analysis of sidelines, building on the idea in #6813 by @joergoster.
Passed non-reg STC:
LLR: 2.95 (-2.94,2.94) <-1.75,0.25>
Total: 166880 W: 42357 L: 42282 D: 82241
Ptnml(0-2): 394, 18685, 45177, 18820, 364
https://tests.stockfishchess.org/tests/view/6a0dea55818cacc1db0abb6a
Failed non-reg LTC:
LLR: -2.97 (-2.94,2.94) <-1.75,0.25>
Total: 890520 W: 224168 L: 225282 D: 441070
Ptnml(0-2): 390, 91902, 261789, 90790, 389
https://tests.stockfishchess.org/tests/view/6a1143ad818cacc1db0ac14c
Opening as draft for discussion on how to proceed. In SinglePV analysis, the patch is completely nonfunctional. But it is maybe a (small?) slowdown because of the increased size of `RootMove`. I am not sure if there as an elegant way to enrich the class only for MultiPV analysis (but the switch can happen at any time through the UCI interface), or to mitigate the speed penalty in some other way.
A local speedup test shows only a small slowdown on my system (but still high error bars):
```
sf_base = 1156928 +/- 1459 (95%)
sf_test = 1155885 +/- 1283 (95%)
diff = -1043 +/- 1777 (95%)
speedup = -0.09021% +/- 0.154% (95%)
```
The PR also adds the new MultiPV mate PV correctness check to the CI.
closes https://github.com/official-stockfish/Stockfish/pull/6886
No functional change
example of using, to avoid mixed usage of std::uint/std::int and uint/int...
```cpp
using u64 = std::uint64_t;
using u32 = std::uint32_t;
using u16 = std::uint16_t;
using u8 = std::uint8_t;
using i64 = std::int64_t;
using i32 = std::int32_t;
using i16 = std::int16_t;
using i8 = std::int8_t;
using usize = std::size_t;
using isize = std::ptrdiff_t;
#if defined(__GNUC__) && defined(IS_64BIT)
__extension__ using u128 = unsigned __int128;
__extension__ using i128 = signed __int128;
#endif
```
closes https://github.com/official-stockfish/Stockfish/pull/6874
No functional change
Follow-up to #6785
After completing an iteration, the engine verifies if a "mate in x" condition is met to stop searching. Currently, it checks is_mate() and is_mated() independently using two separate bulky logic branches. However, VALUE_MATE equals 32000, so both a mate (e.g., 31998) and a mated score (e.g., -31998) satisfy VALUE_MATE - std::abs(score) == 2. By utilizing is_mate_or_mated() and applying std::abs(), we mathematically combine these checks into a single concise expression.
Can someone please help run a matetrack with a --mate 100 limit for this?
closes https://github.com/official-stockfish/Stockfish/pull/6829
No functional change
This patch improves mate finding results by protecting mate scores from being forgotten in a later iteration. This applies especially to aborted searches, but also to completed iterations in e.g. `go depth` scenarios. The only visible change to the user is the final UCI info line.
The case of TB wins/losses was excluded from this patch on purpose. The reason is that in some corner cases related to rounding in some DTZ Syzygy EGTBs, later searches may turn a TB loss into a draw, for example. See #5175 and the example
```
info depth 5 seldepth 5 multipv 1 score cp -20000 nodes 122 nps 8714 hashfull 0 tbhits 26 time 14 pv h1b1 a7b6 f6e5 b6b1 e5f4 c8c3 f4g5 d7h7 g5f4 b1f5 f4f5 c3e5 f5g6 e5f4 g6h7 f4g5 h7h8 b8c7 h8h7 c7d6 h7h8 d6e5 h8h7 e5f6 h7h8 g5g7
info depth 6 seldepth 9 multipv 1 score cp 0 nodes 255 nps 18214 hashfull 0 tbhits 26 time 14 pv h1h2 d7d6 f6g5 a7e7
```
from #5414.
Matetrack for patch:
```
Using ./stockfish on matetrack.epd with --nodes 1000000
Engine ID: Stockfish dev-20260510-dbbc3ed1
Total FENs: 6554
Found mates: 3054
Best mates: 2224
```
This is slightly more/equal compared to master (3040 and 2224), see https://github.com/vondele/matetrack/blob/faf7cf3be95ab7dfdcde6e035de28a30887cd8a5/matetrack1000000.csv#L4368.
Passed STC non-reg:
LLR: 2.96 (-2.94,2.94) <-1.75,0.25>
Total: 108224 W: 27565 L: 27429 D: 53230
Ptnml(0-2): 212, 11755, 30064, 11847, 234
https://tests.stockfishchess.org/tests/view/6a00899e9392f0c31721406c
closes https://github.com/official-stockfish/Stockfish/pull/6816
No functional change
I found these changes useful and natural when working on a patch to improve mate reporting. I can revert some of the changes if maintainers would prefer that. E.g. I am not sure whether `unset_bound_flags()` is worth defining.
closes https://github.com/official-stockfish/Stockfish/pull/6785
No functional change
Failed VVLTC non-regression
https://tests.stockfishchess.org/tests/view/69d562b84088e069540a2288
LLR: -2.96 (-2.94,2.94) <-1.75,0.25>
Total: 386998 W: 99181 L: 99760 D: 188057
Ptnml(0-2): 35, 35792, 122429, 35203, 40
Failed STC non-regression
https://tests.stockfishchess.org/tests/view/69f3c6601e5788938e86a99e
LLR: -2.93 (-2.94,2.94) <-1.75,0.25>
Total: 33696 W: 8492 L: 8795 D: 16409
Ptnml(0-2): 124, 4209, 8504, 3868, 143
Many thanks to Dubslow, Torom, ces42, Shawn, vondele, Disservin and others for discussion.
## Summary
The venerable small net has been around for quite some time now, and while the big net architecture has substantially advanced with TI, the small net has stayed with plain HalfKA. It therefore presents a few burdens: multiple net architectures to maintain, multiple nets to train, and a whole lot of templates to deal with the variable L1 size.
Locally I measure a slowdown of -2.5% in NPS with this branch – and it's probably more on non-AVX512 architectures – but a pure slowdown of that magnitude would lead to more dramatic losses (even at VVLTC) than exhibited in the above tests, suggesting that the small net's lower eval quality is deleterious.
Bonus: Shawn found this interesting PGN among the VVLTC games: https://lichess.org/study/hvo8jflc/OeTOityv `master` seems to misevaluate the fortress because all positions go to small net (the material difference is larger than the threshold).
closes https://github.com/official-stockfish/Stockfish/pull/6796
Bench: 2877007
Recent changes to this part of the code mean that the variable `completedDepth` is no longer neeeded. The only *functional* part of this patch is the effect on the thread voting formula. Basically, compared to master this patch raises the depth weight in the formula uniformly by 1. For very, very low depths this may have a tiny effect on thread selection compared to master. But in general we expect this patch to have no effect on game play at STC and LTC.
Passed STC SMP non-reg:
LLR: 2.95 (-2.94,2.94) <-1.75,0.25>
Total: 284808 W: 72944 L: 72994 D: 138870
Ptnml(0-2): 351, 32192, 77370, 32138, 353
https://tests.stockfishchess.org/tests/view/69eddd011e5788938e86a075
closes https://github.com/official-stockfish/Stockfish/pull/6793
No functional change
Master in `pv()` incorrectly assumes that any root move with `i != pvIdx` has an exact score. This can lead to the suppression of bound flags for (a) root moves that have been partially searched in an iteration but do not end up to be best move, or (b) when an aborted search for `pvIdx > 0` leads to a new best move. This latter case is the cause for issue #6783.
Fixes#6783.
The bug in its incarnation (a) was independently discovered by @ciekce a couple of weeks ago, and we have jointly discussed a patch that fixes it. He is therefore a co-author of the bugfix part of this patch.
While at it, we also fix three minor oversights in the PV roll-back for aborted searches with a loss score:
* We only roll back the PV if the aborted search would report an exact score.
* Master inadvertently may have reported a bound for the rolled-back score+PV.
* We ensure a final UCI info line with the correct search depth and searched nodes statistics etc.
closes https://github.com/official-stockfish/Stockfish/pull/6784
No functional change
Co-Authored-By: Ciekce <44617491+Ciekce@users.noreply.github.com>
For quite some time SF has reported a too low final search depth in rare cases
(in multi-threaded search if a non-main thread was selected). Unfortunately,
the recent refactoring in #6704 means that this now happens for almost every
stopped search.
This PR fixes this issue, which was first spotted and reported by joergoster,
by ensuring that `rootDepth` always holds the last depth for which a search was
started.
Fixes#6756.
closes https://github.com/official-stockfish/Stockfish/pull/6761
No functional change.
This PR is a replacement for #6652 and takes into account the changes made to this part of the code since then.
In multiPV analysis master may suppress genuine proven mated-in scores, if the iteration for a secondary PV line is aborted. In very rare cases, it may also evict a proven mated-in score from the best PV line with a score from an incomplete iteration of a secondary PV line. This PR avoids both.
In addition, we allow best thread selection in multithreaded multiPV searches. This was originally disabled in #524.
Single PV searches are not affected by this patch at all.
Fixes#6642.
closes https://github.com/official-stockfish/Stockfish/pull/6728
No functional change.
Fix MultiArray::at() which was marked noexcept despite delegating to
std::array::at() that throws on out-of-range. Per review feedback, at() now
keeps standard bounds-checking semantics (only noexcept removed), and
operator[] gains an assert for debug-mode safety.
All eight correction history lookups in search.cpp (correction_value and
update_correction_history) are switched from .at(us) to [us].
Passed STC:
https://tests.stockfishchess.org/tests/view/69de7a735c67bee7d241ff92
LLR: 2.96 (-2.94,2.94) <-1.75,0.25>
Total: 77536 W: 20031 L: 19860 D: 37645
Ptnml(0-2): 221, 8411, 21338, 8572, 226
closes https://github.com/official-stockfish/Stockfish/pull/6734
No functional change
Conditions for no pawn pushes, non-pawn material and last captured piece are already such restrictive that we don't need to artificially set checkers bitboard for legal move generation.
Also moving the check for available pawn pushes at first place, as it should already filter about 90% of cases.
Passed STC and LTC non-regression tests.
LLR: 2.96 (-2.94,2.94) <-1.75,0.25>
Total: 171232 W: 44202 L: 44131 D: 82899
Ptnml(0-2): 466, 18809, 47023, 18824, 494
https://tests.stockfishchess.org/tests/view/69d3623b61a12cebe17ededa
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 55662 W: 14424 L: 14247 D: 26991
Ptnml(0-2): 38, 5479, 16609, 5678, 27
https://tests.stockfishchess.org/tests/view/69d64cada07a818b0a25f95e
And also neutral on 10k fixed games test with custom stalemate book.
Elo: 0.24 ± 0.9 (95%) LOS: 69.9%
Total: 10000 W: 4681 L: 4674 D: 645
Ptnml(0-2): 0, 87, 4819, 94, 0
nElo: 1.81 ± 6.8 (95%) PairsRatio: 1.08
https://tests.stockfishchess.org/tests/view/69d7e4183ca80bdf151d43b8
closes https://github.com/official-stockfish/Stockfish/pull/6715
Bench: 2984258
The uci pv suppression and PV roll-back logic in master is a bit
convoluted, which makes it hard to reason about the code. In fact,
subtle bugs that led to wrong mated-in scores in game play or a mismatch
between bestmove and first PV move were only recently fixed. Moreover,
in master the uci pv output through `pv()` is called in four different
places.
This PR proposes to simplify this logic. In this patch, the PV is sent
to the GUI from within `iterative_deepening()` only (a) for fail
highs/lows or (b) when an iteration for a root move is completed. All
other PV outputs are now handled by `start_searching()`, just before the
bestmove extraction. This easily ensures that bestmove (and ponder move)
will always be in sync with the final PV output.
The only noticeable change to master is for multi-threaded searches that
do not involve `limits.depth`. Here master would show both the PV from
the aborted search in main thread, as well as the new PV from the
selected best thread. This patch would only show the latter.
While at it, we also remove the requirement to finish at least a depth 1
search and simplify the logic around stalemates/checkmates at root and
in particular avoid any PVs that start with `Move::none()`.
Passed STC non-reg:
LLR: 2.96 (-2.94,2.94) <-1.75,0.25>
Total: 72416 W: 18784 L: 18609 D: 35023
Ptnml(0-2): 200, 7762, 20127, 7901, 218
https://tests.stockfishchess.org/tests/view/69d3ef0c33584dad27b3c85a
closes https://github.com/official-stockfish/Stockfish/pull/6704
No functional change
Passed simp STC
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 29632 W: 7772 L: 7552 D: 14308
Ptnml(0-2): 76, 3155, 8147, 3349,
Id expect there to be a small speedup (bc the compiler can skip some
stores) but i haven’t tried to measure it
closes https://github.com/official-stockfish/Stockfish/pull/6688
No functional change
In master for various reasons the given ponder move may get out of sync
with (the second move of) the last PV sent to the GUI:
1. If a ponder move is extracted from the TT despite bestmove leading to
a game ending draw.
2. If a ponder move is extracted from the TT from a fail high/low, since
after the ponder move extraction no new PV is sent to the GUI.
3. If `syzygy_extend_pv()` from within the call to `pv()` changes the PV
after the ponder move has been selected.
4. If the PV roll-back to protect mated-in scores differs in the second
move from the final PV sent to the GUI.
This PR keeps ponder move and the last printed PV in sync.
Fixes#6676.
closes https://github.com/official-stockfish/Stockfish/pull/6679
No functional change
