mstembera [Mon, 8 May 2017 03:14:23 +0000 (20:14 -0700)]
Avoid *begin always being included in the sorted list regardless of its value.
This was a minor criticism by @zamar in the original pull request
https://github.com/official-stockfish/Stockfish/pull/1065
necessitating a comment explanation.
joergoster [Thu, 4 May 2017 02:46:40 +0000 (19:46 -0700)]
Fix multiPV issue #502
In general, this patch handles the cases where we don't have a valid score for each PV line in a multiPV search. This can happen if the search has been stopped in an unfortunate moment while still in the aspiration loop. The patch consists of two parts.
Part 1: The new PVIdx was already part of the k-best pv's in the last iteration, and we therefore have a valid pv and score to output from the last iteration. This is taken care of with:
bool updated = (i <= PVIdx && rootMoves[i].score != -VALUE_INFINITE);
Case 2: The new PVIdx was NOT part of the k-best pv's in the last iteration, and we have no valid pv and score to output. Not from the current nor from the previous iteration. To avoid this, we are now also considering the previous score when sorting, so that the PV lines with no actual but with a valid previous score are pushed up again, and the previous score can be displayed.
Testing the release candidate revealed only one minor issue, namely a new warning -Wimplicit-fallthrough (part of -Wextra) triggers in the movepicker. This can be silenced by adding a comment, and once we move to c++17 by adding a standard annotation [[fallthrough]];.
Marco Costalba [Sat, 29 Apr 2017 03:33:30 +0000 (20:33 -0700)]
Retire the misdesigned StepAttacks[] array.
StepAttacks[] is misdesigned, the color dependance is specific
to pawns, and trying to generalise to king and knights, proves
neither useful nor convinient in practice.
So this patch reformats the code with the following changes:
- Use PieceType instead of Piece in attacks_() functions
ss->killers can change while the movepicker is active.
The reason ss->killers changes is related to the singular
extension search in the moves loop that calls search<>
recursively with ss instead of ss+1,
effectively using the same stack entry for caller and callee.
By making a copy of the killers,
the movepicker does the right thing nevertheless.
Use int instead of Value for history related stats.
history related scores are not related to evaluation based scores.
For example, can easily exceed the range -VALUE_INFINITE,VALUE_INFINITE.
As such the current type is confusing, and a plain int is a better match.
the order of elements returned by std::partition is implementation defined (since not stable) and could depend on the version of libstdc++ linked.
As std::stable_partition was tested to be too slow (http://tests.stockfishchess.org/tests/view/585cdfd00ebc5903140c6082).
Instead combine partition with our custom implementation of insert_sort, which fixes this issue.
Implementation based on a patch by mstembera (http://tests.stockfishchess.org/tests/view/58d4d3460ebc59035df3315c), which suggests some benefit by itself.
Higher depth moves are all sorted (INT_MIN version), as in current master.
It is worth noting that an attempt to only increase the bonus passed STC but failed LTC, and
an attempt to remove the cap without increasing the bonus is still running at STC, but will probably fail after more than 100k.
By adding pos.non_pawn_material(pos.side_to_move()) as a precondition in step 13,
which is already in use in Futility Pruning (child node) and Null Move Pruning for similar reasons.
Pawn endgames, especially those with only 1 or 2 pawns, are simply heavily influenced by zugzwang situations.
Since we are using a bitbase for KPK endgames, I see no reason to accept buggy evals as shown in #760
a single Xeon Phi can present itself as a single numa node with up to 288 threads (4 threads per hardware core).
Tested to work as expected with a Xeon Phi CPU 7230 up to 256 threads.
joergoster [Fri, 17 Mar 2017 21:41:08 +0000 (14:41 -0700)]
Pawns count imbalance table
Instead of having a continuous increasing bonus for our number of pawns when calculating imbalance, use a separate lookup array with tuned values.
Idea by GuardianRM.
GuardianRM [Fri, 24 Feb 2017 05:26:59 +0000 (21:26 -0800)]
Pieces protecting king
Initial protective idea by Snicolet for knight, for other pieces too
Patch add penalties and bonuses for pieces, depending on the distance from the own king
snicolet [Sun, 19 Feb 2017 22:25:05 +0000 (14:25 -0800)]
Keep pawns on both flanks
Positions with pawns on only one flank tend to be more drawish. We add
a term to the initiative bonus to help the attacking player keep pawns
on both flanks.
STC: yellowish run stopped after 257137 games
LLR: -0.92 (-2.94,2.94) [0.00,5.00]
Total: 257137 W: 46560 L: 45511 D: 165066
FauziAkram [Sun, 19 Feb 2017 21:56:17 +0000 (13:56 -0800)]
Variable tuning
A tuning patch which cover the following changes:
increase the importance of queen and rook mobility in endgame and
decrease it in mg, since if we use the heavy pieces too early in the game
we will just make opponent develop their pieces by threatening ours.
King Psqt:
1)King will be encouraged more to stay in the first ranks in the MG
2)and will be encouraged more to go to the middle of the board/last ranks in the EG
Bishop scale better in EG
Logical changes on various psqt tables
1/6 of the changes of the last tuning session on mobility tables
VoyagerOne [Wed, 25 Jan 2017 14:32:10 +0000 (09:32 -0500)]
Penalty for a quiet ttMove that fails low
Also the penalty/bonus function is misleading, we
should simply change it to stat_bonus(depth) for
bonus and -stat_bonus(depth+ ONE_PLY) for extra
penalty.
This submitted version (using if (abs(mg + eg) > 1500) )
seems more logical than the following other green simplification (using if (abs(mg)>1500))
since it can happen than mg_value is > eg_value (about 20% of the time)
and the submitted version seems stronger at LTC
After we have taken into account all cheap evaluation
terms, we check whether the score exceeds a given threshold.
If this is the case, we return a scaled down evaluation.
Stéphane Nicolet [Wed, 11 Jan 2017 17:11:17 +0000 (18:11 +0100)]
Check for overflow in Score * int multiplication (#969)
Add asserts to check for overflow in Score * int multiplication.
There is no overflow in current master, but it would be easy to
create one as the scale of the current eval does not leave many
spare bits. For instance, adding the following unused variables
in master at the end of evaluate() (line 882 of evaluate.cpp)
overflows:
New shell scripts for testing, used for travis CI (#957)
Perform more complex verification and validation.
- signature.sh : extract and optionally compare Bench/Signature/Node count.
- perft.sh : verify perft counts for a number of positions.
- instrumented.sh : run a few commands or uci sequences through valgrind/sanitizer instrumented binaries.
- reprosearch.sh : verify reproducibility of search.
These script can be used from directly from the command line in the src directory.
Marco Costalba [Sat, 7 Jan 2017 09:13:49 +0000 (10:13 +0100)]
Drop Stats c'tors
Now taht we correctly value-initialize Thread objects,
we don't need c'tors anymore because tables will be
zero-initialized by the compier when Thread object
is instanced.
It can be used uninitialized in time management.
Fixes all valgrind errors on './stockfish go wtime 8000 btime 8000 winc 500 binc 500'
This is one (of the many) quirks of C++. There is a subtle difference between:
new Foo
new Foo()
The first statement calls the default constructor (that in case of a POD leaves data members
uninitialized), the second one performs a value-initialization (that in case of POD is
equivalent to a zero-initialization)
Aram Tumanian [Wed, 21 Dec 2016 11:40:55 +0000 (13:40 +0200)]
Don't clear EasyMove in search()
EasyMove is cleared after every iteration of the
search if the 3rd move in the PV of the main thread
changes from the previous iteration. Therefore,
clearing EasyMove during a search iteration may be
excessive. The tests show that this is indeed unnecessary.
In the new version the EasyMove variable is used only in
the Thread::search function.
Sergei Antonov [Mon, 12 Dec 2016 15:04:16 +0000 (16:04 +0100)]
Threefold repetition detection
Implement a threefold repetition detection. Below are the examples of
problems fixed by this change.
Loosing move in a drawn position.
position fen 8/k7/3p4/p2P1p2/P2P1P2/8/8/K7 w - - 0 1 moves a1a2 a7a8 a2a1
The old code suggested a loosing move "bestmove a8a7", the new code suggests "bestmove a8b7" leading to a draw.
Incorrect evaluation (happened in a real game in TCEC Season 9).
position fen 4rbkr/1q3pp1/b3pn2/7p/1pN5/1P1BBP1P/P1R2QP1/3R2K1 w - - 5 31 moves e3d4 h8h6 d4e3
The old code evaluated it as "cp 0", the new code evaluation is around "cp -50" which is adequate.
Patch has been rewritten into current form for simplification and
logic slightly changed so that return a draw score if the position
repeats once earlier but after or at the root, or repeats twice
strictly before the root. In its original form, repetition at root
was not returned as an immediate draw.
After retestimng testing both version with SPRT[-3, 1], both passed
succesfully, but this version was chosen becuase more natural. There is
an argument about MultiPV in which an extended draw at root may be sensible.
See discussion here:
Alain SAVARD [Wed, 28 Dec 2016 22:14:09 +0000 (17:14 -0500)]
Small eval cleanup and renaming
Non-functional changes
a) splitting the threat array to avoid using an enum
b) reorder the scores according to functions where they are used.
c) declarations in evaluate_pieces after the const(s) like elsewhere
d) more compact definitions of KingFlank,
now that we need it also for the PanwLessFlank penalty.
e) reuse CenterFiles in evaluate_space
f) move one line inside next popcount
Explicitly use alpha+1 for beta in NonPV search (#939)
Fixes the only exception, in razoring.
The code already does assert(PvNode || (alpha == beta - 1)), and it can be verified by studying the program flow that this is indeed the case, also for the modified line.