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
In setup_benchmark, the variables evaluating the current ply simply mirror the
exact iterative progress of the string arrays loop (since ply just increments
by 1 with every pass). We can delete the separate integer assignment and
execute it securely within the loop declaration.
closes https://github.com/official-stockfish/Stockfish/pull/6732
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
The error is "C1001: Internal compiler error." in uci.cpp on line 370 of
master. For some reason the compiler can't handle the
std::size(hashfullAges) inside the lambda. Older MSVC versions had no
problem.
Most of the warnings are due to implicit type conversions "conversion
from type A to type B, possible loss of data"
many of which have been present for a while.
closes https://github.com/official-stockfish/Stockfish/pull/6464
No functional change
`speedtest [threads] [hash_MiB] [time_s]`. `threads` default to system concurrency. `hash_MiB` defaults to `threads*128`. `time_s` defaults to 150.
Intended to be used with default parameters, as a stable hardware benchmark.
Example:
```
C:\dev\stockfish-master\src>stockfish.exe speedtest
Stockfish dev-20240928-nogit by the Stockfish developers (see AUTHORS file)
info string Using 16 threads
Warmup position 3/3
Position 258/258
===========================
Version : Stockfish dev-20240928-nogit
Compiled by : g++ (GNUC) 13.2.0 on MinGW64
Compilation architecture : x86-64-vnni256
Compilation settings : 64bit VNNI BMI2 AVX2 SSE41 SSSE3 SSE2 POPCNT
Compiler __VERSION__ macro : 13.2.0
Large pages : yes
User invocation : speedtest
Filled invocation : speedtest 16 2048 150
Available processors : 0-15
Thread count : 16
Thread binding : none
TT size [MiB] : 2048
Hash max, avg [per mille] :
single search : 40, 21
single game : 631, 428
Total nodes searched : 2099917842
Total search time [s] : 153.937
Nodes/second : 13641410
```
-------------------------------
Small unrelated tweaks:
- Network verification output is now handled as a callback.
- TT hashfull queries allow specifying maximum entry age.
closes https://github.com/official-stockfish/Stockfish/pull/5354
No functional change
Previously it was possible to also get the node counter after running a bench with perft, i.e.
`./stockfish bench 1 1 5 current perft`, caused by a small regression from the uci refactoring.
```
Nodes searched: 4865609
===========================
Total time (ms) : 18
Nodes searched : 4865609
Nodes/second : 270311611
````
closes https://github.com/official-stockfish/Stockfish/pull/5188
No functional change
Also fixes searchmoves.
Drop the need of a Position object in uci.cpp.
A side note, it is still required for the static functions,
but these should be moved to a different namespace/class
later on, since sf kinda relies on them.
closes https://github.com/official-stockfish/Stockfish/pull/5169
No functional change
- remove the blank line between the declaration of the function and it's
comment, leads to better IDE support when hovering over a function to see it's
description
- remove the unnecessary duplication of the function name in the functions
description
- slightly refactored code for lsb, msb in bitboard.h There are still a few
things we can be improved later on, move the description of a function where
it was declared (instead of implemented) and add descriptions to functions
which are behind macros ifdefs
closes https://github.com/official-stockfish/Stockfish/pull/4840
No functional change
This introduces clang-format to enforce a consistent code style for Stockfish.
Having a documented and consistent style across the code will make contributing easier
for new developers, and will make larger changes to the codebase easier to make.
To facilitate formatting, this PR includes a Makefile target (`make format`) to format the code,
this requires clang-format (version 17 currently) to be installed locally.
Installing clang-format is straightforward on most OS and distros
(e.g. with https://apt.llvm.org/, brew install clang-format, etc), as this is part of quite commonly
used suite of tools and compilers (llvm / clang).
Additionally, a CI action is present that will verify if the code requires formatting,
and comment on the PR as needed. Initially, correct formatting is not required, it will be
done by maintainers as part of the merge or in later commits, but obviously this is encouraged.
fixes https://github.com/official-stockfish/Stockfish/issues/3608
closes https://github.com/official-stockfish/Stockfish/pull/4790
Co-Authored-By: Joost VandeVondele <Joost.VandeVondele@gmail.com>
This is a cleanup PR that prepares the automatic checking of missing or
superfluous #include directives via the include-what-you-use (IWYU) tool
on the CI. Unfortunately, IWYU proposes additional includes for
"namespace std" although we don't need them.
To avoid the problem, the commit removes all "using namespace std"
statements from the code and directly uses the std:: prefix instead.
Alternatively, we could add specific usings (e.g. "using std::string")
foreach used type. Also, a mix of both approaches would be possible.
I decided for the prefix approach because most of the files were already
using the std:: prefixes despite the "using namespace std".
closes https://github.com/official-stockfish/Stockfish/pull/4772
No functional change
since the introduction of NNUE (first released with Stockfish 12), we
have maintained the classical evaluation as part of SF in frozen form.
The idea that this code could lead to further inputs to the NN or
search did not materialize. Now, after five releases, this PR removes
the classical evaluation from SF. Even though this evaluation is
probably the best of its class, it has become unimportant for the
engine's strength, and there is little need to maintain this
code (roughly 25% of SF) going forward, or to expend resources on
trying to improve its integration in the NNUE eval.
Indeed, it had still a very limited use in the current SF, namely
for the evaluation of positions that are nearly decided based on
material difference, where the speed of the classical evaluation
outweights its inaccuracies. This impact on strength is small,
roughly 2Elo, and probably decreasing in importance as the TC grows.
Potentially, removal of this code could lead to the development of
techniques to have faster, but less accurate NN evaluation,
for certain positions.
STC
https://tests.stockfishchess.org/tests/view/64a320173ee09aa549c52157
Elo: -2.35 ± 1.1 (95%) LOS: 0.0%
Total: 100000 W: 24916 L: 25592 D: 49492
Ptnml(0-2): 287, 12123, 25841, 11477, 272
nElo: -4.62 ± 2.2 (95%) PairsRatio: 0.95
LTC
https://tests.stockfishchess.org/tests/view/64a320293ee09aa549c5215b
Elo: -1.74 ± 1.0 (95%) LOS: 0.0%
Total: 100000 W: 25010 L: 25512 D: 49478
Ptnml(0-2): 44, 11069, 28270, 10579, 38
nElo: -3.72 ± 2.2 (95%) PairsRatio: 0.96
VLTC SMP
https://tests.stockfishchess.org/tests/view/64a3207c3ee09aa549c52168
Elo: -1.70 ± 0.9 (95%) LOS: 0.0%
Total: 100000 W: 25673 L: 26162 D: 48165
Ptnml(0-2): 8, 9455, 31569, 8954, 14
nElo: -3.95 ± 2.2 (95%) PairsRatio: 0.95
closes https://github.com/official-stockfish/Stockfish/pull/4674
Bench: 1444646
If a global function has no previous declaration, either the declaration
is missing in the corresponding header file or the function should be
declared static. Static functions are local to the translation unit,
which allows the compiler to apply some optimizations earlier (when
compiling the translation unit rather than during link-time
optimization).
The commit enables the warning for gcc, clang, and mingw. It also fixes
the reported warnings by declaring the functions static or by adding a
header file (benchmark.h).
closes https://github.com/official-stockfish/Stockfish/pull/4325
No functional change
Restore the default NNUE setting (enabled) after a bench command.
This also makes the resulting program settings independent of the
number of FENs that are being benched.
Fixes issue #3112.
closes https://github.com/official-stockfish/Stockfish/pull/3113
No functional change.
Joint work gvreuls / vondele
* Download the default NNUE net in AppVeyor
* Download net in travis CI `make net`
* Adjust tests to cover more archs, speedup instrumented testing
* Introduce 'mixed' bench as default, with further options:
classical, NNUE, mixed.
mixed (default) and NNUE require the default net to be present,
which can be obtained with
```
make net
```
Further examples (first is equivalent to `./stockfish bench`):
```
./stockfish bench 16 1 13 default depth mixed
./stockfish bench 16 1 13 default depth classical
./stockfish bench 16 1 13 default depth NNUE
```
The net is now downloaded automatically if needed for `profile-build`
(usual `build` works fine without net present)
PGO gives a nice speedup on fishtest:
passed STC:
LLR: 2.93 (-2.94,2.94) {-0.50,1.50}
Total: 3360 W: 469 L: 343 D: 2548
Ptnml(0-2): 20, 246, 1030, 356, 28
https://tests.stockfishchess.org/tests/view/5f31b5499081672066537569
passed LTC:
LLR: 2.97 (-2.94,2.94) {0.25,1.75}
Total: 8824 W: 609 L: 502 D: 7713
Ptnml(0-2): 8, 430, 3438, 519, 17
https://tests.stockfishchess.org/tests/view/5f31c87b908167206653757c
closes https://github.com/official-stockfish/Stockfish/pull/2931
fixes https://github.com/official-stockfish/Stockfish/issues/2907
requires fishtest updates before commit
Bench: 4290577
This patch ports the efficiently updatable neural network (NNUE) evaluation to Stockfish.
Both the NNUE and the classical evaluations are available, and can be used to
assign a value to a position that is later used in alpha-beta (PVS) search to find the
best move. The classical evaluation computes this value as a function of various chess
concepts, handcrafted by experts, tested and tuned using fishtest. The NNUE evaluation
computes this value with a neural network based on basic inputs. The network is optimized
and trained on the evalutions of millions of positions at moderate search depth.
The NNUE evaluation was first introduced in shogi, and ported to Stockfish afterward.
It can be evaluated efficiently on CPUs, and exploits the fact that only parts
of the neural network need to be updated after a typical chess move.
[The nodchip repository](https://github.com/nodchip/Stockfish) provides additional
tools to train and develop the NNUE networks.
This patch is the result of contributions of various authors, from various communities,
including: nodchip, ynasu87, yaneurao (initial port and NNUE authors), domschl, FireFather,
rqs, xXH4CKST3RXx, tttak, zz4032, joergoster, mstembera, nguyenpham, erbsenzaehler,
dorzechowski, and vondele.
This new evaluation needed various changes to fishtest and the corresponding infrastructure,
for which tomtor, ppigazzini, noobpwnftw, daylen, and vondele are gratefully acknowledged.
The first networks have been provided by gekkehenker and sergiovieri, with the latter
net (nn-97f742aaefcd.nnue) being the current default.
The evaluation function can be selected at run time with the `Use NNUE` (true/false) UCI option,
provided the `EvalFile` option points the the network file (depending on the GUI, with full path).
The performance of the NNUE evaluation relative to the classical evaluation depends somewhat on
the hardware, and is expected to improve quickly, but is currently on > 80 Elo on fishtest:
60000 @ 10+0.1 th 1
https://tests.stockfishchess.org/tests/view/5f28fe6ea5abc164f05e4c4c
ELO: 92.77 +-2.1 (95%) LOS: 100.0%
Total: 60000 W: 24193 L: 8543 D: 27264
Ptnml(0-2): 609, 3850, 9708, 10948, 4885
40000 @ 20+0.2 th 8
https://tests.stockfishchess.org/tests/view/5f290229a5abc164f05e4c58
ELO: 89.47 +-2.0 (95%) LOS: 100.0%
Total: 40000 W: 12756 L: 2677 D: 24567
Ptnml(0-2): 74, 1583, 8550, 7776, 2017
At the same time, the impact on the classical evaluation remains minimal, causing no significant
regression:
sprt @ 10+0.1 th 1
https://tests.stockfishchess.org/tests/view/5f2906a2a5abc164f05e4c5b
LLR: 2.94 (-2.94,2.94) {-6.00,-4.00}
Total: 34936 W: 6502 L: 6825 D: 21609
Ptnml(0-2): 571, 4082, 8434, 3861, 520
sprt @ 60+0.6 th 1
https://tests.stockfishchess.org/tests/view/5f2906cfa5abc164f05e4c5d
LLR: 2.93 (-2.94,2.94) {-6.00,-4.00}
Total: 10088 W: 1232 L: 1265 D: 7591
Ptnml(0-2): 49, 914, 3170, 843, 68
The needed networks can be found at https://tests.stockfishchess.org/nns
It is recommended to use the default one as indicated by the `EvalFile` UCI option.
Guidelines for testing new nets can be found at
https://github.com/glinscott/fishtest/wiki/Creating-my-first-test#nnue-net-tests
Integration has been discussed in various issues:
https://github.com/official-stockfish/Stockfish/issues/2823https://github.com/official-stockfish/Stockfish/issues/2728
The integration branch will be closed after the merge:
https://github.com/official-stockfish/Stockfish/pull/2825https://github.com/official-stockfish/Stockfish/tree/nnue-player-wip
closes https://github.com/official-stockfish/Stockfish/pull/2912
This will be an exciting time for computer chess, looking forward to seeing the evolution of
this approach.
Bench: 4746616
this patch extends bench to print static evaluations.
./stockfish bench 16 1 1 filename eval
will now print the evaluations for all fens in the file.
This complements the various 'go' flavors for bench and might be useful for debugging and/or tuning.
No functional change.
The current bench is missing a position with high 50 moves rule counter,
making most 'shuffle' tests based on 50mr > N seem non-functional.
This patch adds one FEN with high 50mr counter to address this issue
(taken from a recent tcec game).
Four new FENs:
- position with high 50mr counter
- tactical position with many captures, checks, extensions, fails high/low
- two losses by Stockfish in the S16 bonus games against Houdini
See the pull request for nice comments by @Alayan-stk-2 about each position
in bench: https://github.com/official-stockfish/Stockfish/pull/2338
Bench: 4590210
Initialization of larger hash sizes can take some time.
Don't include this time in the bench by resetting the timer after Search::clear().
Also move 'ucinewgame' command down in the list, so that it is processed
after the configuration of Threads and Hash size.
No functional change.
Preparation commit for the upcoming Stockfish 10 version, giving a chance to catch last minute feature bugs and evaluation regression during the one-week code freeze period. Also changing the copyright dates to include 2019.
No functional change
Add tests for:
- Positions with move list
- Chess960 positions
Now bench covers almost all cases, only few endgames
are still out of reach (verified with lcov)
It is a non functionality patch, but bench
changed because we added new test positions.
bench: 5479946
Rewrite perft to be placed naturally inside new
bench code. In particular we don't have special
custom code to run perft anymore but perft is
just a new parameter of 'go' command.
So user API is now changed, old style command:
$perft 5
becomes
$go perft 4
No functional change.
First step in improving bench to handle
arbitrary UCI commands so to test many
more code paths.
This first patch just set the new code
structure.
No functional change.
Better split code that should be run at
startup from code run at ucinewgame. Also
fix several races when 'bench', 'perft' and
'ucinewgame' are sent just after 'bestomve'
from the engine threads are still running.
Also use a specific UI thread instead of
main thread when setting up the Position
object used by UI uci loop. This fixes a
race when sending 'eval' command while searching.
We accept a race on 'setoption' to allow the
GUI to change an option while engine is searching
withouth stalling the pipe. Note that changing an
option while searchingg is anyhow not mandated by
UCI protocol.
No functional change.
Both Tablebases::filter_root_moves() and
extract_ponder_from_tt(9 were unable to handle
a mate/stalemate position.
Spotted and reported by Dann Corbit.
Added some mate/stalemate positions to bench so
to early catch this regression in the future.
No functional change.
In this position we should have draw for repetition:
position fen rnbqkbnr/2pppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1 moves g1f3 g8f6 f3g1
go infinite
But latest patch broke it.
Actually we had two(!) very subtle bugs, the first is that Position::set()
clears the passed state and in particular 'previous' member, so
that on passing setupStates, 'previous' pointer was reset.
Second bug is even more subtle: SetupStates was based on std::vector
as container, but when vector grows, std::vector copies all its contents
to a new location invalidating all references to its entries. Because
all StateInfo records are linked by 'previous' pointer, this made pointers
go stale upon adding more element to setupStates. So revert to use a
std::deque that ensures references are preserved when pushing back new
elements.
No functional change.
And passed in do_move(), this ensures maximum efficiency and
speed and at the same time unlimited move numbers.
The draw back is that to handle Position init we need to
reserve a StateInfo inside Position itself and use at
init time and when copying from another Position.
After lazy SMP we don't need anymore this gimmick and we can
get rid of this special case and always pass an external
StateInfo to Position object.
Also rewritten and simplified Position constructors.
Verified it does not regress with a 3 threads SMP test:
ELO: -0.00 +-12.7 (95%) LOS: 50.0%
Total: 1000 W: 173 L: 173 D: 654
No functional change.
