This is a non-functional simplification. Instead of passing the piece type
for remove_piece, we can rely on the board. The only exception is en-passant
which must be explicitly set because the destination square for the capture
is not the same as the piece to remove.
Verified also in the Chess960 castling case by running a couple of perft, see
the pull request discussion: https://github.com/official-stockfish/Stockfish/pull/2460
STC
LLR: 2.94 (-2.94,2.94) [-3.00,1.00]
Total: 18624 W: 4147 L: 4070 D: 10407
Ptnml(0-2): 223, 1933, 4945, 1938, 260
http://tests.stockfishchess.org/tests/view/5dfeaa93e70446e17e451163
No functional change
SEE (Static Exchange Evaluation) is a critical component, so we might
indulge some tricks to make it faster. Another pull request #2469 showed
some speedup by removing templates, this version uses Ronald de Man
(@syzygy1) SEE implementation which also unrolls the for loop by
suppressing the min_attacker() helper function and exits as soon as
the last swap is conclusive.
See Ronald de Man version there:
https://github.com/syzygy1/Cfish/blob/master/src/position.c
Patch testes against pull request #2469:
LLR: 2.95 (-2.94,2.94) {-1.00,3.00}
Total: 19365 W: 3771 L: 3634 D: 11960
Ptnml(0-2): 241, 1984, 5099, 2092, 255
http://tests.stockfishchess.org/tests/view/5e10eb135e5436dd91b27ba3
And since we are using new SPRT statistics, and that both pull requests
finished with less than 20000 games I also tested against master as
a speed-up:
LLR: 2.99 (-2.94,2.94) {-1.00,3.00}
Total: 18878 W: 3674 L: 3539 D: 11665
Ptnml(0-2): 193, 1999, 4966, 2019, 250
http://tests.stockfishchess.org/tests/view/5e10febf12ef906c8b388745
Non functional change
Store repetition info in StateInfo instead of recomputing it in
three different places. This saves some work in has_game_cycle()
where this info is needed for positions before the root.
Tested for non-regression at STC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 34104 W: 7586 L: 7489 D: 19029
http://tests.stockfishchess.org/tests/view/5cd0676e0ebc5925cf044b56
No functional change.
In master search() may incorrectly return a draw score in the following
corner case: there was a 2-fold repetition during the game, and the
current position can be reached by a move from a repeated one. This case
is treated as an upcoming 3-fold repetition, which it is not.
Here is a testcase demonstrating the issue (note that the moves
after FEN are required). The input:
position fen 8/8/8/8/8/8/p7/2k4K b - - 0 1 moves c1b1 h1g1 b1c1 g1h1 c1b1 h1g1 b1a1 g1h1
go movetime 1000
produces the output:
[...]
info depth 127 seldepth 2 multipv 1 score cp 0 [...]
bestmove a1b1
saying that the game will be drawn by repetion. However the other possible
move for black, Kb2, avoids repetitions and wins. The patch fixes this behavior.
In particular it finds mate in 10 in the above position.
STC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 10604 W: 2390 L: 2247 D: 5967
http://tests.stockfishchess.org/tests/view/5cb373e00ebc5925cf0167bf
LTC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 19620 W: 3308 L: 3185 D: 13127
http://tests.stockfishchess.org/tests/view/5cb3822f0ebc5925cf016b2d
Bench is not changed since it does not test positions with history of moves.
Bench: 3184182
Instead of looping through kfrom,kto, rfrom, rto, we can use BetweenBB. This is less lines of code and it is more clear what castlingPath actually is. Personal benchmarks are all over the place. However, this code is only executed when loading a position, so performance doesn't seem that relevant.
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 changes 2 parts with regards to static exchange evaluation.
Currently, we do not allow pinned pieces to recapture if *all* opponent
pinners are still in their starting squares. This changes that to having
a less strict requirement, checking if *any* pinners are still in their
starting square. This makes our SEE give more respect to the pinning
side with regards to exchanges, which makes sense because it helps our
search explore more tactical options.
Furthermore, we change the logic for saving pinners into our state
variable when computing slider_blockers. We will include double pinners,
where two sliders may be looking at the same blocker, a similar concept
to our mobility calculation for sliders in our evaluation section.
Interestingly, I think SEE is the only place where the pinners bitboard
is actually used, so as far as I know there are no other side effects
to this change.
An example and some insights:
White Bf2, Kg1
Black Qe3, Bc5
The move Qg3 will be given the correct value of 0. (Previously < 0)
The move Qd4 will be incorrectly given a value of 0. (Previously < 0)
It seems the tradeoff in search is worth it. Qd4 will likely be pruned
soon by something like probcut anyway, while Qg3 could help us spot
tactics at an earlier depth.
STC:
LLR: 2.96 (-2.94,2.94) [0.50,4.50]
Total: 62162 W: 13879 L: 13408 D: 34875
http://tests.stockfishchess.org/tests/view/5c4ba1a70ebc593af5d49c55
LTC: (Thanks to @alayant)
LLR: 3.40 (-2.94,2.94) [0.00,3.50]
Total: 140285 W: 23416 L: 22825 D: 94044
http://tests.stockfishchess.org/tests/view/5c4bcfba0ebc593af5d49ea8
Bench: 3937213
Delay legality check of castling moves at search time,
just before making the move, as is the standard with all
the other move types.
This should avoid an useless and not trivial legality check
when the castling is then not tried later. For instance due
to a previous cut-off.
The patch is also a big simplification and allows to entirely
remove generate_castling()
Bench changes due to a different move sequence out of MovePicker.
STC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 45073 W: 9918 L: 9843 D: 25312
http://tests.stockfishchess.org/tests/view/5c2f176f0ebc596a450bdfb3
LTC:
LLR: 3.15 (-2.94,2.94) [-3.00,1.00]
Total: 10156 W: 1707 L: 1560 D: 6889
http://tests.stockfishchess.org/tests/view/5c2e7dfd0ebc596a450bcdf4
Verified with perft both in standard and Chess960 cases.
Closes https://github.com/official-stockfish/Stockfish/pull/1929
Bench: 3559104
I've gone through the RENAME/REFORMATTING thread and changed everything I could find, plus a few more. With this, let's close the previous issue and open another.
No functional change.
This reverts commit 33d9548218 ,
which crashed in DEBUG mode because of the following assert in position.h
````
Assertion failed: (is_ok(m)), function capture, file ./position.h, line 369.
````
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
For the rationale to allow this, see commit
a66c73deef
This was broken when cuckoo hashing was added, and
subtly broke (for example) lichess' Android application,
thus illustrating the original judgement was sound.
No functional change.
The function Position::has_repeated() is used by Tablebases::root_probe()
to determine whether we can rank all winning moves with the same value, or
if we need to strictly rank by dtz in case the position has already been
repeated once, and we are risking to run into the 50-move rule and thus
losing the win (especially critical in some very complicated endgames).
To check whether the current position or one of the previous positions
after the last zeroing move has already been occured once, we start looking
for a repetition of the current position, and if that is not the case, we
step one position back and repeat the check for that position, and so on.
If you now look at how this was done before the new root ranking patch was
merged two months ago, it seems quite obvious that it is a simple oversight:
108f0da4d7
More specifically, after we stepped one position back with
```
stc = stc->previous;
```
we now have to start checking for a repetition with
```
StateInfo* stp = stc->previous->previous;
```
and not with
```
StateInfo* stp = st->previous->previous;
```
Closes https://github.com/official-stockfish/Stockfish/pull/1625
No functional change
Simplifying away all the progressKey stuff gives exactly the same bench,
without any speed impact. Tested for speed against master with two benches
at depth 22 ran in parallel:
**testedpatch**
Total time (ms) : 92350
Nodes searched : 178962949
Nodes/second : 1937877
**master**
Total time (ms) : 92358
Nodes searched : 178962949
Nodes/second : 1937709
We also tested the patch at STC for no-regression with [-3, 1] bounds:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 57299 W: 11529 L: 11474 D: 34296
http://tests.stockfishchess.org/tests/view/5b015a1c0ebc5914abc126e5
Closes https://github.com/official-stockfish/Stockfish/pull/1603
No functional change.
A position which has a move which draws by repetition, or which could have
been reached from an earlier position in the game tree, is considered to be
at least a draw for the side to move.
Cycle detection algorithm by Marcel van Kervink:
https://marcelk.net/2013-04-06/paper/upcoming-rep-v2.pdf
----------------------------
How does the algorithm work in practice? The algorithm is an efficient
method to detect if the side to move has a drawing move, without doing any
move generation, thus possibly giving a cheap cutoffThe most interesting
conditions are both on line 1195:
```
if ( originalKey == (progressKey ^ stp->key)
|| progressKey == Zobrist::side)
```
This uses the position keys as a sort-of Bloom filter, to avoid the expensive
checks which follow. For "upcoming repetition" consider the opening Nf3 Nf6 Ng1.
The XOR of this position's key with the starting position gives their difference,
which can be used to look up black's repeating move (Ng8). But that look-up is
expensive, so line 1195 checks that the white pieces are on their original squares.
This is the subtlest part of the algorithm, but the basic idea in the above game
is there are 4 positions (starting position and the one after each move). An XOR
of the first pair (startpos and after Nf3) gives a key matching Nf3. An XOR of
the second pair (after Nf6 and after Ng1) gives a key matching the move Ng1. But
since the difference in each pair is the location of the white knight those keys
are "identical" (not quite because while there are 4 keys the the side to move
changed 3 times, so the keys differ by Zobrist::side). The loop containing line
1195 does this pair-wise XOR-ing.
Continuing the example, after line 1195 determines that the white pieces are
back where they started we still need to make sure the changes in the black
pieces represents a legal move. This is done by looking up the "moveKey" to
see if it corresponds to possible move, and that there are no pieces blocking
its way. There is the additional complication that, to match the behavior of
is_draw(), if the repetition is not inside the search tree then there must be
an additional repetition in the game history. Since a position can have more
than one upcoming repetition a simple count does not suffice. So there is a
search loop ending on line 1215.
On the other hand, the "no-progress' is the same thing but offset by 1 ply.
I like the concept but think it currently has minimal or negative benefit,
and I'd be happy to remove it if that would get the patch accepted. This
will not, however, save many lines of code.
-----------------------------
STC:
LLR: 2.95 (-2.94,2.94) [0.00,5.00]
Total: 36430 W: 7446 L: 7150 D: 21834
http://tests.stockfishchess.org/tests/view/5afc123f0ebc591fdf408dfc
LTC:
LLR: 2.96 (-2.94,2.94) [0.00,5.00]
Total: 12998 W: 2045 L: 1876 D: 9077
http://tests.stockfishchess.org/tests/view/5afc2c630ebc591fdf408e0c
How could we continue after the patch:
• The code in search() that checks for cycles has numerous possible variants.
Perhaps the check need could be done in qsearch() too.
• The biggest improvement would be to get "no progress" to be of actual benefit,
and it would be helpful understand why it (probably) isn't. Perhaps there is an
interaction with the transposition table or the (fantastically complex) tree
search. Perhaps this would be hard to fix, but there may be a simple oversight.
Closes https://github.com/official-stockfish/Stockfish/pull/1575
Bench: 4550412
This patch corrects both MultiPV behaviour and "go searchmoves" behaviour
for tablebases.
We change the logic of table base probing at root positions from filtering
to ranking. The ranking code is much more straightforward than the current
filtering code (this is a simplification), and also more versatile.
If the root is a TB position, each root move is probed and assigned a TB score
and a TB rank. The TB score is the Value to be displayed to the user for that
move (unless the search finds a mate score), while the TB rank determines which
moves should appear higher in a multi-pv search. In game play, the engine will
always pick a move with the highest rank.
Ranks run from -1000 to +1000:
901 to 1000 : TB win
900 : normally a TB win, in rare cases this could be a draw
1 to 899 : cursed TB wins
0 : draw
-1 to -899 : blessed TB losses
-900 : normally a TB loss, in rare cases this could be a draw
-901 to -1000 : TB loss
Normally all winning moves get rank 1000 (to let the search pick the best
among them). The exception is if there has been a first repetition. In that
case, moves are ranked strictly by DTZ so that the engine will play a move
that lowers DTZ (and therefore cannot repeat the position a second time).
Losing moves get rank -1000 unless they have relatively high DTZ, meaning
they have some drawing chances. Those get ranks towards -901 (when they
cross -900 the draw is certain).
Closes https://github.com/official-stockfish/Stockfish/pull/1467
No functional change (without tablebases).
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.
