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
Instead of checking whether a threat index is valid and then writing to
a vector if it isn't, we can instead always write to the buffer of the
vector, and only increase the size of the vector if the index is valid.
This saves some branch mispredictions.
passed STC:
https://tests.stockfishchess.org/tests/view/69ceb1689f7a7e3fdfc9a44b
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 205760 W: 53109 L: 52561 D: 100090
Ptnml(0-2): 603, 22552, 56076, 22992, 657
local speedtest shows
```
Result of 100 runs (cycles)
===========================
base (...ockfish.orig) = 296886 +/- 1645
test (...kfish.df0822) = 298862 +/- 1662
speedup % = +0.67 +/- 0.19
[95% CI; t-statistic]
p(speedup > 0) = 1.0000
p(speedup > .5%) = 0.9562
CPU: 16 x Intel(R) Core(TM) Ultra 9 185H
Hyperthreading: on
```
closes https://github.com/official-stockfish/Stockfish/pull/6703
No functional change
[Passed
STC](https://tests.stockfishchess.org/tests/live_elo/69cf6a5d1668971c9da23ae1)
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 51104 W: 13376 L: 13180 D: 24548
Ptnml(0-2): 134, 5555, 13963, 5781, 119
[Passed
LTC](https://tests.stockfishchess.org/tests/live_elo/69d00a7ee2b443cb2670b5c6)
LLR: 2.95 (-2.94,2.94) <-1.75,0.25>
Total: 101256 W: 25867 L: 25732 D: 49657
Ptnml(0-2): 59, 10423, 29520, 10576, 50
When processing a promotion we add/remove nonsensical threats, namely,
piece threats of pawns on the 1st or 8th rank. This is not only
inefficient but also seems to lead to an obscure bug during a
promotion–capture + `double_inc_update` that causes a lingering invalid
feature. Currently – as an artifact of the training process – these
parts of the net are filled with random values in [-1,1], which is why
there is a bench change.
@ces42 did a helpful analysis:
> checking bench with depth 17 actually shows that the bug in has been
there all the time since 8e5392d7 (when threat inputs was merged into
master)
> i.e. double_inc_update for ThreatFeatureSet has always been slightly
bugged
After this patch is applied we'll be able to better compress the net by
filling the invalid features with 0, w/o changing bench. I expect a
15%-ish size savings.
closes https://github.com/official-stockfish/Stockfish/pull/6701
Bench: 2700393
After 415f2ef1. Now works again with CXX paths like `A:\path\to\g++`
under MSYS2. The cache files are no longer read in the rule itself and
only used as markers for change detection.
closes https://github.com/official-stockfish/Stockfish/pull/6699
No functional change
```
clang++: error: unknown argument: '-fno-ipa-cp-clone'
clang++: error: unknown argument: '-fno-ipa-cp-clone'
make[1]: *** [bitboard.o] Error 1
```
The flag is unsupported by Clang, which pretends to be GCC on macOS, so
only passing -fno-ipa-cp-clone to real GCC fixes macOS builds.
closes https://github.com/official-stockfish/Stockfish/pull/6698
No functional change
[STC](https://tests.stockfishchess.org/tests/live_elo/69cb991bc025c305b7daa219)
LLR: 2.97 (-2.94,2.94) <0.00,2.00>
Total: 47520 W: 12558 L: 12219 D: 22743
Ptnml(0-2): 190, 5474, 12102, 5795, 199
[LTC](https://tests.stockfishchess.org/tests/live_elo/69cc6b42731dfb72d2e92128)
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 123756 W: 31996 L: 31497 D: 60263
Ptnml(0-2): 81, 13512, 34221, 13955, 109
Adds features for pawns pushing against other pawns. For indexing, we
use the existing threats feature set by adding the forward square to the
squares threatened by a pawn. This threat is only enabled when another
pawn occupies that square. We then add appropriate logic changes in
`update_piece_threats` and `features/full_threats.cpp`.
The wonderful Ciekce (of Stormphrax) and peregrine (of Reckless) also
had similar ideas with adding forward pawn movements to TI, and I
appreciate their giving me motivation to figure out `nnue-pytorch` and
test it.
closes https://github.com/official-stockfish/Stockfish/pull/6694
Bench: 2926703
Makes eval calls more cache-friendly on a hot path.
Hints of a speedup found locally:
```
Result of 30 runs
==================
base (...ockfish-base) = 1754124 +/- 2887
test (...h-eval-order) = 1757848 +/- 2412
diff = +3724 +/- 1368
speedup = +0.0021
P(speedup > 0) = 1.0000
CPU: 16 x arm
Hyperthreading: off
```
Passed STC:
https://tests.stockfishchess.org/tests/view/69c84f3cf40d95b975597c21
LLR: 3.36 (-2.94,2.94) <0.00,2.00>
Total: 335840 W: 87022 L: 86257 D: 162561
Ptnml(0-2): 932, 36760, 91816, 37435, 977
closes https://github.com/official-stockfish/Stockfish/pull/6693
No functional change
Passed non-regression STC at
tests.stockfishchess.org/tests/view/69c9b8c3a54395a761335044 (set to
gainer bounds, *mea culpa*)
There's a major performance penalty when compiling the fish with GCC 16.
@Torom 's measurements (similar to mine, but more precise):
```
GCC 15 (master) vs. 16 (master)
sf_base = 2549174 +/- 2984 (95%)
sf_test = 2435456 +/- 2239 (95%)
diff = -113718 +/- 3599 (95%)
speedup = -4.46100% +/- 0.141% (95%)
GCC 15 (master) vs. 16 (patch)
sf_base = 2544107 +/- 3091 (95%)
sf_test = 2568837 +/- 3164 (95%)
diff = 24729 +/- 4359 (95%)
speedup = 0.97204% +/- 0.171% (95%)
```
Based on looking at the disassembly + profiling data of the builds, the
root cause appears to be overly aggressive code duplication from
constant propagation of various functions especially in `search.cpp`.
The worst offender is `Worker::search`, which I think is being cloned
and optimized based on the value of `cutNode`. `perf stat` indicates
that the instruction cache miss rate goes up by ~50% on the GCC 16
build.
closes https://github.com/official-stockfish/Stockfish/pull/6691
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
STC
```
LLR: 5.45 (-2.94,2.94) <0.00,2.00>
Total: 612192 W: 158602 L: 157260 D: 296330
Ptnml(0-2): 1634, 67783, 166027, 68911, 1741
```
@Torom measured:
```
sf_base = 2380902 +/- 1016 (95%)
sf_test = 2391426 +/- 1065 (95%)
diff = 10524 +/- 1336 (95%)
speedup = 0.44203% +/- 0.056% (95%)
```
I've verified that `_mm512_slli_epi16(v, 0)` gets optimized out in GCC
9+ and clang 4+. Added constants to types.h so that people messing
around with the layout of `Move` don't have to know about this part of
the code.
closes https://github.com/official-stockfish/Stockfish/pull/6670
No functional change
Ever since #5094 master only lets the main thread terminate `go mate`
searches, reverting the earlier improvement #1215.
This PR restores the old logic. So any thread that found a "mate in x"
can now stop the search.
To make this work robustly, we need to guard against inexact mate scores
in the best thread selection. In addition, in contrast to time limits,
the main thread may now not complete a d1 search for a mated-in
position.
In master an aborted d1 search may have a PV beginning with
`Move::none()`, in which case no thread selection is performed. See
#623. We fix this bug here by checking if `lastBestPV` is empty or not.
For interrupted d1 searches we now label mated-in scores as inexact.
While at it, we also simplify the logic for detecting if we can
terminate a go mate x search, using the fact that threads.stop can only
be false if we have a completed iteration with a valid score.
The PR has no effect on game play, but should slightly improve general
mate finding and speed up multi-threaded `go mate` searches.
We also add a corresponding matecheck run to the CI. This only involves
61 mates up to mate-in-2. Test runs with the first 50 or 100 mates from
`mates2000.epd` did at times not finish within 30 minutes on my fork or
in local tests, possibly due to search explosions for some mate-in-3
positions.
closes https://github.com/official-stockfish/Stockfish/pull/6668
No functional change
fixes#6663, #6664, and a million others issues raised over the years.
This is similar to
https://github.com/official-stockfish/Stockfish/pull/4563/changes but
more conservative and errors are split between "Unsupported position"
and "Invalid FEN".
The FEN parser needs to be strict as a foundation for safety. It does
not specify much of the semantics, so this step is fairly simple. Parts
after the ep square are optional, however, since it's common, for
example in EPD notation.
Errors arising from positional semantics that were previously bucketed
under invalid FENs are now reported as "Unsupported position". Only
positions that are potentially problematic are designated as
unsupported. It is NOT guided by illegality of the position, but instead
by the ability of the engine to handle them correctly. This means that
some checks from the previous PR were removed.
std::exit is used instead of std::terminate so atexit handles will be
called.
Probably wise to run copilot on this or smth because I scribbled it
without much thought.
With these small changes and reduced, less controversial, scope I hope
this PR will finally make it and we will be done with these weekly
issues. I'll wait with putting it on fishtest until there's approval.
closes https://github.com/official-stockfish/Stockfish/pull/6665
No functional change
Thanks to MinetaS for discovering this and finding the culprit commit,
specific to this architecture. 9f42980 was subtly incorrect, leading to
spurious wrong benches on avx512icl. Because this piece of code has twice been
the source of wrong benches, I also added a brief comment explaining why it
needs to be this way, and added a sample position to bench.
closes https://github.com/official-stockfish/Stockfish/pull/6654
Bench: 2288704
Currently, the engine updates each level of the continuation history stack (ply -1, -2, -3, etc.) with fixed weights that are independent of one another. The idea behind CMHC is to scale the magnitude of these updates based on the consistency of the move's performance across the stack.
By tracking how often a move has historically performed well (positive history) within the current continuation context, we can adjust the update magnitude dynamically. Moves that demonstrate "continuity" (appearing as strong candidates across multiple historical contexts) receive more significant updates, while unproven or inconsistent moves are dampened.
Passed STC:
LLR: 2.94 (-2.94,2.94) <0.00,2.00>
Total: 103264 W: 26954 L: 26541 D: 49769
Ptnml(0-2): 338, 11910, 26728, 12313, 343
https://tests.stockfishchess.org/tests/view/69a81702fac54339cf435a87
Passed LTC:
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 47406 W: 12259 L: 11920 D: 23227
Ptnml(0-2): 22, 5052, 13237, 5349, 43
https://tests.stockfishchess.org/tests/view/69aab480bd35724e4b290498
closes https://github.com/official-stockfish/Stockfish/pull/6653
Bench: 2356418
Co-Authored-By: Daniel Samek <samekdaniel@email.cz>
This PR removes the variable threads.abortedSearch introduced in #4990 and
relies on threads.stop and completedDepth instead.
The logic in master is only guaranteed to work in single threaded search. For
multiple threads, as soon as threads.stop is set, some of the threads may abort
their search, and so their mated-in scores are not reliable.
Compared to master, this patch prevents unproven mated-in scores in these
multi-threaded scenarios:
in game play (when time management is used)
when the uci commands quit or stop are received
when maximum depth is reached
for go mate commands
The patch will change the bestmove in those very rare situations where the bug
occurs in master. This is because bestmove is taken from rootMoves[0].pv[0],
which will now no longer contain unproven mated-in PVs. In that sense the patch
is "functional". But of course, it does not change bench.
While at it, we also fix a bug in master that suppressed PV output for inexact
mated-in scores on receiving threads.stop. In cases where uciScore < score with
score not being a proven loss, and where the new bestmove was different from
the last completed iteration, this led to a bestmove output that did not match
the final PV (which was for an exact score).
Fixes#6293.
Fixes#6626.
closes https://github.com/official-stockfish/Stockfish/pull/6636
No functional change
