When we have more then 2 threads then we do an array
access with index 'Threads[slave].activeSplitPoints - 1'
This should be >= 0 because we tested the variable just
few statements before, but because is a shared variable
it could be that the 'slave' thread set the value to zero
just after we test it, so that when we use the decremented
variable for array access we crash.
Bug spotted by Bruno Causse.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Unify start loop for master and slave threads. Also guarantee
that all the 'stop' flags are set to false before first slave
is started, should be no harm because only master thread can
reset 'stop' flag of slaves to true, so should be no race but
better safe then sorry.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Only pure blocking evasions are candidate
for pruning.
After 998 games at 1+0
Mod vs Orig +215 =596 -187 +10 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In move_to_san() we create by copy a new position just
to detect if move gives check. This could be very costly in
line_to_san() that calls move_to_san() for every move, so
create the position only once and pass a reference to move_to_san()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When pondering threads are put to sleep, but when thinking
the threads are parked in idle_loop in a tight polling loop
checking for workIsWaiting falg.
So before we set the slave's flag workIsWaiting we have to
guarantee that all the slave data is already setup because
slave can start in any moment from there.
Rearrange the last loop to fix this race.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Once we have allocated our slave threads and we have removed
master from available threads we can safely remove the lock
so that the lenghty search stack copy operation will not
impact lock contention.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
A pointer is enough because after a split point has been
setup master and slaves thread end up calling sp_search() or
sp_search_pv() and here a full copy of split point position is
done again, note that even master does another copy (of itself)
and this is done before any do_move() call so that master Position
is never updated between split() and sp_search().
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And detach splitPoint Position from the master one.
So we duplicate StateInfo only once in split() instead
of one for each thread in sp_search
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Renamed a bit the functions to be more clear what
we actually are doing when we craete a Position object
and explained how StateInfo works.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also removed some trailing whitespaces and aligned
indentation to current standard.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Only the previous, the current and the next ply SearchStack
are copied.
This reduces split overhead especially at low depth (high ply)
and with many threads.
Possibly no functional change (it is not easy to prove in SMP)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The init_eval() function corrupted the static array castleRightsMask[]
in the Position class, resulting in instant crashes in most Chess960
games. Fixed by repairing the damage directly after the function is
called. Also modified the Position::to_fen() function to display
castle rights correctly for Chess960 positions, and added sanity checks
for uncastled rook files in Position::is_ok().
It is a good programming practice to verify a system
call has indeed succeed.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When a search fails high then sp->alpha is increased and
slave threads are requested to stop.
So we have to check for a stop request before to start a search
otherwise we could end up with sp->alpha >= sp->beta
leading to an assert in debug run in search_pv().
This patch fixes the assert and get rid of some of possible races.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
maximum depth during a "go movetime ..." search. This prevents
Stockfish from hanging forever after finding a mate in two or
three while running a test suite at a level of a few seconds
per move.
No functional change when playing games at normal time controls.
In qsearch() try to get a cutoff with the help of an
extra check if we are already very near.
Small increase in actual games but a good result in tactical
test sets where this patch makes SF more tactical.
Mod vs Orig +197 =620 -181 +6 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Louis Zulli reports a miscompile with g++-4.4 from MacPorts.
Namely enum Value is compiled as unsigned instead of signed integer
and this yields an issue in score_string() where float(v) is incorrectly
casted when Value v is negative.
This patch ensure that compiler choses a signed variable to store a Value.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Value is never used un-initialized, but MSVC is not
smart enough to detect itself :-(
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use the same scoring system used for evasions. Small if any
increase, but should be in at least for completeness.
After 999 games at 1+0
Mod vs Orig +208 =601 -190 +6 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that we have position static score we don't
need to call evaluate() a second time.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This will allow to have wider access to attack
information, for instance from MovePicker.
Note that 'eval' field become obsolete, it is kept
just becasue when we get a position score from TT
we update 'eval' even without an EvalInfo object.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In sp_search_pv() we do a LMR search using sp->alpha, at the end
we detect a fail high with condition (value > sp->alpha), but if
another thread has increased sp->alpha during our LMR search we
could miss to detect a fail high event becasue value will be equal
to old alpha and so smaller then new one.
This patch fixes this SMP-bug and changes also the non SMP versions
of the search to keep code style in sync.
Bug spotted by Bruno Causse.
No functional change (for single CPU case)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
futilityValue is now calculated immediately after
staticValue, so remove small bunch of unused code
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
MSVC raises an "use of partially uninitialized variable" for futilityValue
and staticValue but this is not rue becasue when !isCheck variables
are never used, anyhow silence the warning.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is less prone to bugs because now it's up to the
compiler don't forget this important initialization.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
No change in functionality signature
The only functional change is that when we reach PLY_MAX,
we now return VALUE_DRAW instead of evaluating position.
But we reach PLY_MAX only when position is dead drawn and
transposition table is filled with draw scores, so this
shouldn't matter at all.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This way razoring is always based on exact evaluation and
follows simple formula.
Joona's test results are positive:
32-bit 1CPU:
Mod - Orig: 1073 - 993
64-bit 4CPU:
Mod - Orig: 759 - 721
Functionality Signature: 11448962
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Don't clamp to zero if a move continues to fail.
After 946 games at 1+0
Mod vs Orig +208 =562 -176 +12 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also retire razoring margins vector and use
a simpler formula instead.
Now that we use a more accurate static evaluation
try to avoid useless null searches when we are well
below beta. And for teh same reason increase a bit
the razoring.
After 972 games at 1+0
Mod vs Orig +224 =558 -190 +12 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It means we have already received "stop" or "quit" commands.
This fixes an hang in tactical test in Fritz GUI. Bug
introduced by previous bug fix :-(
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
According to UCI standard once engine receives 'go infinite'
command it should search until the "stop" command and do not exit
the search without being told so, even if PLY_MAX has been reached.
Patch is quite invasive because it cleanups some hacks used
by fixed depth and fixed nodes modes, mainly during benchmarks.
Bug found by Pascal Georges.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After deep tests Louis Zulli found on his OCTAL machine that
best setup for an 8 core CPU is as following
"Threads" = 8
"Minimum Split Depth" = 6 or 7 (mSD)
"Maximum Number of Threads per Split Point" = not important (MNTpSP)
Here are testing results:
mSD7 (8 threads) vs mSD4 (8 threads): 291 - 120 - 589
mSD6 vs mSD7: 168 - 188 - 644
mSD6-MNTpSP5 vs mSD6-MNTpSP6: 172 - 172 - 656
SF-7threads vs SF-8threads: 179 - 204 - 617
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So to have the same layout and be as much similar as
possible. The only functional change is that now we
try ttMove as first also in PV nodes and at the end
we save the ttMove, as it happens in search. This
should have almost zero impact on ELO but it seems
the correct thing to do.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is an important design change because we know
compute evaluation in each node.
This is a 2.0 type change!
After 977 games at 1+0
Mod vs Orig +236 =538 -202 51.74% +12 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If after 'moves' there is a space then we crash.
The problem is that operator>>() trims whitespaces so that
after 'moves' has been extract we are still not at eof()
but remaining string contains only spaces. So that the next
extarction operation uip >> token ends up with unchanged token
value that remains 'moves', this garbage value is then feeded
to RootPosition.do_move() through move_from_string() that does
not detect the invalid move value leading to a crash.
This bug is triggered by Shredder 12 interface under Mac that
puts a space after 'moves' without any actual move list.
Bug fixed by Justin Blanchard
After reviewing UCI parsing code I spotted other possible weak
points due to the fact that we don't test if the last extract
operation has been succesful. So I have extended Justing patch
to fix the remaining possible holes in uci.cpp
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
According to standard en-passant is recorded in fen string regardless
of whether there is a pawn in position to make an en passant capture.
Instead internally we set ep square only if the pawn can be captured.
So teach from_fen() to correctly handle this difference.
Bug reported and fixed by Justin Blanchard.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Try to get a position evaluation better then
the quick one with the help of the TT table.
This allows the null search conditions and
chosen reductions to be more accurate.
After 908 games at 1+0
Mod vs Orig +209 =526 -173 +14 ELO
Functionality Signature: 16627355
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Linear rule, less aggressive then Dann's one.
It seems it scales well with depth. We will need to
verify against weaker engine if it keeps the score.
After 999 games at 1+0 on my Dual Core
Mod vs Orig +232 =534 -207 +9 ELO
After 1000 games by Martin Thoresen on his QUAD at 1+0
Mod vs Orig 521/479 52.10%
Functionality Signature: 17655312
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The most interesting thing is a bit of rewrite
and semplification in connected_moves()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
