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
After merging the HalfKA and Threats accumulators (7c7fe322) and the
subsequent removal of the double-incremental/fused update, a number of
NNUE helpers and fields became unreachable. Each was verified to have
zero callers/readers across the source tree:
- FusedUpdateData logic in FullThreats: the fused-update branch of
append_changed_indices and the FusedUpdateData parameter are unused;
the accumulator update no longer passes fused data.
- FullThreats::requires_refresh: never called. The live king-bucket
refresh check is HalfKAv2_hm::requires_refresh (PSQFeatureSet), used
in nnue_accumulator.
- HalfKAv2_hm::append_active_indices: never called. The live
active-index builder is FullThreats::append_active_indices
(ThreatFeatureSet).
- DirtyThreats::us, prevKsq and ksq: written in do_move but only read by
the now-removed FullThreats::requires_refresh. Removing them also
drops three stores from the do_move path.
- Unused feature Name constants and the unused FtOneVal / HiddenMaxVal
constants in nnue_common.h.
- Two stale feature-header banner comments.
closes https://github.com/official-stockfish/Stockfish/pull/6898
No functional change
The FEN validation check intended to reject pawns on the first or eighth rank uses the `Rank` enum values in a bitwise OR operation:
`if (pieces(PAWN) & (RANK_1 | RANK_8))`
`RANK_1 | RANK_8` evaluates to the integer `0 | 7 == 7` instead of a bitboard, so the expression only tests squares A1, B1 and C1. As a result, unsupported positions with pawns elsewhere on the first or eighth rank are silently accepted. For instance, `position fen 3P3k/8/8/8/8/8/8/3K4 w - - 0 1` is accepted even though the pawn on d8 makes the position unsupported.
Use the `Rank1BB | Rank8BB` bitboard constants so any pawn on the first or eighth rank are correctly rejected.
closes https://github.com/official-stockfish/Stockfish/pull/6887
No functional change
Passed STC
https://tests.stockfishchess.org/tests/view/6a2682ce351b79f679cc47c5
LLR: 2.93 (-2.94,2.94) <0.00,2.00>
Total: 57248 W: 14730 L: 14399 D: 28119
Ptnml(0-2): 145, 6159, 15698, 6464, 158
Reordering operations in `do_move` allows us to effectively prefetch the TT entry earlier, since the piece moving helpers don't actually modify the position key. I suspect that with threat inputs, `put_piece` and friends got a lot more expensive, and so this helps us a lot.
vondele's machine:
==== master ====
1 Nodes/second : 294311526
2 Nodes/second : 297068312
3 Nodes/second : 297418763
Average (over 3): 296266200
==== pfearly ====
1 Nodes/second : 303986449
2 Nodes/second : 304221719
3 Nodes/second : 305302969
Average (over 3): 304503712 (+2.78%)
Locally, `bench`:
Result of 200 runs
speedup = +0.0158
P(speedup > 0) = 1.0000
As expected it helps even more in a large-hash, NUMA setting.
closes https://github.com/official-stockfish/Stockfish/pull/6891
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
Over the past few months our attacks and bitboard logic got a bit more involved and target specific, so I think it makes sense to split this up a bit more into separate files.
While it's not in an ideal state I think it's first step and can be improved later, `pawn_attacks_bb` could also be moved into the attacks.h but this was more of an oversight from my side.
closes https://github.com/official-stockfish/Stockfish/pull/6833
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
Passed STC
https://tests.stockfishchess.org/tests/view/69e82e200e9667dd5a765631
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 115424 W: 29950 L: 29822 D: 55652
Ptnml(0-2): 318, 12712, 31544, 12800, 338
Passed AVX2-only STC
https://tests.stockfishchess.org/tests/view/69e953080e9667dd5a7657c0
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 134720 W: 34773 L: 34665 D: 65282
Ptnml(0-2): 344, 14958, 36694, 14974, 390
Now that our L1 size is much smaller, the overhead of tracking diffs for
`double_inc_update` is a larger share of the cost.
Interestingly ces42 measured a -0.9% slowdown locally (avxvnni build).
Meanwhile Torom and I find this branch to be 2% faster. I'm not sure exactly
what's going on but more tests may be in order
closes https://github.com/official-stockfish/Stockfish/pull/6776
No functional change
Piece promotion is generated, and then type_of(promotion) is used to retrieve
the underlying type. The class Move provides .promotion_type() natively,
meaning we can cache the type directly and avoid extracting it indirectly
afterward. By extracting pt = m.promotion_type(); up front, we don't need to
call type_of(promotion) iteratively inside the assertions or conditions below.
closes https://github.com/official-stockfish/Stockfish/pull/6759
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
[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
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
[Passed STC SMP](https://tests.stockfishchess.org/tests/view/694e506c572093c1986d7276):
```
LLR: 2.97 (-2.94,2.94) <0.00,2.00>
Total: 14992 W: 3924 L: 3653 D: 7415
Ptnml(0-2): 20, 1547, 4090, 1820, 19
```
[Passed LTC SMP](https://tests.stockfishchess.org/tests/live_elo/694ead61572093c1986d7365):
```
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 41146 W: 10654 L: 10342 D: 20150
Ptnml(0-2): 17, 3999, 12225, 4319, 13
```
[Passed a sanity check STC SMP post-refactoring](https://tests.stockfishchess.org/tests/view/69503997572093c1986d763a):
```
LLR: 2.94 (-2.94,2.94) <0.00,2.00>
Total: 46728 W: 12178 L: 11863 D: 22687
Ptnml(0-2): 82, 5093, 12685, 5436, 68
```
(The large gain of the first STC was probably a fluke, and this result
is more reasonable!)
After shared correction history, Viz suggested we try sharing other
histories, especially `pawnHistory`. As far as we're aware, sharing
history besides correction history (like Caissa does) is novel. The
implementation follows the same pattern as shared correction history –
the size of the history table is scaled with
`next_power_of_two(threadsInNumaNode)` and the entry is prefetched in
`do_move`.
A bit of refactoring was done to accommodate this new history. Note that
we prefetch `&history->pawn_entry(*this)[pc][to]` rather than
`&history->pawn_entry(*this)` because unlike the other entries, each
entry contains multiple cache lines.
closes https://github.com/official-stockfish/Stockfish/pull/6498
Bench: 2503391
Co-authored-by: Michael Chaly <Vizvezdenec@gmail.com>
We did quite a few tests because this is a pretty involved change with
unknown scaling behavior, but results are decent.
[STC 10+0.1 1th, non-regression](https://tests.stockfishchess.org/tests/live_elo/6941ce3b46f342e1ec210180)
```
LLR: 2.93 (-2.94,2.94) <-1.75,0.25>
Total: 83200 W: 21615 L: 21452 D: 40133
Ptnml(0-2): 247, 9064, 22844, 9169, 276
```
[STC 5+0.05 8th](https://tests.stockfishchess.org/tests/live_elo/693dc38346f342e1ec20f555)
```
LLR: 3.48 (-2.94,2.94) <0.00,2.00>
Total: 58536 W: 15067 L: 14688 D: 28781
Ptnml(0-2): 87, 6474, 15781, 6825, 101
```
[LTC 20+0.2 8th](https://tests.stockfishchess.org/tests/live_elo/693f2afb46f342e1ec20f847)
```
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 27716 W: 7211 L: 6925 D: 13580
Ptnml(0-2): 8, 2674, 8207, 2962, 7
```
[LTC 10+0.1 64th](https://tests.stockfishchess.org/tests/live_elo/694003aa46f342e1ec20fac4):
```
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 16918 W: 4439 L: 4182 D: 8297
Ptnml(0-2): 3, 1493, 5213, 1744, 6
```
[NUMA test, 5+0.05 256th](https://tests.stockfishchess.org/tests/view/6941ee4e46f342e1ec210203)
```
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 7124 W: 1910 L: 1678 D: 3536
Ptnml(0-2): 0, 560, 2211, 790, 1
```
[LTC 60+0.6 64th](https://tests.stockfishchess.org/tests/live_elo/6940a85346f342e1ec20fcde):
```
LLR: 2.94 (-2.94,2.94) <0.50,2.50>
Total: 15504 W: 4045 L: 3826 D: 7633
Ptnml(0-2): 0, 1002, 5530, 1219, 1
```
Bonus (courtesy of Viz): The 1 double kill in this last test was master
blundering a cool mate in 3: https://lichess.org/jyNZuRl4
Basically the idea here is to share correction history between threads.
That way, T1 can use the correction values produced by T2, which already
searched positions with that pawn structure etc., so that T1 can search
more efficiently. The table size per thread is about the same, so we
shouldn't get a large increase in hash collisions; in fact, I'd expect a
lower collision rate overall.
Although I came up with and implemented the idea independently,
[Caissa](https://github.com/Witek902/Caissa) was the first engine to
implement corrhist sharing (and corrhist in the first place) – this idea
is not completely novel.
The table size is rounded to a power of two. In particular, it's `65536
* nextPowerOfTwo(threadCount)`. That way, the indexing operation becomes
an AND of the key bits with a mask, rather than something more expensive
(e.g., a `mul_hi64`-style approach or a modulo).
The updates are racy, like the TT, but because `entry` is hoisted into a
register, there's no risk of writing back a value that's out of the
designated range `[-D, D]`. Various attempts at rewriting using atomics
led to substantial slowdowns, so we begrudgingly ignored the functions
in thread sanitizer, but at some point we'd like to make this better.
We allocate one shared correction history per NUMA node, because the
penalty associated with crossing nodes is substantial – I get a 40% hit
with NPS=4 and 256 threads, which is intolerable. With separate tables
per NUMA node I get a 6% penalty for nodes per second, which isn't ideal
but apparently compensated for.
closes https://github.com/official-stockfish/Stockfish/pull/6478
Bench: 2690604
Co-authored-by: Disservin <disservin.social@gmail.com>
Remove a couple unused things, mark `noRaysContaining` as
`[[maybe_unused]]` (suggested by Viz) to silence a warning on GCC 10,
update a comment, and replace inline constants
closes https://github.com/official-stockfish/Stockfish/pull/6460
No functional change
passed on avx512ICL:
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 30240 W: 8026 L: 7726 D: 14488
Ptnml(0-2): 95, 3235, 8171, 3513, 106
https://tests.stockfishchess.org/tests/view/69281d9ab23dfeae38cfeeb8
passed on generic architectures:
LLR: 2.94 (-2.94,2.94) <0.00,2.00>
Total: 73184 W: 19183 L: 18821 D: 35180
Ptnml(0-2): 258, 7988, 19744, 8338, 264
https://tests.stockfishchess.org/tests/view/6928ba11b23dfeae38cff276
subsequent cleanups tested as:
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 72480 W: 18678 L: 18502 D: 35300
Ptnml(0-2): 242, 7925, 19718, 8125, 230
https://tests.stockfishchess.org/tests/view/692a26adb23dfeae38cff566
We add an argument noRaysContaining, which skips all discoveries which contain
all bits in the argument; if the argument is from | to, then this will
eliminate the discovery. Separately, on AVX512ICL we can speed up the
computation of DirtyThreats by moving from a pop_lsb loop to a vector
extraction with vpcompressb. See PR for details.
closes https://github.com/official-stockfish/Stockfish/pull/6453
No functional change
During recent work on threat inputs, there was a hard-to-debug crash in tbprobe
caused by incorrectly computing st->materialKey. This PR adds correctness
checking to pos_is_ok and a faster check in tbprobe that will actually run in
CI (because pos_is_ok checking is disabled even in debug mode, for performance
purposes).
closes https://github.com/official-stockfish/Stockfish/pull/6410
No functional change
The contents of DirtyThreats gets memcpyed twice for each call to do_move.
(Return value optimization doesn't apply to the do_move function itself because
it constructs a std::pair, so it gets copied; and the calls to reset also
require a copy.) This patch inserts the dirty info in place.
Sometimes the caller of do_move ignores the DirtyThreats info, so we pass in
scratch objects. I found that stack-allocating these scratch objects was bad on
Fishtest, so I begrudgingly put them in the Position struct. Both templating
the do_move function on whether dirty threats are needed and putting a
null-check branch for each use of dirty threats were slowdowns locally. Of
course, nothing prevents a future attempt at cleaning this up.
passed STC
LLR: 2.96 (-2.94,2.94) <0.00,2.00>
Total: 68448 W: 17770 L: 17418 D: 33260
Ptnml(0-2): 198, 7425, 18630, 7769, 202
https://tests.stockfishchess.org/tests/view/6914c01a7ca87818523318ba
closes https://github.com/official-stockfish/Stockfish/pull/6414
No functional change
Only a modest amount of work happens between the transposition table prefetch
and the probe, so the probe still often stalls waiting for DRAM. The vast
majority of the time (in particular, if !checkEP), the key is known much
earlier in the do_move function and the latency can be better hidden.
passed STC SMP
https://tests.stockfishchess.org/tests/view/68f337c528e6d77fcffa066a
LLR: 2.95 (-2.94,2.94) <0.00,2.00>
Total: 65256 W: 16806 L: 16462 D: 31988
Ptnml(0-2): 76, 7386, 17362, 7726, 78
but failed to gain STC
https://tests.stockfishchess.org/tests/view/68f3378328e6d77fcffa0665
LLR: -2.94 (-2.94,2.94) <0.00,2.00>
Total: 109824 W: 28523 L: 28618 D: 52683
Ptnml(0-2): 311, 11799, 30788, 11702, 312
In local tests, the speedup grows with thread count
closes https://github.com/official-stockfish/Stockfish/pull/6372
No functional change
for positions such as:
position fen rr6/2q2p1k/2P1b1pp/bB2P1n1/R2B2PN/p4P1P/P1Q4K/1R6 b - - 2 38 moves f7f5
position fen 8/p2r1pK1/6p1/1kp1P1P1/2p5/2P5/8/4R3 b - - 0 43 moves f7f5
position fen 4k3/4p3/2b3b1/3P1P2/4K3/8/8/8 b - - moves e7e5
ep is not possible for various reasons. Do not set the ep square and the do not change the zobrist key, as if ep were possible.
This fixes 3-fold detection, which could in rare cases be observed as a warning generated by fastchess for PVs that continued beyond an actual 3-fold.
Also fixes wrong evals for certain moves e.g. a2a1 for
position fen 4k3/1b2p2b/8/3P1P2/4K3/8/8/r7 b - - 0 1 moves e7e5 e4e3 a1a2 e3e4 a2a1 e4f3 a1a2 f3e4
fixes https://github.com/official-stockfish/Stockfish/issues/6138
originally written and tested by rn5f107s2, with small buglet as
https://tests.stockfishchess.org/tests/view/685af90843ce022d15794400
failed STC
LLR: -2.93 (-2.94,2.94) <-1.75,0.25>
Total: 130688 W: 33986 L: 34362 D: 62340
Ptnml(0-2): 414, 13292, 38302, 12928, 408
https://tests.stockfishchess.org/tests/view/688bcb35502b34dd5e7111c9
passed LTC
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 99018 W: 25402 L: 25273 D: 48343
Ptnml(0-2): 54, 9097, 31089, 9204, 65
https://tests.stockfishchess.org/tests/view/688c613c502b34dd5e7112d3https://github.com/official-stockfish/Stockfish/pull/6211
Bench: 2946135
Co-authored-by: Joost VandeVondele <Joost.VandeVondele@gmail.com>
