Marco Costalba [Fri, 26 Apr 2013 10:12:53 +0000 (12:12 +0200)]
Fix a crash introduced few days ago
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.
Marco Costalba [Thu, 25 Apr 2013 19:51:05 +0000 (21:51 +0200)]
Store Eval::Info in Search::Stack
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.
Marco Costalba [Thu, 25 Apr 2013 10:43:55 +0000 (12:43 +0200)]
Expose EvalInfo struct to search
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.
Marco Costalba [Thu, 25 Apr 2013 09:39:06 +0000 (11:39 +0200)]
Merge Joona's increased static null pruning
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
Marco Costalba [Tue, 9 Apr 2013 21:18:28 +0000 (23:18 +0200)]
Simplify and speed up previous patch
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.
Don't treat king safety differently in AnalysisMode
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.
Gary Linscott [Mon, 18 Mar 2013 15:35:19 +0000 (11:35 -0400)]
Add KNPKB endgame
In a game vs Junior, SF had the option to trade into a winning
pawn endgame, and failed to do so. PGN at bottom.
This FEN was one key position: 8/2Nb1k2/6pp/4Pp2/5K1P/5PP1/8/8 w - - 5 62.
SF master chooses h5 here, with a fail high, which goes into the drawn KNPKB
ending. With the patch, SF correctly chooses Ke3, which maintains chances to win.
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.
Marco Costalba [Mon, 4 Mar 2013 08:27:00 +0000 (09:27 +0100)]
Simplify "easy move" detection
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.
Marco Costalba [Mon, 4 Mar 2013 07:58:57 +0000 (08:58 +0100)]
Avoid locking/unlocking in a tight loop
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.
jundery [Thu, 28 Feb 2013 05:18:11 +0000 (22:18 -0700)]
Remove strange use of the ternary operator
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.
Marco Costalba [Sat, 16 Feb 2013 11:42:22 +0000 (12:42 +0100)]
Account for gamePly after each move
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().
Marco Costalba [Fri, 15 Feb 2013 15:21:15 +0000 (07:21 -0800)]
Merge Gary's king safety tweak
Still well within error bars, so probably not a big improvement,
but may be worthwhile. I will let the test keep running. The idea
for the tweak came from the TCEC game against Houdini. Stockfish saw
it as a draw, well past when it should have seen problems. King safety
was off, since it was QN and pawns only, but in fact the king was
quite vulnerable.
PGN of game against houdini. Moves 55-59 it was seeing a draw, and
Houdini was seeing a good sized advantage for black. With the change,
Stockfish now recognizes that moving the king there is a bad idea.
Marco Costalba [Fri, 15 Feb 2013 07:56:04 +0000 (08:56 +0100)]
Further speed up bitbase generation
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.
Marco Costalba [Wed, 13 Feb 2013 11:26:08 +0000 (12:26 +0100)]
Speedup KPK bitbase of 25%
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>
Marco Costalba [Sun, 10 Feb 2013 18:10:01 +0000 (19:10 +0100)]
Rename and de-templetize sort()
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.
Marco Costalba [Sat, 9 Feb 2013 15:22:47 +0000 (16:22 +0100)]
Further simplify first_entry()
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.
Marco Costalba [Fri, 8 Feb 2013 07:49:36 +0000 (08:49 +0100)]
Workaround value-initialization in MSVC
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.