- Fix a skill level problem: Don't allow move pruning at root node
- Revert "Fix profile build for gcc on Mac OSX". Results for a faster binary in x86-64.
- Fix a MSVC warning
Bench: 8918745
Seems to be a performance regression for standard build.
For SF6 people compiling on Mac OSX using profile-build option
just need to make necessary adjustments manually...
No functional change
Resolves#223
Warning is C4512 (assignment operator could not be generated)
Now, apart the foreign syzygy code, everything compiles
without warnings at warning level 4.
Backported from C++11 branch.
No functional change.
- Fix a compilation issue related to BMI2 PEXT instruction
- Retrieve a ponder move from TT if PV is only one move long
Bench: 8080602
No functional change
This intrinsic to call BMI2 PEXT instruction is
defined in immintrin.h. This header should be
included only when USE_PEXT is defined, otherwise
we define _pext_u64 as 0 forcing a nop.
But under some mingw platforms, even if we don't
include the header, immintrin.h gets included
anyhow through an include chain that starts with
STL <algorithm> header. So we end up both defining
_pext_u64 function and at the same time defining
_pext_u64 as 0 leading to a compile error.
The correct solution is of not using _pext_u64 directly.
This patch fixes a compile error with some mingw64
package when compiling with x86-64.
No functional change.
Resolves#222
In case we stop the search during a fail-high
it is possible we return to GUI without a ponder
move. This patch try harder to find a ponder move
retrieving it from TT. This is important in games
played with 'ponder on'.
bench: 8080602
Resolves#221
On platforms where size_t is 32 bit, we
can have an overflow in this expression:
(mbSize * 1024 * 1024)
Fix it setting max hash size of 2GB on platforms
where size_t is 32 bit.
A small rename while there: now struct Cluster
is definied inside class TranspositionTable so
we should drop the redundant TT prefix.
No functional change.
On Android-ARM current TB code crashes at
random times even in single thread mode.
Reported, debugged, fixed and verified
by Peter Osterlund.
No functional change.
Resolves#201
This optimization is aimed at old hardware only (withouth popcount), and even on
non popcount compile (ARCH=x86-64), it provides no mesurable speedup:
stat test master diff
mean 2,341,779 2,354,699 -12,920
stdev 12,910 14,770 18,150
speedup -0.55%
P(speedup>0) 23.8%
No functional change.
Resolves#187
- Change UCI::value() signature
This function should only return the value,
lowerbound and upperbound info is up to the
caller because it requires external knowledge,
out of the scope of this little helper.
- Retire 'key' command
It is not an UCI command and is absolutely
useless: never used.
- Comments fixing and other trivia
No functional change.
And reshuffle a bit the functions to place
them in a consistent order.
To be on the safe side, patch has been
validated for no regression/crashes with
a small 8K games test with 3 threads:
ELO: 3.98 +-4.4 (95%) LOS: 96.3%
Total: 8388 W: 1500 L: 1404 D: 5484
No functional change.
It is up to material (and pawn) table look up
code to know where the per-thread tables are,
so change API to reflect this.
Also some comment fixing while there
No functional change.
Move all in evaluation.
Simplify the code and concentrate in a single place
all the logic behind space evaluation, making it much
more clear.
Verified also at STC it does not regress due to a possible
slow down:
LLR: 3.91 (-2.94,2.94) [-3.00,1.00]
Total: 65744 W: 13285 L: 13194 D: 39265
No functional change.
Commenst are obsolete now, an updated description
would be quite obscure, so better let the code
to talk and remove them all together.
No functional change.
Use the same template of other pawns moves generation,
make the code more uniform, simplify generate_promotions
that has now been renamed.
No functional change (verified also with perft).
In some UNIX systems "rm" prompts user for confirmation.
However "rm -f" is always a guaranteed forced deletion.
Also move gcc profiling hack under the correct target
No Functional change
Resolves#168
ShelterWeakness and Stormdanger array are now indexed additionally by
file pair (a/h,b/g,c/f,d/e). The special case of king blocking a pawn
is incorporated in the StormDanger array. Finally the 93 parameters
are tuned by SPSA on LTC.
STC
ELO: 3.46 +-2.2 (95%) LOS: 99.9%
Total: 40000 W: 8275 L: 7877 D: 23848
LTC
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 10311 W: 1876 L: 1721 D: 6714
Bench: 9498821
Resolves#163
The evaluation is already done by the specialized
function, don't need to add something elese later.
With this patch following positions are evaluated
correctly as draws:
8/6p1/1Pkp1p1p/2nNn2P/2P1K1P1/8/8/3B4 w - - 7
8/1k4p1/1P1p1p1p/3NnK1P/2P3P1/1n6/4B3/8 w - -
Verified it not regress with an STC test:
LLR: 3.15 (-2.94,2.94) [-3.00,1.00]
Total: 49812 W: 10095 L: 10016 D: 29701
Reported by Arjun Temurnikar.
bench: 8289983
In particular seems more natural to return
bool and TTEntry on the same line, actually
we should pass and return them as a pair,
but due to limitations of C++ and not wanting
to use std::pair this can be an acceptable
compromise.
No functional change.
Resolves#157
In some cases we want to go direcly to the moves loop
without checking for early return. The patch make this
logic more clear and consistent.
Tested for no regression, passed STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 25282 W: 5136 L: 5022 D: 15124
and LTC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 72007 W: 12133 L: 12095 D: 47779
bench: 9316798
This patch replaces RKISS by a simpler and faster PRNG, xorshift64* proposed
by S. Vigna (2014). It is extremely simple, has a large enough period for
Stockfish's needs (2^64), requires no warming-up (allowing such code to be
removed), and offers slightly better randomness than MT19937.
Paper: http://xorshift.di.unimi.it/
Reference source code (public domain):
http://xorshift.di.unimi.it/xorshift64star.c
The patch also simplifies how init_magics() searches for magics:
- Old logic: seed the PRNG always with the same seed,
then use optimized bit rotations to tailor the RNG sequence per rank.
- New logic: seed the PRNG with an optimized seed per rank.
This has two advantages:
1. Less code and less computation to perform during magics search (not ROTL).
2. More choices for random sequence tuning. The old logic only let us choose
from 4096 bit rotation pairs. With the new one, we can look for the best seeds
among 2^64 values. Indeed, the set of seeds[][] provided in the patch reduces
the effort needed to find the magics:
64-bit SF:
Old logic -> 5,783,789 rand64() calls needed to find the magics
New logic -> 4,420,086 calls
32-bit SF:
Old logic -> 2,175,518 calls
New logic -> 1,895,955 calls
In the 64-bit case, init_magics() take 25 ms less to complete (Intel Core i5).
Finally, when playing with strength handicap, non-determinism is achieved
by setting the seed of the static RNG only once. Afterwards, there is no need
to skip output values.
The bench only changes because the Zobrist keys are now different (since they
are random numbers straight out of the PRNG).
The RNG seed has been carefully chosen so that the
resulting Zobrist keys are particularly well-behaved:
1. All triplets of XORed keys are unique, implying that it
would take at least 7 keys to find a 64-bit collision
(test suggested by ceebo)
2. All pairs of XORed keys are unique modulo 2^32
3. The cardinality of { (key1 ^ key2) >> 48 } is as close
as possible to the maximum (65536)
Point 2 aims at ensuring a good distribution among the bits
that determine an TT entry's cluster, likewise point 3
among the bits that form the TT entry's key16 inside a
cluster.
Details:
Bitset card(key1^key2)
------ ---------------
RKISS
key16 64894 = 99.020% of theoretical maximum
low18 180117 = 99.293%
low32 305362 = 99.997%
Xorshift64*, old seed
key16 64918 = 99.057%
low18 179994 = 99.225%
low32 305350 = 99.993%
Xorshift64*, new seed
key16 65027 = 99.223%
low18 181118 = 99.845%
low32 305371 = 100.000%
Bench: 9324905
Resolves#148
Currently Search::RootMoves is accessed and even
modified by TB probing functions in a hidden
and sneaky way.
This is bad practice and makes the code tricky.
Instead explicily pass the vector as function
argument so to clarify that the vector is modified
inside the functions.
No functional change.
Simplified also some logic while there.
TBLargest needs renaming too, but itis for
a future patch because touches also syzygy
directory stuff.
No functional change.
We really don't need to uglify in this way
our nice count() API with this ad-hoc hack.
So remove the hack and use the already
existing infrastructure.
No functional change.
Resolves#134
Updating the makefile so that the clean and gcc-profile-clean targets also
remove the profiling data files in the syzygy directory.
No functional change.
Resolves#136
Just like in Physics, the ratio of 2 things in the same unit, should be
without unit.
Example use case:
- Ratio of a Depth by a Depth (eg. ONE_PLY) should be an int.
- Ratio of a Value by a Value (eg. PawnValueEg) should be an int.
Remove a bunch of useless const while there.
No functional change.
Resolves#128
Adds support for Syzygy tablebases to Stockfish. See
the Readme for information on using the tablebases.
Tablebase support can be enabled/disabled at the Makefile
level as well, by setting syzygy=yes or syzygy=no.
Big/little endian are both supported.
No functional change (if Tablebases are not used).
Resolves#6
It is possible that we won't have a ponder move if our PV
is too short. In that case, just don't print a ponder move.
No functional change
Resolves#130
When evaluating double pawns we use always
lsb() to extract the frontmost square.
This breaks evaluation color symmetry as is
possible to verify with an instrumented evaluate()
Value evaluate(const Position& pos) {
Value v = do_evaluate<false>(pos);
Position p = pos;
p.flip();
assert(v == do_evaluate<false>(p));
return v;
}
Passed no regression test:
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21035 W: 4244 L: 4122 D: 12669
LTC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 39839 W: 6662 L: 6572 D: 26605
bench: 8255966
It is a non functional change, but because
we now skip copying of pv[] in SpNode, patch
has been tested for regression with 3 threads:
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 54668 W: 9873 L: 9809 D: 34986
No functional change.
This is a regression from 428962a
We have to cast to char here, otherwise the compiler
interprets it as an integer, and writes a number.
No functional change
Resolves#122
This is the first of a patch series to
rearrange and simplify accurate PV.
In this patch there is simple coding
style and reformatting stuff.
Verified with fishtest it does not crash
with MAX_PLY = 8
No functional change.
Previously token would keep its value from the previous line when an empty
line was input, leading to unexpected behaviour.
No functional change
Resolves#119
When comparing to a Depth it is more
consistent to use DEPTH_MAX instead
of a int.
This is a subtle difference because we use
ply and depth almost interchangably in SF,
but they are different. FOr counting plies
makes ense to continue using ints, while
for Depth we have our specific enum.
This cleanly fixes a new Clang 3.5 warning:
No functional change.
This gives SF accurate PVs, such that the evaluation of the leaf node in
the PV matches the score backed up to the root (99% of the time.
q-search will use the value stored in the hash table instead of the eval
value sometimes).
One drawback is that fail-high/low only get a minimal 2 move PV.
It doesn't add any additional overhead to the non-PV codepath except an
extra eight bytes to the SearchStack structure in multi-threaded
searches.
A core part of this is not pruning based on TT score in PV nodes. This
was measured as not being a regression at multiple TCs, except for one
exception, fast TC with huge hash, which is not realistic for longer
searches.
STC - 1 thread, 128 mb hash
ELO: 1.42 +-3.1 (95%) LOS: 81.9%
Total: 20000 W: 4078 L: 3996 D: 11926
STC - 3 thread, 128 mb hash
ELO: -3.60 +-2.9 (95%) LOS: 0.8%
Total: 20000 W: 3575 L: 3782 D: 12643
STC - 3 thread, 8 mb hash
ELO: 0.12 +-2.9 (95%) LOS: 53.3%
Total: 20000 W: 3654 L: 3647 D: 12699
LTC - 3 thread, 32mb hash
ELO: 2.29 +-2.0 (95%) LOS: 98.8%
Total: 35740 W: 5618 L: 5382 D: 24740
Bench: 6984058
Resolves#102
Daniel Jose reported that it was an elo gain in his engine:
http://www.talkchess.com/forum/viewtopic.php?t=54290
STC: Hash=16
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 33067 W: 6670 L: 6571 D: 19826
LTC: Hash=64
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 41181 W: 7008 L: 6920 D: 27253
And another one to verify no regression with hash pressure:
STC: Hash=4
LLR: 2.96 (-2.94,2.94) [-4.00,0.00]
Total: 25085 W: 5059 L: 4991 D: 15035
Verified that qsearch does not explode after this patch (recapture threshold).
Bench 7962287
Resolves#112
Real men jump/branch by hand...but
we prefer the humble way.
Moved also some uci info code where it
belongs, while there.
No functional change.
Resolves#110
16MB for 1" searches is more comensurate with the average use case.
And 16 is the default used by stockfish bench, so it makes sense to be
consistent, if only to have the same minimum memory requirement for using
SF and compiling it with PGO.
No functional change.
Resolves#109
Now we can directly replace it with
the definition resulting in simpler
and possibly faster code because
PvNode is evaluated at compile time.
No functional change.
* remove some erroneous comments, that were based on the ONE_PLY == 2.
* rename hd to d, because there's no more half-depth in SF.
* rescope variables into the for loops.
* reindent the for loops correctly.
* add a comment to explain the eval improving part (not so obvious to read
this code as array has 4 dimensions).
No functional change.
This should reduce search inconsistencies, and doesn't seem to have a measurable ELO Impact:
STC with Hash=16
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 49264 W: 10076 L: 10007 D: 29181
LTC with Hash=64
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 82149 W: 14044 L: 14023 D: 54082
Plus an extra test, to make sure it doesn't regress with strong hash pressure:
STC with Hash=4
LLR: 2.95 (-2.94,2.94) [-4.00,0.00]
Total: 21498 W: 4327 L: 4246 D: 12925
Bench: 7302735
Resolves#100
Idea is to apply king safety later in the endgame. Previously, we didn't
apply KS in a RR vs. Q ending for example, which causes poor play.
Now we calculate king attacks when the attacking side has a queen or more.
STC with 8moves_v3
LLR: 3.06 (-2.94,2.94) [0.00,4.00]
Total: 38481 W: 6228 L: 5952 D: 26301
LTC with 2moves_v1
LLR: 2.95 (-2.94,2.94) [0.00,4.00]
Total: 51053 W: 8670 L: 8353 D: 34030
Bench: 7514010
Resolves#98
UCI specification is not clear on the size of
integers that are exchanged in the protocol, so
instead of a simple int, assume 'nodes' is a
int64_t because we need a bigger size to store
this value in many real cases, especialy with
very long searches.
No functional change.
Resolves#75
Clang 3.5 issues warning on constructs like: abs(f1 - f2). The thing is that
f1 and f2 are enum types, and the range given (all positive) allows the
compiler to choose an unsigned type (efficiency being one reason to prefer
unsigned arithmetic). If f1 < f2 are unsigned, then f1 - f2 wraps around zero
and the abs() becomes a no-op. It's the reinterpretation of the unsigned
result (large value) as a signed int that happens to give the correct result,
thanks to 2's complement. This is all tricky and dangerous!
In the spirit of the standard, we assume nothing on the signedness of enums,
and simply calculate the rank and file distances as:
- rank_dist(r1, r2) = r1 < r2 ? r2 - r1 : r1 - r2
- file_dist(f1, f2) = f1 < f2 ? f2 - f1 : f1 - f2
this logic can in fact be applied to any enum we may use, so for better
generality and to avoid code duplication, we use a template function diff()
here.
No functional change.
Resolves#95
This area has become obscure and tricky over the course of incremental
changes that did not respect the original's consistency and clarity. Now,
it's not clear why we use MAX_PLY = 120, or why we use MAX_PLY+6, among
other things.
This patch does the following:
* ID loop: depth ranges from 1 to MAX_PLY-1, and due to TT constraint (depth
must fit into an int8_t), MAX_PLY should be 128.
* stack[]: plies now range from 0 to MAX_PLY-1, hence stack[MAX_PLY+4],
because of the extra 2+2 padding elements (for ss-2 and ss+2). Document this
better, while we're at it.
* Enforce 0 <= ply < MAX_PLY:
- stop condition is now ss->ply >= MAX_PLY and not ss->ply > MAX_PLY.
- assert(ss->ply < MAX_PLY), before using ss+1 and ss+2.
- as a result, we don't need the artificial MAX_PLY+6 range. Instead we
can use MAX_PLY+4 and it's clear why (for ss-2 and ss+2).
* fix: extract_pv_from_tt() and insert_pv_in_tt() had no reason to use
MAX_PLY_PLUS_6, because the array is indexed by plies, so the range of
available plies applies (0..MAX_PLY before, and now 0..MAX_PLY-1).
Tested with debug compile, using MAX_PLY=16, and running bench at depth 17,
using 1 and 7 threads. No assert() fired. Feel free to submit to more severe
crash-tests, if you can think of any.
No functional change.
SSE4.2 has nothing to do with POPCNT. We must dispell this myth, because
Stockfish is a reference and many will copy this mistake if they see it in Stockfish:
* SSE is an SIMD instruction set, relative to vectorization (using special 128-bit registers).
* POPCNT/LZCNT work on normal registers (eg. AL, AX, EAX, RAX).
The confusion comes from the fact that, in the Intel product line, it just
so happens that SSE4.2 and POPCNT/LZCNT came out at the same time. But this
is not true for AMD. For example, all AMD Pheniom II have SSE3 but no
POPCNT/LZCNT, and that is why the modern compile uses -msse3 -popcnt and not -msse4.2.
No functional change.
Resolves#86
Objects that are only accessible at file-scope should be put in the anonymous namespace.
This is what the C++ standard recommends, rather than using static, which is really C-style and results in static linkage.
Stockfish already does this throughout the code. So let's weed out the few exceptions,
because... they have no reason to be exceptional.
No functional change.
Resolves#84
It is more idiomatic, we didn't used it
in the past because Position::pretty(Move)
had a calling argument, but now we can.
As an added benefit, we avoid a lot of string
copies in the process because now we avoid
std::ostringstream ss.
No functional change.
This commit fixes two issues:
1) Don't print PVs after the search has been interrupted
This solves the "mate 0 upperbound" scores that sometimes
creep up when a multi-PV analysis gets interrupted with
the `stop` command.
2) Print multipv before score
Shredder Classic fails to identify the main PV
(the one with multipv 1) if `score` comes first.
This leads to an eval graph that doesn't reflect
the scores actually reported by Stockfish when
doing a multiPV analysis.
No functional change
Closes#76
Helper function should just know how to find the
biggest piece type in a bitboard. All the threat
logic and data shoud be in evaluate_threats().
This nicely separates the scope of the two functions
in a more consistent way and simplifies the code.
No functional change.
Use the max_threat() helper function to estimate more precisely the
best hanging piece threat. Also retunes the Threat array using SPSA.
STC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 7598 W: 1596 L: 1468 D: 4534
LTC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 7896 W: 1495 L: 1350 D: 5051
Bench: 6816504
Resolves#73
Instead of hard-code the weights in a big table,
we prefer to calculate them out of few parameters
at startup. This allows to keep low the number of
independent parameters and hence is good for tuning
and for a better insight in the meaning of the numbers.
No functional change.
Make even more clear what are the terms that
contribute to evaluate connected pawns, and
completely separate them from the weights
that are now fully looked up in a table.
For future tuning makes sense to init the table with
a formula instead of hard-code it. This allows to
reduce problem space cardinality and makes tuning
easier.
And fix a MSVC warning while there:
warning C4804: '>>' : unsafe use of type 'bool' in operation
No functional change.
These two notions are very correlated. Since connected has the most
generality, it makes sense to generalize it to encompass what is
covered by candidate.
STC:
LLR: 4.03 (-2.94,2.94) [-3.00,1.00]
Total: 11970 W: 2577 L: 2379 D: 7014
LTC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 13194 W: 2389 L: 2255 D: 8550
bench 7328585
Now that half plies have been removed from the engine, we can encode
TT depth into an int8_t.
Range is -128 to +127, so it goes still further than the previous
limit of 121 plies (with ONE_PLY == 2 where depth - DEPTH_NONE was
encoded as an uint8_t).
No functional change.
Resolved#60
Now that half-plies are no more used we can simplify
the code assuming that ONE_PLY is 1 and no more 2.
Verified with a SMP test:
LLR: 2.95 (-2.94,2.94) [-4.50,0.00]
Total: 8926 W: 1712 L: 1607 D: 5607
No functional change.
On top of previous patch, rename time variables to
reflect the simplification of UCI parameters.
It is more correct to use as varibales directly the
corresponding UCI option, without intorducing redundant
intermediate variables.
This allows also to simplify the code.
No functional change.
In case of a succesful late join we set again
'searching' flag, so we can restart search
immediately without an useless lock/unlock
cycle.
No functional change.
Now "Write Search Log" will pring moves in UCI format, consistent with all the rest. This functionality is
not aimed at end-users anyway. It's hardly useful at all, in fact. Also, pretty-printing SAN moves is
something that better belongs in the GUI than in the engine.
No functional change.
Where they better belong.
Also, this removes '#include <string>' from types.h, which reduces the amount of code to compile (every
translation unit includes types.h).
No functional change.
Unify various perft functions and move all the code
to search.cpp.
Avoid perft implementation to be splitted between
benchmark.cpp (where it has no reason to be) and
search.cpp
No functional and no speed change (tested).
So that one can redirect cout to /dev/null and only print print cerr in the terminal (for more accurate speed
tests).
Suggested by Marco.
No functional change.
The compiler tries to cast Options["Hash"] into a string, using:
Option::operator std::string() const {
assert(type == "string");
return currentValue;
}
And, as expected, the assert() fails.
std::to_string() would be the right solution, but it's C++11. And using a stringstream is too much code to
achieve so little. Let's keep it the way it was: hardcoded (ie. default hash defined in two places).
No functional change.
The eval already returns zero in KK, KBK, KNK (see material.cpp). The difference is:
- we lose the "TB pruning" benefit of the draw rule (ie. search goes on even if eval is zero)
- we gain some speed by removing a useless test from the hot path
STC:
LLR: 0.05 (-2.94,2.94) [-3.00,1.00]
Total: 128000 W: 21357 L: 21560 D: 85083
LTC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 33023 W: 4613 L: 4509 D: 23901
bench 7461881
First, remove some dead code (function never called with a Move argument).
Then, remove printing of legal moves, which does not belong here. Let's keep commands orthogonal and minimal:
- the "d" command should display the board, nothing more, or less.
- "perft 1" will display the list of legal moves.
No functional change.
Stockfish allocates the default hash (32MB) in main(), before entering UCI::loop(). If there is not enough
memory, the program will crash even before UCI::loop() is entered and the GUI is given a change to specify a
lower Hash value.
This defective design could be resolved by doing a lazy allocation upon "isready" command, as the UCI protocol
guarantees that "isready" will be sent at least once before any search. But it's a bit cumbersome when using
Stockfish "manually" to have to remember to type "isready" everytime.
So leave the current design, but reduce the default hash to 16MB instread of 32MB. In order to perform such
quick searches (depth=13), there is no reason to use so much Hash anyway. Another benefit is to introduce a
bit of hash pressure in bench, which increases chances to detect rare bugs related to TT replacement, for
example.
This is not a functional change, although it obviously changes the bench.
bench 7461879
Instead of defining it both in ucioption.cpp and benchmark.cpp. Obviously changing the default Hash will
change the bench as a result.
No functional change.
The main purpose of perft is to help debugging. But without the breakdown in sum of perft(N-1), it is a
completely useless debugging tool.
So perft now displays the breakdown, and divide is therefore removed.
No functional change.
After commit 94b1bbb68b, in case available root moves are less than multiPV, we
could never reach condition:
PVIdx + 1 == multiPV
and as a consequence UCI output is not printed.
Fixed suggested by Joerg Oster.
No functional change.
Despite being neutral at STC, it turned out to be regressive at LTC:
40k games at LTC with Hash=8
ELO: -2.06 +-1.9 (95%) LOS: 1.4%
Total: 39720 W: 5740 L: 5976 D: 28004
40k games at LTC with Hash=128
ELO: -2.69 +-1.9 (95%) LOS: 0.2%
Total: 39149 W: 5702 L: 6005 D: 27442
bench 7477963
Also raise the admissible bounds to (-100,100), as there is no reason to prevent users from using high
values if they want to.
Does not regress in self play:
ELO: 0.10 +-2.0 (95%) LOS: 53.7%
Total: 40000 W: 7084 L: 7073 D: 25843
master vs SF 3
ELO: 182.86 +-2.7 (95%) LOS: 100.0%
Total: 40000 W: 21843 L: 2541 D: 15616
Contempt = 20 vs SF 3
ELO: 189.25 +-2.8 (95%) LOS: 100.0%
Total: 40000 W: 22721 L: 2859 D: 14420
Diff is therefore 6.4 +/- 3.9 elo against a 180-190 elo weaker engine, which is significantly positive,
as expected. This elo difference is likely understated, because of FishTest aggressive draw adjudication
though.
We could push Contempt further, but after 20cp, it would get in the way of FishTest draw adjudication
rule, and is likely to reduce the testing throughput as a result.
bench 8198667
Steamline a bit the implementation of
skill levels. As a side effect we can
retire MultiPV global and use a local
variable instead.
No functional change.
- Currently broken
- Never been really useful
- Does not work well with new splitting model
Verified for no regression at STC with 3 threads:
LLR: 2.96 (-2.94,2.94) [-6.00,0.00]
Total: 81905 W: 12122 L: 12381 D: 57402
No functional change
Bitboard init code is already noteasy to follow,
so don't make it even harder using 'smart' code.
Also reindent a while loop in standard way.
No functional change.
With Eelco's patch "Don't special case for abs(beta) >= VALUE_MATE_IN_MAX_PLY" condition "abs(ttValue) < VALUE_KNOWN_WIN" has been removed from singular extension search, and condition "abs(beta) < VALUE_KNOWN_WIN" was added to the SingularExtensionNode definition.
This might lead to problems, especially in positions, where a mate is due.
For example, this position 5rk1/4K1pp/8/5PPP/8/8/8/1R6 w - - 12 1 triggers an assert.
stockfish: search.cpp:434: Value {anonymous}::search(Position&, Search::Stack*, Value, Value, Depth, bool) [with {anonymous}::NodeType NT = (<unnamed>::NodeType)2u; bool SpNode = false]: Assertion `-VALUE_INFINITE <= alpha && alpha < beta && beta <= VALUE_INFINITE' failed.
So let's re-insert the removed condition.
First spotted by Uri Blass, fix by me.
Bench: 8759675
* /boot/common was removed from Haiku
* The equivalent path now that package management has been implemented is /boot/system/non-packaged
No functional change
Bench: 8759681
The assert:
assert(ttValue != VALUE_NONE);
Could fire for multiple reasons (although is very rare),
for instance after an IID we can have ttMove != MOVE_NONE
while ttValue is still set at VALUE_NONE.
But not only this, actually SMP is a source of corrupted
ttValue and anyhow we can detect the condition:
ttMove != MOVE_NONE && ttValue == VALUE_NONE
even north of IID.
Reported by Ronald de Man.
It is so rare that bench didn't change.
bench: 7710548
Remove from the search this special case and apply
null search and razoring also in mate positions.
Tested in no-regression mode and passed both
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 65431 W: 10860 L: 10810 D: 43761
and LTC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 34928 W: 4814 L: 4713 D: 25401
This patch kicks in only in mate positions and in
these cases it seems beneficial in finding mates
faster as Yery Spark measured on the Chest mate suite:
Total number of positions 6425
Fixed nodes 200K per position
master: 1049
new: 1154
And also the 5446 'hard' positions again with 2000K nodes
(those not found by both engines in 200K nodes):
master: 1069
new: 1395
bench: 7710548
It seems this flag is only for gcc and
yields a warning under OSX Mavericks:
clang: warning: argument unused during compilation: '-ansi'
No functional change.
Here MSVC is worried that
StepAttacksBB[PAWN][psq]
could overflow, so change psq initialization
to clarify psq is always less than 64.
No functional change.
Don't take the split lock if we don't have
available slaves (about 30-40% of times).
This new condition allows to retire the now
redundant one on number of threads.
No functional change.
Split previous patch in 2 steps: first remove
the MOVE_NULL hack, then retire nullChild.
The first step is a prerequisite
for second one and affects bench.
The second step (next patch) just removes nullChild
without affecting bench.
bench: 8205159
Are broken for big-endian case and
I have verified with MSVC 2013 Premium
bench is correct and there is no
miscompilation, so the main reason
to change the original code drops.
No functional change.
Another attempt at retiring current asymmetric
king evaluation and use a much simpler symmetric
one. As a good side effect we can avoid recalculating
eval after a null move.
Tested in no-regression mode and passed
STC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21580 W: 3752 L: 3632 D: 14196
LTC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 18253 W: 2593 L: 2469 D: 13191
And a LTC regression test against SF DD to
verify we don't have regression against
weaker engines due to some kind of 'contempt'
effect:
ELO: 54.69 +-2.1 (95%) LOS: 100.0%
Total: 40000 W: 11072 L: 4827 D: 24101
bench: 8205159
Before it was working by accident in case of
see_sign() and failing with see() due to how
castle moves are coded (king captures the rook).
Better to explicitly filter out castling moves
and use see() without any surprise/trick.
No functional case.
Book handling belongs to GUI, we kept this code
for historical reasons, but nowdays there is
really no need of this old, (mostly) unused
and especially incorrect designed functionality.
It is up to the GUI to choose the book (far easier for
the user) and to select the book parameters. In no
place, including fishtest, TCEC, rating lists, etc.
the "own book" is used, moreover currently SF is
released without any book and even if in the future we
bundle a book in the release package, it will be the GUI
that will take care of it.
This corrects a wrong design decision that Galurung
and later Stockfish inherited from what was common
practice many yeas ago.
No functional change.
There is really little that user can achieve (apart
from a weakened engine) tweaking these parameters
that are already tuned and have no immediate or visible
effect.
So better do not expose them to the user and avoid the
typical "What is the best setup for my machine?" kind of
question (by far the most common, by far the most useless).
No functional change.
To show perft numbers for each move. Just
use 'divide' instead of 'perft', for instance:
position startpos moves e2e4 e7e5
divide 4
Inspired by Ronald de Man.
No functional change.
Retire current asymmetric king evaluation
and use a much simpler symmetric one.
As a side effect retire the infamous
'Aggressiveness' and 'Cowardice' UCI
options.
Tested in no-regression mode,
Passed both STC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 33855 W: 5863 L: 5764 D: 22228
And LTC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 40571 W: 5852 L: 5760 D: 28959
bench: 8321835
At root we start counting plies from 1,
instead pv[] array starts from 0. So
the variable 'ply' we use in extract_pv_from_tt
to index pv[] is misnamed, indeed it is
not the real ply, but ply-1.
The fix is to leave ply name in extract_pv_from_tt
but assign it the correct start value and
consequentely change all the references to pv[].
Instead in insert_pv_in_tt it's simpler to rename
the misnamed 'ply' in 'idx'.
The off-by-one bug was unhidden when trying to use
'ply' for what it should have been, for instance in
this position:
position fen 8/6R1/8/3k4/8/8/8/2K5 w - - 0 1
at depth 24 mate line is erroneusly truncated due
to value_from_tt() using the wrong ply.
Spotted by Ronald de Man.
bench: 8732553
If razoring conditions are satisfied and
depth is low, then directly drop in qsearch.
Passed both STC
LLR: 2.98 (-2.94,2.94) [-1.50,4.50]
Total: 12914 W: 2345 L: 2208 D: 8361
And LTC
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 50600 W: 7548 L: 7230 D: 35822
bench: 8739659
When there aren't legal moves after
a search, instead of returning imediately,
save bestValue in TT as in the usual case.
There is really no reason to special case
this one.
With this patch is fully fixed (again) follwing
position:
7k/6p1/6B1/5K1P/8/8/8/8 w - - 0 1
Also in SMP case.
bench: 8802105
After last Joona's patch there is no measurable
difference between the option set or unset.
Tested by Andreas Strangmüller with 16 threads
on his Dual Opteron 6376.
After 5000 games at 15+0.05 the result is:
1 Stockfish_14050822_T16_on : 3003 5000 (+849,=3396,-755), 50.9 %
2 Stockfish_14050822_T16_off : 2997 5000 (+755,=3396,-849), 49.1 %
bench: 880215
Instead of waiting to be allocated, actively search
for another split point to join when finishes its
search. Also modify split conditions.
This patch has been tested with 7 threads SMP and
passed both STC:
LLR: 2.97 (-2.94,2.94) [-1.50,4.50]
Total: 2885 W: 519 L: 410 D: 1956
And a reduced-LTC at 25+0.05
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 4401 W: 684 L: 566 D: 3151
Was then retested against regression in 3 thread case
at standard LTC of 60+0.05:
LLR: 2.96 (-2.94,2.94) [-4.00,0.00]
Total: 40809 W: 5446 L: 5406 D: 29957
bench: 8802105
On a final fixed game number test it failed
to prove better than standard version.
STC 15+0.05
ELO: -0.86 +-1.7 (95%) LOS: 15.8%
Total: 57578 W: 10070 L: 10213 D: 37295
bench: 8802105
Unfortunatly we have a slow down that causes
a regression in STC with no-regression mode:
LLR: -2.96 (-2.94,2.94) [-3.00,1.00]
Total: 22454 W: 3836 L: 4029 D: 14589
bench: 8678654
The bug was found to be elsewhere. This version
is correct and also is able to detect as draw
positions like:
8/8/5b2/8/8/4k1p1/6P1/5K2 b - - 6 133
bench: 8678654
Position is win also if strong side has a bishop
and a knight (plus other material, otherwise
KBNK would be triggered instead of KXK).
This fixes a subtle bug where a search on position
k7/8/8/8/8/P7/PB6/K7 b - - 6 1
Instead of returning a draw score, suddendly returns
a big score. This happens because at one point in
search we reach this position:
8/Pk6/8/8/8/4B3/P7/K7 w - - 3 8
Where white can promote. In case of rook promotion (and also in case of
queen promotion) the resutling position gets a huge static eval that is
above VALUE_KNOWN_WIN (from the point of view of white). So for rook
promotion it is
&& futilityBase > -VALUE_KNOWN_WIN
that prevents futility pruning in qsearch. (Removing this condition indeed
lets the problem occur). Raising the static eval for K+B+N+X v K to a value
higher than VALUE_KNOWN_WIN fixes this particular problem without having to
introduce an extra futility pruning condition in qsearch.
I just checked and it seems K+R v K, K+2B v K and even K+B+N v K already get
a huge static eval. Why not K+B+N+P v K?
I think this fix corrects an oversight. There is special code for KBNK, but
KBNXK is handled by KXK, so the test for sufficient material should also test
for B+N.
bench: 8678654
In the (rare) cases when the two conditions
are true, then fully check again with a slow
but correct MoveList<LEGAL>(pos).size().
This is able to detect false positives like
this one:
8/8/8/Q7/5k1p/5P2/4KP2/8 b - - 0 17
When we have a possible simple pawn push that
is not stored in attacks[] array. Because the
third condition triggers very rarely, even if
it is slow, it does not alters in a measurable
way the average speed of the engine.
bench: 8678654
Currently a stealmate position is misevaluated
in a negative/positive score, this leads qsearch(),
that does not detects stealmates too, to return the
wrong score and this yields to some kind of endgames
to be completely misevaluated.
With this patch is fully fixed follwing position
7k/6p1/6B1/5K1P/8/8/8/8 w - - 0 1
Also in SMP case.
Correct root cause analysys by Ronald de Man.
bench: 8678654
After reverting to the original Tord's
endgame, a search on position
7k/6p1/6B1/5K1P/8/8/8/8 w - - 0 1
Reports, correctly, a draw score instead of
an advantage for white.
Issue reported by Uri Blass.
bench: 8678654
Remove the optimization for Intel, is not
standard and can break at any time, moreover
our release build is not done with Intel C++
anymore so we don't need to sqeeze the extra
speed out from this compiler.
No functional change.
If we return from split with a stale value
due to a stop or a cutoff upstream occurred,
then we exit moves loop and save a stale value
in TT before returning search().
This patch, from Joona, fixes this.
bench: 8678654
We can never have bestValue == -VALUE_INFINITE at
the end of move loop because if no legal move exists
we detect it with previous condition on !moveCount,
if a legal move exists we never prune it due to
futility pruning condition:
bestValue > VALUE_MATED_IN_MAX_PLY
So this code never executes, as I have also verified
directly.
Issue reported by Joona.
No functional change.
Intel compiler is very picky:
"error: this operation on an enumerated type requires an
applicable user-defined operator function"
Reported by Tony Gaor.
No functional change.
Put the division at the end to reduce
rounding errors. This alters the bench
due to different rounding errors, but
should not alter ELO in any way.
bench: 7615217
This apparentely silly tweak allows
to speed up the bench by almost 3%.
Not clear why, repeating with perft,
the speed up vanishes.
Suggested by Jonathan Calovski.
No functional change.
Believed to be a speed optimization as benched
on Windows with bench realtime affinity 0x1 deleting
highest and lowest runs:
Base Test
1549259 1608202
1538115 1583934
1543168 1556938
1536365 1554179
1533026 1582010
Signature remains unchanged and gives anywhere from 1-2% nps
boost in analysis depending on number of cores used.
No functional change.
This is a very discussed patch with many
argumentations pro and against. The fact is
it passed both STC:
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 16305 W: 3001 L: 2855 D: 10449
And LTC
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 34273 W: 5180 L: 4931 D: 24162
Although it is true that a correct test should
include foreign engines, we commit it anyhow so
people can test it out in the wild, under broader
conditions.
bench: 7384368
Idea from Lyudmil Tsvetkov.
The value seems to be raised a bit abruptly, but as
Gary said, a blocked pawn on the sixth rank has been
instrumental in limiting king mobility in multiple
losses that I've seen from SF. A blocked pawn on fifth
rank is much less serious on the king safety impact.
Passed both STC
LLR: 2.97 (-2.94,2.94) [-1.50,4.50]
Total: 14551 W: 2750 L: 2607 D: 9194
and LTC
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 43595 W: 6917 L: 6618 D: 30060
And even a retest at 60" fixed games 40K
ELO: 1.79 +-1.9 (95%) LOS: 97.0%
Total: 39889 W: 6018 L: 5813 D: 28058
bench: 7154916
Adding BMI1 allows the compiler to use _blsr_u64
automatically (the advertised 0.3% speed gain).
I verified that the compiler does not use this
instruction with the -mbmi2 flag only. Also, all
processors supporting BMI2 is also supporting BMI1.
No functional change
Prefer
file_of(s) < file_of(ksq)
to the inidrect
file_of(ksq) < FILE_E
To evaluate if semiopen side to check is the left side.
Also other small touches while there.
No functional change.
Right now the Makefile is cluttered with OS X equivalents
of all the x86 targets. We can get rid of all of them and
just check UNAME against "Darwin" for the few OS X-specific
things we need to do.
We also disable Clang LTO when using BMI2 instructions. For
some reason, LLVM cannot find the PEXT instruction when using
LTO. I don't know why, but disabling LTO for BMI2 fixes it.
No functional change.
Intel Haswell and newer CPUs can calculate sliders
attacks using special PEXT asm instructions instead
of magic bitboards. This gives a +3% speed up.
To enable it just compile with ARCH=x86-64-bmi2
No functional change.
Retire software pext and introduce hardware
call when USE_PEXT is defined during compilation.
This is a full complete implementation of sliding
attacks using PEXT.
No functional change.
Reshuffle functions to define them in reverse
calling order (C style).
This allow us to define templates before they are
used. Currently it is not like this, for instance
evaluate_pieces is defined after do_evaluate that
calls it. This happens to work for some strange
reason (two phase lookup?) but we want to avoid
code that works 'by magic'.
As a nice side-effect we can now remove the function
prototypes.
No functional change.
This is more consistent with what other engines are doing.
Often people thinks that SF's scores are overblown. In the
end, it just boils down to the arbitrary way of rescaling them.
No functional change.
I have noticed that increasing the bench depth produces
progressively smaller and slightly faster executables at
the cost of longer compile times. Also using bench "time"
instead of "depth" seems to produce slightly smaller/faster
executables given comparable compile times.
I have made a new Makefile that generates smaller and
about 1% to 2% faster profile executables at only a
little extra compile time. On my mobile 2GHz i7 a
full profile build time goes from 3'48" to 4'13" and
the exe goes down by 5% from 416,310 bytes to 395,567
bytes.
No functional change.
Small simplification.
Passed SPRT(-3,1) both at STC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 17051 W: 3132 L: 3005 D: 10914
and LTC:
LLR: 4.55 (-2.94,2.94) [-3.00,1.00]
Total: 24890 W: 3842 L: 3646 D: 17402
The rationale behind this is that I've never managed to add a
Queen on 7th rank bonus in DiscoCheck, because it never showed
to be positive (evne slightly) in testing. The only thing that
worked is Rook on 7th rank.
In terms of SF code, it seemed natural to group it with QueenOnPawn
as well as those are done together. I know you're against groupping
in general, but when it comes to non regression test, you are being
more conservative by groupping. If the group passes SPRT(-3,1) it's
safer to commit, than test every component in SPRT(-3,1) and end up
with the risk of commiting several -1 elo regression instead of just
one -1 elo regression.
In chess terms, perhaps it's just easier to manouver a Queen (which
can more also diagonaly) than a Rook. Therefore you can let the search
do its job without needing eval ad-hoc terms to guide it. For the Rook
which takes more moves to manouver such eval terms can be (marginally)
useful.
bench: 7473314
We chose this instead of negamax sign convention
(ie. from the point of view of the side to move)
because it is more in line to how the eval
table is presented.
Also some tweak to formatting while there.
No functional change.
In some legal positions like this one:
R6R/3Q4/1Q4Q1/4Q3/2Q4Q/Q4Q2/Np1Q4/kB1N1KB1 b -- 0 1
We can have a very high score, in this case 30177 and 29267
for midgame and endgame respectively, and because
VALUE_INFINITE = 30001 we have an assert in interpolate()
Midgame and endgame scores are stored in 16 bit signed integers
so we can rise VALUE_INFINITE a little bit. This does not fix
the possibility of overflow in general case, just makes the
condition more difficult to trigger and anyhow better uses all
the score width.
Raising VALUE_INFINITE to 32000 seems to fix the problem for this
particular case.
No functional change.
Tested directly at LTC because previous long
test series on this topic shows it is TC dependant.
Tested with no-regression mode because gets rid of
an ugly and ad-hoc rule.
Test at LTC:
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 67918 W: 10590 L: 10541 D: 46787
bench: 7926803
Here the new idea is to link pinned pieces
with king safety.
Passed both STC
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 10047 W: 1867 L: 1737 D: 6443
And LTC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 10419 W: 1692 L: 1543 D: 7184
bench: 8325087
We add a penalty for each pawn which is not protected by another pawn
of the same color.
Passed both short TC
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 12107 W: 2411 L: 2272 D: 7424
And long TC
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 9204 W: 1605 L: 1458 D: 6141
bench: 7682173
We want all the UCI options are printed in the order in which are
assigned, so we use an index that, depending on Options.size(),
increases after each option is added to the map. The problem is
that, for instance, in the first assignment:
o["Write Debug Log"] = Option(false, on_logger);
Options.size() can value 0 or 1 according if the l-value (that
increments the size) has been evaluated after or before the
r-value (that uses the size value).
The culprit is that assignment operator in C++ is not a
sequence point:
http://en.wikipedia.org/wiki/Sequence_point
(Note: to be nitpick here we actually use std::map::operator=()
that being a function can evaluate its arguments in any order)
So there is no guarantee on what term is evaluated first and
behavior is undefined by standard in this case. The net result
is that in case r-value is evaluated after l-value the last
idx is not size() - 1, but size() and in the printing loop
we miss the last option!
Bug was there since ages but only recently has been exposed by
the removal of UCI_Analyze option so that the last one becomes
UCI_Chess960 and when it is missing engine cannot play anymore
Chess960.
The fix is trivial (although a bit hacky): just increase the
last loop index.
Reported by Eric Mullins that found it on an ARM and MIPS
platforms with gcc 4.7
No functional change.
Thanks to std::bitset we can easily increase
the limit of active threads above 64.
Thanks to Lucas Braesch for pointing at the
correct solution of using std::bitset.
No functional change.
Using memset on a std::vector is undefined behavior,
so manually init all the data memebers of LimitsType.
Bug intorduced in 41641e3b1e
No functional change.
Because we test for available slaves before
entering split(), we almost always allocate a
slave, only in the rare case of a race (less
then 2% of cases) this is not true, but to
special case this occurrence is not worth
the added complexity.
bench: 7451319
Split delta value in aspiration window so that when
search depth is less than 24 a smaller delta value
is used. The idea is that the search is likely to
be more accurate at lower depths and so we can exclude
more possibilities, 25% to be exact.
Passed STC
LLR: 2.96 (-2.94, 2.94) [-1.50, 4.50]
Total: 20430 W: 3775 L: 3618 D: 13037
And LTC
LLR: 2.96 (-2.94, 2.94) [0.00, 6.00]
Total: 5032 W: 839 L: 715 D: 3478
Bench: 7451319
During endgame initialization we get the material
hash key of each endgame forging and ad-hoc position
that in same cases is illegal (leaves teh king under
capture). This is not a problem for the material key,
but rises an assert when SF is run in debug mode with
'testKingCapture' set in pos_is_ok().
So rewrite the code to always produce legal positions.
No functional change.
Big simplification of pawn move check.
Code has been tested with a brute force approach: for
every position reached during a bench search, the function
has been called for each combinations of Move(from, to)
and verified the result is the same of old code.
Actually this function is very critical becuase is the
one that ensures corrupted TT moves are discarded, so
to properly test it a simple bench is not enough.
Verified also speed is not changed.
No functional chnage.
It has been obsoleted out already some time ago
and currently there is no point in changing eval
score according to if we are in game or analyzing.
So retire the option.
No functional change.
Try to avoid repetition draws at early midgame,
this should give an edge against weaker opponents
and reduce draw rate.
Tested for regressions with SPRT[-3, 1] and
passed both short TC
LLR: 2.95 (-2.94,2.94) [-3.00,1.00]
Total: 68498 W: 12928 L: 12891 D: 42679
And long TC
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 40212 W: 6386 L: 6295 D: 27531
bench: 7990513
When running the following position:
8/kPp5/2P3p1/p1P1p1P1/2PpPp2/3p1p2/3P1P2/5K2 w - - 0 1
An assert is raised at depth 92:
assert(-VALUE_INFINITE <= alpha && alpha < beta && beta <= VALUE_INFINITE);
This is because it happens that beta = 29832,
so rbeta = 30032 that is > VALUE_INFINITE
Bug spotted and analyzed by Uri, fix suggested by Joerg.
Other fixes where possible but this one is pointed
exactly at the source of the bug, so it is the best
from a code documentation point of view.
bench: 8430785
Under some very rare case 100 plies of search
could be not enough. Increasing more could lead
to crashes due to reached stack size limit on
some platforms.
Strongly requested by Uri.
bench: 8430785
Currently king has no material key associated because
it can never happen to find a legal position without
both kings, so there is no need to keep track of it.
The consequence is that a position with only the two
kings has material key set at zero and if the material
hash table is empty any entry will match and this is
wrong.
Normally bug is hidden becuase the checking for a draw
with pos.is_draw() is done earlier than evaluate() call,
so that we never check in gameplay the material key of a
position with two kings.
Nevertheless the bug is there and can be reproduced setting
at startup a position with only two kings and typing
'eval' from prompt.
The fix is very simple: add a random key also for the king.
Also fixed the condition in material.cpp to avoid asserting
when a 'just 2 kings' postion is evaluated.
No functional change.
Actually MultiCut is too different from current scheme.
Note that neither ProbCut is exactly what we do because
we try just a handful of captures instead of all moves,
nevertheless it seems more in line with what we do.
Suggested by Joona.
No functional change.
Makes more sense than returning a draw score. Tested
with reduced MAX_PLY = 30 and passed both short TC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 17434 W: 3345 L: 3194 D: 10895
And long TC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 2610 W: 488 L: 373 D: 1749
With current limit of MAX_PLY = 100 the patch should not
introduce any measurable change, nevertheless is the correct
approach.
Idea of returning eval is from Michel Van den Bergh.
bench: 8430785
Fix small overflow error while converting
magic boosters from right rotate to left rotate,
in particular booster 38 was converted to 4122
instead of the corrcet value 26.
Formula used was:
s1 = original & 63, s2 = (original >> 6) & 63;
new = (64 - s1) | ((64 - s2) << 6);
Instead of:
s1 = original & 63, s2 = (original >> 6) & 63;
new = ((64 - s1) & 63) | (((64 - s2) & 63) << 6);
This has no impact in number of cycles needed, but
just in the resultig number that yields to a rotate
amount bigger than 63.
Spotted by Ehsan Rashid.
No functional change.
Latest master triggers a compiler warning due
to comparing int64_t to uint64_t.
notation.cpp: In Funktion »std::string pretty_pv(Position&, int, Value, int64_t, Move*)«:
notation.cpp:230:30: Warnung: Vergleich zwischen vorzeichenbehafteten und vorzeichenlosen Ganzzahlausdrücken [-Wsign-compare]
This patch should fix it.
No functional change.
When initializing the magic numbers used to
compute sliding attacks, we endless generate a
random and test it as a possible magic.
In the general case this takes a lot of iterations,
but here, insteaad of picking a casual random, we
rotate it a couple of times and generate a number that
we know has a good probability to be a magic candidate.
This is becuase the quantities by which we rotate the
number are known in advance to produce quickly a good
canidate.
The patch, inspired by DON, just moves the shuffle to RKISS
changing the boosters to take in account a left rotation
instead of a right rotation as in the original.
No functional change.
Although does not change ELO level, it seems
verification is useful in many zugzwang positions
as reported by many sources.
So revert this simplification.
bench: 8430785
Another SEE speed up that passed the SPRT short TC test!
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 81337 W: 15060 L: 14745 D: 51532
No functional change.
Actually race conditions do exist in an engine, just
think for a moment to TT concurrent access. Racy code
is not a problem per se, if the consequences are well
known and correctly handled.
In case of TT access we ensure that the TT move is validated
before to be tried, here we just retry the same move in less
that 1 case out of a million: this is totally harmless considering
that very probably the second time the move is tried we get
immediately a TT hit and search quickly returns.
So we simplify the code for no harm.
No fuctional change (in single thread case)
Tested with SPRT in simplification mode [-4.00,0.00],
this ensures that the patch is (very probably) not
a regression.
Passed both short TC
LLR: 2.95 (-2.94,2.94) [-4.00,0.00]
Total: 27543 W: 4278 L: 4209 D: 19056
And long TC
LLR: 2.95 (-2.94,2.94) [-4.00,0.00]
Total: 39483 W: 7325 L: 7305 D: 24853
bench: 8347121
Hopefully this patch makes the code more:
* Self-documenting: Null search is always a zero window search,
because it is testing for a fail high. It should never be done
on a full window! The current code only works because we don't
do it at PV nodes, and therefore (alpha, beta) = (beta-1, beta):
that's the kind of "clever" trick we should avoid.
* Idiot-proof: If we want to enable null search at PV nodes, all we
need to do now is comment out the !PvNode condition. It's that simple!
In theory, null search should not be done at PV nodes, because PV nodes
should never fail high. But in practice, they DO fail high, because of
aspiration windows, and search inconsistencies, for example. So it makes
sense to keep that flexibility in the code.
No functional change.
Use ralpha instead of rbeta
* rbeta is confusing people. It took THREE attempts to code razoring
at PV nodes correctly in a recent test, because of the rbeta trick.
Unnecessary tricks should be avoided.
* The more correct and self-documenting way of doing this, is to say
that we use a zero window around alpha-margin, not beta-margin.
The fact that, because we only do it at PV nodes, alpha happens to be
beta-1 and that the current stuff with rbeta works, may be correct,
but is confusing.
Remove the misleading and partially erroneous comment about returning
v + margin:
* comments should explain what the code does, not what it could have done.
* this comment is partially wrong in saying that v+margin is "logical",
and that it is "surprising" that is doesn't work.
From a theoretical perspective, at least 3 ways of doing this are equally
defendable:
1/ fail hard: return alpha: The most conservative. We bet that the search
will fail low, but we don't know by how much and don't want to take risks.
2/ aggressive fail soft: return v (what the current code does). This
corresponds to normal fail soft, with the added assumption that we don't
care about the reduction effect (see below point 3/)
3/ conservative fail soft: return v + margin. If the reduced search (qsearch)
gives us a score <= v, we bet that the non reduced search will give us a
score <= v + margin.
* Saying that 2/ is "logical" implies that 1/ and 3/ are not, which is
arguably wrong. Besides, experimental results tell us that 2/ beats 3/,
and that's not something we can argue against: experimental results are
the only trusted metric.
* Also, with the benefit of hindsight, I don't think the fact that 2/ is
better than 3/ is surprising at all. The point is that it is YOUR turn to
move, and you are assuming that by NOT playing (and letting the opponent
capture your hanging pieces in QS) you cannot generally GAIN razor_margin(depth).
No functional change.
With some positions like
8/8/8/2p2K2/1pp5/br1p1b2/2p2r2/qqkqq3 w - -
The eval score is higher than VALUE_INFINITE because
is the sum of VALUE_KNOWN_WIN plus a big material
advantage. This leads to an assert. Here are the
steps to reproduce:
Compile SF with debug=yes then do
./stockfish
position fen 8/8/8/2p2K2/1pp5/br1p1b2/2p2r2/qqkqq3 w - -
go depth 1
This patch fixes the issue in this case, but do exsist
other positions for which the patch is not enough and
we will need to limit the eval score to be sure not
overflow the limit.
Note that is not possible to increase the value of
VALUE_INFINITE because should remain within int16_t
type to be stored in a TT entry.
bench: 7356053
Depth is already dependent on the actual value
of ONE_PLY, in particular can be expressed like:
Depth = n * ONE_PLY
And because formula is used to calculate R that is
also dependent on the value of ONE_PLY and can be
expressed like:
R = x * ONE_PLY
We don't want to divide depth by a 'ply' value but
directly by an integer number.
Spotted by sf-x
No functional change.
Instead of a fixed reduction of ONE_PLY, now
Null move dynamic reduction based on value can
grow larger in case we are above beta of a value
much higher then PawnValueMg.
Note that now an eval returning VALUE_KNOWN_WIN
makes null search to drop in qsearch.
Passed both short TC:
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 26141 W: 4871 L: 4699 D: 16571
And long TC:
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 33695 W: 5309 L: 5056 D: 23330
bench: 7356053
Verified there are no hidden bugs and is
actually a speed optimization:
Fixed games at 15+0.05 TC
ELO: 1.72 +-2.9 (95%) LOS: 87.5%
Total: 20000 W: 3741 L: 3642 D: 12617
No functional change
When time remaining is less than Emergency Move Time,
we won't even complete one iteration and engine reports
a stale +M0 score.
To reproduce run "go wtime 10"
info depth 1 seldepth 1 score mate 0 upperbound nodes 2 nps 500 time 4 multipv 1 pv a2a3
info nodes 2 time 4
bestmove a2a3 ponder (none)
This patch fixes the issue.
Tested by Binky at very short TC: 0.05+0.05
ELO: 5.96 +-12.9 (95%) LOS: 81.7%
Total: 1458 W: 394 L: 369 D: 695
And at a bit longer TC:
ELO: 1.56 +-3.7 (95%) LOS: 79.8%
Total: 16511 W: 3983 L: 3909 D: 8619
bench: 7804908
TCEC season 3, which is due to start in a few weeks, just
had its server upgraded to 64GB RAM and will therefore allow
16GB hash to be used per engine.
This is almost the upper limit without changing the
type of size and hashMask. After this we need to
move to uint64_t instead of uint32_t.
No functional change.
Retire KmmKm evaluation function. Instead give a very drawish
scale factor when the material advantage is small and not much
material remains.
Retire NoPawnsSF array. Pawnless endgames without a bishop will
now be scored higher. Pawnless endgames with a bishop pair will
be scored lower. The effect of this is hopefully small.
Consistent results both at short TC (fixed games):
ELO: -0.00 +-2.1 (95%) LOS: 50.0%
Total: 40000 W: 7405 L: 7405 D: 25190
And long TC (fixed games):
ELO: 0.77 +-1.9 (95%) LOS: 78.7%
Total: 39690 W: 6179 L: 6091 D: 27420
bench: 7213723
When we have a fail-high of a quiet move, store it in
a Followupmoves table indexed by the previous move of
the same color (instead of immediate previous move as
is in countermoves case).
Then use this table for quiet moves ordering in the same
way we are already doing with countermoves.
These followup moves will be tried just after countermoves
and before remaining quiet moves.
Passed both short TC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 10350 W: 1998 L: 1866 D: 6486
And long TC
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 14066 W: 2303 L: 2137 D: 9626
bench: 7205153
This hacky rule allows to get an about right eval out of this position:
r2qk2r/ppp2p2/2npbn2/2b1p3/2P1P1P1/2NB1PPp/PPNP3K/R1BQ1R2 b kq - 0 13
And, more importantly, passed both short TC:
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 6239 W: 1249 L: 1127 D: 3863
And long TC:
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 38371 W: 6165 L: 5888 D: 26318
bench: 8183238
As pointed out by Joona, Lucas and otehr people in
the forum, this endgame is not a known, there are many
positions where it takes more than 50 moves to claim the
win and becasue exact rules is not possible better to
retire and allow the search to workout the endgame for us.
bench: 8502826
While editing original Uri's messy patch
I have incorrectly simplified the logic
condition. Here is the correct original
version, as it was tested.
bench: 8502826
Remove the easy move code and add the condition to
play instantly if only one legal move is available.
Verified there is no regression at 60+0.05
ELO: 0.17 +-1.9 (95%) LOS: 57.0%
Total: 40000 W: 6397 L: 6377 D: 27226
bench: 8502826
If we are still at first move, without a fail-low and
current iteration is taking too long to complete then
stop the search.
Passed short TC:
LLR: 2.97 (-2.94,2.94) [-1.50,4.50]
Total: 26030 W: 4959 L: 4785 D: 16286
Long TC:
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 18019 W: 2936 L: 2752 D: 12331
And performed well at 40/30
ELO: 4.33 +-2.8 (95%) LOS: 99.9%
Total: 20000 W: 3480 L: 3231 D: 13289
bench: 8502826
First tested with 50K games at very short TC of 5+0.05
ELO: 3.11 +-2.0 (95%) LOS: 99.9%
Total: 49665 W: 10941 L: 10497 D: 28227
Then retested with usual SPRT at short TC
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 16875 W: 3198 L: 3049 D: 10628
And at long TC
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 5890 W: 985 L: 857 D: 4048
bench: 7800379
In case ply is very high, function will round
to zero (although mathematically it is always
bigger than zero). On my system this happens at
movenumber 6661.
Although 6661 moves in a game is, of course,
probably impossible, for safety and to be locally
consistent makes sense to ensure returned value
is positive.
Non functional change.
Function move_importance() is already always
positive, so we don't need to add a constant
term to ensure it.
Becuase move_importance() is used to calculate
ratios of a linear combination (as explained in
previous patch), result is not affected. I have
also verified it directly.
No functional change.
Drop a useless parameter. This works because ratio1 and ratio2
are ratios of linear combinations of thisMoveImportance and
otherMovesImportance and so the yscale cancels out.
Therefore the values of ratio1 and ratio2 are independent
of yscale and yscale can be retired.
The same applies to yshift, but here we want to ensure
move_importance() > 0, so directly hard-code this safety
guard in function definition.
Actually there are some small differences due to rounding errors
and usually are at most few millisecond, that's means below 1% of
returned time, apart from very short intervals in which a difference
of just 1 msec can raise to 2-3% of total available time.
No functional change.
The flag raises also in case of a pawn duo, i.e.
when we have two adjacent pawns on the same rank,
and not only in case of a chain, i.e. when the two
pawns are on a diagonal line.
See this for a reference:
http://en.wikipedia.org/wiki/Connected_pawns
Renaming suggested by Ralph.
No functional change.
Use a skew-logistic function to replace the
MoveImportance[] array.
Verified it does not regress at fixed number
of games both at short TC:
LLR: -2.91 (-2.94,2.94) [-1.50,4.50]
Total: 39457 W: 7539 L: 7538 D: 24380
And long TC:
ELO: -0.49 +-1.9 (95%) LOS: 31.0%
Total: 39358 W: 6135 L: 6190 D: 27033
bench: 7335588
Passed short TC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 18331 W: 3608 L: 3453 D: 11270
And scored above 50% on a very long test in long TC
LLR: -2.97 (-2.94,2.94) [0.00,6.00]
Total: 51533 W: 8181 L: 8047 D: 35305
bench: 7335588
In endgame it's better to have pawns on both wings.
So give a bonus according to file distance between left
and right outermost pawns.
Passed both short TC
LLR: 2.97 (-2.94,2.94) [-1.50,4.50]
Total: 39073 W: 7749 L: 7536 D: 23788
And long TC
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 6149 W: 1040 L: 910 D: 4199
bench: 7665034
Reduce eval discontinuity becuase now we kick in
king safety evaluation in many more cases.
Passed both short TC:
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 8708 W: 1742 L: 1613 D: 5353
And long TC:
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 6743 W: 1122 L: 990 D: 4631
bench: 6835416
Tighter lower bound for pawn attacks so to
activate king safety in some cases like here:
6k1/2B3p1/2Pp1p2/2nPp3/2Q1P2K/P2n1qP1/R6P/1R6 w
Original patch by Chris, further simplified by
Jörg Oster.
Passed both short TC
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 30171 W: 5887 L: 5700 D: 18584
And long TC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 20706 W: 3402 L: 3204 D: 14100
bench: 7607562
Add a bonus according if the attacking
pieces are minor or major.
Passed both short TC
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 13142 W: 2625 L: 2483 D: 8034
And long TC
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 18059 W: 3031 L: 2844 D: 12184
bench: 7425809
A great simplification that shows no regression
and it seems even a bit scalable.
Tested with fixed number of games:
Short TC
ELO: 0.60 +-2.1 (95%) LOS: 71.1%
Total: 39554 W: 7477 L: 7409 D: 24668
Long TC
ELO: 2.97 +-2.0 (95%) LOS: 99.8%
Total: 36424 W: 5894 L: 5583 D: 24947
bench: 8184352
Change updating rule after a TT hit to match
the same one at the end of the search.
Small change in functionality, but we want to
have uniform rules in the code.
bench: 7767864
We already update killers so it is natural to extend to
history and counter move too.
Passed both short TC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 52690 W: 9955 L: 9712 D: 33023
And long TC
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 5555 W: 935 L: 808 D: 3812
bench: 7876473
After a fail high in LMR, if reduction is very high do
a research at lower depth before teh full depth one.
Chances are that the re-search will fail low and the
full depth one is skipped.
Passed both short TC:
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 11363 W: 2204 L: 2069 D: 7090
And long TC:
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 7292 W: 1195 L: 1061 D: 5036
bench: 7869223
Since all ENPASSANT moves are now considered dangerous, this
change of order should give a slight speedup.
Also simplify futilityValue formula.
No functional change.
We avoid to use an ad-hoc table at the cost of a
relative_rank() call in advanced_pawn_push().
On my 32 bit system it is even slightly faster (on 64bit
may be different). This is the speed in nps alternating
old and new bench runs:
new
368890
368825
369972
old
367798
367635
368026
No functional change.
Instead of a passed pawn now we just require the pawn to
be in the opponent camp to be considered a dangerous
move. Added some renaming to reflect the change.
Passed both short TC test
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 10358 W: 2033 L: 1900 D: 6425
And long TC
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 21459 W: 3486 L: 3286 D: 14687
bench: 8322172
To align to same named Position function and
avoid using std::cout directly.
Also remove some stale <iostream> include while
there.
No functional change.
Add a Mac SSE4.2 target. Also change the Mac OS X minimum version to
10.6. Rationale: 97% of Macs run at least 10.6, version 10.9 is now
free, and using 10.6 as the minimum version gives a small 5% boost in
benchmark speed over versions using 10.0 as the minimum version.
Finally, enable Clang’s Link Time Optimization when compiling for the
Mac.
No functional change.
An old idea retested at SPRT(0, 3) with 60+0.05 TC:
LLR: 2.95 (-2.94,2.94) [0.00,3.00]
Total: 98872 W: 15549 L: 15123 D: 68200
This is a very small elo increase patch so it really
stresses the limits of fishtest.
bench: 8596156
It seems to intorduce a regression when tested
with 3 threads at 15+0.05:
ELO: -2.26 +-2.2 (95%) LOS: 2.4%
Total: 30000 W: 4813 L: 5008 D: 20179
bench: 8331357
Tested setting FakeSplit to true and running
./stockfish bench 128 2
There is a different signature with and without
the patch so it affects functionality but
only in SMP case.
bench: 8331357
SMP case is very tricky and raises an assert in stage_moves():
assert(stage == KILLERS_S1 || stage == QUIETS_1_S1 || stage == QUIETS_2_S1)
So rewrite the code to just return moves[] when we are sure
we are in quiet moves stages.
Also rename stage_moves to quiet_moves to reflect that.
No functional change (but needs testing in SMP case)
Use MovePicker moves[] to access already tried
quiet moves. A bit of care shall be taken
to avoid calling stage_moves() when we are still
at ttMove stage, because moves are yet to be
generated. Actually our staging move generation
makes this code a bit more tricky than what I'd
like, but removing an ausiliary redundant
array like quietsSearched[] is a good thing.
Idea by DiscoCheck
bench: 9355734
Use the newly introduced LineBB[] to simplify this
super hot-path function.
Verified with perft we don't have any speed regression, although
the number of squares removed is less than before in case of
contact check.
Insipred by DiscoCheck implementation.
Perft numbers are the same, but we have an harmless functional
change due to reorder of moves, because now some illegal moves
are no more detected at generation time, but in the search.
bench: 8331357
This seems a die hard idea :-)
Passed both short TC
LLR: 2.97 (-2.94,2.94) [-1.50,4.50]
Total: 17485 W: 3307 L: 3156 D: 11022
And long TC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 38181 W: 6002 L: 5729 D: 26450
bench: 8659830
Actually, it is not used, as both arrays have the
same values. Some local tests in either direction
showed no improvement.
Also some minor corrections in the comments.
No functional change.
Previously some squares could be "incorrectly" awarded
to a pinned piece.
e.g. in 3k4/1q6/3b4/3Q4/8/5K2/B7/8 b - - 0 1 the black
bishop get 4 squares too many and the white queen gets 6.
Passed both short TC.
LLR: 2.97 (-2.94,2.94) [-1.50,4.50]
Total: 4871 W: 934 L: 817 D: 3120
And long TC:
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 38968 W: 6113 L: 5837 D: 27018
bench: 9282549
But compensate by reducing rook and queen
value by 53 = (160 / 3)
Material imbalances are affected as follows:
Red. Major Rook Queen Total
QRR +160 -2*53 -53 +1
QR +160 -53 -53 +54
RR +160 -2*53 0 +54
R 0 -53 0 -53
Q 0 0 -53 -53
so that the imbalance changes by at most 54 + 53 = 107 units.
This corresponds to appromximately 3.5cp in the final evaluation.
Verified with fixed number 40000 games at both short
and long TC it does not regress.
Short TC 15+0.05
ELO: 1.93 +-2.1 (95%) LOS: 96.6%
Total: 40000 W: 7520 L: 7298 D: 25182
Long TC 60+0.05
ELO: -0.33 +-1.9 (95%) LOS: 36.5%
Total: 39663 W: 6067 L: 6105 D: 27491
bench: 6703846
As, Gary (that analyzed the bug) says:
SF does not print a PV when the original best move fails low,
we hit our time allowance, and stop the search. The output from
the SF search is below. It was failing low on Ne1 at depth 34.
Then, we get bestmove Qd3, but no PV change.
info depth 34 seldepth 45 score cp 38 upperbound nodes 483484489 nps 15464575 time 31264 multipv 1 pv f3e1 h5h4 e1d3 h4g3 f2g3 a6f6 f1f6 e7f6 d1a4 f6e7 a1f1 d8f8 a4b3 b7b6 b3c2 f7f6 c2a4 h3g5 b2b3 g5f7 a4c6 f7d6 h1g2 f6f5 e4f5 d6f5
info depth 34 seldepth 45 score cp 38 upperbound nodes 483484489 nps 15464575 time 31264 multipv 1 pv f3e1 h5h4 e1d3 h4g3 f2g3 a6f6 f1f6 e7f6 d1a4 f6e7 a1f1 d8f8 a4b3 b7b6 b3c2 f7f6 c2a4 h3g5 b2b3 g5f7 a4c6 f7d6 h1g2 f6f5 e4f5 d6f5
info depth 34 seldepth 47 score cp 30 upperbound nodes 2112334132 nps 17255517 time 122415 multipv 1 pv f3e1 h5h4 d1a4 a6f6 e1d3 d8f8 a4c2 h4g3 f2g3 f6f1 a1f1 h7g8 b2b3 f7f6 a3a4 b7b6
info depth 34 seldepth 47 score cp 30 upperbound nodes 2112334132 nps 17255517 time 122415 multipv 1 pv f3e1 h5h4 d1a4 a6f6 e1d3 d8f8 a4c2 h4g3 f2g3 f6f1 a1f1 h7g8 b2b3 f7f6 a3a4 b7b6
info nodes 18235667001 time 969824
bestmove e2d3 ponder c8d7
Looking at the code, if we hit Signals.stop, we return from id_loop
before printing any PV. It is possible for us to have resorted the
RootMove list though, which will change the move that is actually
played.
No functional change.
1/ eval margin and gains removed:
16bit are now free on TT entries, due to the removal of eval margin. may be useful
in the future :) gains removed: use instead by Value(128). search() and qsearch()
are now consistent in this regard.
2/ futility_margin()
linear formula instead of complex (log(depth), movecount) formula.
3/ unify pre & post futility pruning
pre futility pruning used depth < 7 plies, while post futility pruning used
depth < 4 plies. Now it's always depth < 7.
Tested with fixed number of games both at short TC:
ELO: 0.82 +-2.1 (95%) LOS: 77.3%
Total: 40000 W: 7939 L: 7845 D: 24216
And long TC
ELO: 0.59 +-2.0 (95%) LOS: 71.9%
Total: 40000 W: 6876 L: 6808 D: 26316
bench 7243575
1/ eval margin and gains removed:
- gains removed by Value(128): search() and qsearch() now behave consistently!
2/ futility_margin()
- testing showed that there is no added value in this weird (log(depth), movecount)
formula, and a much simpler linear formula is just as good. In fact, it is most
likely better, as it is not yet optimally tuned.
- the new simplified formula also means we get rid of FutilityMargins[], its
initialization code, and more importantly ss->futilityMoveCount, and the hacky
code that updates it throughout the search().
- the current formula gives negative futility margins, and there is a hidden interaction
between the move coutn pruning formula and the futility margin one: what happens is
that MCP is supposed to be triggered before we use the non-sensical negative futility
margins.
3/ unify pre & post futility pruning
- pre futility pruning (what SF calls value based pruning) used depth < 7 plies,
while post futility pruning (what SF calls static null move pruning) used depth < 4 plies.
- also the condition depth < 7 in pre futility pruning was not obvious, and it seemd
to be depth < 16 (futility_margin() returns an infinite value when depth >= 7).
Tested with fixed number of games both at short TC:
ELO: 0.82 +-2.1 (95%) LOS: 77.3%
Total: 40000 W: 7939 L: 7845 D: 24216
And long TC
ELO: 0.59 +-2.0 (95%) LOS: 71.9%
Total: 40000 W: 6876 L: 6808 D: 26316
bench: 10206576
RedundantRook and RedundantQueen replaced by simple
variable RedundantMajor. Also the SameColor coefficient
for Queen<->Queen has been set by definition to 0.
The remaining 5 parameters:
LinearCoefficients[ROOK]
LinearCoefficients[QUEEN]
QuadraticCoefficientsSameColor[ROOK][ROOK]
QuadraticCoefficientsSameColor[QUEEN][ROOK]
RedundantMajor
are sufficient to equate the material imbalances for the
5 common material configurations of R, RR, Q, QR and QRR
to any desired values simultaneously.
With the chosen parameters there should be no functional
change unless one side has more than 2 rooks or more
than 1 queen. For example bench from the start position
using the commands:
./stockfish
go depth 16
produces identical output except for one extra node
in the last iteration.
bench: 8198094
Coefficients for Bishop<->BishopPair and Bishop<->Bishop
are also pretty much redundant. By altering the values
in LinearCoefficients[] these coefficients can be zeroed
without changing the imbalance calculations in any position
with less than 3 bishops for one side.
bench: 7995098
First coefficient in the SameColor array does an
equivalent job when folded into the LinearCoefficients
array.
All of the diagonal terms in the OppositeColor array
are redundant due to cancellation.
No functional change.
In case we find a very good move after a
troubled start, we don't return immediately
anymore.
Tested directly at long TC where it passed:
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 13910 W: 2397 L: 2228 D: 9285
bench: 7995098
And remove a complex (and broken) formula.
Indeed previous code was broken in case of TC with big
time increments where available_time() was too similar
to total time yielding to many time losses, so for instance:
go wtime 2600 winc 2600
info nodes 4432770 time 2601 <-- time forfeit!
maximum search time = 2530 ms
available_time = 2300 ms
For a reference and further details see:
https://groups.google.com/forum/?fromgroups=#!topic/fishcooking/dCPAvQDcm2E
Speed tested with bench disabling timer alltogheter vs timer set at
max resolution, showed we have no speed regressions both in single
core and when using all physical cores.
No functional change.
A combo of two patches that failed SPRT with score
higher than 50% but togheter they succeed:
SPRT at 60+0.05
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 7312 W: 1276 L: 1139 D: 4897
bench: 8029334
If the game got late enough that move_importance(currentPly) * slowMover / 100
rounds to 0, then we ended up dividing 0 by 0 when only looking 1 move ahead.
This apparently caused the search to almost immediately abort and Stockfish
would blunder in long games. So convert thisMoveImportance to a double.
No functional change.
This seems more a material imbalance topic,
anyhow test is good and so patch is applied
as is.
Passed both short TC:
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 17391 W: 3548 L: 3393 D: 10450
And long TC:
LLR: 3.00 (-2.94,2.94) [0.00,6.00]
Total: 34660 W: 5972 L: 5700 D: 22988
bench: 8291883
Dumb down a bit the code and trade some possible
speed (but this is far from hot path anyhow) for
some added readability for the layman.
No functional change.
The normalising transformation is computed all at
once by the helper function get_flip_sq and then
applied immediately to the relevant squares as soon
as they are loaded from the position class.
bench: 8350690
New formula mathces the old formula until d = 45
Test code:
int main() {
for(int d=1; d<=45; d++)
{
int a = int(log(double(d * d) / 2) / log(2.0) + 1.001);
int b = int(2.9 * log(double(d)));
if (a != b) std::cout << d << std::endl;
}
return 0;
}
bench: 8455956
This is the first chain bonus version
from Ralph that also passed both
Short TC:
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 23460 W: 4727 L: 4556 D: 14177
And long TC:
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 31858 W: 5497 L: 5240 D: 21121
And performed better against current
committed version, always at 60secs:
LLR: -2.94 (-2.94,2.94) [-3.00,3.00]
Total: 26301 W: 4477 L: 4580 D: 17244
This test was done by Leonid.
bench: 8455956
After 40K games at 60 secs, result is still
not clear, but not a regression against SF 4
After
ELO: 50.11 +-2.1 (95%) LOS: 100.0%
Total: 40000 W: 10547 L: 4817 D: 24636
Before
ELO: 49.51 +-2.1 (95%) LOS: 100.0%
Total: 40000 W: 10483 L: 4821 D: 24696
So re-apply the patch to avoid to
special-case this one.
bench: 7403882
It was never updated !
Currently it only affects evaluate_passed_pawns()
and in particularly the rule to increase the bonus
if we have more non-pawn pieces. We could simply use
popcount() instead and avoid the little slowdown
in put_piece() and remove_piece(), but this would
leave a very subtle and tricky hole where people
are forced to remember that pos.count<ALL_PIECES>()
does not work. This is not obvious and so dangerous.
Thanks to Ronald de Man for spotting this.
bench: 7931424
Due to a strange issue (bug?) the ternary
operator does not return a BitCountType for
icc, so revert to the expression.
The same patch was already applied in
9749f1f14c
Thanks to NssY Wanyonyi for pointing out
this.
No functional change.
This reverts commit 4bc2374450 for
two reasons.
First regression testing shows almost equal
score:
Before the patch:
ELO: 49.75 +-2.5 (95%) LOS: 100.0%
Total: 27205 W: 7113 L: 3244 D: 16848
After the patch:
ELO: 48.87 +-2.9 (95%) LOS: 100.0%
Total: 20860 W: 5478 L: 2563 D: 12819
Second, and more sensible to me, this patch
increases safe check bonuses to 4 times their
original value (!) and considering:
- Values were already well tuned
- Values are highly critical
- King safety is highly critical, very TC
dependent and very difficult to test
- Our testing coverage is partial (self-testing,
blitz times)
I think is better to be safe than sorry and so
I revert the patch.
bench: 8440524
Passed both short TC:
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 10466 W: 2087 L: 1953 D: 6426
And long TC:
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 26334 W: 4540 L: 4310 D: 17484
And also proved stronger than a slightly
different patch, also succesful against master:
https://github.com/mcostalba/Stockfish/commit/dc6830a3b4ed12
But losing against current one in a match
at 60secs with SPRT [-3, 3]:
LLR: -2.96 (-2.94,2.94) [-3.00,3.00]
Total: 44484 W: 7360 L: 7463 D: 29661
bench: 9160831
Also the drawing criteria has been slightly loosened.
It now detects a draw if the king is ahead of all the
pawns and on the same file or the adjacent file.
bench: 7700683
This lost position 8/8/3q4/8/5k2/2P1R3/2K2P2/8 w - - 0 1
was previously evaluated as a draw.
The king and rook need to be correctly placed with
respect to the _same_ pawn.
(Note also that the check for the pawn being on RANK_2
in the old version is redundant: it must be on RANK_2 if
it hopes to protect a rook on RANK_3)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Good both at short TC:
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 5448 W: 1133 L: 1012 D: 3303
And at long TC:
LLR: 2.95 (-2.94,2.94) [0.00,6.00]
Total: 40509 W: 6836 L: 6541 D: 27132
bench: 7700683
Don't need a struct here. Speed test shows
result is teh same. Moreover RKISS is used
mainly at startup to compute magics, so
prefer to keep it simple...RKISS ;-)
Also some assorted triviality while there.
No functional change.
These two changes go in opposite directions and it
seems that the combination is stronger than original.
Here are the positive tests at various TC:
15+0.05
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 24561 W: 4946 L: 4772 D: 14843
60+0.05
LLR: 2.96 (-2.94,2.94) [0.00,6.00]
Total: 15259 W: 2598 L: 2423 D: 10238
40/30
LLR: 2.96 (-2.94,2.94) [-3.00,3.00]
Total: 2570 W: 527 L: 422 D: 1621
Unfortunately there is also a bad result
with one sec time increment that needs
to be further investigated:
12+1
LLR: -2.97 (-2.94,2.94) [-3.00,3.00]
Total: 2694 W: 438 L: 543 D: 1713
bench: 8340585
Rationale:
- Speed of double and float is about the same (not on the hot path anyway)
- Double makes code prettier (no need to write 1.0f, just 1.0)
- Only practical advantage of float is to use less memory, but since we never
store large arrays of double, we don't care.
No functional change.
Increase bench default depth from 12 to 13 and
add 15 new endgame positions to have broader
coverage and also more reliable nps calulcation
used for fishtest framework.
Due to the new endgame positions, where nps is higher,
the total nps is increased of about 15%.
Thanks to Lucas and Jörg for the suggestions.
No functional change, but bench number is now:
bench: 8336338
For some users -stack_size,0x4000 does not work,
so revert for now.
osX 10.6.8
gcc version 4.7.3 (MacPorts gcc47 4.7.3_2)
g++: error: unrecognized command line option '-stack_size,0x4000'
make[2]: *** [stockfish] Error 1
make[1]: *** [gcc-profile-make] Error 2
make: *** [profile-build] Error 2
No functional change.
This reverts commit 800410eef1 and instead increases
stack size.
I went through the old emails with Daylen that reported the
crash issue on Mac OS X and was fixed by 0049d3f337.
It was reported default stack size for a thread in Mac OS X is 8
megabytes while the patch that we are reverting allows to reduce
stack size at max of about 217KB, so the reason for the crash was
only marginal in MAX_MOVES value. On those emails Daylen also
hinted how to increase stack size for Mac OS X to 16MB.
So prefer to increase stack size to 16MB instad of re-inventing
the wheel and do our home grown stack as we did with the patch
that we are now reverting (it will remain anyhow in git history
for documentation purposes).
No functional change.
Unify extensions between PV and not PV nodes
and remove all but check extensions.
This is a simplification so tested at fixed number
of games where proved to not regress.
About 45k games at 15+0.05
ELO: 1.23 +-2.0 (95%) LOS: 88.5%
Total: 45643 W: 9107 L: 8946 D: 27590
About 45k games at 60+0.05
ELO: 1.07 +-1.8 (95%) LOS: 87.8%
Total: 46786 W: 7728 L: 7584 D: 31474
bench: 3172206
If the uci option 'Best Book Move' is set to true the lookup still
returns a move at random instead of the move with the highest
weight.
No functional change.
This should be enough for any legal position, even
the handcrafted ones, like the one presented by Reuven:
1Q5R/4Q1K1/B1Q5/B4Q2/N2Q4/pQ4Q1/pn2Q3/krQ4R w - -
Where currently we crash. This reverts the patch
0049d3f337 of 8/4/2012 where stack
was shrinked due to crashes while in deep analysys.
No functional change.
This greately reduces stack usage and is a
prerequisite for next patch.
Verified with 40K games both in single and SMP
case that there are no regressions.
No functional change.
This is an even safer setup proposed and tested
by Alexandre Meirelles.
Regression testing of 40K games at 10+0.05 show
result is stable both against current master:
ELO: -0.29 +-2.2 (95%) LOS: 39.7%
Total: 40000 W: 8010 L: 8043 D: 23947
and again original master (the one with smallest
time parameters):
ELO: 1.71 +-2.2 (95%) LOS: 93.8%
Total: 40000 W: 8325 L: 8128 D: 23547
Alexandre verified with LittleBlitzer time losses are
greately reduced with this setup:
Games Completed = 2100 of 3000 (Avg game length = 35.745 sec)
Settings = RR/128MB/15000ms+50ms/M 1000cp for 12 moves, D 150 moves/
Time = 39200 sec elapsed, 16800 sec remaining
1. Stockfish 190913 1091.5/2100 803-720-577 (L: m=313 t=1 i=0 a=406) (D: r=278 i=91 f=136 s=8 a=64) (tpm=212.5 d=14.75 nps=925427)
2. Houdini 2.0 w32 1008.5/2100 720-803-577 (L: m=250 t=299 i=0 a=254) (D: r=278 i=91 f=136 s=8 a=64) (tpm=204.1 d=12.04 nps=1326351)
No functional change.
Goes in the direction of avoiding time losses and seems
equivalent after almost 40K games at super fast TC of 10+0.05
ELO: 2.61 +-2.2 (95%) LOS: 99.1%
Total: 39869 W: 8258 L: 7959 D: 23652
No functional change.
Goes in the direction of avoiding time losses and seems
equivalent after almost 40K games at super fast TC of 10+0.05
ELO: 2.41 +-2.3 (95%) LOS: 98.1%
Total: 37222 W: 7843 L: 7585 D: 21794
No functional change.
The ideal setting for super-blitz might be something like:
"Emergency Base Time" = 50
"Emergency Move Time" = 5
This would give a total emergency time buffer of:
50 + 40 * 5 = 250 ms
This setup replaces the previous half cooked hack
"Don't blunder under extreme time pressure".
Test results are very good at super blitz, but keep good even
at 60 secs.
At 5+0.05
ELO: 24.30 +-2.4 (95%) LOS: 100.0%
Total: 37802 W: 10060 L: 7420 D: 20322
At 15+0.05
ELO: 13.41 +-2.9 (95%) LOS: 100.0%
Total: 22271 W: 4853 L: 3994 D: 13424
At 60+0.05
ELO: 5.30 +-3.2 (95%) LOS: 99.9%
Total: 16000 W: 2897 L: 2653 D: 10450
No functional change.
Instead of current code, give a bonus according to the frontmost
square among candidate + passed pawns.
This is a big simplification that removes a lot of accurate code
substituting it with a statistically based one using the common
'bonus' scheme, leaving to the search to sort out the details.
Results are equivalent but code is much less and, as an added bonus,
we now store candidates bitboard in pawns hash and allow this
info to be used in evaluation. This paves the way to possible
candidate pawns evaluations together with all the other pieces,
as we do for passed.
Patch passed short TC
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 16927 W: 3462 L: 3308 D: 10157
Then failed (quite quickly) at long TC
LLR: -2.95 (-2.94,2.94) [0.00,6.00]
Total: 8451 W: 1386 L: 1448 D: 5617
But when ran with a conclusive 40K fixed games at 60 secs it proved
almost equivalent to original one.
ELO: 1.08 +-2.0 (95%) LOS: 85.8%
Total: 40000 W: 6739 L: 6615 D: 26646
bench: 3884003
Now that we use pre-increment on enums, it
make sense, for code style uniformity, to
swith to pre-increment also for native types,
although there is no speed difference.
No functional change.
ENABLE_OPERATORS_ON has incorrect definitions of
post-increment and post-decrement operators.
In particularly the returned value is the variable
already incremented/decremented, while instead they
should return the variable _before_ inc/dec.
This has no real effect because are only used in loops
and where the returned value is never used, neverthless
it is wrong. The fix would be to copy the variable to a
dummy, then inc/dec the variable, then return the dummy.
So instead, rename to pre-increment that can be implemented
without the dummy, actually the current implementation
it is already the correct pre-increment, with the only change
to return a reference (an l-value) and not a copy, so
to properly mimic the pre-increment on native integers.
Spotted by Kojirion.
No functional change.
We always attempt to keep at least this emergencyBaseTime
at clock. But if available time is very low it means that
we will force ourself to play immediately to satisfy the
emergencyBaseTime constrain and so leading to blunders.
Patch is good at short and very short TC (15secs and 5secs respectively)
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 26590 W: 5426 L: 5245 D: 15919
LLR: 2.96 (-2.94,2.94) [-1.50,4.50]
Total: 5767 W: 1397 L: 1268 D: 3102
Instead seems has no influence at longer TC (60 secs)
LLR: -2.96 (-2.94,2.94) [0.00,6.00]
Total: 79862 W: 13623 L: 13339 D: 52900
So it is committed to have a broader testing but is
to be consider still EXPERIMENTAL and can be reverted
easily.
No functional change.
In case we have less then 10ms to think as soon as
we wake up the timer, it immediately fires and calls
check_time() where due to condition:
elapsed > TimeMgr.maximum_time() - 2 * TimerResolution
the stop flag is set and search returns immediately, without
actually search anything.
Here the somewhat hacky fix is to start the timer after
at least one iteration as been completed.
No functional change.
The possible maximum mobility cardinality (plus one in case of
zero squares available) is:
- Knights: max. 8 squares -> max. 9 entries
- Bishops: max. 13 squares -> max. 14 entries
- Rooks: max. 14 squares -> max. 15 entries
- Queen: max. 27 squares -> max. 28 entries
So remove the extra entries in the table.
Spotted by Dariusz Orzechowski.
No functional change.
Union of
- LMR >= 3 plies from Gary tests.stockfishchess.org/tests/view/522522960ebc595d328fcafd
- allows() tweak from Reuven tests.stockfishchess.org/tests/view/5225fa1c0ebc595d328fcb53
Both passed Step I and failed Step II.
Instead this union passed both short TC:
LLR: 2.95 (-2.94,2.94)
Total: 14525 W: 3063 L: 2874 D: 8588
And long TC
LLR: 2.94 (-2.94,2.94)
Total: 31075 W: 5566 L: 5308 D: 20201
bench: 4238160
Idea is sound but implementation is partial. Ryan and Joona noticed that
we leave an hole in material table. Also we got another report by an user
of an odd behaviour. Namely, if you start stockfish and from the prompt
give 'bench' you get 3453941, then if you run again bench you get 3453940.
The reason is that two different positions with the same number of pieces,
but one with a bishop pair and another without have the same material key.
But after Eelco patch also different material imbalance and this yields
to this issue.
Restesting at long TC shows the patch does not really contribute at
ELO improvement. Actually patch failed at long TC.
LLR: -2.97 (-2.94,2.94)
Total: 23109 W: 4104 L: 4092 D: 14913
So revert.
bench: 3453945
Prefer pos.bishop_pair() to pos.count<BISHOP>(WHITE) > 1
because the first checks that the two bishops are on
different color squares.
Although the change seems to kick in only in very rare cases,
quite surprisingly it was able to pass SPRT test at short TC.
LLR: 2.95 (-2.94,2.94)
Total: 39818 W: 8174 L: 7956 D: 23688
bench: 3453941
This was thought to be a draw but the bishops generally win. However,
it takes up to 66 moves. The position in the diagram was thought to be
a draw for over one hundred years, but tablebases show that White wins
in 45 moves. All of the long wins go through this type of semi-fortress
position. It takes several moves to force Black out of the temporary
fortress in the corner; then precise play with the bishops prevents Black
from forming the temporary fortress in another corner (Nunn 1995:265ff).
Before computer analysis, Speelman listed this position as unresolved,
but "probably a draw" (Speelman 1981:109).
bench: 3453945
STANDALONE-TOOLCHAIN.html in Android NDK says:
It is recommended to use the -mthumb compiler flag to force the generation
of 16-bit Thumb-1 instructions (the default being 32-bit ARM ones).
If you want to target the 'armeabi-v7a' ABI, you will need ensure that the
following two flags are being used:
CFLAGS='-march=armv7-a -mfloat-abi=softfp'
Note: The first flag enables Thumb-2 instructions, and the second one
enables H/W FPU instructions while ensuring that floating-point
parameters are passed in core registers, which is critical for
ABI compatibility. Do *not* use these flags separately!
Thanks to Peter Osterlund for pointout this doc and for showing me
an example Makefile to follow.
No functional change.
Becuase castle is coded as "king captures the rook"
the to_sq(move), A1/8 or H1/8 is empty after the move,
leading to assert assert(p != NO_PIECE) in color_of().
Teach allows() asserts about castle and fix the crash.
Bug reported by Ryan Takker and tracked down by Tom Vijlbrief.
No functional change.
If our rook is behind a passed pawn, all
squares are defended.
One of the longest tests to pass !
Passed both short TC
LLR: 2.97 (-2.94,2.94)
Total: 44560 W: 9518 L: 9281 D: 25761
And long TC
LLR: 2.96 (-2.94,2.94)
Total: 61348 W: 11618 L: 11192 D: 38538
bench: 3787694
With
position fen 7k/8/8/8/8/7P/6K1/7B w - - 0 1
go depth 25
The evaluation at depth 22 is not draw as it should be. The reason is that
when search reaches the position 8/6kP/8/8/8/3B4/6K1/8 w - - 0 1 if white plays
h8R or h8N then we get a position that is a "KNOWN_WIN" and is _not_ a check, so
futility pruning in qsearch kicks in and black may think that it is "futile"
to reply Kxh8 since, according to the logic of the code, it cannot raise the score
back towards a draw.
bench: 4728533
The case of two lone kings on the board is already considered
by the "No pawns" scaling factor rules in material.cpp as is
KBK and KNK.
Moreover we had a small leak in endgames map because for
KK endgame it happens white and black material keys are the
same (both equal to zero), so when adding the black endgame in
Endgames::add() we were overwriting the already exsisting
white one, leading to a memory leak found by Valgrind.
So remove the endgames althogheter and rely on scaling
to correctly set the endgames value to a draw.
No functional change.
Instead of classical flags, throw an
exception when we want to immediately halt
the search. Currently only one type
is used for both UCI stop and threads
cut off.
No functional change.
Idea originated from a post of Don Dailey
on talkchess and reported by Eelco.
This is the last succesful attempt of a long
series of trials (as usually happens, the
'idea' alone is not enough).
Passed both short 15secs TC
LLR: 2.97 (-2.94,2.94)
Total: 7629 W: 1645 L: 1515 D: 4469
And long 60secs TC
LLR: 2.96 (-2.94,2.94)
Total: 10218 W: 1932 L: 1775 D: 6511
bench: 4944581
With the new automatic setting of split depth
instead of a default, the user no longer needs
guidance on setting the split point.
Also threads now defaults to one.
No functional change.
Search::RootColor is a global parameter set
before to start a search, it is not something
trace() should change.
This patch allows to add trace() calls, for
debugging, inside search itself without altering
the bench, and also ensures that the values
returned by trace() and evaluate() are fully
equivalent.
No functional change.
The rounding formula is different between
positive and negative scores due to the
GrainSize/2 term that is asymmetric.
So use truncation instead of rounding. This
guarantees that evaluation is rounded to zero
in the same way for both positive and negative
scores.
Found with position's flip
bench: 4634244
Because VALUE_NONE is 30002, it happens that
after a check the next move is never an improving
one.
After this patch bench signature is independent from
VALUE_NONE actual value.
bench: 4303194
Apply to LMR the same Eelco's idea
applied to move count pruning.
This is the result of a series of
attempts started by Thomas Kolarik.
Passed both short TC
LLR: 2.95 (-2.94, 2.94)
Total: 5675 W: 1241 L: 1117 D: 3317
And long TC:
LLR: 2.95 (-2.94, 2.94)
Total: 8748 W: 1689 L: 1539 D: 5520
bench: 4356801
Not a real functional change, but bench changed due to different piecelist
reordering. To verify it a temporary my canonicalize_rooks function was
written as follows. It just ensures that the rook on the "smaller" square
is listed first.
void Position::canonicalize_rooks(Color c)
{
if (pieceCount[c][ROOK] == 2)
{
Square s0 = pieceList[c][ROOK][0];
Square s1 = pieceList[c][ROOK][1];
if (s0 > s1)
{
pieceList[c][ROOK][0] = s1;
pieceList[c][ROOK][1] = s0;
index[s0] = 1;
index[s1] = 0;
}
}
}
With this both bench and the test on Chess960 positions
./stockfish bench 128 1 8 Chess960.epd file > /dev/null
Gives same result.
bench: 4424151
Set threads number always to 1 at startup and let the
user explicitly to chose the number of threads.
Also preserve the useful behavior of automatically set
"Min Split Depth" according to the requested threads,
indeed this parameter is too technical for a casual user,
so, when left to zero, we set it on a sensible value.
No functional change
The new Position methods add_piece, move_piece, and remove_piece
now manage the member variables pieceList, pieceCount, and index,
and 9 blocks of code in Position that used to manipulate those
data structures by hand now call the new methods.
There is a slightly slowdown (< 1%) on Clang and on perft,
but the cleanup compensates the little speed loss.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Introduce ThreadBase struct that is search
agnostic and just handles low level stuff,
and derive all the other specialized classes
form here.
In particular TimerThread does not hinerits
anymore all the search related stuff from Thread.
Also some renaming while there.
Suggested by Steven Edwards
No functional change.
At thread creation start_routine() is called
and from there the virtual function idle_loop()
because we do this inside Thread c'tor, where the
virtual mechanism is disabled, it could happen that
the base class idle_loop() is called instead.
The issue happens with TimerThread and MainThread
where, at launch, start_routine calls
Thread::idle_loop instead of the derived ones.
Normally this bug is hidden because c'tor finishes
before start_routine() is actually called in the
just created execution thread, but on some platforms
and in some cases this is not guaranteed and the
engine hangs.
Reported by Ted Wong on talkchess
No functional change.
The endgame king + minor vs king is erroneusly
detected as king + minor vs king + minor
Here the fix is to detect king + minor earlier,
in particular to add these trivial cases to
endgame evaluation functions.
Spotted by Reuven Peleg
bench: 4727133
And #ifdef instead of #if defined
This is more standard form (see for example iostream file).
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A big simplification and removing of useless code.
Finished at 50% both at short TC (with SPRT) than
at long TC at fixed number of games:
ELO: -0.14 +-3.4 (95%) LOS: 46.8%
Total: 15206 W: 2836 L: 2842 D: 9528
bench: 5059948
This reverts commit 4b3a0fdab0.
As Gary says: " It failed when I tried it at long TC previously, and only
barely passed this time. Some anecdotal evidence is that it hurts vs other
engines as well (the Lightspeed rating list showed a 16 elo drop from previous
best version - still +- 5 error bars on both, but that's still significant)"
I also agree that if we have some doubts (like in this case) it is better to
be safe than sorry.
bench: 4615572
Reduces the influence of PSQT for entries such as
the extended center and the h-file.
Passed both short TC test:
LLR: 2.95 (-2.94,2.94)
Total: 23919 W: 5207 L: 5029 D: 13683
And long TC one:
LLR: 2.96 (-2.94,2.94)
Total: 5762 W: 1108 L: 974 D: 3680
Bench: 4617880
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This very speed critical code was full of clever (!)
tricks and subtle details.
So I have rewritten it in a more straithforward way
and, as very often happens, result is even faster
than original.
No functional change.
This reverts commit 3e95800814
For some reason it fails the short TC test:
LLR: -2.96 (-2.94,2.94)
Total: 20033 W: 4214 L: 4265 D: 11554
bench: 4769737
It is somewhat unbilievable but our SEE is broken !
If the first SEE move is a king capture and square is
defended then SEE continues instead of breaking.
The bug shows only on normal SEE, not see_sign() so
probing with a:
dbg_hit_on_c(slIndex==1, captured == KING);
reports just a tiny:
Total 3465656 Hits 6646 hit rate (%) 0
Bug was there since Retire seeValues[] and move PieceValue[] out of Position of 26/6/2011 (!)
although for some reason didn't show immediately, indeed the
bougous patch was a "No functional change" (!!)
bench: 4699504
It is somewhat unbilievable but our SEE is broken !
If the first SEE move is a king capture and square is
defended then SEE continues instead of breaking.
The bug shows only on normal SEE, not see_sign() so
probing with a:
dbg_hit_on_c(slIndex==1, captured == KING);
reports just a tiny:
Total 3465656 Hits 6646 hit rate (%) 0
Bug was there since 351ef5c85b of 26/6/2011 (!)
although for some reason didn't show immediately, indeed the
bougous patch was a "No functional change" (!!)
bench: 4793754
Without patch we have 333198 nps, with patch 334249.
A very small +0.3%, not a lot manily becuase this is a
side path that is taken very few times.
Anyhow idea is correct becuase first 'quick' condition
has an hit rate of about 95%.
No functional change.
But still keep the same original
margin for score.
Passed both short TC test
LR: 2.95 (-2.94,2.94)
Total: 3710 W: 845 L: 726 D: 2139
And long TC
LLR: 2.95 (-2.94,2.94)
Total: 57859 W: 10939 L: 10532 D: 36388
bench: 4769737
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Partially revert previous patch and use
unlikey() just as code annotation.
Actually it is better to rely on a profiler for branch prediction:
http://blog.man7.org/2012/10/how-much-do-builtinexpect-likely-and.html
"In fact, even when only one in ten thousand values is nonzero,
we're still at only roughly the break-even point"
No functional change,
It is somewhat redundant and could make SF
name too long, so use just Version, in case
of a signature build Version will be set to
'sig-xxx' otherwise, if left empty, we fall
back on usual date stamp.
No functional change.
When compiling with:
make signature-build ARCH=xxx COMP=xxx
After binary has been roduced, it will be run to
get the signature 'stockfish bench' and this
number will be used as Version, so that it
will be easy to track the original sources
from a binary.
No functinal change.
Due to a strange issue (bug?) the ternary
operator does not return a BitCountType for
icc, so revert to the expression used
before bcbc9bfd1f
No functional change.
Here speed up is the name of the game.
Speed up is gained:
- Removing the useless enoughMaterial code
- Limiting trapped rook evaluation to where it counts
Tested at long TC:
LLR: 2.97 (-2.94,2.94)
Total: 10061 W: 1948 L: 1790 D: 6323
bench: 4558173
Thanks to Don, Miguel, Louis and the other people
of talkchess forum for the suggestion:
http://www.talkchess.com/forum/viewtopic.php?t=48612
Also sync polyglot.ini with current UCI options
No functional change.
Unfortunatly a reverse test at long TC failed:
master^ vs master
LLR: 1.37 (-2.94,2.94)
Total: 33682 W: 6294 L: 6071 D: 21317
So becuase short TC score is 50% there is a good
possibility patch is not scalable.
So revert it.
bench: 4507288
Do not use threat move to detect the condition. This
let us to retire the big allows() function.
Test at short TC was within 50% score:
LLR: -2.95 (-2.94,2.94)
Total: 38272 W: 7941 L: 7940 D: 22391
To be verified with reverse long TC
bench: 4191565
Here the main difference is that now we center
aspiration window on last returned score. This allows
to simplify handling of mate scores.
We have done a reversed SPRT tests, where we wanted to
verify if master is stronger than this patch.
Long TC: master vs this patch (reverse test)
LLR: -2.95 (-2.94,2.94)
Total: 37992 W: 7012 L: 6920 D: 24060
bench: 4507288
Temporary revert aspiration window patch
so to be visible to everybody: it will be
re-applied with next patch
No functional change (together with next one)
Here the main difference is that now we center
aspiration window on last returned score. This allows
to simplify handling of mate scores.
We have done a reversed SPRT tests, where we wanted to
verify if master is stronger than this patch.
Long TC: master vs this patch (reverse test)
LLR: -2.95 (-2.94,2.94)
Total: 37992 W: 7012 L: 6920 D: 24060
bench: 4507288
A simplification of the 'dangerous' definition.
Seems neutral at reverse test at long TC
master vs patch
LLR: -2.96 (-2.94,2.94)
Total: 16974 W: 3122 L: 3139 D: 10713
bench: 4689029
Function calloc() already initializes memory to
zero, so avoid calling clear() afterwards.
Also some renaming while there (inspired by DiscoCheck).
No functional change.
Here we skip the call to pos.attacks_from<ROOK>(s) in the 98%
of cases, testing the first 2 members first. Unfortunatly
code is a bit triky and not clear. So we give up to the
speed optimization in exchange of more code clarity.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So when we are doing a LMR search at the parent ALL node.
This patch didn't prove stronger at 60" TC
LLR: -2.97 (-2.94,2.94)
Total: 22398 W: 4070 L: 4060 D: 14268
But, first, it scores at 50%, second (and most important for me) the opposite,
i.e. normal reduction when parent node is not reduced, seems very bad:
LLR: -2.95 (-2.94,2.94)
Total: 7036 W: 1446 L: 1534 D: 4056
According to Don, this idea of increased reduction of CUT nodes
works because if parent node is reduced, missing a cut-off due to
reduced depth search (meaning position is somehow tricky) forces
a full depth research at parent node, giving due insight in this
set of sensible positions.
IOW if we expect a node to fail-high at depth n, then we assume it
should fail-high also at depth n-1, if this doesn't happen it means
position is tricky enough to deserve a research at depth n+1.
bench: 4687419
We got a good result from this tweak, in line with
what was already found by Don Dailey.
At short TC:
LLR: 2.95 (-2.94,2.94)
Total: 13097 W: 2742 L: 2598 D: 7757
At long TC:
LLR: 2.97 (-2.94,2.94)
Total: 7281 W: 1408 L: 1265 D: 4608
bench: 5108393
Follow Don Dailey definition of cut/all node:
"If the previous node was a cut node, we consider this an ALL node.
The only exception is for PV nodes which are a special case of ALL nodes.
In the PVS framework, the first zero width window searched from a PV
node is by our definition a CUT node and if you have to do a re-search
then it is suddenly promoted to a PV nodes (as per PVS search) and only
then can the cut and all nodes swap positions. In other words, these
internal search failures can force the status of every node in the subtree
to swap if it propagates back to the last PV nodes."
http://talkchess.com/forum/viewtopic.php?topic_view=threads&p=519741&t=47577
With this definition we have an hit rate higher than 90% on:
if (!PvNode && depth > 4 * ONE_PLY)
dbg_hit_on_c(cutNode, (bestValue >= beta));
And an hit rate of just 28% on:
if (!PvNode && depth > 4 * ONE_PLY)
dbg_hit_on_c(!cutNode, (bestValue >= beta));
No functional change.
Fix was wrong becuase search starts from ss+1,
code is a bit tricky here, so rewrite in a way
to be more easy to read and understand.
Spotted by Eelco.
No functional change.
In case of we pick a sub-optimal move be
sure to print this, and not the best one
on seach log file.
Bug spotted by Guenther Demetz.
No functional change.
Search is started after setting a position and
issuing UCI 'go' command. Then if we stop the search
and call 'go' again without setting a new position it
is assumed that the previous setup is preserved, but
this is not the case because what happens is that
SetupStates is reset to NULL, leading to a crash as
soon as RootPos.is_draw() is called because st->previous
is now stale.
UCI protocol is not very clear about requiring that a
position is setup always before launching a search,
so here we easy the life of GUI developers assuming
that the current state is preserved after returning
from a 'stop' command.
Bug reported by Gregor Cramer.
No functional change.
A small number of tests with simulated
annealing at 15s indicated these values
may be better
And this is verified at long 60+0.05 TC
LLR: 2.95 (-2.94,2.94)
Total: 40658 W: 7821 L: 7501 D: 25336
bench: 4931544
This patch is the sum of:
- Grainsize of 4 instead of 8
- Removing "depth < DEPTH_ZERO"
- Change DEPTH_QS_RECAPTURES = -5 to -7
All the patches individually failed to pass SPRT but scored
around 50%.
Together they pass easily short TC:
LLR: 2.96 (-2.94,2.94)
Total: 4429 W: 964 L: 844 D: 2621
And with some difficult long TC of 60+0.05:
LLR: 2.95 (-2.94,2.94)
Total: 64133 W: 11968 L: 11532 D: 40633
bench: 4821467
Add MOVE_NONE at the tail, this allows to loop
across MoveList checking for *it != MOVE_NONE,
and because *it is used imediately after compiler
is able to reuse it.
With this small patch perft speed increased of 3%
And it is also a semplification !
No functional change.
Most of the time we cut-off earlier, at captures, so this
results in useless work.
There is a small functionality change becuase 'ss' can change
from MovePicker c'tor to when killers are tried due, for
instance, to singular search.
bench: 4603795
Very good at long 60"+0.05 TC
LLR: 2.95 (-2.94,2.94)
Total: 5954 W: 1151 L: 1016 D: 3787
[edit: slightly changed form original patch to avoid useless loop
across killers when killer is MOVE_NONE]
bench: 4327405
Performed more or less well at short TC
LLR: 2.95 (-2.94,2.94)
Total: 50517 W: 9815 L: 9574 D: 31128
And a bit better at long TC
LLR: 2.96 (-2.94,2.94)
Total: 15564 W: 2805 L: 2624 D: 10135
bench: 4375253
It seems that do not limiting checking the
trapped rook only on rank 1 improves the
score.
At long TC
LLR: 2.97 (-2.94,2.94)
Total: 6581 W: 1346 L: 1204 D: 4031
bench: 4985012
Don't update refutation table in case of
previous move is MOVE_NULL or MOVE_NONE
and don't try refutation if is already
a killer move.
Pass both short TC
LLR: 2.96 (-2.94,2.94)
Total: 4310 W: 953 L: 869 D: 2488
And long one
LLR: 2.95 (-2.94,2.94)
Total: 6707 W: 1254 L: 1184 D: 4269
bench: 4785954
Very good result both at short TC 15+0.05
LLR: 2.95 (-2.94,2.94)
Total: 2803 W: 596 L: 483 D: 1724
And at long TC 60+0.05
LLR: 2.95 (-2.94,2.94)
Total: 2862 W: 548 L: 431 D: 1883
bench: 4329221
Unortunatly we have no guarantee that the call to
operator~(Color c) is resolved at compile time.
Perhaps the solution would be to use C++11 const_expr,
but for now simply use the good old-style ternary operator
that works as expected.
No functional change.
A rook is trapped if on rank 1 as is the king.
Currently the condition aloows for the rook
to be also in front of the pawns as long
as king is on first rank.
Verified with short TC test:
LLR: -1.71 (-2.94,2.94)
Total: 23234 W: 4317 L: 4317 D: 14600
Here what it counts is that after 23K games
result is equal.
bench: 4696542
When it is already defined(_WIN32).
According to Microsoft documentation:
http://msdn.microsoft.com/en-us/library/b0084kay.aspx
_WIN32 Defined for applications for Win32 and Win64. Always defined.
_WIN64 Defined for applications for Win64.
Patch suggested by Joona.
No functional change.
This info is normally printed together with
PV info in uci_pv() but when search is stopped,
for instance when max search time is reached,
uci_pv is not called and we miss this bits.
Suggested by gravy_train
No functional change.
But this time do not play with pointers, in
particular do not assume that size_t is an
unsigned type of the same width as pointers.
This code should be fully portable.
No functional change.
This fixes an assert while testing with debug on.
Assert was due to static null pruning returning value
eval - futility_margin(depth, (ss-1)->futilityMoveCount)
That was sometimes higher than VALUE_INFINITE triggering
an assert at the caller site.
Because eval con be equal to ttValue and anyhow is read from
TT that can be corrupted in SMP case, we need to sanity
check it before to use.
bench: 4176431
Let TT clusters (16*4=64 bytes) to hold on a singe cache line.
This avoids the need for the double prefetch.
Original patches by Lucas and Jean-Francois that has also tested
on his AMD FX:
BIG HASHTABLE
./stockfish bench 1024 1 18 > /dev/null
Before:
1437642 nps
1426519 nps
1438493 nps
After:
1474482 nps
1476375 nps
1475877 nps
SMALL HASHTABLE
./stockfish bench 128 1 18 > /dev/null
Before:
1435207 nps
1435586 nps
1433741 nps
After:
1479143 nps
1471042 nps
1472286 nps
No functional change.
Crash is due to uninitialized ss->futilityMoveCount that
when happens to be negative, yields to an out of range
access in futility_margin().
Bug is subtle because it shows itself only in SMP case.
Indeed in single thread mode we only use the
Stack ss[MAX_PLY_PLUS_2];
Allocated at the begin of id_loop() and due to pure
(bad) luck, it happens that for all the MAX_PLY_PLUS_2
elements, ss[i].futilityMoveCount >= 0
Note that the patch does not prevent futilityMoveCount
to be overwritten after, for instance singular search
or null verification, but to keep things readable and
because the effect is almost unmeasurable, we here
prefer a slightly incorrect but simpler patch.
bench: 4311634
Instead of a pointer. This should fix the issue of
remaining with a stale pointer when for instance calling
IID, but also null search verification, singular search
and razoring where we call search with the same ss
pointer. In this case ss->ei is overwritten in the
search() call and upon returning remains stale.
This patch could have a performance hit because Eval::Info
is big (176 bytes) and during splitting we copy 4 ss entries.
On the good side, this patch is a clean solution.
Proposed by Gary.
No functional change.
Allow to use EvalInfo struct, populated by
evaluation(), in search.
In particular we allocate Eval::Info on the stack
and pass a pointer to this to evaluate().
Also add to Search::Stack a pointer to Eval::Info,
this allows to reference eval info of previous/next
nodes.
WARNING: Eval::Info is NOT initialized and is populated
by evaluate(), only if the latter is called, and this
does not happen in all the code paths, so care should be
taken when accessing this struct.
No functional change.
The idea is to fail high more easily in static
null test if in the parent node we are already
very deep in the move list, so the propability
to fail high there is very low.
[edit: I have slightly changed the functionality
moving
ss->futilityMoveCount = moveCount;
At the end of the pruning code, this should not affect
ELO in anyway, but makes code more natural and logic]
Test with SPRT is good at 15+0.05
LLR: 2.96 (-2.94,2.94)
Total: 50653 W: 10024 L: 9780 D: 30849
And at 60+0.05
LLR: 2.97 (-2.94,2.94)
Total: 40799 W: 7227 L: 6921 D: 26651
bench: 4530093
When we use sysconf(_SC_NPROCESSORS_ONLN) to get number of
cores, we have to include sysconf library that is unistd.h
Sometimes it happens to work just becuase unistd.h indirectly
included by some other libraries, but not always.
Reported and fixed by Eyal BD
No functional change.
The idea is to penalize a bishop in case of
its pawns are on the same colored squares.
Good at short 15"+0.05 TC
LLR: 2.95 (-2.94,2.94)
Total: 4252 W: 925 L: 806 D: 2521
And at longer 60"+0.05 TC
LLR: 2.95 (-2.94,2.94)
Total: 15006 W: 2743 L: 2564 D: 9699
bench: 5274705
It seems stronger both at fast 15+0.05 TC with fixed game number test:
ELO: 2.74 +-2.7 (95%) LOS: 97.6%
Total: 24000 W: 4698 L: 4509 D: 14793
And also at long 60+0.05 TC with SPRT
LLR: 3.05 (-2.94,2.94)
Total: 38986 W: 6845 L: 6547 D: 25594
bench: 5157061
Use an optinal argument instead of a template
parameter. Interestingly, not only is simpler,
but also faster, perhaps due to less L1 instruction
cache pressure because we don't duplicate the very
used SEE code path.
No functional change.
Rationale:
- Current settings seem to make engine *significantly* weaker in analysis mode.
- In practice this setting only has effect when king safety scores are high.
- Even in analysis mode its far more important to know if one side is getting mated,
rather than get evaluation correct with 1cp accuracy.
No functional change
According to Jean-Paul this setup should be stronger
than default.
And SPRT test seems to confirm it:
At fast TC 15"+0.05
ELO: 3.33 +-2.7 (95%) LOS: 99.2%
Total: 25866 W: 5461 L: 5213 D: 15192
At longer TC 60"+0.05
ELO: 7.27 +-5.0 (95%) LOS: 99.8%
Total: 6544 W: 1212 L: 1075 D: 4257
bench: 5473339
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Increasing depth limit to 10 plies seems stronger
after 16K games at 15"+0.05 (ELO: +3.56) and also
repeating the test at 60"+0.05 TC:
ELO: 2.08 +-3.1 (95%) LOS: 90.9%
Total: 16000 W: 2641 L: 2545 D: 10814
Moreover setting the limit to 12 is proved stronger
then limit set to 10 by direct SPRT test at 15"+0.05:
ELO: 2.56 +-2.0 (95%) LOS: 99.5%
Total: 46568 W: 9240 L: 8897 D: 28431
So we directly set the limit to 12, the strongest setup.
bench: 4361224
Master IID formula is depth / 2
Previous patch is depth - 4 * ONE_PLY
This one is the middle way:
(dept/2 + depth-4*ONE_PLY)/2 -> depth-2*ONE_PLY-depth/4
After 16000 games at 60+0.05 th 1
ELO: 4.08 +-3.1 (95%) LOS: 99.5%
Total: 16000 W: 2742 L: 2554 D: 10704
bench: 4781239
Same idea of 5af8179647
in qsearch() but applied to search()
After 15500 games at 15+0.05
ELO: 4.48 +-3.4 (95%) LOS: 99.5%
Total: 15500 W: 3061 L: 2861 D: 9578
bench: 4985829
Always before pruning the move, it's important to check that:
bestValue > VALUE_MATED_IN_MAX_PLY
See example position:
8/2p1p3/P1NpP3/3k4/1P1BN3/2P1P3/2Q5/6K1 w - - 0 1
http://support.stockfishchess.org/discussions/problems/268-wrong-declaring-a-forced-mate-in-3-moves
This problem was present in 2.3.1, then it was fixed by my patch.
After 24000 games at 15+0.05
ELO: 2.40 +-4.4 (95%) LOS: 95.7%
Total: 24000 W: 4774 L: 4608 D: 14618
bench: 4465997
This reverts commit a24da071f0
Seems a regression when tested against 2.3.1
With this patch, have after 20000 games at 60+0.05, we have
ELO: 13.42 +-4.8 (95%) LOS: 100.0%
Total: 20000 W: 3746 L: 2974 D: 13280
Instead with the patch reverted:
ELO: 16.62 +-4.8 (95%) LOS: 100.0%
Total: 20000 W: 3816 L: 2860 D: 13324
Although we are within error bounds here we take the conservative
approach of not introducing changes that are not proved stronger
It doesn't mean that the change shall be weaker, simply that we
don't want to take any risk.
No functional change.
Here the rational seems to be that if after one try easy
move detection fails then the easy move is not so easy :-)
After 15563 games at 60+0.05
ELO: 3.04 +-5.5 (95%) LOS: 97.0%
Total: 15563 W: 2664 L: 2528 D: 10371
No functional change.
Increase MaxRatio to use more time when in trouble.
After 16000 games at 60+0.05
ELO: 4.89 +-5.4 (95%) LOS: 99.9%
Total: 16000 W: 2700 L: 2475 D: 10825
No functional change.
This seems good at short TC controls.
After 10000 games at 20+0.05
ELO: 9.56 +-6.8 (95%) LOS: 100.0%
Total: 10000 W: 1949 L: 1674 D: 6377
Testing at long TC and regression testing is still
ongoing. So this is a bit speculative commit and
could be reverted in the future.
Also re-testing at long TC the SEE pruning in PV nodes
seems less effective (perhaps even a regression, but
still ongoing) so disabled for now.
bench: 4968764
This reverts commit 0d68b523a3.
After easy move semplification this machinery is not
needed anymore (because of we don't need to know if a
root move is a recapture)
No functional change.
Detect a move as easy only if it is the only one ;-)
or if is much better than remaining ones after we
have spent 20% of search time.
Tests are ongoing, but it seems this semplification
stands. Anyhow it is experimental for now and could
be reverted/improved with further work Gary is
testing right now.
No functional change.
After previous patch if split point master is
waiting for job and "Use Sleeping Threads" is
false (our condition for official releases) then
it will lock/unlock splitPoint mutex in a super
tight loop badly affecting performance.
Rewrite the code to lock only when we are about
to finish. Note that race condition on slavesMask
is anyhow fixed.
No functional change.
There is no point searching a move that is forced.
It wastes time while allowing computer opponents to
fill hash with 100% accuracy.
[edit: Condition moved together with "easy move" ones]
Bench identical: 4922272
We detect an easy move as a recapture with an
high margin on the second best move.
Unfortunatly the recapture detection is broken
becuase we identify as a recapture any move that
follows an opponent's previous capture !
This patch fix the logic to correctly detect a
real re-capture.
No functional change.
Note that we read shared data without lock
protection, so code is theoretically prone to
torn reads. But, first splitPoint pointer
never changes, and alpha is of integer type so
it is read in a single DWORD access.
No functional change.
This patch is actually the sum of two contributions that
have been tested independently:
1) Pruning of negative SEE moves in PV
After 10000 games at 20+0.05
ELO: 5.18 +-7 (95%) LOS: 99.2%
Total: 10000 W: 1952 L: 1803 D: 6245
2) Remove of bestValue > VALUE_MATED_IN_MAX_PLY condition
After 23000 games at 20+0.05
ELO: 1.63 +-4 (95%) LOS: 88.1%
Total: 23000 W: 4232 L: 4124 D: 14644
The whole patch as been re-tested at long TC with positive results:
After 10000 games at 60+0.05
ELO: 4.31 +-7 (95%) LOS: 98.3%
Total: 10000 W: 1765 L: 1641 D: 6594
kf = (kf == FILE_A) ? kf++ : ....
is tricky becuase kf is updated twice and it happens
to do the right thing just by accident.
Rewrite in a better way.
Spotted by pdimov
No functional change.
Give a bonus if a bishop can pin a piece or can
give a discovered check through an x-ray attack.
Seems good after 24000 games at 15"+0.05 (single thread):
ELO: 12.30 +- 99%: 5.79 95%: 4.40 LOS: 100.00%
Total: 24000 W: 4931 L: 4082 D: 14987
bench: 4917064
Rename startPosPly to gamePly and increment/decrement
the variable after each do/undo move. This adds a little
overhead in do_move() but we will need to have the
game ply during the search for the next patches now
under test.
Currently we don't increment gamePly in do_null_move()
becuase it is not needed at the moment. Could change
in the future.
As a nice side effect we can now remove an hack in
startpos_ply_counter().
No functional change.
Another trick, along the same lines of previous
patch. This time we first check positions with
white side to move that, becuase we start with
pawn on rank 7, are easily classified as wins,
then black ones.
Number of cycles reduced to 15 !
Becuase now it is faster we can remove a lot of
code to detect theoretical draws. We will calculate
them anyhow, although a bit slower, but the speed
up trick more than compensates it.
Verified that generated bitbases match original ones.
No functional change.
Change the way the index is coded so that
now looping from 0 to IndexMax generates
the pawns from RANK_7 down to RANK2.
Becuase positions with pawns at RANK_7
are easily classified as wins/draws, this
small trick allows to reduce the number
of needed iterations from 30 down to 26!
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Rename to insertion_sort so to avoid confusion
with std::sort, also move it to movepicker.cpp
and use the bit slower std::stable_sort in
search.cpp where it is used in not performance
critical paths.
No functional change.
We can encode the ClusterSize directly in the
hashMask, this allows to skip the left shift.
There is no real change, but bench number is now
different because instead of using the lowest order
bits of the key to index the start of the cluster,
now we don't use the last two lsb bits that are
always set to zero (cluster size is 4). So for
instance, if 10 bits are used to index the cluster,
instead of bits [9..0] now we use bits [11..2].
This changes the positions that end up in the same
cluster affecting TT hits and so bench is different.
Also some renaming while there.
bench: 5383795
Do a 32bit bitwise 'and' instead of a 64bit
subtract and bitwise 'and'.
This is possible because even in the biggest
hash table case (8GB) the number of entries
is 2^29 so storable in an unsigned int.
No functional change.
Save the current active position in each Thread
instead of keeping a centralized array in struct
SplitPoint.
This allow to skip a memset() call at each split.
No functional change.
Obsolete renmant of when position was directly
passed to the search instead of being copied
for the main thread as is now.
From Jundery.
No functional change.
The syntax splitPoints() should force the compiler to
value-initialize the array and because there is no
user defined c'tor it falls back on zero-initialization.
Unfortunatly this is broken in MSVC compilers, because
value initialization for non-POD types is not supported,
so left splitPoints un-initialized and add in split()
initialization of slavesPositions, that is the only
member not already set at split time.
This fixes an assert under MSVC when running with
more than one thread.
Spotted and reported by Jundery.
No functional change.
It is a draw if pawns are on G or B files, weaker pawn is
on rank 7 and bishop can't attack the pawn.
No functional change (because it is very rare and does not appear in bench)
To return a pointer to the available
thread instead of a bool. This allows
to simplify the core loop in split().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This function "returns" two values: bestValue and bestMove
Instead of returning one and passing as pointer the other
be consistent and pass as pointers both.
No functional change.
Currently a ttMove is reduced with ss->reduction = DEPTH_ZERO,
so it is actually not reduced (as it should be), but the
trick works just becuase it happens that ttMove is the first
to be tried and
reduction(depth, 1)
Always returns zero. So explicitly forbid reduction of ttMove
in the LMR condition. This is much clear and self-documented.
No functional change.
Surprisingly this rare case was not considered
when scoring a capture.
Also take in account that in the promotion case
we gain a new piece (typically a queen) but we
lose the promoting pawn.
These small issues were present since Glaurung times!
Found while browsing DiscoCheck sources
bench: 5400063
Handling of History and Gains is almost the same, with
the exception of the update logic, so unify both
classes under a single Stats struct.
No functional change.
And handle the castle directly in do/undo_move().
This allow to greatly simplify the code.
Here the beast is the nasty Chess960 that is
really tricky to get it right because could be
that 'from' and 'to' squares are the same or
that king's 'to' square is rook's 'from' square.
Anyhow should work: verified on all Chess960
starting positions.
No functional and no speed change also in Chess960.
Use a more traditional approach, along the same lines
of do_move().
It is true that we copy more in do_null_move(), but we
save the work in undo_null_move(). Speed test shows the
new code to be even a bit faster.
No functional change.
We have only one call place so inline its content.
BTW, function is already declared as FORCE_INLINE.
Also some small refactoring while there.
No functional change.
When a thread is allocated a bit is set in slavesMask.
This bit corresponds to the thread's index field that,
because it happens to be the position in the threads
array, eventually it is equal to the loop index 'i'.
But instead of relying on this 'coincidence', explicitly
use the 'idx' field so to clarify slavesMask usage.
Backported from c++11 branch.
No functional change.
Intel Compiler has 'invented' this pearl:
warning #1476: field uses tail padding of a base class
Just becuase we have subclassed MainThread and added
the field 'bool thinking'.
Pure nosense. Silence the warning.
No functional change.
Fix again TimerThread::idle_loop() to prevent a
theoretical race with 'exit' flag in ~Thread().
Indeed in Thread d'tor we raise 'exit' and then
call notify() that is lock protected, so we
have to check again for 'exit' before going to
sleep in idle_loop().
Also same change in Thread::idle_loop() where we
now check for 'exit' before to go to sleep.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Great code simplification: - instead do not futility
prune threat refutations. allows_move() is therefore removed.
4000 games at 50,000 nodes/move:
1085-989-1926 [51.2%] LOS=98.3%
4000 games in 10"+0.1"
756-751-2493 [50.1%] LOS=55.1%
EDIT: I have retested the patch of Lucas in a slightly different form
(without pruning in PvNode) and test mre or less confirms that
60 lines of code are totally unuseful:
After 6195 games at 15"+0.05"
1333 - 1325 - 3537 ELO 0
bench 5140990
Silly logic bug introduced in dda7de17e7
Timer thread, when msec = 0, instead of going
to sleep, calls check_time() in an endless loop.
Spotted and reported by snino64 due to abnormally
high CPU usage.
No functional change.
Subclass MainThread and TimerThread and declare
idle_loop() virtual. This allow us to cleanly
remove a good bunch of hacks, relying on C++
polymorphism to do the job.
No functional change.
Rename it is_finished and use it only in main
thread to signal search is finished. This allows
us to simplify the complex SMP logic.
Ultra tricky patch: deep test is required under
wide conditions like pondering on and option
"Use Sleeping Threads" set to false.
No functional change.
This reverts commit 869c924410
I misunderstood here. Actually it can happen that
thread is created but still not entered idle_loop
and at the same time start_searching() is called.
Becuase 'do_sleep' is set start_searching() will
set it to false and start the search, but when,
at last, the thread enters idle_loop(), resets
the flag and goes to sleep: not what we want.
Revert the hack waiting for a better solution
in the next patches.
No functional change.
Finally we can now merge the 'ponderhit' case with
'stop' and 'quit'.
The patches have been done step by step to help debugging
becuase this is really tricky code.
No functional change.
Reset Limits.ponder only if search continue, but if
we are going to stop the search there is no need
(and is also confusing) to clear the 'ponder' flag.
This mimics the behaviour upon rceiving 'stop' when
pondering.
No functional change.
In SAN notation when two pieces of the same type can
move to a given destination square, a disambiguation
additional info (like starting file) shall be added
to the SAN move.
If one of the two pieces is pinned, the corresponding
move _could_ be illegal and in this case disambiguation
is not needed. But to be pinned alone it is not enough
to deduce that the move is illegal, for instance in this
position:
R3rk2/2r6/8/8/8/8/8/K7 b - - 0 1
The move Rc8 is ambiguous although the rook in e8 is pinned
and the correct SAN notation should be Rcc8.
No functional change.
Don't wait for the search to finish after a 'stop'
command, but keep processing the GUI input if any.
Also explicitly wake up the main thread (that could be
sleeping) after a 'stop' or 'quit' command and do not
rely on wait_for_search_finished() doing it for us.
This patch cleans up the code and functions's definitions,
but it is risky and needs a good test under different
conditions to be sure it does not introduces hungs up.
No functional change.
Revert patch c581b7ea36
Seems a regression after testing from Gary:
ELO: 7.24 +- 99%: 17.03 95%: 12.93
LOS: 97.86%
Wins: 439 Losses: 381 Draws: 1962
And mine:
After 5410 games at 15"+0.05
Wins: 936 Losses: 1141 Draws: 3333 ELO -13
Moreover we know that there is a regression in the range
of patches which include the fromNull patch.
Probably this is not the only regression since 2.3.1 and
perhaps the idea under fromNull is good, but at the moment,
while in deep regression hunting, better to be on the safe
side and revert it entirely.
My guess on why this is a regression is that using the
negated evaluation of previous ply in case of null search
fails to take in account the king safety asymmetry between
the two colors. This is of course just a guess.
bench 5503830
They are not self-describing and create a lot of user
requests about them.
Given that the values are already well tuned there
is no need to expose them as UCI options.
No functional change.
Sometimes is faster, but not always and on very long mates
produces strange scores probably due to truncation of PV
artifacts.
So simply perform normal search also in case of UCI 'mate x'
command, with the only difference that when a mate in x is
found search returns immediately.
No functional change.
Now that we can call 'bench' command also from
interactive terminal it makes no more sense to
exit the application if the user types a wrong
file name.
No functional change.
Since &-ing with SpaceMask restricts the set to the home
half of the board, it is possible to use just one popcount
instead of 2 by shifting "safe" to the other half of the
board. This gives a small speedup especially on systems
where hardware popcount is not available.
Patch kindly sent by Richard Vida.
No functional change.
It is now possible to run SF on a 'mate in x' testsuite.
For instance in case of a file with fen strings of
positions with mate in 10 we can now 'bench' on it:
stockfish bench 128 1 10 mate_in_10.epd mate
No functional change.
Following a user request I added the handling of UCI:
go mate x
Currently we just return from a PV node if x moves have been
done. Probably not the best approach. I have looked at Fruit/Toga
sources and there is even simpler: engine falls back on a fixed
depth search.
No functional change.
And return on using TT as backing store for position
evaluations.
Tests (even on single thread) show eval cache was a regression.
In multi thread result should be even worst because eval cache
is a per-thread struct, while TT is shared.
After 4957 games at 15"+0.05 (single thread)
eval cache vs master 969 - 1093 - 2895 -9 ELO
So previous reported result of +18 ELO was probably due to an
issue in the testing framework (a bug in cutechess-cli) that
has been fixed in the meanwhile.
bench: 5386711
In case of null search at low depths returns a fail low
due to a threat then, rather than return beta-1 (to cause
a re-search at full depth in the parent node), we set a flag
threatExtension = true (false by default) that will cause
moves that prevent the threat to be extended of one ply
in the following search.
Idea and patch is by Lucas Braesch.
Lucas also did the tests:
1500 games in 5"+0.05":
SF_threatExtension vs SF_20121222: 366 - 331 - 803 [51.2%] LOS=90.8%
3000 games in 10"+0.1":
SF_threatExtension vs SF_20121222: 610 - 559 - 1831 [50.8%] LOS=93.2%
Tests confirmed by Gary after 10570 games,
ELO: 2.79 +- 99%: 8.72 95%: 6.63
LOS: 94.08%
Wins: 1523 Losses: 1438 Draws: 7607
And finally by me at 15"+0.05, single thread, 3824 games
threatExtension vs master 768 - 692 - 2364 +7 ELO
bench 4918443
This is difficult code becuase a bug here could lead
to very subtle crashes in case of SMP games where we
have TT move corruption due to concurrent access.
Anyhow I have fully verified te code throwing at it
random moves. It shoudl work.
No functional change.
Test by Joona prooves the new feature don't value 70 added lines of code.
Grand totals after 10040 games (crashes: 0) for tt_both
master_9edc7 - 6a93488_6a934: 1756 - 1688 - 6596 ELO +2 (+- 2.7)
Confirmed by test of Gary:
After 8680 games:
ELO: 0.80 +- 99%: 9.62 95%: 7.31
LOS: 65.38%
Wins: 1288 Losses: 1268 Draws: 6130
Thanks a lot to both for testing it !!!
bench 5149248
This is more complex than what I'd like but I
was unable to split in small chunks.
Here we add 2 slots to TTEntry (valueUpper and depthUpper)
so that sizeof(TTEntry) returns to the original 16 bytes
and we can pack exactly 4 entries in a 64 bytes cache line.
Now we save an upper bound score alongside a lower (exact)
score. The idea is to increase TT cut-offs rates becuase
there is now an higher probability for a node to use TT info.
This patch is highly experimental and probably needs further
steps as is hinted by an unrealistic bench number:
bench: 2022385
In almost all cases we already know in advance that
color_of() argument is different from NO_PIECE.
So avoid the check for NO_PIECE in color_of() and
test at caller site in the very few places where
this case could occur.
As a nice side effect, this patch fixes a (bogus)
warning under some versions of gcc.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use an eval cache instead of TT to store node
position evaluations.
It is already an improvment and, because it frees
two TT entry slots, paves the way to extend TT to
store both upper and lower bounds.
After 4855 games, single thread, 15"+0.05
Mod vs Orig 1165 -920 - 2770 ELO +18
bench: 5149248
And document why this is an hard limit. It
seems for some (lucky) people 32 threads
are not enough.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In search(), after we evalute the position, in case there
isn't any TT entry we create one with just the evaluation
score.
This patches removes that code. The reason becuase the patch
deserves a single commit it is becuase introduces a (very small)
functional change due to the fact that the total number of
TT stores is less now and this slightly alters the TT hits
of our benchmark.
bench: 4983262
Rely fully on eval cache. Note that we still save eval
info to TT, this is not needed at this moment and will be
removed in future patches. We keep it so to have a "non
functional change" patch.
No functional change.
With this patch series we want to introduce a per-thread
evaluation cache to store node evaluation and do not
rely anymore on the TT table for this.
This patch just introduces the infrastructure.
No functional change.
In case of a RootNode or a SpNode move has
been already checked for legality so we can
skip a redundant check.
Spotted by Frank Genot.
No functional change.
In qsearch we should update the bestValue as we do
in case of futilityValue < beta, also when pruning
moves with non-positive see.
Spotted by Lucas Braesch
Bench: 5695710
Send the PV lines to GUI only once at the end of the
PV search loop or just in case of long searches.
We need to sync also sending of "currmove" info to
avoid sending info on current move without first
informing the GUI on the PV line we are searching on.
No functional change.
At this point we have already verified (value > alpha)
and this implies, in case of a non-PV node, where search
window size is zero, that value >= beta.
This is not so self-evident, so document the code with
an assert condition.
No functional change.
Let the caller to decide where to redirect (cout or cerr) the
ASCII representation of the position. Rename the function to
reflect this.
Renamed also from_fen() and to_fen() to set() and fen() respectively.
No functional change.
In case a PvNode node has a static evaluation above alpha but
no available moves we want to flag the node as BOUND_EXACT,
not as BOUND_UPPER as is currently.
The behaviour was recently introduced with patch d471c49700
of 3/10/2012
Spotted by Hongzhi Cheng.
bench: 5558464
Both Lucas re-test and Jean-Francois confirrm it
is a regression.
Here Jean-Francois's results after 3600 games :
Score of 96d3b1c92b vs 3b87314: 690 - 729 - 2181 [0.495] 3600
ELO: -3.86 +- 99%: 14.94 95%: 11.35
LOS: 15.03%
Wins: 690 Losses: 729 Draws: 2181 Total: 3600
Bench: 5404066
Don't prune and eventually extend check moves of type
DISCO_CHECK (pun intended, Lucas will understand :-) ).
Patch from Lucas Braesch that has also tested it:
Result: 879-661-2137, score=52.96%, LOS=100.00% (time 10"+0.1")
I have started a verification test right now.
bench: 6004966
From Jean-Francois's
Final result after 5000 games :
Score of c581b7e vs a878312: 1163 - 970 - 2867 [0.519] 5000
ELO: 13.35 +- 99%: 12.71 95%: 9.65
LOS: 100.00%
Wins: 1163 Losses: 970 Draws: 2867 Total: 5000
From me
After 3266 games at 20"+0,05
Score of c581b7e vs a878312: 612 - 607 - 2047
So no regression at longer TC and perhaps a little gain at
fast TC.
In this case we try a rather drastic
approach: we simply don't futility prune
in qsearch when arriving from a null move.
So we save evaluating and also save to mess
with eval margins at all because margin is used
only in futility.
Also accuracy should not be affected, actually it
improves because we don't prune anything anymore.
bench: 5404066
Performs well at very short TC of 40/4+0.05 (courtesy of Jean-Francois):
Wins: 2503 Losses: 2146 Draws: 5581 Total: 10230 +12 ELO
But is poor at longer TC of 20"+0.05
Wins: 321 Losses: 373 Draws: 1141 Total: 1808 -10 ELO
The patch was clearly a tradoff between speed and accuracy and
the most interesting part of it are test results that can be
commented as follows:
- A short TC is very sensible to any speed increase
- A longer TC is more sensible to accuracy and less to speed
So a patch that does not change speed is suitable to be tested at
short TC, while a speed/accuracy compromise patch is IMO better to
be tested at longer TC to verify loss of accuracy can be tolerated.
In this case the revert is only temporary. We will come back again
once we will be able to preserve the evaluation margin.
bench: 5809010
Reuse the evaluation of the parent with inverted sign and
set margin to zero (this is an hack!).
This is done only in qsearch where almost 15% of calls are
from a null move. In normal search the number of nodes where
(ss-1)->currentMove == MOVE_NULL
is almost zero and so there is no need of using this trick.
The big advantage of this patch is a speed-up due to skipped
evaluate() calls, that are very costly.
Functionality is of course affected and we will need to proper
test it later. For now we just register a 3-4% speed up.
Suggested by Hongzhi Cheng.
bench: 5051328
When testing if a move blocks the threat path there is no
reason to require the threat to be a slider. Indeed threat
can be a double pawn push like in this example:
r1bq1rk1/ppp1np1p/4n1p1/3p4/3P2Q1/2P1B3/PPBN2PP/R4RK1 w - - 0 16
Where white's move Rf6 blocks the threat f5.
As a nice side effect we can retire the now useless helper
piece_is_slider().
This patch kicks in only very rare cases, indeed the bench is
still the same!
bench: 5809010
There is no guarantee that split() consumes all the node's
moves. Indeed split() can return without performing any job
for instance because MAX_SPLITPOINTS_PER_THREAD is reached
or becuase no available threads are found (this latter case
is much more common).
So search must continue in those cases and we cannot force
exiting from move's loop.
Bug introduced by 1ac417edb8 of 5/10/2012
Spotted by Frank Genot.
No functional change.
If a previous move attacks the king (with the piece
of the threat move removed) then must be a discovered
check, otherwise it means that first move gave check
and we were not able to do a null move.
Also renamed stuff to better document the function's
context.
No functional change.
When testing if a piece is moving through the squares
vacated by a previous move there is no reason to require
the piece to be a slider, indeed we can have a double
pawn push like in this example:
r1q2rk1/2p1bppp/2Pp4/pN5b/Q1P1p3/4B2P/PP1R1PP1/1K5R w - - 3 18
Where black's move f5 is connected to previous move Be7 that
frees the path.
Or we can have a castle move:
r1bqkb1r/pppp1ppp/2n1pn2/1B6/4P3/2N2N2/PPPP1PPP/R1BQK2R b KQkq - 5 1
Where a previous move Bb5 allows the white to castle king side.
This time patch is mine ;-)
new bench: 5809010
We send to GUI multi-pv info after each cycle,
not just once at the end of the PV loop. This is
because at high depths a single root search can
be very slow and we want to update the gui as
soon as we have a new PV score.
Idea is good but implementation is broken because
sort() takes as arguments a pointer to the first
element and one past the last element.
So fix the bug and rename sort arguments to better
reflect their meaning.
Another hit by Hongzhi Cheng. Impressive!
No functional change.
When checking if the moving piece p1 in a previous
move m1 defends the destination square of a move m2
we have to use the occupancy with the from square of
m2 removed so to take in account the case in which
f2 will block an x-ray attack from p1.
For instance in this position:
r2k3r/p1pp1pb1/qn3np1/1N2P3/1p3P2/2B5/PPP3QP/R3K2R b KQ - 1 9
The move eXf6 is connected to the previous move Bc3 that
defends the destination square f6.
With this patch we have about 10% more moves detected as
'connected'. Anyhow the absolute number is very low, about
4000 more moves out of 6M nodes searched.
Another issue spotted by Hongzhi "Hawk Eye" Cheng ;-)
new bench: 5757373
On Intel, perhaps due to 'lea' instruction this way of
zeroing the lsb of *b seems faster than a shift+negate.
On perft (where any speed difference is magnified) I
got a 6% speed up on my Intel i5 64bit.
Suggested by Hongzhi Cheng.
No functional change.
Compiler complies that 'cnt' is initialized but
unused (in !CheckThreeFold case). Moving the
definition of 'cnt'out of the loop seems to do
the trick.
No functional change.
Instead of use a variable so to resolve many conditions
already at compile time. In quiesce is also where we
have most of the InCheck nodes and is one of the most
performance critical code paths.
Speed up of 1.5% with Clang and 1% with gcc
Suggested by Hongzhi Cheng.
No functional change.
When checking if a move defends the threatened piece
we correctly remove from the occupancy bitboard the
moved piece. This patch removes from the occupancy also
the threatening piece so to consider the cases of moves
that defend the threatened piece x-raying through the
threat move.
As example in this position:
r3k2r/p1ppqp2/Bn4p1/3p1n2/4P1N1/5Q1P/PPP2P1P/R3K2R w KQkq - 1 10
The threat black move is dxe4. With this patch we include
(and so don't prune) white's Bb7 that would be pruned otherwise.
The number of affected position is very low, around 1% of cases,
so we don't expect ELO changes, neverthless this is the logical
and natural thing to do.
Patch suggested by Hongzhicheng.
new bench: 5323798
ReducedStateInfo is a redundant struct that is also
prone to errors, indeed must be updated any time is
updated StateInfo. It is a trick to partial copy a
StateInfo object in do_move().
This patch takes advantage of builtin macro offsetof()
to directly calculate the number of quad words to copy.
Note that we still use memcpy to do the actual job of
copying the (48 bytes) of data.
Idea by Richard Vida.
No functional and no performance change.
Only in not performance critical code like pretty_pv(),
otherwise continue to use the good old C-style arrays
like in extract/insert PV where I have done some code
refactoring anyhow.
No functional change.
Silly typo (introduced in e304db9d1e) completely
messed up move notation in case of promotions causing
"Illegal move" warning in cutechess-cli.
Reported by Jörg Oster.
No functional change.
In multi-threads runs with debug on we experience some
asserts due to the fact that TT access is intrinsecally
racy and its contents cannot be always trusted so must
be validated before to be used and this is what the
patch does.
No functional case.
And restore old behaviour of not returning from a RootNode
without updating RootMoves[].
Also renamed is_draw() template parameters to reflect a
'positive' logic (Check instead of Skip) that is easier
to follow.
New bench: 5312693
When signal 'stop' is raised we return bestValue
that could be still set at -VALUE_INFINITE and
this triggers an assert. Fix it by returning
a value we know for sure is not +-VALUE_INFINITE.
Reported by 平岡拓也 Hiraoka.
No functional change.
Note that the actual pickup is done in the class
d'tor so to be sure it is always triggered, even
in case of a sudden exit due to a 'stop' signal.
No functional change.
Function move_to_san() requires the Position to be
passed by referenced because a do/undo move is done
inside the function to detect a possible mate and to
add to the san string the corresponding '#' suffix.
Instead of passing a copy of current position pass
directly the original position object after const
casting it. This has the advantage to avoid a costly
Position copy, on the down side a bench test could
report different searched nodes if print(move) is
used, due to the additionals do_move() calls.
No functional change.
Existing Makefile is buggy for PowerPC, it has no
SSE, yet it is given it if Prefetch is enabled,
because it isn't ARMv7.
Patch from Matthew Brades.
No functional change.
We can have ttValue == VALUE_NONE when we use a TT
slot to just save a position static evaluation, but
in this case we also save DEPTH_NONE so to avoid
using the ttValue in search. This happens to work,
but due to a number of lucky and tricky cases that
we now documnet through a bunch of asserts and a
little change to value_from_tt() that has no real
effect but clarifing the code.
No functional change.
On some platforms 128 MB of RAM for TT is too much,
so run 'bench' with the default 32 MB size.
No functional change although of course now 'bench'
reports a different number: 5545018
Use popcount() instead in the only calling place.
It is used only at initialization so there is no
speed regression and anyhow even initialization
itself is not slowed down: magic bitboard setup
stays around 175 msec on my slow 32bit Core Duo.
No functional change.
Simplify the code and doing this introduce a couple
of (very small) functional changes:
- Always compare to depth even in "mate value" condition
- TT cut-off in qsearch also in case of PvNode, as in search
Verified against regression with 2500 games at 30"+0.05
on 2 threads: 451 - 444 - 1602
Functional changed: new bench is 5544977
As Joona says: "The problem is that when doing full
window search (-VALUE_INFINITE, VALUE_INFINITE), and
pruning all the moves will return fail low which is
mate score, because only clause touching alpha is
"mate distance pruning". So we are returning mate score
although we are just pruning all the moves. In reality
there probably is no mate in sight.
Bug spotted and fixed by Joona.
Implement lsb/msb using armv7 assembly instructions.
msb is the easiest one, using a gcc intrinsic that generates
code using the ARM's clz instruction. lsb is also using this
clz instruction, but with the help of ARM's 'rbit' (bit
reversing) instruction. This leads to a >2% speed gain.
I also renamed 'arm-32' to the more meaningfull 'armv7' in the Makefile
No functional change.
Port to qsearch() the same changes we recently
added to search().
Overall this search refactoring series shows
almost 2% speed up on gcc compile.
No functional change.
When using the Makefile (as for the mingw case),
IS_64BIT and USE_BSFQ are already set with
ARCH=x86-64 and do not need to be redefined.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
First disable Contempt Factor during analysis, then
calculate the modified draw score from the point of
view of the player, so from the point of view of
RootPosition color.
Thanks to Ryan Taker for suggesting the fixes.
No functional change.
These kind of arch specific code is really nasty
to make it right becuase you need to verify on
all the platforms.
Now should compile properly also on ARM
Reported by Jean-Francois.
No functional change.
In particular on ARM processors. Original patch by
Jean-Francois, sligtly modified by me to preserve
the meaning of NO_PREFETCH flag.
Verified with gcc, clang and icc that prefetch instruction
is correctly created.
No functional change.
A simplification and also a small speed-up of
about 1% mainly due to reducing calls to
thisThread->cutoff_occurred().
Worst case split point recovering time after a
cut-off occurred is limited to 3 msec on my slow
PC, and usually is below 1 msec, so it seems safe
to remove the cutoff_occurred() check.
No functional change.
This is very crude and very basic: simply in case
of a draw for repetition or 50 moves rule return
a negative score instead of zero according to the
contempt factor (in centipawns). If contempt is
positive engine will try to avoid draws (to use
with weaker opponents), if negative engine will
try to draw. If zero (default) there are no changes.
No functional change.
Use a value related to PawnValue instead.
This is a different patch from previous one because
could affect game play and skill levels, although
in a mostly unmeasurable way. Indeed thresold has
been raised so easy move is a bit harder to trigger
and skill level is a bit more prone to blunders.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Show the real value in the code, not hide it
behind a variable name, especially when there
is only once occurence.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Extend for an extra half-ply in case the node is (probably)
going to fail high. In this case the added overhead is limited.
A novelity is the way this patch has been tested: Always in
self-play but with a much longer TC to allow the singular
extension to fully kick in and also (my impression) to have
less noisy results.
Ater 1015 games on my QUAD at 60"+0.05
Mod vs Orig 173 - 150 - 692 ELO +8
Handle also the SMP case. This has been quite tricky, not
trivial to enforce the node limit in SMP case becuase
with "helpful master" concept we can have recursive split
points and we cannot lock them all at once so there is the
risk of counting the same nodes more than once.
Anyhow this patch should be race free and counted nodes are
correct.
No functional change.
As long as isPvMove (renamed to pvMove) is set after
legality check, we can postpone legality even in PV case.
Patch aligns the PV case with the common non-pv one.
No functional change.
Patch and tuning by Gary Linscott from an idea of Ryan Taker.
Double tested by Gary:
Wins: 3390 Losses: 2972 Draws: 11323
LOS: 99.999992%
ELO: 8.213465 +- 99%: 6.746506 95%: 5.124415
Win%: 51.181792 +- 99%: 0.969791 95%: 0.736740
And by me:
After 5612 games 1255 1085 3271 +11 ELO
We have a crash with this position:
rkqbnnbr/pppppppp/8/8/8/8/PPPPPPPP/RKQBNNBR w HAha -
What happens is that even if we are castling QUEEN_SIDE,
in this case we have kfrom (B8) < kto (C8) so the loop
that checks for attackers runs forever leading to a crash.
The fix is to check for (kto > kfrom) instead of
Side == KING_SIDE, but this is slower in the normal case of
ortodhox chess, so rewrite generate_castle() to handle the
chess960 case as a template parameter and allow the compiler
to optimize out the comparison in case of normal chess.
Reported by Ray Banks.
Broken by commit a44c5cf4f7 of 3 /12 / 2011 that
was labeled "No functional change" because our 'bench'
test didn't triggered that particular endgame. Indeed
we need to run a specific bench on a set of endgames
position when touching endgame.cpp because normal bench
does not cover endgames properly.
Found by MSVC 2012 code analyzer.
It seems Intel is unable to properly workout templates with 'static'
storage specifier.
Workaround using an anonymous namespace instead.
No functional change.
Currently we exit the loop when
abs(bestValue) >= VALUE_KNOWN_WIN
but there is no logical reason for this. It seems more
natural to re-search again with full open window.
This has practically no impact in most cases, we have a
'no functional change' running 'bench' command.
The trick here is to check for legality only in the
(rare) cases we have pinned pieces or a king move
or an en-passant.
This trick is able to increase the speed of perft
of more then 20%!
No functional change.
Simply reshuffling the code inverting the condition in next_attacker()
yields a miraculous speed up of more than 3% under gcc!
On my laptop a bench run goes from 320Knps to 330Knps
No functional change.
This allows to reduce the scanning for new X-ray attacks
according to the capturing piece type.
It seems to be just a very small speed increase in MSVC 64
bit and gcc 32 bit, I guess cache issues value more than some
instruction less to execute (as usual).
No functional change.
It is very difficult and risky to assure
that a running thread doesn't access a global
variable. This is currently true, but could
change in the future and we don't want to rely
on code that works 'by accident'. The threads
are still running when ThreadPool destructor is
called (after main() returns) and this could
lead to crashes if a thread accesses a global
that has been already freed. The solution is to
use an exit() function and call it while we are
still in main(), ensuring global variables are
still alive at threads termination time.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When many threds concurrently print you need to serialize
the access to std::cout to avoid output lines are intermixed
with the contents of each thread.
This is not strictly needed at the moment because
only main thread prints out, although some ad-hoc
test could trigger UCI::loop() printing while searching.
Anyhow we want to lift this pretty avoidable constrain
also as a prerequisite for future work.
This patch just introduces the support, next one will enable
the serialization.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Before the search we setup the starting position doing all the
moves (sent by GUI) from start position to the position just
before to start searching.
To do this we use a set of StateInfo records used by each
do_move() call. These records shall be kept valid during all
the search because repetition draw detection uses them to back
track all the earlier positions keys. The problem is that, while
searching, the GUI could send another 'position' command, this
calls set_position() that clears the states! Of course a crash
follows shortly.
Before searching all the relevant parameters are copied in
start_searching() just for this reason: to fully detach data
accessed during the search from the UCI protocol handling.
So the natural solution would be to copy also the setup states.
Unfortunatly this approach does not work because StateInfo
contains a pointer to the previous record, so naively copying and
then freeing the original memory leads to a crash.
That's why we use two std::auto_ptr (one belonging to UCI and another
to Search) to safely transfer ownership of the StateInfo records to
the search, after we have setup the root position.
As a nice side-effect all the possible memory leaks are magically
sorted out for us by std::auto_ptr semantic.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Should be more clear from where the 'magic' numbers
come from.
Also bit of reformat while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of just size(). Although code is longer,
should be more immediate to understand when reading.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
To mimics C++11 std::mutex and std::condition_variable,
also rename locks and condition variables to be more
uniform across the classes.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Reduce of one instruction. It seems a tad faster on
the profiler now. Very slightly but anyhow it is a
code semplification.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This better mimics std::vector::operator[] and
fixes a warning with MSVC 64bit.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Further reduce redundancy in move generation.
Veirifed no speed regression on MSVC, Clang and gcc.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Greatly simplify these very performace critical functions.
Amazingly we don't have any speed regression actually under
MSVC we have the same assembly for generate_moves() !
In generate_direct_checks() 'target' is calculated only
once being a loop invariant.
On Clang there is even a slight speed up.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Are used by Position but do not belong to that class,
there is only one instance of them (that's why were
defined as static), so move to a proper namespace instead.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Check we are the last slave of the split point
before to wake up the master. This should avoid
spurious wakes up.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We can detect the split point master also from within idle_loop,
so we can call the function without parameters and remove an
overloaded member hack in Thread class.
Note that we don't need to take a lock around curSplitPoint
when entering idle_loop() because if we are the master then
curSplitPoint cannot change under our feet (because is_searching
is set and so we cannot be reallocated), if we are a slave
we enter idle_loop() only upon Thread creation and in that case
is always splitPointsCnt == 0. This is true even in the very rare
case that curSplitPoint != NULL, if we have been already allocated
even before entering idle_loop().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Directly use the underlying std::map instead and avoid
a useless inheritance.
As a nice side-effect Options global object has now a
default c'tor avoiding possible issues with globals
initializations.
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Templetize MovePicker::next_move() member function instead. It
is easier and we also avoid the forwarding of MovePicker() c'tor
arguments in the common case.
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
First change: If Haiku is host platform, change
installation prefix to /boot/common/bin
Second change: Only link in pthreads if Haiku isn't
host platform.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And group there all the formatting functions but
uci_pv() that requires access to search.cpp variables.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use the helpers to format the PV info but without
writing to output stream (file or cout). Message
formatting and sending are two logically different
task.
Incidentaly reintroduce the pretty_pv() name,
from Glaurung memories :-)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Simplifies the code and seems more natural.
We have a very small fucntional change becuase now
at PV nodes castles are extended one ply anyhow.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With warning level 4 MSVC complains that a default
assignment operator could not be generated due to
member 'file' is a reference (warning C4512).
Use a pointer instead of a reference and move
struct Tie outisde class Logger while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems more accurate: lsb is clear while 'first
bit' depends from where you look at the bitboard.
And fix compile in case of 64 bits platforms that
do not use BSFQ intrinsics.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of -O4 option that does not work with both mingw and
Linux gcc (tested with Clang 3.1).
As reported by Reed Kotler:
Turns out that -O4 is not a valid option for clang unless you have
the proper gold linker and plugins built. That's because -O4 enables
LTO, which writes out bitcode files during the compile, and then loads
those and optimizes them during the link phase.
It requires a linker that supports LLVM's LTO. There is a plugin for
Gold available as part of LLVM.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We have a signed integer here so let the return type
take in account that.
Found by Clang with -Weverything option.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Return a reference instead of void so to enable
chained assignments like
"p = q = Position(...);"
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Although a (little) functional change, we have no ELO change
but formula it is now more clear.
After 13019 games at 30"+0.05
Mod vs Orig 2075 - 2088 - 8856 ELO 0 (+- 3.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems the standard behaviour as implemented
in most engines although UCI protocol does not
specify what to do upon "ucinewgame" command.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Calculate the attacks only for the piece to disambiguate,
not for all.
Also some reformatting while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
At endgame time push the king near his pawns (actually
one of them).
Original idea is from Critter (although slightly different),
implementation is mine and is completely different from the
original, in particular it is different the algorithm to
compute the minimum distance from pawns.
After 19895 games at 15"+0.05
Mod vs Orig 3638 - 3248 - 13009 ELO +7
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Error is due to ambiguous overloading of operator^
because we have both the built-in operator^(int, int)
and the user defined operator^(Bitboard, Square).
This error does not trigger when using Makefile becuase,
due to luck, the user defined operator^(Bitboard, Square)
happens to be always defined _after_ opposite_colors() so
that compiler does not claim. But in case of Microsoft MSVC
we don't have a Makefile and the order of files compilation
is chosen by the compiler (in an unpredictable way). So it
could still happen that error is not detected (as in my case),
but in another case the order of compilation of the files could
be so that at some point both operator^ were defined before
opposite_colors() and this triggers the error.
The fix is much simpler than the explanation :-)
Reported by Quocvuong82.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Currently when we fail to open a book file, for instance
if it doesn't exsist, we leave Book::open() with ifstream
failbit set. If then the book file is added, we correctly
open it at next attempt, but failbit is still set so that
after opening we exit because ifstream::good() returns false.
The fix is to reset failbit upon exiting.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In case a book entry has 'count' field set to 0
we crash. Spotted by Clang's static analyzer.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So to be fully in sync with StateInfo, and move struct
to position.h, just below StateInfo.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Revision 2aac860db3 of 27 / 4 / 2012
changed can_castle() signatures from bool to int and
this broke the code that calculates polyglot hash key
in book.cpp
Instead of directly fixing the code we prefer to change
castling rights definitions to align to the polyglot ones
(as we did in previous patch). After this step we can simply
take internal castle rights as they are and use them
directly to calculate polyglot book hash key, as we do
in this patch that fixes the regression.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
To be aligned with PolyGlot book castle right definitions.
This will be used by next patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Early classify as known draws the positions
where white king is trapped on the rook file.
Suggested by Dan Honeycutt.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After extensive testing (I was off line this week and let
the test go on) it seems this change is useless:
After 33968 games on a QUAD (4 threads) at 15"+0.05
Mod vs Orig 5425 - 5550 - 22993 ELO -1 (+-2.1)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Only in case of promotion we care about an upper case
promotion piece char, so std::transform() is overkill
for the task.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Assumption: Junior sends promotions according to the side to move (ucase/lcase).
Fact: Stockfish generally handles promotion lcase.
Patch: Handling position fen input moves always with lcase promotions.
Ported back by Portfish. No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems ADL lookup is broken with the STLPort library. Peter says:
The compiler is gcc 4.4.3, but I don't know how many patches they
have applied to it. I think gcc has had support for Koenig lookup
a long time. I think the problem is the type of the vector iterator.
For example, line 272 in search.cpp:
if (bookMove && count(RootMoves.begin(), RootMoves.end(), bookMove))
gives the error:
jni/stockfish/search.cpp:272: error: 'count' was not declared in this scope
Here RootMoves is:
std::vector<RootMove> RootMoves;
If std::vector<T>::iterator is implemented as T*, then Koenig lookup
would fail because RootMove* is not in namespace std.
I compile with the stlport implementation of STL, which in its vector
class has:
typedef value_type* iterator;
I'm not sure if this is allowed by the C++ standard. I did not find
anything that says the iterator type must belong to namespace std.
The consensus in this thread
http://compgroups.net/comp.lang.c++.moderated/argument-dependent-lookup/433395
is that the stlport iterator type is allowed.
Report and patch by Peter Osterlund.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Let first argument to be the 'color'. This allows to align
pos.pieces() signatures with piece_list(), piece_count() and
all the other functions where 'color' argument is passed as
first one.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Hex representation doesn't add any value in those cases.
Preserve hex representation where more self-documenting
for instance for binary masks values.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems to increase SMP performances. To note that
this patch goes in the opposite direction of "Active
reparenting" where we try to reparent an idle slave
as soon as possible. Instead here we prefer to keep
it idle instead of splitting on a shallow / near the
leaves node.
After 11550 games on a QUAD (4 threads) at 15"+0.05
Mod vs Orig 1972 - 1752 - 7826 ELO +6 (+-3.6)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Set optimization level to 4 and get a 2.564% faster binary:
Stockfish (Clang, Level 4) bench:
$ make build ARCH=osx-x86-64 COMP=clang
(Clang does not support PGO)
Average of 4 trials:
Total time (ms): 5137.5
Nodes searched: 5631135
Nodes/second: 1096084.5
Stockfish (Clang, Level 3) bench:
$ make build ARCH=osx-x86-64 COMP=clang
(Clang does not support PGO)
Average of 4 trials:
Total time (ms): 5269.25
Nodes searched: 5631135
Nodes/second: 1068679.25
Stockfish (GCC, PGO) bench:
$ make profile-build ARCH=osx-x86-64
Average of 4 trials:
Total time (ms): 5286
Nodes searched: 5631135
Nodes/second: 1065292.25
Suggestion and performance tests by Daylen Yang.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A PvNode that givesCheck has been already granted
an extension of ONE_PLY in previous condition.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Makefile modified to support the clang compiler under Mac.
This was tested using clang 4 under Mountain Lion, but should
also work fine under Lion and possibly under Snow Leopard.
It requires the 'Xcode 4.x Command Line Tools' to be installed.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Since revision 374c9e6b63
we use also castling information to calculate king safety.
So before to reuse the cached king safety score we have to
veify not only king position, but also castling rights are
the same of the pre-calculated ones.
This is a very subtle bug, found only becuase even after
previous patch, consecutives runs of 'bench' _still_ showed
different numbers. Pawn tables are not cleared between 'bench'
runs and in the second run a king safety score, previously
evaluated under some castling rights, was reused by another
position with different castling rights instead of being
recalculated from scratch.
Bug spotted and tracked down by Balint Pfliegel
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we can call bench multiple times
from command prompt we need to ensure searched
nodes remain constant across different runs.
Spotted by Blint Pfliegel.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After 6K games at 60" + 0.1 on QUAD with 4 threads
this implementation fails to show a measurable increase,
result is well within error bar.
Perhaps with 8 or more threads resut is better but we
don't have the hardware to test. So retire for now and
in case re-add in the future if it proves good on big
machines.
The only good news is that we don't have a regression and
implementation is stable and bug-free, so could be reused
somewhere in the future.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The check for detecting when a split point has all the
slaves still running is done with:
slavesMask == allSlavesMask
When a thread reparents, slavesMask is increased, then, if
the same thread finishes, because there are no more moves,
slavesMask returns to original value and the above condition
returns to be true. So that the just finished thread immediately
reparents again with the same split point, then starts and
then immediately exits in a tight loop that ends only when a
second slave finishes, so that slavesMask decrements and the
condition becomes false. This gives a spurious and anomaly
high number of faked reparents.
With this patch, that rewrites the logic to avoid this pitfall,
the reparenting success rate drops to a more realistical 5-10%
for 4 threads case.
As a side effect note that now there is no more the limit of
maxThreadsPerSplitPoint when reparenting.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Check for a cutoff occurred also high in
the tree and not only at current split
point.
This avoids some more wasted reparenting.
No functional chnage.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is more correct given what the function does. In
particular single_bit() returns true also in case of
empty bitboards.
Of course also the usual renaming while there :-)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of reparenting to oldest split point, try to reparent
to latest. The nice thing here is that we can use the YBWC
helpful master condition to allow the reparenting of a split
point master as long as is reparented to one of its slaves.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And update master->splitPointsCnt under lock
protection. Not stricly necessary because
single_bit() condition takes care of false
positives anyhow, but it is a bit tricky and
moving under lock is the most natural thing
to do to avoid races with "reparenting".
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In Young Brothers Wait Concept (YBWC) available slaves are
booked by the split point master, then start to search below
the assigned split point and, once finished, return in idle
state waiting to be booked by another master.
This patch introduces "Active Reparenting" so that when a
slave finishes its job on the assigned split point, instead
of passively waiting to be booked, searches a suitable active
split point and reprents itselfs to that split point. Then
immediately starts to search below the split point in exactly
the same way of the others split point's slaves. This reduces
to zero the time waiting in idle loop and should increase
scalability especially whit many (8 or more) cores.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Apart from the semplification it is now more clear that
the actual Tempo added was half of the indicated score.
This is becuase at start compute_psq_score() added half
Tempo unit and in do_move() white/black were respectively
adding/subtracting one Tempo unit.
Now we have directly halved Tempo constant and everything
is more clear.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is still enabled during fixed limit search so to
use it during fixed depth/nodes/time matches.
Bug reported by Daylen.
No functional changes.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Shrinking from [16] to [2][2] is able to speedup
perft of start position of almost 5% !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Shrink dimensions of the biggest stack consumers arrays.
In particular movesSearched[] can be safely shrinked
without any impact on strenght or risk of crashing.
Also MAX_PLY can be reverted to 100 with almost no impact
so to limit search recursion and hence stack allocation.
A different case is for MAX_MOVES (used by Movepicker's
moves[]), because we know that do exsist some artificial
position with about 220 legal moves, so in those cases SF
will crash. Anyhow these cases are never found in games.
An open risk remains perft, especially run above handcrafted
positions.
This patch originates from a report by Daylen that found
SF crashing on his Mac OS X 10.7.3 while in deep analysys
on the following position:
8/3Q1pk1/5p2/4r3/5K2/8/8/8 w - - 0 1
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now a fen file with Chess960 positions is
correctly parsed. But it is mandatory to set
"UCI_Chess960" option _before_ to call bench.
Note that this was not needed/possible before
adding the possibility to call 'bench' from
command prompt.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we can call bench on current position
we can directly use it to perform our perft.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we can call bench from command prompt
has a sense to teach bench to run the current
set position. To do this is enough to call bench
with 'current' as fen source parameter.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After extensive test Gary says:
"So, after 16k games at 10"+1" on an i7, the undefended rook test
looks to be not good (albeit by a very small margin).
3063 - 3093 - 9844 (-1).
I doubt that is causing the regression, but even so, it looks like
it's not worth keeping, and we can go back to the simpler undefended
minors check."
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Unfortunatly accessing thread local variable
is much slower than object data (see previous
patch log msg), so we have to revert to old code
to avoid speed regression.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Much faster then pthread_getspecific() but still a
speed regression against the original code.
Following are the nps on a bench:
Position
454165
454838
455433
tls
441046
442767
442767
ms (Win)
450521
447510
451105
ms (pthread)
422115
422115
424276
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After we release the SplitPoint lock the master, suppose
is main thread, can safely return and if a "quit" command
is pending, main thread exits and associated Thread object
is freed. So when we access master->is_searching a crash
occurs.
I have never found such a race that is of course very rare
becuase assumes that from lock releasing we go to sleep for
a time long enough for the main thread to end the search and
return. But you can never know, and anyhow a race is a race.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So to avoid a crash when setting the moves in
UCI "position startpos moves ...." command.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
But use the newly introduced local storage
for this. A good code semplification and also
the correct way to go.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use thread local storage to store a pointer to the thread we
are running on. This will allow to remove thread info from
Position class.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With this change sources are fully endianess
independent, so we can simplify the Makefile.
Somewhat surprisingly we don't have any speed
regression !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Erroneusly adds default positions to the fens
loaded from external file.
Bug introduced in adb71b8096
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The 2 overload functions map() accept a pointer to
EndgameBase<Value> or a pointer to EndgameBase<ScaleFactor>.
Because Endgame<E> is derived from one of them we can
directly use a pointer to this class to resolve the
overload as is needed in Endgames::add().
Also made class Endgames fully parametrized and no more
hardcoded to the types (Value or ScaleFactor) of endgames
stored.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A std::set (that is a rb_tree) seems really
overkill to store at most a handful of moves
and nothing in the common case.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is possible to start with 'stockfish', then from
command prompt type 'bench' and SF will do what you expect.
Old behaviour is anyhow preserved. As a bonus we can now
start from command line any UCI command understood by
Stockfish. The difference is that after execution of a
command from arguments SF quits, while at the end of the
same command from prompt SF stays in UCI loop.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Allows some code semplification and avoids directly
allocation and managing heap memory.
Also the usual renaming while there.
No functional change and no speed regression.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In particualr before to wake up main thread that
could take some random time. Until we don't reset
search time we are not able to correctly track
the elapsed search time and this can be dangerous
under extreme time pressure.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We need to wake up main thread if it is sleeping
waiting for stop or ponderhit, so we cannot skip
calling wait_for_search_finished().
Found by Othello1984.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In this case SF stop searching and goes sleeping
waiting for a stop / ponderhit before to return
best move. So when a "stop" arrives we need to wake
up the main thread again.
Another regression introduced by 3aa471f2a9,
hopefully the last one.
Thanks to Otello1984 to reporting this.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Renamed stuff and added comments. The aim is to make more
readable, at least by me ;-) , this newly added part of code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Incredible typo from my side!
The 2 tables are completely different, one counts 1s the
other returns the msb position. Even more incredible
the 'stockfish bench' command returns the same number
of nodes!!!
Spotted by Justin Blanchard.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add more detailed pawn shelter/storm evaluation
After 10670 games at 10"+0.05
Mod vs Orig 2277 - 1941 - 6452 ELO +11 !!!
The first real increase since 2.2.2, congratulations Gary !!!
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fixes a not so rare crash (once every 100 games)
newly introduced. Unfortunatly I am still not
able to figure out why :-(
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
There is no need to "invent" different names
from the original UCI parameters.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Penalty for undefended rook
Almost no change at longer TC, but perhaps there
is a tiny increase....
After 17522 games at 10"+0.05
Mod vs Orig 3064 - 2967 - 11491 ELO +2
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When quitting we should avoid RootPosition to be
destroyed while threads are still running, leading
to a crash. In case of a "stop" or "ponderhit"
command there is no need for the UI thread to wait.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And add final touches to this long patch series.
All the series has been verified against regression with
20K games at fast TC.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We have a clash with start_fn defined both as a
Thread memeber and as a function pointer type in
pthread_create().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We cannot set do_sleep flag of main thread before
"bestmove" is sent to GUI, otherwise GUI could send
immediately the next "go" command that triggers
start_thinking() and because do_sleep is set UI
thread resets the flag to launch a new search. But
when shortly after main thread returns to main_loop()
flag is incorrectly reset and main thread goes to sleep
hanging the engine.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We store pointers instead of Thread objects because
Thread is not copy-constructible nor copy-assignable
and default ones are not suitable. So we cannot store
directly in a std::vector.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Associate platform OS thread to the Thread class instead of
creating it from ThreadsManager.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Split the data allocation, now done (mostly once)
in read_uci_options(), from the wake up and sleeping
of the slave threads upon entering/exiting the search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems is the cause of strange and rare hangs
reported by some users where Stockfish stops
responding to GUI. It is not clear why but for
the moment revert the patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Not correct warning about use of an uninitialized
variable. The warning is not correct becuase we can
never reach the warned code when in SpNode, anyhow
the fix is simple.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A somewhat tricky function pointer cast allows us
to move the platform specifics to lock.h, the cast
is tricky because return type is not the same of the
casted function in Linux (for Windows return type is
a DWORD that is a long) but this should not be a
problem as long as the size is the same;
From: http://stackoverflow.com/questions/188839/function-pointer-cast-to-different-signature
"OpenSSL was only casting functions pointers to
other function types taking and returning the same
number of values of the same exact sizes, and this
(assuming you're not dealing with floating-point)
happens to be safe across all the platforms and
calling conventions I know of. However, anything
else is potentially unsafe."
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This should avoid some aliasing issues
with TT table access.
After 3913 games at 10"+0.05
Mod vs Orig 662 - 651 - 2600 ELO +0 (+- 6.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Even if not under attack. This seems to be good
especially on openings.
After 12112 games at 10"+0.05
Mod vs Orig 2175 - 1997 - 7940 ELO +5 (+- 3.7)
[Patch series from Gary, little edited by me]
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We need splitted Tie classes because MSVC stream library
takes a lock on buffer both on reading and on writing and
this causes an hang because, while searching, the I/O
thread is locked on getline() and when main thread is
trying to std::cout() something it blocks on the same
lock waiting for I/O thread getting some input and
releasing the lock.
The solution is to use separated streambuf objects for
cin and cout.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A big code simplification and cruft removing, make
Logger class a singleton and fully self conteined.
Also add direction indicators (">>" and "<<") to
better differentiate input and output lines in the
log file.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Using "o" as a parameter with the on_xxx(const UICOption& o)
functions is a bit dangerous because of confusion with "0".
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Some trial was needed to find the correct recipe but now
we log both stdin and stdout to file "io_log.txt".
Link http://spec.winprog.org/streams/ was very useful
to understand the details of iostreams implementation.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
By means of "Use Debug Log" UCI option it is possible to toggle
the logging of std::cout to file "out.txt" while preserving
the usual output to stdout. There is zero overhead when logging
is disabled and we achieved this without changing a single line
of exsisting code, in particular we still use std::cout as usual.
The idea and part of the code comes from this article:
http://groups.google.com/group/comp.lang.c++/msg/1d941c0f26ea0d81
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In particular before initialization. So that SF
seems more snappy at startup.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
As it was in Glaurung times. Also rearranged order
so that byTypeBB[0] is accessed before byTypeBB[x]
to be more cache friendly. It seems there is even
a small speedup.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also some microoptimizations, were there from ages
but hidden: the renaming suddendly made them visible!
This is a good example of how better naming lets you write
better code. Naming is really a kind of black art!
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Stick to UCI protocol that says:
* by default all the opening book handling is done by the GUI,
but there is an option for the engine to use its own book
("OwnBook" option, see below)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
UCI protocol it is not clear about what the engine
should be supposed to do when "ucinewgame" is
received. Stockfish simply sets the position to
start FEN, but it is redundant becuase the GUI always
resends the position after "ucinewgame" command, so
it seems we can safely ignore that command.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When a button fires UCIOption::operator=() is called and from
there the on_change() function. Now it happens that in case of
a button the on_change() function resets option's value to
"false" triggering again UCIOption::operator=() that calls again
on_change() and so on in an endless loop that is experienced
by the user as an application hang.
Rework the button logic to fix the issue and also be more clear
about how button works.
Reported by several people working with Scid and tracked down
to the "Clear Hash" UCI button by Steven Atkinson.
Bug recently introduced by 2ef5b4066e.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now we are forced to just use C++ iostream becuase
buffers are independent and using C library functions
like printf() or scanf() could yield to issues.
Speed up of about 1%.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Result of t.time * 1000 should be a 64 bit value, not an int.
Bug reported by several users.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fine tune newly introduced pinner bonus score:
After 34696 games at 2"+0.05
Mod vs Orig 7474 - 7087 - 20135 ELO +3 (+- 2.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So to be done only once at startup and in the (unlikely)
cases that a relevant UCI parameter is changed, instead
of doing it at the beginning of each search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Introduce 'on change' actions that are triggered as soon as
an UCI option is changed by the GUI. This allows to set hash
size before to start the game, helpful especially on very fast
TC and big TT size.
As a side effect remove the 'button' type option, that now
is managed as a 'check' type.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Self-documenting code instead of a tricky
bitwise tweak, not known by everybody.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add a bonus if a slider is pinning an enemy piece.
Idea from Critter.
After 27443 games at 2"+0.05
Mod vs Orig 5900 - 5518 - 16025 ELO +4 (+- 2.7)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Introduce and use a new Time class designed after
QTime, from Qt framework. Should be a more clear and
self documented code.
As an added benefit we now use 64 bits internally to get
millisecs from system time. This avoids to wrap around
to 0 every 2^32 milliseconds, which is 49.71 days.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Visual Studio 11 is worried that shift result could
overflow an integer, this is impossible becuase max
value of the shift is 4, but compiler cannot know it.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Pre compute castle path so to quickly test
for impeded rule.
This speeds up perft on starting position
of more than 2%.
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix this warning with MSVC 64 bits:
warning C4244: '=' : conversion from 'std::streampos' to 'size_t',
possible loss of data
Point is that std::streampos could be negative, while size_t
is always non-negative.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And introduce SPlitPoint bestMove to pass back the
best move after a split point.
This allow to define as const the search stack passed
to split.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is a prerequisite for next patch and simplifies
the function. testing at ultra fast TC shows no
regression.
After 24302 games at 2"+0.05
Mod vs Orig 5122 - 5038 - 13872 ELO +1 (+- 2.9)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Reverse the meaning of castleRightsMask[sq] so that now
is stored the castling right that will be removed in
case a move starts from or arrives to sq square. This
allows to simplify the code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of by square. This is a more conventional
approach, as reported also in:
http://chessprogramming.wikispaces.com/Zobrist+Hashing
We shrink zobEp[] from 64 to 8 keys at the cost of an extra
'and 7' at runtime to get the file out of the ep square.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We shouldn't need lock protection to increment
splitPointsCnt and set curSplitPoint of masterThread.
Anyhow because this code is very tricky and prone to
races bound the change in a single patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When updating castleRights in do_move() perform only one
64bit xor with zobCastle[] instead of two.
The trick here is to define zobCastle[] keys of composite
castling rights as a xor combination of the keys of the
single castling rights, instead of 16 independent keys.
Idea from Critter although implementation is different.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Because TT table is shared tte->move() could change
under our feet, in particular we could validate
tte->move() then the move is changed by another
thread and we call pos.do_move() with a move different
from the original validated one !
This leads to a very rare but reproducible crash once
every about 20K games.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
There is no need to limit the maximum ply searched to
100, with deep exclusion search extensions we could
reach it even with much smaller search depths.
The only drawback is an increase in stack usage, but
is limited mainly to id_loop(), in particular the
recursive search() functions are not affected.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Replace a 64 bit 'and' by two 32 bits ones and
use unsigned instead of int.
This simple patch increases perft speed of 6% on
my Intel Core 2 Duo !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
But only when needed, after a split point. This behaviour
does not apply when useSleepingThreads is false, becuase
in this case threads are not woken up at split points so
must be already running.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Rule says should be reset only after a capture and/or
a pawn move.
This incredible bug was here since Glaurung times !
Spotted by Kiriakos.
No functional change in the test bench because we
don't reach the 50 moves limits.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With default value of 100 no change in regard of current
behaviour. Increasing the value makes SF to think a
longer time for each move. Decreasing the value makes SF
to move faster.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This method belongs to Thread, not to ThreadsManager.
Reshuffle stuff in thread.cpp while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Release split point lock before to wake up
master thread. This seems to increase speed
in case "sleeping threads" are used:
After 7792 games with 4 threads at very fast TC (2"+0.05)
Mod vs Orig 1722 - 1627 - 4443 ELO +4 (+- 5.1)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When allocating a slave we set both is_searching
and splitPoint under lock protection.
Unfortunatly the order in which the variables are
set is not defined. This article was very clarifying:
http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for-multi-threaded-programming/
So when in idle loop we test for is_searching and then
access splitPoint, it could happen that splitPoint is still
not updated leading to a possible crash.
Fix the race lock protecting splitPoint access.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With current code we could raise bestValue above beta,
not what is intended for.
Spotted by Richard Vida.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Simplify and streamline the code. Verified all the
resulting bitbases are not changed.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix an issue where the log file stores an incorrect +0.00
eval after a search has been stopped.
Bug reported by Ajedrecista.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This allows to retire ClearMaskBB[] and use just
one SquareBB[] array to set and clear a bit.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With this we should compeltely remove the need
of installing third party POSIX threads library
when compiling with mingw-gcc under Windows.
Spotted by Trung Tu.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is not clear the advantage and we don't want
to risk of introducing regressions on this
very critical parameter. So revert to old limit.
After 16003 games
Mod vs Orig 2496 - 2421 - 11086 ELO +1 (+-3)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Apart from some renaming the biggest change
is the retire of split_point_finished()
replaced by slavesMask flags. As a side
effect we now take also split point lock
when allocation available threads.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of Posix threads. This seems to fix time
losses of the gcc compiled version for Windows.
The patch replaces the MSVC specific _MSC_VER flag
with _WIN32 and _WIN64 that are defined both by
MSVC and mingw-gcc.
Workaround found by Jim Ablett.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of by SEE. Almost no ELO change but it is
a bit easier and is a more natural choice given
that good captures are ordered in the same way.
After 10424 games
Mod vs Orig 1639 - 1604 - 7181 ELO +1 (+-3.8)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
pass references (Windows style) instead of
pointers (Posix style) as function arguments.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The previous line, LDFLAGS += $(CXXFLAGS), does not make sense, and
breaks profile-build, thus changing it into: LDFLAGS += -flto.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Integrate TT_MOVE step into the first state. This allows to
avoid the first call to next_phase() in case of a TT move.
And use overflow detection instead of the bunch of STOP_XX
states to detect end of moves.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In case of a PvNode could happen that alpha == beta - 1,
for instance in case the same previous node was visited
with same beta during a non-pv search, the node failed low
and stored beta-1 in TT. Then the node is searched again
in PV mode, TT value beta-1 is retrieved and updates alpha
that now happens to be beta-1.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Almost no functional change because multiple recaptures
to same square are very rare, but neverthless it seems
the correct thing to do.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
They are almost the same, if the function arguments would
have been the same they could even be integrated.
Also a bit of renaming while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And increase LMR limit. Tests show no change ELO wise,
but we prefer to take the risk to commit anyhow becuase
is a 'prune reducing' patch.
After 10749 games
Mod vs Orig: 1670 - 1676 - 7403 ELO 0 (+-3.7)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Skip calling promotion generation functions in
the very common case of no possible promotion
evasion. Also retire generate_pawn_captures()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix an incorrect warning: 'bm may be used uninitialized'
with the old but still commonly used gcc 4.4
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we don't support anymore popcount detection
at runtime this target is obsolete.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use $(CXX) instead of assuming compiler name is 'gcc'
Spotted by Louis Zulli.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
All the piece dependant data is passed now as
function arguments so that the code is exactly
the same for bishop and rook.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is more correct and specific. Another naming
improvement while reading Critter sources.
No functional changes.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And directly pass RootMoves instead of SearchMoves
to main thread. A class declaration is better suited
in a header and slims a bit the fatty search.cpp
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We just need to verify if a legal move is among the
SearchMoves, so we don't need a vector for this.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
New gcc 4.7 complains about casting a volatile pointer
to void* so assign the variables directly.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It could lead to terrible mistakes otherwise, as
it happened during a game on playchess when on
this position (after white's f4):
2q4r/4b1k1/p3rpp1/3np2p/PpNpNP1P/1P1P2PQ/2P1R3/4R1K1 b - - 0 1
SF moves immediately e5xf4 instead of the correct f5.
In general during engine matches it is impossible the
opponent leaves a piece hanging or anyhow starts a
clear losing sequence. So avoid to fall in subtle traps.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It should help to avoid recalculating check squares
of sliding attackers for queen when already done for
bishops and rooks. Of course this helps when there are
bishop, rook and queen on the board !
Fixed also a subtle bug (use of same variable b in while
condition and in condition body) introduced recently by
revision d655147e8c that triggers
in case we have at least 2 non-pawn discovered check pieces.
This is very rare that's why didn't show in the node count
verification where we actually have a case of 2 dc pieces
in position 14, but one is a pawn.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
During generation of non-captures checks (in qsearch)
we don't consider castling moves that give check,
this patch includes also this rare case. Verified with
perft that all the non-capture checks are now generated.
There should be a very little slowdown due to the extra
work, but actually I failed to measure it. I don't expect
any ELO improvment, there is even no functional change on
the standard depth 12 search, it is just to have a correct
move generator.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The full movegen patch series shows a speed up of almost
6% (!) on perft, and code is much more readable too.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
MV_CHECK is an alias of the more appropiate named
MV_NON_CAPTURE_CHECK so use only the latter.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And make CRITICAL_SECTION locks the only option for Windows.
This guarantees backward compatibility with all the Windows
versions (even XP and older) and an hassle free experience
when compiling for Windows. Tests performed by Ingo and
reported on talkchess confirm there is no speed penalty
against the most modern SRW locks:
http://www.talkchess.com/forum/viewtopic.php?t=41835&start=20
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
On that platform non-bracketed casting are not supported.
Reported by Richard Lloyd.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Further increase safety against time losses. After this
change (tested on LittleBlitzer and cutechess) I had no
more time losses at 2" and 1"+0.02 TC both on Windows
and Linux on more than 10000 games.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We try hard not to lose on time even under extreme
time pressure. We achieve this through 3 different but
coordinated steps:
1) Increase max frequency of timer events
2) Quickly return after a stop signal
3) Take in account timer resolution
With these SF played under LittleBlitzer at 1"+0.02 and 3"+0
without losing on time even one game.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Before creating main thread we set its do_sleep flag to true,
then thread is created and it will go to sleep in main_loop()
after resetting do_sleep.
But if after the setting of do_sleep and before its resetting
the UI thread calls start_thinking() it will not wait on:
if (!asyncMode)
while (!main.do_sleep)
cond_wait(&sleepCond, &main.sleepLock);
as it should but will immediately return before the main thread has
started the search. This very rare race show itself during bench,
when the first position is erroneusly skipped so that bench node count
results of 5309038 instead of the correct 5457475.
The patch is somewhat tricky, but is simple and it works!
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Locals left and right shadow two same named
variables in the std::ifstream base class.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems it yields to missing wake-up events with the
result of SF loosing on time as reported by many people.
So revert the patch and use a more robust approach: assume
there can be spurious wake ups events and make the code to
work also in those cases.
While debugging I found that WaitForSingleObject() had wrong
parameter 0 instead of INFINITE yielding to a crash while
exiting under Windows, strangely unnoticed til now.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The aim is to have shorter names without losing
readibility but, if possible, increasing it.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Retire open(), close() and name() from public visibility
and greately simplify the code. It is amazing how much
can be squeezed out of an already mature code !
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In Windows when OLD_LOCKS is defined we use SetEvent() to mimic
the semantic of the POSIX pthread_cond_signal().
Unfortunatly there is not a direct mapping because with SetEvent()
the state of an event object remains signaled until it is set
explicitly to the nonsignaled state or until a single waiting thread
has been released. Instead in case of pthread_cond_signal(), if there
are no waiting threads it has no effect. What we may want is something
like PulseEvent() instead of SetEvent(). Unfortunatly it is documented
by Mcrosoft as 'unreliable' due to spurious wakes up that could
filter out the signal resetting. So we opt to reset manually any
pending signaled state before to go to sleep.
This fixes the strange misbehaves during 'stockfish bench'
when using OLD_LOCKS under Windows.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that HasPopCnt is a compile time constant we can
centralize and unify the BitCountType selection.
Also rename count_1s() in the more standard popcount()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It was meant to build a single binary optimized
for any kind of CPU: with and without hardware POPCNT.
This is a nice idea but in practice was never used, or
people builds binary with popcnt enabled or not, mainly
according to their type of CPU. And it was also never
used in the official Jim's builds where, in case, would
be easier for a number of reasons, do build two different
versions: with and without SEE42 support.
So retire this feature and simplify the code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This leads to a further and unexpected simplification
of this already very size optimized code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Currently after a 'quit' command UI thread raises stop
signal, exits from uci_loop() and calls Threads.exit()
while the search threads are still active.
In Threads.exit() main thread is asked to terminate, but
if it is parked in idle_loop() it will exit and free its
resources (in particular the shared Movepicker object) while
sibling slaves are still active and this leads to a crash.
The fix is to let the UI thread always wait for main thread
to finish the search before to return from uci_loop().
Found by Valgrind when running with 8 threads.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Greatly improves the usage. User defined conversions
are a novelity for SF, another amazing C++ facility
at work !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The condition for a mate score was wrong:
abs(v) < VALUE_MATE - PLY_MAX * ONE_PLY
instead of
abs(v) < VALUE_MATE_IN_PLY_MAX
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
During the search we score a mate as "plies to mate
from the root" to compare in an homogeneous way the
values returned by different sub-trees. However we
store in TT a mate score as "plies to mate from the
current position" the let the TT value remain valid
across the game.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is simple but somewhat tricky code that deserves
a bit of documentation. A bit of renaming while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add the check that alpha < beta - 1 if and only if PvNode is true.
The current code would not flag PvNode and alpha == beta - 1. In
other words, the || is not an exclusive OR!.
Also sync assert conditions of search() and qsearch()
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Make a better use of C++ operators overloading to
streamline the APIs.
Also sync polyglot.ini file while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I am not able to reproduce the speed regression anymore,
and also we were using cout even before speed regression
so probably the reason is not there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Be consistent with the way these operators are defined
in plain C (and in C++).
Spotted by Lucas Braesch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We don't use killers to order evasions, so it
seems natural do not consider an evasion cut-off
move as a possible killer. Test shows almost no
change, as it should be becuase this is a really
tiny change, but neverthless seems the correct
thing to do.
After 11893 games
Mod vs Orig 1773 - 1696 - 8424 ELO +2 (+-3.4)
Idea from Critter.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Take advantage of argument-dependent lookup (ADL) to
avoid specifying std:: qualifier in some STL functions.
When a function argument refers to a namespace (in this
case std) then the compiler will search the unqualified
function in that namespace too.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Partially revert efd2167998
Without this patch SF does not send "bestmove" to GUI.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Seems sensibly faster: On a
./stockfish bench > /dev/null
We have +2% on mingw and even +5% on MSVC !
Also removed the nice but complex enum set960 machinery,
use directly the underlying move_to_uci() function.
Speed regression reported by Heinz van Saanen.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Do not return the book move if is not among the
RootMoves,in particular if we have been asked to
search on a move subset with "searchmoves" then
return book move only if it is among this subset.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Introduce pv_info_to_log() and pv_info_to_uci() and
greatly cleanup this stuff.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Actually after last patch it happens that delta
starts always with the fixed value of 16.
So further remove useless code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems that we just need to look at previous score to
compute aspiration window size.
After 5350 games:
Mod vs Orig 800 - 803 - 3647 ELO +0 (+- 5.2)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Diretcly use the underlying std::vector<Move> and the
STL algorithms. Also a bit of cleanup while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is ok to redirect st pointer to startState, but the latter
should be updated with the content pointed by the st of the
original position. The bug is hidden when startState and *st
are the same as is the case of searching from start position,
but as soon as moves are made (as is the case when splitting)
the bug leads to a crash.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The Position object used by UI thread is a local variable in
uci_loop(), so after receiving "quit" command the function
returns and the position is freed from the stack.
This should not be a problem becuase in start_thinking() we copy
the position to RootPosition that is the one used by main search
thread. Unfortunatly we blindly copy also StateInfo pointer that
still points to the startState struct inside UI position. So the
pointer becomes stale as soon as UI thread leaves uci_loop() and
because this happens while main search thread is still recovering
after the 'stop' signal we have a crash.
The fix is to update the pointer to the correct startState after
the copy.
Found with Valgrind.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Comments should be informative but not pedantic / obvious.
The only exception is the function description where we
indulge a bit on the "chatty" side, but has always been like
this since Glaurung times, so we continue with this tradition.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Tested togheter with previous patch; shows no regression and
is a semplification.
After 5817 games:
Mod vs Orig 939 - 892 - 3986 ELO +2 (+- 5.1)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Triggered by a comment of Eelco on talkchess. Also
a bit of cleanup while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Consider negative captures as good if
still enough to reach beta.
After 7502 games:
Mod vs Orig 1225 - 1158 - 5119 ELO +3 (+- 4.5)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A cast rarely is the right solution. In this case was enough
to redifine 3 variables with type size_t instead of int
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems we don't have any added value. Note that the
moves that were used to be extended are still flagged
as dangerous so to avoid at least pruning them.
After 9555 games
Mod vs Orig 1562 - 1540 - 6453 ELO +0 (+- 4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
As Heinz says:
"Function empty() should have a constant run-time even
on lousy compilers and you spare the not.
The change is even measurable: + 100-150 nodes/sec. Wow:-)"
Patch from Heinz van Saanen
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is more idiomatic for a functor (a function object) as are
the endgames.
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A pinned piece cannot move and so does not play any role
in SAN disambiguation.
Reported by Steven Edwards.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It was never clear to me why we needed this trick, and now
that we rely only on C++ std::getline() and std::cout for
input / output it is even more a mistery what this code does.
So disable it and wait to see if someone screams ;-)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Detach from the UI thread the input arguments used by
the search threads so that the UI thread is able to receive
and process any command sent by the GUI while other threads
keep searching.
With this patch there is no more need to block the UI
thread after a "stop", so it is a more reliable and
robust solution than the previous patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Unfortunatly xboard sends immediately the new position to
search after sending "stop" when we have a ponder miss.
Becuase main thread position is not copied but is referenced
directly from root position and the latter is modified by
the "position.." UCI command we end up with the working position
that changes under our feet while the search is still recovering
after the "stop" and this causes a crash.
This happens only with the (broken) xboard, native UCI does not
have this problem.
Reported by otello1984
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fixes an hang when playing with ponder ON. Perhaps there is still
a very small race but now it seems engine does not hang anymore.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Move global search-related variables under "Search" namespace.
As a side effect we can move uci_async_command() and
wait_for_stop_or_ponderhit() away from search.cpp
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use the starting thread to wait for GUI input and instead use
the other threads to search. The consequence is that now think()
is alwasy started on a differnt thread than the caller that
returns immediately waiting for input. This reformat greatly
simplifies the code and is more in line with the common way
to implement this feature.
As a side effect now we don't need anymore Makefile tricks
with sleep() to allow profile builds.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If we are pondering we will stop the search only when
GUI sends "ponderhit" or "stop" commands or when we reach
maximum depth. In all the other cases we continue to search
so there is no need to verify for available time.
Also better clarify why wait_for_stop_or_ponderhit() before
to exit in some cases.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In case there is only 1 legal move we will stop the
search at depth 10 anyway because the exclusion search
probe will fail low.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In the "easy move" paradigm we verify if the best move has
ever changed over a good number of iterations and if the
biggest part of the searched nodes are consumed on it.
This is a kind of hacky ad indirect heuristic to deduce
if one move is much better than others.
Rewrite the early stop condition to verify directly if one
move is much better than others performing an exclusion
search.
Idea to use exclusion search for time management if form Critter.
After 12245 games at 30"+0.1
Mod vs Orig 1776 - 1775 - 8694 ELO +0 (+-3.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Tuned with CLOP against a pool of 3 engines. Result
verified with a direct match:
After 11720 games at 10"+0.1
Mod vs Orig 1922 - 1832 - 7966 ELO +2 (+-3.6)
So no change in self match but if CLOP is right it should
be a bit better against an engine pool.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The value returned by root search it is actually
our best value, so rename the variable to reflect this.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of binding link time optimization to the choice of
popcount support, do the right thing and add -flto option
when gcc 4.5 or later is detected.
Although it should be supported also under mingw, it happens
that it doesn't, at least on my 4.6.1 due to some known bugs.
Thanks to Mike for helping me with this patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Remove the bonus for no *friendly* pieces in the pawn's path and
reduce a bit the bonus based on kings proximity.
This patch is part of to the ongoing effort to remove form evaluation
all the terms that do not add value.
After 16284 games:
Mod vs Orig 2728 - 2651 - 10911 ELO +1 (+- 3.1)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After a "stop" due to a ponder miss Xboard sends
immediately the new position to search, without
waiting for engine to effectively stop the search.
It is not clear if this is a GUI bug (as I suspect)
or allowed behaviour, but because it won't be fixed
anyway workaround this issue making listener thread
to switch to in-sync mode as soon as a "stop" command
is received.
Thanks to Mike Whiteley for reporting this.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If GUI sends stop while we are waiting for
a command do not reply with a silly:
Unknown command: stop
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Oliver reports profile builds error with new gcc 4.6, he says:
"We need to add -lgov with profile-generate AND profile-use.
So it has to be added to the second stage of building too.
The problem occurred first with the introduction of gcc4.6 and
I think this is because the previous version did find the gcov
library automatically. gcc4.6 needs more precise options and
does less guesses. I have seen it in debian, Ubuntu and also with
mingw on Windows. And all use gcc4.6."
This patch fixes the issue.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After async I/O patches 'bench' changed behaviour and now waits for
input at the end of the test run. This is due to listener thread stay
blocked on std::getline() even after test run is finished, as soon as
we feed something the thread unblocks and then quickly exits.
This is not a big problem, but has the bad side effect of breaking
profile builds that hang forever at the end of the test run.
The tricky workaround is to create a pipe that connects to stockfish
input and then, when test run is finished, breaking the pipe: this
makes std::getline() immediately return.
So this patch adds a 'sleep 10' piped into 'stockfish bench' test run
command. After 10 seconds sleep ends, the pipe breaks and 'bench'
finishes as usual.
Thanks to Oliver Korff for reporting the issue, and to Mike Whiteley
for having co-authored this solution.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use do_uci_async_cmd() instead of process input commands
directly and clarify that what we are waiting for is
something that is able to raise StopRequest flag.
Also fix some stale comments in do_uci_async_cmd(). Here
we need to reset Limits.ponder only upon receiving "ponderhit".
In the case of "quit" or "stop" resetting Limits.ponder has no
effect because the search is going to be stopped anyway.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The timer will be fired asynchronously to handle
time management flags, while other threads are
searching.
This implementation uses a thread waiting on a
timed condition variable instead of real timers.
This approach allow to reduce platform dependant
code to a minimum and also is the most portable given
that timers libraries are very different among platforms
and also the best ones are not compatible with olds
Windows.
Also retire the now unused polling code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With our new listener thread we don't need anymore
this ugly and platform dependent code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of polling for input use a dedicated listener
thread to read commands from the GUI independently
from other threads.
To do this properly we have to delegate to the listener
all the reading from the GUI: while searching but also
while waiting for a command, like in std::getline().
So we have two possible behaviours: in-sync mode, in which
the thread mimics std::getline() and the caller blocks until
something is read from GUI, and async mode where the listener
continuously reads and processes GUI commands while other
threads are searching.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The std::min() template function requires both arguments
to be of the same type.
But here we have the integer MAX_THREADS compared to a long:
long sysconf(int name);
So cast to integer and fix the compile.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add comments and rename stuff to better clarify what the
magic bitboard initialization code does.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We test if the piece moved in 'to' attacks the square 's' with:
bit_is_set(attacks_from(piece, to), s))
But we should instead consider the new occupancy, changed after
the piece is moved, and so test with:
bit_is_set(attacks_from(piece, to, occ), s))
Otherwise we can miss some cases, for instance a queen in b1 that
moves in c1 is not detected to attack a1 while instead she does.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is not possible to unify due to the fact that the
sequence steps are reversed. What we can do is to try
to sync comments and code as much as we can to easy
reading and documentation.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is called only in do_move() that now has been fully
expanded. This is the most time consuming function of
all the engine.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Is called in just one place. And rename set_castle() in the
now free to use and more appropiate set_castle_right().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
They don't add any value given that the corresponding
table has global visibility anyhow.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is a prerequisite to allow changing piece values
at runtime, needed for tuning.
Also use scores instead of separated midgame and endgame values.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
First tuning with CLOP against a pool of 3 engines. Result
verified with a direct match:
After 8736 games at 10"+0.1
Mod vs Orig 1470 - 1496 - 5770 ELO -1 (+-4.3)
So no change in self match but if CLOP is right it should
be a bit better against an engine pool.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In particular add that we can have an harmless false positive
in case StopRequest or thread.cutoff_occurred() are set.
Reported by David Lee.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
As a side effect now log file is open and closed every
time it is used instead of remaining open for the whole
thinking time.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Mainly used to log stuff to a file while playing, when
stdout is used for the comunication with the GUI.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Build broken by commit 3141490374
where we renamed move_is_ok() in is_ok() and this clashes
with the same named method in Position that overrides the
move's one causing compile errors.
The fix is to rename the method in Position.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Justin reports that it breaks the compilation on Fedore 15 and as Tom says:
-static is only needed to work around the gcc on ubuntu 11.10 beta bug.
If -static introduces issues on its own then it is better to remove it.
It will not be needed in most environments.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Partially revert 1036cadcec because UCI protocol
in case of multipv explicitly requires:
for the best move/pv add "multipv 1" in the string when you send the pv.
in k-best mode always send all k variants in k strings together.
Thanks to Justin Blanchard for pointing this out.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is enabled when selecting x86-64-modern target, this gives
another nice speed up:
On a Core i5-2500 (3300 Mhz, Sandy Bridge):
64 bit download version: 1597151 n/s
-flto : 1659664 n/s
-flto -msse3: 1732344 n/s
Patch suggested by Tom Vijlbrief.
Also unify flto, popcount and msse3 optimization under "modern"
target, note that this can break the "modern" build on old gcc that
do not support -flto option: in this case update gcc ;-) or default
to the standard build.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Just by adding the -flto option to CXXFLAGS link command
we can gain a few percent in speed.
On a Core i5-2500 (3300 Mhz, Sandy Bridge):
64 bit download version:
Without -flto: 1597151 n/s
With -flto : 1659664 n/s
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This was not clear to someone on talkchess and actually
is not trivial to understand.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems we have a very rare crash under Linux, once
every 10K games without this patch.
Is faster to wake up all the threads, especially on SMP,
where the threads can then exit in parallel while the main
thread is waiting for the next one to terminate.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Almost no increase but seems the logic thing to do.
After 16707 games 2771 - 2595 - 11341 ELO +3 (+- 3.2)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Restore old locking scheme changed with
commit 1e92df6b20.
This seems to prevent a very rare crash that occurs
once every 5-10K games.
With this patch we have no crashes after 33K games.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is more natural than using the family subtype and also
use two single maps instead of a std::pair.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When initializing endgames map we build a faked FEN string
in mat_key() to get the position hash's key.
This fen string lacks full move numbers, so when parsing the
fen in Position::from_fen() we leave startPosPly un-initialized.
Spotted by Valgrind (this is a kind of bug that is almost impossible
for humans to find).
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Another stupid remark to quiet out:
remark #2259: non-pointer conversion from "int" to "UINT16={unsigned short}"
may lose significant bits
In this case icc always converts to an integer the result of a shift operation
if the bit size of the operand is smaller, hence the warning when assignin
back to n.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we have just two mutually exclusive thread's states
we can repleace them by a simple boolean.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that Rml ordering is based on normal MovePicker logic,
apart for the ttMove that is given, we can avoid to score
all the root moves at depth 1. We only need it for easy move
detection logic, but in this case we just need to score the
first two best moves and not all the Rml set.
No regression after 6400 games
Mod vs Orig 1052 1012 4336 ELO +2 (+- 4.9)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Allocation of pawn and material hash tables should
be strictly bounded to the change of the number of
activeThreads, so move the code inside set_size().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use proper way to detect for thread terimnation instead of
our homegrown flag.
It adds more code than it removes and adds also platform specific
code, neverthless I think is the way to go becuase Thread::TERMINATED
flag is intrinsecly racy given that when we raise it thread is still
_not_ terminated nor it can be, and also we don't want to reinvent
the (broken) wheel of thread termination detection when there is
already available the proper one.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Was used to prevent issues when creating multiple threads
on Windows, but now it seems we can remove it safely.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we can split at root it happens that SendSearchedNodes
works only once at the end of the iteration, but this is useless
becuase speed info is sent anyhow toghter with the pv line.
So retire for now, waiting to find something SMP compatible.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Change qsearch() to reflect alpha update logic
of search().
To be consistent changed also moves loop condition and
futility pruning condition.
No regression after 5072 games at TC 10"+0.1
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Start a slave as soon as is allocated.
No functional change with faked split.
Regression tested the full split() series and after
2000 games no regression and no crash.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When calling split or we immediately return because unable to
find available slaves, or we start searching on _all_ the moves
of the node or until a cut-off occurs, so that when returning
from split we immediately leave the moves loop.
Because of this we don't need to change alpha inside split() and
we can use a signature similar to search() so to better clarify
that split() is actually a search on the remaining node's moves.
No functional change with faked split.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Allocate and initialize a new split point
out of lock becuase modified data is local to
master thread only.
Also better document why we need a lock at
the end of split().
No functional change with faked split.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When StopRequest is raised we cannot immediately exit the
move loop but first we need to update bestValue so to avoid
assert:
assert(bestValue > -VALUE_INFINITE && bestValue < VALUE_INFINITE);
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Another great success by Joona !
After 5876 games at 10"+0.1
Mod vs Orig: 1073 - 849 - 3954 ELO +13 (+- 5.2)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we don't special case the root moves anymore
we don't need to pass NodeType anymore as template parameter,
a simple bool to detect a SpNode will be enough.
Spotted by Joona.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After issuing "go"-command, at the end of the search
SF shows: "Unknown command: ...".
Spotted by Joona.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of our home grown function to perform a case
insensitive compare on option names as required by UCI
protocol.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems FritzGUI already remembers the old lines, so
we just need to update PV info only for the new lines.
Also introduced prevScore field in RootMove to avoid
a bulk copy of Rml.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This should fix following issue:
Suppose the search with MultiPVIteration == 0 returns an exact score
move = Nxf4, score = 100
Now search with MultiPVIteration == 1 and get two scores
move = Qg8, score = 150
move = Ra1, score = 180
If we now reorder all the moves in one step we end up with
pv[0] = Ra1, pv[1] = Qg8
Instead reordering as the current patch we end up in:
pv[0] = Ra1, pv[1] = Nxf4
preserving the first searched move.
No functional change in single PV.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This patch temporarily breaks MultiPV and searchmove
features, but they will be re-implemented in future
patches.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We missed to set chess960 flag into the std::stringstream used to
setup the PV line.
Bug introduced with commit f803f33e63
of 30/12/2010 when we started to print PV line into a std::stringstream
instead of directly into cout, where the chess960 flag is correctly set.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
As a side effect now root position can be directly
allocated on the stack and doesn't need to be defined
static anymore.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is misnamed because it is not a parser, perhaps a
tokenizer, anyhow better call it for what it is, an
input string stream.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Return the correct number of played plies at the end
of the setup moves. Currently it always returns 0 when
starting from start position, like in real games.
We fix this adding st->pliesFromNull that starts from 0
and is incremented after each setup move and is never
reset in the setup phase but only once search is started.
It is an hack because startpos_ply_counter() will return
different values during the search and is correct only
at the beginning of the search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This fixes a regression on real games due to the fact that
we have some mismatches:
history[st->gamePly - i] != stp->key
when st->gamePly - i == 0,this is due to a nasty bug I have
introduced when using std::vector<> as StateInfo backup. The
point is that StateInfo keeps inside a pointer to the previous
StateInfo in a kind of linked list. But when std::vector<> is
resized reallocates a larger chunk of memory and moves the
data there so these pointers became stale.
This patch fixes the issue.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We just need startup value to calculate available
thinking time. So remove from state.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Avoid the ugly and anyhow incorrect hard limit on the
maximum number of moves and allow to handle an arbitrary
number of moves to search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This allow to retire do_setup_move() and also to simplify
draw detection logic becuase now we always have:
Min(st->rule50, st->gamePly) = st->rule50
This was already true when starting from starting position,
but now is true even when starting from a FEN string because
now we take in account fullmove number in counting gamePly so
that it is always.
st->rule50 <= st->gamePly
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fixes the reported KNNK ending problem:
http://talkchess.com/forum/viewtopic.php?t=39347
Joona says:
Now I finally had a time to take a look at on this issue.
I've reproduced the problem starting from this position:
1B6/1B2k3/P7/1P3p2/1K6/8/4b3/4b3 w - - 6 85
I made Stockfish play as white and Fruit as black.
I repeated test ten times and once SF was not able to deliver mate.
But I observed several times that SF had reported on last something like mate in 10.
However next time it played move with score mate in 15.
Easiest way to solve the problem is attached as a patch. I tested it several times and SF always
ended up playing the optimal move. Of course the downside is that now delivering mate
takes a bit longer, but IMO it's better to lose once in a while by time in sudden death
game than not being able to deliver simple mate with long time controls.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We have a bug (possibly because of returning draw from
root move list), it is possible to see when looking at
games with a GUI, we can see rarely but consistently the
score return as #0 for many depths until it comes back to
normal values.
Revert patches until it is not fixed.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Running following command:
position startpos moves e1e8
Makes SF to assert in debug mode in do_move() but to accept
bad input and continue in release mode where probably it is
going to crash little later.
So validate input before to feed do_move().
Suggestion by Yakovlev Vadim.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It's only necessary to do the checking at the end of every non-const
member (including the constructors and from_fen()) of class Position.
Once the post-condition of every modifier guarantees the class invariant,
we don't need to verify sanity of the position as preconditions for outside
callers such as movegen, search etc. For non-class types such as Move and
Square we still need to assert of course.
Suggested by Rein Halbersma.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After previous patch is no more needed to pass
the color, becuase it is always the side to move.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It worked by accident because we always called both directions,
but definition was wrong.
Functional change due to different generation order, but
perft numbers are the correct ones.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Functional change due to the fact that now pick_best() is
stable, but should be no change in strenght.
Example code and ideas by Rein Halbersma.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And pass correct currentPly to TimeManager::init().
This restores old behaviour, in particular now black has
a different timing than white becuase is no more:
currentPly = 2 * fullMoveNumber;
but becomes
2 * (fullMoves - 1) + int(sideToMove == BLACK)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
No functional change also in faked split mode
To be sure verified in real games with 4 threads TC 2"+0.1
After 11125 games 2497 - 2469 - 6159 ELO +0 (+- 4.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix also an incredible 3% speed regression by an almost
never called function. I guess this is due to mingw very low
quality standard libraries implementation.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And also tolerate a 0 value for full move number.
Revert BUG_41 patch, now we set initial King file only
if a castling is possible, so we don't need the fix
anymore in case of correct FEN.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If we cannot castle castleRightsMask[] could be not valid,
for instance when king initial file is FILE_A as queen rook.
In this case castleRightsMask[] at initialQRFile is different
from the expected (ALL_CASTLES ^ WHITE_OOO).
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
See:
http://talkchess.com/forum/viewtopic.php?t=39327
After 8130 games on QUAD at 20"+0.1
1342 - 1359 - 5429 ELO +0 (+- 4.4)
Tried also version with just king settings changed:
After 5932 games 962 - 1052 - 3918 ELO -5 (+- 5.2)
And with just mobility settings changed:
After 4114 games 618 - 619 - 2877 ELO +0 (+- 5.9)
Frank has tested only 1200 games, but at longer TC and
against many engines, so because PHQ results are not worst
than other combination and not worst than original let's
commit his version.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We miss to account as a capture a promotion capture !
Incredible bug in this critical function that is here
since a long time (b50921fd5c of 21/10/2009 !!)
This patch fixes the bug and readds the faster
move_is_capture_or_promotion() that slightly increases
overall speed.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is now much more modular than before and also we
always send the seldepth when we send the depth, this
avoids to make seldepth disappearing from GUI at the
start of a new iteration.
Print also fails high/low pv lines at high enough
search depths.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Easy, almost trivial simplification, I don't understand
how I missed this before !!
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This trivial change gives an impressive 2,5% speedup !!!!
Also retire one unused move_gives_check() overload.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Restore original behaviour, before root unification and
remove a now useless ugly hack for alpha in multi-pv case.
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is not correct to use an iterator stick on a vector that
is sorted becuase iterator is invalidated in general case.
It happens to work by accident because iterators are implemented
as pointers and so they behave in the same (correct) way then
using array indices, but the latters are the correct thing to use.
Also better document the code.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is a fossil from the root_search() era, no more
needed today.
Spotted by Onno
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This avoids search explosion in qsearch for some
patological cases like:
r1n1n1b1/1P1P1P1P/1N1N1N2/2RnQrRq/2pKp3/3BNQbQ/k7/4Bq2 w - - 0 1
After 9078 games 20"+0.1 QUAD:
Mod vs Orig 1413 - 1319 - 6346 ELO +3 (+- 4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The trick is to classify more position at first cycle,
so to reduce following work. Speed up is of about 50% !
Also some cleanup while there.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Seems there is no regression so prefer to prune less.
After 8278 games
Mod vs Orig 1246 - 1265 - 5767 +0 ELO (+- 4.2)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We should start from i = 0, it works by accident because
static storage BSFTable[] is init to zero by default.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In case we have more than one promotion move, prefer
the one that captures the biggest piece.
Almost no functional change, anyhow I don't expect any
ELO change, it is just the correct thing to do.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems much worst in number of nodes seacrhed to reach
the depth and anyhow does not give any advantage to the
Onno's oroginal one.
So revert by now and perhaps readd when we find something
clearly better.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Allow to choose among 4096 instances of pseudo-random
sequences instead of the previous 64 so the probability
to find a better sequence increases and actually we have
a much better 64 bit case and we can also use the 64 bit
version of pick_magic() also for 32 bits and althoug sub
optimal, because now we can have more choices results are
even slightly better also for 32 bit.
Use also a faster submask().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After 12613 games at 20"+0.1 on QUAD
Mod vs Orig 1870 - 1863 - 8880 ELO +0 (+- 3.3)
So no performance change but it is a code semplification
and also is more easy to understand.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Good result for 32 bit case where computation is very fast,
still not satisfying on 64 bit case where the magics seem
a bit harder to get.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Due to a -2% speed penalty. This patch takes the best
of the previous series without the regression due to
introduction of Magic struct.
Speedup against previous revision is of almost 3% !!!!
No functional change both in 32 and 64 bits.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We can calculate them counting the masks bits.
Also small tweak to sliding_attacks()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Here the idea is to test probcut not only after bad
captures, but after any bad move, i.e. any move that
leaves the opponent with a good capture.
Ported by a patch from Onno, the difference from
original version is that we have moved probcut after
null search.
After 7917 games 4 threads 20"+0.1
Mod vs Orig: 1261 - 1095 - 5561 ELO +7 (+- 4.2) LOS 96%
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We really want PV moves and also Split Point moves to be
legal to avoid messing the move counter and corresonding
PV move detection or shared Split Point's counter variable.
This fixes a real bug where a position with only one move
allowed returns bestValue == -VALUE_INFINITE if the move
turns out to be illegal.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We disjoint pseudo legal detection from full legal detection.
It will be used by future patches.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When we are in check and we move the king then testing with
pl_move_is_legal(m, pinned) is not enough becuase we cannot
rely on attackers_to() but we have to explicitly remove the
king form the occupied bitboard to catch as invalid moves like
b1a1 when opposite queen is on c1.
Our move generator already produces correct evasions so we
just need to add the extra verification to move_is_legal().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Found another missed control in move_is_legal() thanks to
brute force testing.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add a new check in move_is_legal()
Avoid useless casting in move.h while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is interesting the fact that we need to test for
move_is_castle(m) anyway and not relying on testing
if destination square is attacked. Indeed the latter
condition fails if the castling rook is attacked,
castling is coded as "king captures the rook" but it
is legal in that case.
Verified no functional change with beginning of the series.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Remove the check for castling moves because it is
already implicit in the check for king moves and castling
is so rare that doing the check is just a slow down.
Thanks to Marek Kwiatkowski.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The structure of move is changed so should also the two
functions. It happens that it works by accident !
Bug spotted by Marek Kwiatkowski
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We already test this condition in see_sign() and
so it is almost always a redundant verification.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Bug is subtle because appears only under MSVC 32 bits in
optimized version, hence was missed before.
Bug is due to the fact that evaluation order of terms of a
sum is undefined by the standard, so in get_int() we have:
return 256 * get_int<n-1>() + bookFile.get();
And if get() is evaluated before get_int() we have a corrupted
key.
The patch rewrites the code in a more natural and predictable way.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If blockSq is already on rank 8, blockSq + pawn_push(Us) is on rank 9,
outside of board. It does not make sense to measure king distance to
a field outside the board.
Bug spotted by Fruity:
http://open-chess.org/viewtopic.php?f=5&t=1156&start=10
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Almost useless for the user and now is in sync with
the material value that is already weighted.
A small speedup of 0,4% because we avoid an apply_weight()
call in a fast path.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also simplify tracing because evaluate_unstoppable_pawns()
return always zero if both colors have non pawn material.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Extensive test series on tweaking history limit and bonus
formula. At the end this was the best.
After 11959 games:
Mod vs Orig 2087 - 1934 - 7938 ELO +4 (+- 3.7) LOS 92%
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Node count is different just becuase now we don't log on
"bench.txt" file anymore so that we avoid some calls to
pretty_pv() that calls Position::do_move().
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This prevent crashing on mobile devices with limited RAM,
currently with MAX_THREADS = 32 we would need 44MB that
could be too much for a poor cellphone.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This change allows to remove some quite a bit of code
and seems the natural thing to do.
Introduced file thread.cpp to move away from search.cpp a lot
of threads related stuff.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
According to Heinz's tests current setup is in fact too
strong for weak players. This seems the best according
to his tests.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems gain is practically unuseful, so remove.
After 13554 games:
Mod vs Orig 2252 - 2319 - 8983 ELO -1 (+- 3.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Shrink search() signature for better readibility.
We get also a nice 1.3% speed increase.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Unfortunatly icc does not understand that weakerSide and
strongerSide belongs to the base class :-(
So we have define them in the derived class.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we prefetch in material hash table we
can increase its size and gain something.
Hit rate is now of 98% from 92%
Speedup of 0.8%
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Prefetch both pawn and material tables in do_move() and
prefetch always, not only after a pawn move or a capture.
Speed up of 0,7%
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The only interesting thing is that a backward or isolated
pawn cannot be a candidate passer, so code this condition.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems we have a lot of totally useless code !
After 8577 games 1504 - 1451 - 5622 ELO +2 (+- 4.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add blunder cabability to skill level feature.
The idea is that instead of choosing the best move at the end
of the ID loop, we now do this at a randomly chosen sampling depth
dependent on SkillLevel, so that at low skill levels we sample when
ID loop has reached only a small depth and so we have an higher
probability to pick up a blunder.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The idea is to try a shallow search with reduced beta
on bad captures so to quickly prune them out in case
are really bad.
After 5529 games 966 - 868 - 3695 ELO +6 (+- 5.4) LOS 91%
Tested also version without upper limitation to 8 plies:
After 8780 games 1537 - 1398 - 5850 ELO +5 (+- 4.3) LOS 93%
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Rescaled Skill level from 0 to 20. At level 19 is still
comparable with Crafty 20.14, while at low levels strength
increase is now less steep.
Thanks to Joona and Heinz for testing and valuable
comments.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is now possible to adjust skill level of Stockfish
from 10 (full strength) to 0.
Skill adjustment is done in such a way that is CPU speed and
time control largely independent, at least at low skills. It
means that given a skill we have same play level on a mobile
phone and on a super OCTAL CPU, at 1' per game or at 180'.
At skill 9 strength is that of an average engine, I have used
Crafty 20.14 to tune and we are more or less there. At skill 0
engine is pretty weak but still shows a realistic play.
When skill is not used we don't have any impact on the regular
code.
Idea to use MultiPV is from Heinz van Saanen, implementation and
formulas by me.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is how Shredder, Rybka and others do and
avoids user is confused by a fail high (sent to GUI)
followed by a fail low (not sent).
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Has been reported by Justin Blanchard that
this creates problems on some buggy GUI.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With off-by-one bug in InFrontBB[] loop fixed.
Also use int instead of File to workaround a bug
in mingw 4.4.0 in first loop that cycles forever.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
No speed regression after 8731 games:
Mod vs Orig 1394 - 1342 - 5995 ELO +2 (+- 4.1)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Otherwise in case we change an option with setoption and
then ask for "eval" command the evaluation is not updated.
Spotted by Justin Blanchard.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix a corner case where we start aspiration window and
suddendly we get a VALUE_KNOWN_WIN / MATE score, this makes
aspiration to blow up in a series of researches loops.
Exit aspiration loop in that case.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use first iteration to get a proper startup score
and possibly detect an easy move.
After 5180 games:
Mod vs Orig 847 - 823 - 3510 ELO +1 (+- 5.5)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Because we are never in check there and evaluation cannot
return a mated value the condition is useless.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This patch is based on Justin Blanchard's original
work and allows to breakdown evaluation in its sub terms and
show to the user.
Tracing code has zero speed impact when not used.
Note that tracing code is not thread-safe, but this
should not be a problem given the typical usage scenario.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use Square instead. At the end is the same because we were
anyway foreseen operators on mixed terms (Square, SquareDelta).
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we don't have anymore a search stack array in
SplitPoint we can rename this data member to somthing more
usual.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use a local variable instead. To make it work we need to
correctly init next ply search stack at the beginning of the
search because now that ss is allocated on the stack instead
of on the global storage it contains garbage.
As a side effect we can peform a fast search stack
init in id_loop().
With this patch size of SplitPoint goes from 71944 to 136 bytes,
and consequently size of Thread goes from 575568 to 1104 bytes.
Finally the size of ThreadsManager that contains all the thread
info goes from 9209248 to just 17824 bytes !!
No functional change also in faked split.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This allow us to restore the old depth 12 benchmark
and fixes one and for all the depth mess.
Test confirms no regression:
After 5658 games 892 - 924 - 3842 ELO -1 (+- 5.2)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Note that this introduces an asymmetry in which best move
is searched deeper then others also in MultiPV, but this is
not an error per se.
No functional change when MultiPV = 1
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This patch removes a condition that allows a PV entry to remain
in TT across games for an unlimited time.
Although this produces a nice ELO boost in the long term it
is an artifact that affects tests results bewteen version
with and without this feature.
So remove now and readd before to release because it actually
seems a strong feature.
As example a verification tournament against SF 2.0.1 starting around
+10 ELO after 4K games sligltly climbed to +21 ELO after 14K games !!!
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Skip writing fail high/low sequences. Note that we don't need
fail high/low markers anymore in pretty_pv().
No functional change but some do/undo move sequences.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
First search should be done at iteration = 1, not 2. So offset
the variable by one.
As a nice side effect now search correctly stops at PLY_MAX
included, not after searching (PLY_MAX - 1) as before.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Interestingly this patch will make people complain search depth
is reduced against 2.0.1 ;-) but actually it is only an artifact.
Spotted by Joona.
No functional change apart from a different do / undo move
sequence due to teh fact that we don't call pv_info_to_uci()
anymore before entering id loop.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
No functional change apart form move reordering because
pv_info_to_uci() performs a do / undo_move sequence.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Biggest advantage is be able to analize positions
without "loss of memory" when goind back/forth in
a position.
Patch has proven to fix analysys problems and is even
worths some elo points.
After 5811 games Mod- Orig:
1037 - 902 - 3872 +8 ELO (+- 3.6) LOS 97%
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Warning C4146: unary minus operator applied to
unsigned type, result still unsigned.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We dont' call MovePicker from print() anymore, so that
reentrancy check in now not needed.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This let us get rid of number_of_evasions()
After 5487 games
Mod- Orig: 851 - 852 - 3784 +0 ELO (+- 3.7)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Move qsearch scoring functionality out of RootMoveList
initialization. Will be needed by future patches.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is used for secondary scoring so it does not
changes the fact that Rml[0].pv[0] is always tried
as first anyhow.
It happens this is even a no functional change patch
becuase we reinsert PV in TT after a search so that
TT move is actually Rml[0].pv[0].
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So to better spot where the differences really
count. Also add some more additional cleanup.
Harmless functional change and no regression.
After 5780 games
Mod- Orig: 931 - 955 - 3894 ELO -1 (+- 3.6)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In case of a Root node we can leave with bestValue set
to -VALUE_INFINITE if search is stopped by the GUI and
stopReques flag is raised.
This patch fixes the issue.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A bit of template magic to restore a proper and readable moves
'while' loop that now is again 'similar' to the one that used
to be in search().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Teach search() to behave as a root node if requested.
Just added code, but still no functional change.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And sync root_search() with search()
After 9384 games Mod - Orig:
1532 - 1433 - 6419 ELO +3 (+- 2.8)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Returns an int64_t while we want a simple int.
This occurs only when compiling with MSVC on a 64 bit platform.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Respin this old idea. Earlier we tried only
with < 1000 games and result was inconclusive.
After 5845 games
Mod vs Orig: 935 - 936 - 3974 ELO (+-3.6)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If size_t is defined as a 32 bit quanitity then we have an
overflow in the left term of the while condition if mbSize
is bigger then 2048.
For instance if mbSize is 2049 then when newSize will reach
0x80000000 (2048MB) comparison is still true, 'while' loops
again and we have an overflow in the expression (2*newSize)
so that result is 0 and at that point 'while' keeps looping
forever hanging the application.
This patch fixes the bug and also makes operator new do not
throw an exception upon failure but return a NULL pointer
instead.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In input_available() we use function select(), so
we have to set as unbuffered also C library I/O
functions otherwise we can miss some input.
For instance in case GUI sends "go infinite\nstop\n" we
parse the "go infinite" but then input_available() under Linux
is unable to detect that we still have "stop" to be processed.
This is because "select" uses file descriptors instead of file
pointers. So it cannot know about the buffer associated to a file
pointer.
This patch, by BB+, should fix the problem.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also let do_setup_move() don't reuse same StateInfo so that
we can remove the check about different StateInfo objects
before memcpy() in do_move.
Functional change due to harmless additionals
do_move() / undo_move() steps.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Actually it is san.cpp renamed. Because now has the move
conversions functions and doesn't have any more the bulky
move_from_san(), it is better to call it move.cpp
Remove san.h while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use a reverse logic: among the list of generated legal moves
transformed in UCI coordinate notation find the one that
matches the given string.
It is a bit slower, but here is not performance critical and
is much more simplified then before.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This partially reverts 1e7aaed8bc keeping the conversion
functions from/to move to uci string in the same file.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I got report from Werner that Shredder Gui has problems with
UCI values which maximum value is greater than 30000.
Of course it's stupid to change engine to fix a GUI's bug,
but on the other hand 30000 ms as maximum value is clearly enough,
so why not to be merciful
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Added checking of (stdin->_cnt > 0) from Greko.
This seems to greatly improve responsivness when running
under console. Now while running a 'stockfish bench', any key
press immediately is detected by SF while before there was a
delay of some fraction of a second.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is an old Glaurung bug that prevented a Polyglot
book move to be read correctly in case of underpromotion.
This patch fixes the bug restoring support for both
queen and underpromotions.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
MSVC (and possibly other compilers) does not inline
as requested, so force it to do so.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Move the castling condition test out of the
function. This avoids a function call most of
the times.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Functional change due only to moves reorder. Anyhow after
5242 games at 15"+0.1 TC verified we have no regression.
Mod vs Orig 994 - 958 - 3290 +2 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Shrink of 1 bit so to fit a move in an uint_16 and
possibly a MoveStack in an uint_32.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A bit faster on 32 bits machines, more similar to
TranspositionTable::first_entry() and same result.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
There was a strange "int16" type and "int64_t"
was defined twice.
Spotted by Joona.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We never reach a position where rule50 > 100.
When rule50 == 100, it's either draw or mate and
there is no way search could go deeper.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We must be able to filter out also moves where move_is_ok()
is false.
And actually we are. Tested on all the default position injecting
a number from -1000000 to 1000000 casted to a Move.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
As is defined now is always true, tested with:
for (long i=-1000000; i < 1000000; i++)
if (!move_is_ok(Move(i)))
exit(0);
Reason is that move_from() and move_to() already truncate the
input value to something in the range [0, 63] that is always
a possible square.
So change definition to something useful.
The same applies also to square_is_ok()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now first move has moveCount == 1 also in root_search()
Also added small readibility touches.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Should be already inlined by the compiler when
optimizing but better safe than sorry ;-)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Keep the isChess960 flag inside Position so that is
copied with the Position, but esplicitly highlight the
fact that a FEN string has not enough information to detect
Chess960 in general case. To do this add a boolean argument
isChess960 to from_fen() function so to self document this
shortcoming of FEN notation.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems HP's ANSI C++ doesn't understand very well
standard function-style cast.
Reported by Richard Lloyd.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And set them as default.
Introduce compile switch OLD_LOCKS to allow to fallback on
compatible locks supported by Windows XP and older versions.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Introduced by me in before 1.9 and found by Tord that says:
The 'isChess960' slot in the 'Position' class is currently
set depending on the initial files of the rooks, and not on the value
of the UCI_Chess960 parameter. This is incorrect, as there are lots of
Chess960 positions where the rooks start on the usual files. As a
consequence (unless I am missing something), Stockfish will occasionally
output castling moves as e1g1/e1c1 rather than the correct e1h1/e1a1 format
in Chess960 games. It is possible that some or even most GUIs are robust
enough to accept both notations, but I wouldn't bet on it. And in any case,
Stockfish's behavior clearly violates the protocol.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This time I have removed the function alltogether !
Sorry to work above a patch of UncombedCoconut (Justin Blanchard)
but I couldn't resist ;-)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When we log best and ponder move to a file before to
return from think we change the position. If position is
then not resended by GUI, as for manual user input we got
an error:
justinb@malibu:~$ stockfish
Stockfish 2.0 JA 64bit by Tord Romstad, Marco Costalba, Joona Kiiski
setoption name Use Search Log value true
go depth 1
info depth 1
info depth 1 seldepth 1 multipv 1 score cp 72 time 59 nodes 20 nps 338 pv g1f3
info depth 2
info depth 2 seldepth 2 multipv 1 score cp 12 time 59 nodes 44 nps 745 pv g1f3 g8f6
info nodes 84 nps 1423 time 59
bestmove g1f3 ponder g8f6
go depth 1
info depth 1 score mate 0
info nodes 87 nps 0 time 0
bestmove (none) ponder (none)
Bug spotted and fixed by UncombedCoconut.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And set "Use Sleeping Threads" to true because it keeps
much more responsive and cool my QUAD during tests :-)
It will be reverted back before to release that's the
reason to bundle it here.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Always same siganture: 7224363
Hopefully some more bug fixed and restored
compatibility with WIndows XP.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
They are not compatible with Windows XP
Revert to old CRITICAL_SECTION locks and events.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Second parameter of insertion_sort() is a pointer to the
element _after_ the last of the list, e.g. end() when sorting
all items.
If we want to sort say the first 2 moves we should write:
sort_multipv(2);
So, becuase in root moves loop move counter 'i' starts
from 0, we need to pass:
sort_multipv(i+1);
To sort up to move 'i' included.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It doesn't seem to have any meaning.
Also add a FIXME on the MaxNodes condition that now is broken
in SMP case due to known issue with pos.nodes_searched()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We were not quitting the engine after a "quit" command
while still in the book and pondering.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If flag StopOnPonderhit is set it means that we UseTimeManagement
and also we are at Iteration >= 3.
So we can safely simplify the formula.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is called only from one place, so move code there.
Add a bit of renaming and documentation while at there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And set "Use Sleeping Threads" to true because it keeps
much more responsive and cool my QUAD during tests :-)
It will be reverted back before to release that's the
reason to bundle it here.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It was introduced by patch 66d16592 of 22/3/2009 merging
from Glarurung iPhone.
Tord says:
That change is only found in the Glaurung iPhone app, and not
in the latest Glaurung UCI source code. I don't remember why
this was added (and the iPhone app, unlike the UCI engine,
was never version controlled), but it was almost certainly
because it was somehow needed in the communication between
the engine and the iPhone GUI, and that it was never meant to be
included in the UCI engine. My guess is that it has something to
do with castling moves being entered as e1-g1 in the GUI, but
represented as e1-h1 in the chess engine.
Removing it in Stockfish should be completely safe, and won't harm
the iPhone version. Initially the iPhone GUI called functions in the
chess engine for checking for legality of moves, writing the move
list in SAN format, and various other tasks, but this is no longer
the case in the current version.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is the "selective search depth in plies" and we set
equal to PV line length.
Tested that works under FritzGUI.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems a more appropiate place (IMHO) and helps to clarify
that idle_loop() should return a move, not a score.
Fix also handling of stalemate positions (we were not
sending any score) and we don't need to wait on "ponderhit",
this is done when returning in think().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We have a small functionality change in case we have a
fail-high so that both rml[].pv and pv[] are updated, but if,
after researching, we have a fail-low then rml score is updated
again but pv[] remains the same and coming back from search we
used a PV line that has failed-low (after having failed-high).
With this patch we always use the 'correct' PV line, i.e. the
line with highest score at the end of the whole search.
Retire also redundant RootMove's 'move' member and directly
use pv[0] instead.
Define symbol '<' to mean 'minor of', as it should be. Its meaning
was reversed to be used with std::sort() that sorts in ascending order
while we want a descending order.
But now that we use our own sorting code we don't need this
trick anymore.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When a reference breaks things !
Here we take a reference (that is a pointer) to an
entry in a vector that changes below us --> BOOM !
References are essential but should be considered with
care in C++ because could lead to nasty surprises.
Restored functionality.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of a default member by member copy use set_pv()
to copy the useful part of pv[] array and skip the remaining.
This greatly speeds up sorting of root move list !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is redundant being a move list ;-)
Also better document the two scores used by root list.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Function ray_bb() was used just in one endgame where can
be used squares_in_front_of() instead.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Remark 1418: external function definition with no prior declaration
and
Remark 1419: external declaration in primary source file
Can be safely ignored because are pure idiocy.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Namely we use only two types of depth in TT:
DEPTH_QS_CHECKS or DEPTH_QS_NO_CHECKS.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And rename move_from/to_string() in a more specific
move_from/to_uci() that is a simple coordinate notation.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use one sleep lock per thread insted of a single one shared.
Also renamed WaitLock in SleepLock.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
By mean of an an UCI option it is possible let the available
threads to sleep, this should help with Hyper Threading although
is not the best solution when number of threads equals number
of available cores.
Option is disabled by default.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
No speed regression and no functional change.
After 7826 games Mod- Orig:
1188 - 1230 - 5408 ELO -1 (+- 3.1)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Non-PV LMR is left unchanged.
After 8819 games
Mod- Orig: 1442 - 1343 - 6034 ELO +3 (+- 2.9) LOS 86%
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Bug introduced in 9dcc2aad98
We can be asked to open a non-exsistent file,
in this case we should gracefully handle the
case and silently return instead of exiting.
Bug discovered and bisected down by Joona.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This time we try very hard to avoid false positives.
The obvious downside is that we also miss many true
winning positions.
After 10544 games on RC
Mod- Orig: 1744 - 1646 - 7154 ELO +3 (+- 2.7) LOS 83%
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After 2036 games Mod- Orig:
381 - 278 - 1377 ELO +17 (+- 6.2) LOS 99%
One of the biggest increases ever !
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When MultiPV > 1, always take bestmove from the RootMoveList
(and don't bother with a ponder move). Without that the bestmove
is most probably incorrect.
Patch from Peter Petrov.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When we store this value in TT we cut this to 9 bits,
so we need a smaller variable otherwise comparisons
like:
replace->generation() == generation
Are always false if generation is bigger then the maximum
TT storable value.
This fixes a very nasty and difficult to spot bug (2 weeks for
regression hunting).
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is a redundant boiler plate, just call initialization and
resource release directly from main()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Correctly handle uci option names in a case insensitive way.
Alos fix some indentation while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Was introduced by 403db5a6e9
on 1/12/2009 to correctly handle "loose on time"
LSN filtering functionality, but is now unused.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This L1/L2 optimization has an incredible +4.7% speedup
in perft test where this function is the most time consumer.
Verified a speed up also in normal bench, although smaller.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
UCI protocol uses "true" and "false" for check and button types,
so store that values instead of "1" and "0", this simplifies a
bit the code.
Also a bit strictier option's type checking in debug mode.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This patch greatly cleanups generation of pawn moves but
we change the order in which moves are generated so there is
a change in functionality, but not in perft.
The only real functionality change is that now when type == CHECK
we generate knight underpromotion captures only if give check and
not always as before.
Perft is 2% faster and fully verified.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now stockfish bench' defaults to
stockfish bench 128 1 12 default depth
that is the most used line (at least by me)
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Let to sleep even split point master, it will be waken
up by its slaves when they return from the search.
This time let it be enabled by an UCI option, so
people is free to test it on their Hyper Thread box.
Option is disabled by default.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Search at fixed depth with one thread must be
reproducible so remove randomess from time().
Also better license description.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I finally got SF to compile under MinGW (after adding pthread libraries)
and here are the fixed warnings.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And also store the node counter in Position and not in Thread.
This will allow to properly count nodes also in sub trees with
SMP active.
This requires a surprisingly high number of changes
in a lot of places to make it work properly.
No functional change but node count changed for obvious reasons.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This was subtle and google was my friend.
The leak was in _dl_allocate_tls called by pthread_create() and
is due to the fact that threads are created in joinable state so that
once terminated are not freed. To make the thread to release
its resources upon termination we should set them in detached state.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix warning: "Source and destination overlap in memcpy"
This happens when we call multiple time do_move() with the
same state, for instance when we don't need to undo the move.
This is what valgrind docs say:
You don't want the two blocks to overlap because one of them could
get partially overwritten by the copying.
You might think that Memcheck is being overly pedantic reporting this
in the case where 'dst' is less than 'src'. For example, the obvious way
to implement memcpy() is by copying from the first byte to the last.
However, the optimisation guides of some architectures recommend copying
from the last byte down to the first. Also, some implementations of
memcpy() zero 'dst' before copying, because zeroing the destination's
cache line(s) can improve performance.
In addition, for many of these functions, the POSIX standards have wording
along the lines "If copying takes place between objects that overlap,
the behavior is undefined." Hence overlapping copies violate the standard.
The moral of the story is: if you want to write truly portable code, don't
make any assumptions about the language implementation.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems we have a speed regression under Linux, anyhow
commit and revert to leave some documentation in case we
want to try again in the future.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Let to sleep even split point master, it will be waken up
by its slaves when they return from the search.
With this patch we get maximum HT speedup
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It has more sense to treat the two evaluation metrics
in the same way.
As a side effect now we use the correct eval margin when
pruning in a SplitPoint node.
No functional change in single thread.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix the movcount updating bug and let search() to completely
subsititute sp_search().
No functional change even with fakes split.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
There is a bug in the conversion that is triggered when testing
with faked split and that I missed somehow :-(
To allow proper testing on cluster restore old sp_search()
until I don't fiugre up what's happened.
Restored to be functional equivalent to old behaviour both in
single thread and in faked split.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When entering and exiting from think() we don't need any special
wake up / sleeping code because we want available threads to keep
sleeping.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This simple patch has devastating consequences ;-)
Now an available thread goes to sleep and is waked up after
being allocated.
This patch allows Stockfish to dramatically increase performances
on HyperThreading systems.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
They are fast and also have the same semantic of Linux ones.
This allow to simplify the code and especially to use
SleepConditionVariableSRW() to wait on a condition releaseing the lock,
this has the same semantic as pthread_cond_wait().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is a prerequisite for future work and anyhow removes
a state flag, so it is good anyhow.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is redundant and complicates the already complicated
SMP code for no reason.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We already do this for locks. Also rename SitIdleEvent
in WaitCond to be uniform with Lunix naming.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is the native way done in Windows and we will use it
for future work, so change Linux to do the same.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Plus some other icc warnings popped up with new and strictier
compile options.
No functional and speed change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It has more sense to treat the two evaluation metrics
in the same way.
As a side effect now we use the correct eval margin when
pruning in a SplitPoint node.
No functional change in single thread.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Rewrite sp_search() to have same signature of search()
This is the first prerequistite step toward unification.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Actually it is an error to update back moveCount value after split()
because it is used in update_history() to access movesSearched[]
array. But becasue this vector is not updated in the split point
we end up with an access of stale data.
Bug has been hidden til now because we 'forgot' to update
moveCount before returning from split().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Mostly suggested by Justin (UncombedCoconut), the 0ULL -> 0 conversion
is mine.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Language guarantees that c'tor is called, but without any c'tor
it happens to work by accident because OS zeroes out the freshly
allocated pages. The problem is that if I deallocate and allocate
again, the second time pages are no more newly come by the OS and
so could contain stale info.
A practical case could be if we change TT size or numbers of
threads on the fly while already running.
Bug spotted by Justin Blanchard.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Fix release to workaround chess960 on some GUIs
Signature is:
stockfish bench 128 1 12 default depth
Node counts: 10914593
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Get rid of macros and use templates instead,
this is safer and allows us fix the warning:
ISO C++ forbids braced-groups within expressions
That broke compilation with -pedantic flag under
gcc and POPCNT enabled.
No functional and no performance change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Joona Kiiski