This is a non-functional code style change which provides a safe access handler for LineBB.
Also includes an assert in debug mode to verify square correctness.
closes https://github.com/official-stockfish/Stockfish/pull/2719
No functional change
There is an ambiguity between global and std clamp implementations when compiling in c++17,
and on certain toolchains that are not strictly conforming to c++11.
This is solved by putting our clamp implementation in a namespace.
closes https://github.com/official-stockfish/Stockfish/pull/2572
No functional change.
This is a non-functional speed-up: master has to access SquareBB twice while this patch
determines opposite_colors just using the values of the squares. It doesn't seem to change
the overall speed of bench, but calling opposite_colors(...) 10 Million times:
master: 39.4 seconds
patch: 11.4 seconds.
The only data point I have (other than my own tests), is a quite old failed STC test:
LLR: -2.93 (-2.94,2.94) [-1.50,4.50]
Total: 24308 W: 5331 L: 5330 D: 13647
Ptnml(0-2): 315, 2577, 6326, 2623, 289
http://tests.stockfishchess.org/tests/view/5e010256c13ac2425c4a9a67
Closes https://github.com/official-stockfish/Stockfish/pull/2498
No functional change
As Stockfish developers, we aim to make our code as legible and as close
to simple English as possible. However, one of the more notable exceptions
to this rule concerns operations between Squares and Bitboards.
Prior to this pull request, AND, OR, and XOR were only defined when the
Bitboard was the first operand, and the Square the second. For example,
for a Bitboard b and Square s, "b & s" would be valid but "s & b" would not.
This conflicts with natural reasoning about logical operators, both
mathematically and intuitively, which says that logical operators should
commute.
More dangerously, however, both Square and Bitboard are defined as integers
"under the hood." As a result, code like "s & b" would still compile and give
reasonable bench values. This trap occasionally ensnares even experienced
Stockfish developers, but it is especially dangerous for new developers not
aware of this peculiarity. Because there is no compilation or runtime error,
and a reasonable bench, only a close review by approvers can spot this error
when a test has been submitted--and many times, these bugs have slipped past
review. This is by far the most common logical error on Fishtest, and has
wasted uncountable STC games over the years.
However, it can be fixed by adding three non-functional lines of code. In this
patch, we define the operators when the operands are provided in the opposite
order, i.e., we make AND, OR, and XOR commutative for Bitboards and Squares.
Because these are inline methods and implemented identically, the executable
does not change at all.
This patch has the small side-effect of requiring Squares to be explicitly
cast to integers before AND, OR, or XOR with integers. This is only performed
twice in Stockfish's source code, and again does not change the executable at
all (since Square is an enum defined as an integer anyway).
For demonstration purposes, this pull request also inverts the order of one AND
and one OR, to show that neither the bench nor the executable change. (This
change can be removed before merging, if preferred.)
I hope that this pull request significantly lowers the barrier-of-entry for new
developer to join the Stockfish project. I also hope that this change will improve
our efficiency in using our generous CPU donors' machines, since it will remove
one of the most common causes of buggy tests.
Following helpful review and comments by Michael Stembera (@mstembera), we add
a further clean-up by implementing OR for two Squares, to anticipate additional
traps developers may encounter and handle them cleanly.
Closes https://github.com/official-stockfish/Stockfish/pull/2387
No functional change.
This is a non-functional simplification.
backmost_sq and frontmost_sq are redundant. It seems quite clear to always use frontmost_sq and use the correct color.
Non functional change.
Only called with Squares as argument, so remove unused variants.
As this is just syntax changes, only verified bench at high depth.
No functional change.
Adding a clamp function makes some of these range limitations a bit prettier and removes some #include's.
STC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 28117 W: 6300 L: 6191 D: 15626
http://tests.stockfishchess.org/tests/view/5c9aa1df0ebc5925cfff8fcc
Non functional change.
This is a non-functional code style change.
If we add an accessor function for SquareBB we can consolidate all of the asserts. This is also a bit cleaner because all SquareBB accesses go through this method making future changes easier to manage.
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 63406 W: 14084 L: 14045 D: 35277
http://tests.stockfishchess.org/tests/view/5c9ea6100ebc5925cfffc9af
No functional change.
This is non-functional. These 5 arrays are simple enough to calculate real-time and maintaining an array for them does not help. Decreases the memory footprint.
This seems a tiny bit slower on my machine, but passed STC well enough. Could someone verify speed?
STC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 44745 W: 9780 L: 9704 D: 25261
http://tests.stockfishchess.org/tests/view/5c47aa2d0ebc5902bca13fc4
The slowdown is minimal even in 32 bit case (thanks to @mstembera for testing):
Compiled using make build ARCH=x86-32 CXX=i686-w64-mingw32-c++ and benched
This patch only:
```
Results for 40 tests for each version:
Base Test Diff
Mean 1455204 1450033 5171
StDev 49452 34533 59621
p-value: 0.465
speedup: -0.004
```
No functional change.
This patch saves (4-1) * 64 * 64 = 12KiB of cache.
STC
LLR: 2.95 (-2.94,2.94) [0.00,4.00]
Total: 176120 W: 38944 L: 38087 D: 99089
http://tests.stockfishchess.org/tests/view/5c4c9f840ebc593af5d4a7ce
LTC
As a pure speed up, I've been informed it should not require LTC.
No functional change
This is a non-functional simplification that removes the AdjacentFiles array.
This array is simple enough to calculate that the pre-calculated array provides
no benefit. Reduces the memory footprint.
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 74839 W: 16390 L: 16373 D: 42076
http://tests.stockfishchess.org/tests/view/5c3d75920ebc596a450cfb67
No functionnal change
Exclude doubly protected by pawns squares when calculating attackers on
king ring. Idea of this patch is not to count attackers if they attack
only squares that are protected by two pawns.
STC
LLR: 2.95 (-2.94,2.94) [0.00,5.00]
Total: 70040 W: 15476 L: 15002 D: 39562
http://tests.stockfishchess.org/tests/view/5c0354860ebc5902bcee1106
LTC
LLR: 2.96 (-2.94,2.94) [0.00,5.00]
Total: 16530 W: 2795 L: 2607 D: 11128
http://tests.stockfishchess.org/tests/view/5c0385080ebc5902bcee14b5
This is third king safety patch in recent times so we probably need
retuning of king safety parameters.
Bench: 3057978
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
In current master, the function make_bitboard() does nothing apart from
helping initialize the SquareBB[] array. This seems like an unnecessary
abstraction layer.
The advantage of make_bitboard() is we can define a bitboard, in a simple
and general way, not only from a single square but also from a list of
squares. It is more elegant, faster and readable than combining multiple
SquareBB explicitly, but the last complex use case in evaluation was
simplified away a few months ago.
If make_bitboard() becomes useful again to define complicated bitboards,
it will be easy enough to reintroduce it using this pull request as
an implementation reference.
No functional change.
When we are using the "Bitboard + Square" overloaded operators,
the compiler uses the interpediate SquareBB[s] to transform the
square into a Bitboard, and then calculate the result.
For instance, the following code:
```
b = pos.pieces(Us, PAWN) & s
```
generates in fact the code:
```
b = pos.pieces(Us, PAWN) & SquareBB[s]`
```
The bug introduced by Stéphane in the previous patch was the
use of `b = pos.pieces(Us, PAWN) & (s + Up)` which can result
in out-of-bounds errors for the SquareBB[] array if s in the
last rank of the board.
We coorect the bug, and also add some asserts in bitboard.h to
make the code more robust for this particular bug in the future.
Bug report by Joost VandeVondele. Thanks!
Bench: 5512000
This is a patch to fix issue #1498, switching the time management variables
to 64 bits to avoid overflow of time variables after 25 days.
There was a bug in Stockfish 9 causing the output to be wrong after
2^31 milliseconds search. Here is a long run from the starting position:
info depth 64 seldepth 87 multipv 1 score cp 23 nodes 13928920239402
nps 0 tbhits 0 time -504995523 pv g1f3 d7d5 d2d4 g8f6 c2c4 d5c4 e2e3 e7e6 f1c4
c7c5 e1g1 b8c6 d4c5 d8d1 f1d1 f8c5 c4e2 e8g8 a2a3 c5e7 b2b4 f8d8 b1d2 b7b6 c1b2
c8b7 a1c1 a8c8 c1c2 c6e5 d1c1 c8c2 c1c2 e5f3 d2f3 a7a5 b4b5 e7c5 f3d4 d8c8 d4b3
c5d6 c2c8 b7c8 b3d2 c8b7 d2c4 d6c5 e2f3 b7d5 f3d5 e6d5 c4e5 a5a4 e5d3 f6e4 d3c5
e4c5 b2d4 c5e4 d4b6 e4d6 g2g4 d6b5 b6c5 b5c7 g1g2 c7e6 c5d6 g7g6
We check at compile time that the TimePoint type is exactly 64 bits long for
the compiler (TimePoint is our alias in Stockfish for std::chrono::milliseconds
-- it is a signed integer type of at least 45 bits according to the C++ standard,
but will most probably be implemented as a 64 bits signed integer on modern
compilers), and we use this TimePoint type consistently across the code.
Bug report by user "fischerandom" on the TCEC chat (thanks), and the
patch includes code and suggestions by user "WOnder93" and Ronald de Man.
Fixes issue: https://github.com/official-stockfish/Stockfish/issues/1498
Closes pull request: https://github.com/official-stockfish/Stockfish/pull/1510
No functional change.
Make kingRing always eight squares, extending the bitboard to the
F file if the king is on the H file, and to the C file if the king
is on the A file. This may deal with cases where Stockfish (like
many other engines) would shift the king around on the back rank
like g1h1, not because there is some imminent threat, but because
it makes king safety look a little better just because the king ring
had a smaller area.
STC:
LLR: 2.96 (-2.94,2.94) [0.00,5.00]
Total: 34000 W: 7167 L: 6877 D: 19956
http://tests.stockfishchess.org/tests/view/5ab8216d0ebc5902932cbe64
LTC:
LLR: 2.96 (-2.94,2.94) [0.00,5.00]
Total: 22574 W: 3576 L: 3370 D: 15628
http://tests.stockfishchess.org/tests/view/5ab84e6a0ebc5902932cbe72
How to continue from there?
This patch probably makes it easier to tune the king safety evaluation,
because the new regularity of the king ring size will make the king
safety function more continuous.
Closes https://github.com/official-stockfish/Stockfish/pull/1512
Bench: 5934103
To more clearly distinguish them from "const" local variables, this patch
defines compile-time local constants as constexpr. This is consistent with
the definition of PvNode as constexpr in search() and qsearch(). It also
makes the code more robust, since the compiler will now check that those
constants are indeed compile-time constants.
We can go even one step further and define all the evaluation and search
compile-time constants as constexpr.
In generate_castling() I replaced "K" with "step", since K was incorrectly
capitalised (in the Chess960 case).
In timeman.cpp I had to make the non-local constants MaxRatio and StealRatio
constepxr, since otherwise gcc would complain when calculating TMaxRatio and
TStealRatio. (Strangely, I did not have to make Is64Bit constexpr even though
it is used in ucioption.cpp in the calculation of constexpr MaxHashMB.)
I have renamed PieceCount to pieceCount in material.h, since the values of
the array are not compile-time constants.
Some compile-time constants in tbprobe.cpp were overlooked. Sides and MaxFile
are not compile-time constants, so were renamed to sides and maxFile.
Non-functional change.
The NO_BSF does not cover any real life use-case today. The only compilers that
can compile SF today, with the current Makefile and no source code changes, are
either GCC compatible (define __GNUC__) or MSVC compatible (define _MSC_VER). So
they all support LSB/MSB intrinsics.
This patch simplifies away the software fall-backs of LSB/MSB that were still
in Stockfish code, but unused in any of the officially supported compilers.
Note the (legacy) MSVC/WIN32 case, where we use a 32-bit BSF/BSR solution, as
64-bit intrinsics aren't available there.
Discussed in: https://github.com/official-stockfish/Stockfish/pull/1447
and: https://github.com/official-stockfish/Stockfish/pull/1479
No functional change.
This is one of the most difficult to understand but also
most important and speed critical functions of SF.
This patch rewrites some part of it to hopefully
make it clearer and drop some redundant variables
in the process.
Same speed than master (or even a bit more).
Thanks to Chris Cain for useful feedback.
No functional change.
Currently the NORTH/WEST/SOUTH/EAST values are of type Square, but conceptually they are not squares but directions. This patch separates these values into a Direction enum and overloads addition and subtraction to allow adding a Square to a Direction (to get a new Square).
I have also slightly trimmed the possible overloadings to improve type safety. For example, it would normally not make sense to add a Color to a Color or a Piece to a Piece, or to multiply or divide them by an integer. It would also normally not make sense to add a Square to a Square.
This is a non-functional change.
Avoid constructing, passing as a parameter and binding a useless empty tuple of pointers in the qsearch move picker constructor.
Also reformat the scoring function in movepicker.cpp and do some cleaning in evaluate.cpp while there.
No functional change.
