Add an UCI option "Zugzwang detection" OFF by default that
enables correct detection of zugzwang.
This is just to let 1.7.1 be 100% compatible with 1.7 and
should be removed after release.
Verified 100% functional equivalent to 1.7
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It fails in test position:
8/7P/8/8/K2b4/p7/1k6/1B6 b - -
Not clear why but we revert because it fixes the issue.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In this case use a normal VALUE_TYPE_LOWER TT type instead of
VALUE_TYPE_NS_LO. This allow us to TT cut-off in a bit more nodes.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This patch fixes an issue with zugzwang well explained by Tord:
"Assume that a zugzwang position occurs at iteration N,
at a search depth d, with d < 6*OnePly. The null move search
fails high, and no verification search is done, because the
depth is too small. The position gets stored in the transposition
table with a good score and a depth of d.
Now, consider what happens when the same position occurs at iteration
N+1, this time with a depth of d+OnePly (i.e. one ply deeper than at
the previous iteration). Once again, the null move search fails
high. The point is that the verification search will also fail high,
because of an instant transposition table cutoff caused by the value
stored in the TT during the previous iteration."
With this patch we simply do not allow TT cutoffs at the root node
of a null move verification search if the TT value was found by a
null search.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Add VALUE_TYPE_NS_LO to enum ValueType and use it when
saving in TT a value from a null search.
Currently no action is performed, the next patch will enable
the new type.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Reported warning is:
warning #2514-D: pointless comparison of unsigned
integer with a negative constant
Spotted by Richard Lloyd.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Under gcc we have:
warning: dereferencing type-punned pointer will break
strict-aliasing rules
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Use full depth, not reduced one. This allows
to avoid to do a null search when in the same
position and at the same or bigger depth the
null search failed high.
A very small increase, if any.
After 963 games at 1+0
Mod vs Orig: +158 =657 -147 +4 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In this form it is even more evident we have some
issue there to be fixed sooner then later....
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After previous patch we don't need any more the call parameters.
This fixes a couple of warnings under MSVC.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After 500 games at 5+0 on my QUAD (3 days) there
is no difference with old version, so probably it
is a feature that doesn't scale with search depth.
So revert for now, perhaps we should readd under a
different form.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
MSVC complains about an implicit conversion from double to int.
Also small comments tweaks.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In the beginning use milder reduction and at the end be
more aggressive.
After 1500 games on Joona's QUAD
Mod - Orig: 791 - 720 +16 elo
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Causes very small functional change which is not observable with
our usual set of test positions.
However change is observable fx. with following position:
4k3/3r4/5Q2/6K1/8/8/8/8 w - - 0 1
go depth 24
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
If null search fails high return null value instead of beta.
With TT hash there may be a small advantage for fail-soft since
storing slightly better bounds may cause slightly more hash hits.
After 990 games at 1+0
Mod vs Orig +171 =665 -154 +6 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Array castleRightsMask[] is not static because it can
be different for different positions, so let it be
a Position member data. This allows to remove tricky
hacks to take in account that although it was defined
static it could change.
Theoretically now copying a position is a bit slower because
we need to copy also an array of 64 integers, but because in
split() we don't copy the position anymore, but just keep the
pointer, the added burden is not mesurable even in MP case.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also don't use __cpuid() intrinsic for Intel under
Linux because gives wrong results when detecting HT,
use the gcc version instead. Finally clean up the code.
Error was due to changed __cpuid() signature for
gcc compiler.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This will allow to use the function also for other
purposes then detecting POPCNT.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It will be used by future patches and also rearranges some
half cooked code that mistakenly ended up in master in the
past.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Currently, in case of fail high/low we research with
a window increased by 2*AspirationDelta at first
attempt, this patch instead makes the research be
done with an increase of just AspirationDelta size,
in case of a consecutive fail we will widen to
2*AspirationDelta and so on.
After Joona's test:
Orig - Mod: 850 - 890
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
One for failing highs and one for failing lows, this
should reduce average window size in case of positions
that fail first high and then low (or the contrary).
After ~2000 games on Joona's quad we have:
Mod - Orig: 1012- 975 (+6 elo)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead to leave uninitialized or scattered in the code
as is the case for ExtraSearchTime.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Currently we use original sorting after a fail low to
research at wider window. This patch instead sorts the
moves according to the last available move's scores.
Strangely no functional change, but should be.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Changed FutilityMarginsMatrix dimensions to be a power of two
so that compiler can produce a faster accessing code.
Introduced print_pv_info() to remove some redundant code in
root_search()
Remaining stuff is triviality and documentation tweaks.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Under Windows we use CreateThread() to setup threads and
we pass a pointer to a variable that receives the thread
identifier, but this parameter is optional and we don't
use it, so remove it.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It happens that NULL is 0, but the conventional meaning is of
a zero pointer, so repleace with an explicit 0 integer value.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Consider sligtly negative captures as good if at low
depth and far from beta.
After 999 games at 1+0
Mod vs Orig +169 =694 -136 +11 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It raises an assert under Windows, it is not clear why but it
happens that idle_loop() is called with incorrect threadID and
the assert triggered is:
assert(threadID >= 0 && threadID < MAX_THREADS);
So revert the patch for now, but we should understand why it
fails.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We can't do it with full guarantee anyway because
there is always a possible race between the setting of
state to THREAD_SLEEPING and actual sleeping.
So just remove the not perfect code to avoid misunderstandings.
This reflects what we have done in wake_sleeping_threads() in
the previous patch.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Currently there is no guarantee that threads are sleeping
when calling wake_sleeping_threads() because put_threads_to_sleep()
returns without waiting for threads to actually sleep.
Assert can be easily triggered calling put_threads_to_sleep() and
wake_sleeping_threads() in a tight loop.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This option likely has very low meaning for playing strength and style,
so I see no need to keep this configurable
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
search() is used as a "leading star" and other routines
are modified according to it.
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Was broken and fixing would be too messy.
Now this option is only activated in single thread mode
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Just to avoid misunderstandings.
True staticValue is available through search stack
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Bug spotted by Jouni Uski and fix suggested by Pablo Vazquez
Also add note that we are not always handling fifty move rule correctly
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I cannot see connection between the two.
Also add one FIXME for illogical behaviour
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I don't know if enumerating sections is a good idea,
but for me code is more readable this way
No functional change
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
I cannot see any reason to do this. Even this is not enough to fix
theoretical race case on Windows which doesn't seem to cause any
problems in practice anyhow
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Resurrect extra check for sleeping in POSIX code.
This necessary to prevent ugly races between
thread_broadcast and thread_cond_wait.
After thread has woken up, it marks itself as available.
Another thread must not do this, because of possible race.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
So we can use a const value instead of a pointer in
split().
Also pass NULL instead of a faked address of alpha in
case split is called from a non-PV node.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Intrinsic __popcnt64() returns an unsigned __int64, cast
to an integer and silence the warning.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Joona says that sp_update_pv() does not pass the split point
boundaries, so there is no risk to corrupt data from another
split point. Also the race on thread_should_stop() is harmless
because of this.
So revert the patch and come back to single lock.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Joona says:
1. We should not be afraid of "AllThreadsShouldExit" flag.
Because when this is set to true we _must_not_ be searching (= All
splits must have been undone).
And if we are not searching it's impossible that some other thread
could give us work to do. So setting state to THREAD_AVAILABLE
doesn't do any harm. If you want to add check for this, you could do
it like this:
if (threads[threadID].state == THREAD_WORKISWAITING)
{
+ assert(!AllThreadsShouldExit)
threads[threadID].state = THREAD_SEARCHING;
2a. If waitSp->cpus == 0, setting state to THREAD_AVAILABLE makes
no harm either, because helpful master concept dictates that _only_
our own slave can book us. If we don't have any slaves, noone has the
right to book us.
2b. If point (2a) is not correct then your extra check only adds extra race:
In smp code checking for waitSp->cpus > 0 is not enough. It's possible that
our slave immediately exits and another thread
books us as a slave when our state is still
THREAD_AVAILABLE. So instead of adding extra level of security we have
just introduced extra race.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
When we found a cut-off then lock all the split point chain,
not only current one to avoid races in case two threads running
on different split points where one is ancestor then the other,
find a beta cut-off at the same time, in this case we want only
one to call sp_update_pv().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is a per split-point request, not per-thread. When we find
a beta cut-off in current thread's split point or in or in some
ancestor of the current split point then threads should stop
immediately the search and return to idle_loop().
The check is done by thread_should_stop() that now looks only
at split point's chain.
No functional change and a good semplification.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is easier to follow and also reduces the points
where state changes to mainly idle_loop() and split().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is because when we are below 4 * OnePly, the null move
will directly jump to qsearch and if we are below beta,
our opponent is above beta and will get immediate
stand pat cut off.
So basically this patch is just optimizing away useless
evaluation calls. dbg_hit_on() runs show that this heuristic
is correct >99% of cases. Transposition table probably causes
some inaccurary?
After 1148 games on QUAD
mod-orig: 583 - 565 +5 elo
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In this case is dangerous because in split() we reset the flag to
false, but if it was set due to a cut-off higher in the tree we
completely miss that and go on with the full search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of other flags this is not a state flag, i.e. does
not defines a state for the thread, but a request because
after we raise 'stopRequest' flag the corresponding thread is
not stopped, but continues to run for a while until it returns
from sp_search() in idle_loop.
It is important the name reflects this.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Among them 'stop' and 'printCurrentLineRequest' could have
random value, so reset to a known state before to leave the
search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is the first nice effect of previous patch !
Because thread_should_stop() should be declared 'const' we
need to remove the setting of 'stop' flag to true that
turns out to be a bug because thread_should_stop() is called
outside from lock protection while 'stop' flag is a volatile
shared variable so cannot be changed when not in lock.
Note that this bugs fires ONLY when we use more then 2 threads,
so commonly only in a QUAD or OCTAL machine.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Main aim of this patch is to consolidate all the thread related stuff
behind a single class interface so to avoid messing with global flags
and having thread code scattered among non-thread related stuff.
Another advantage is that now access to thread's variables is
more controlled, in particular we can differentiate between
read and write accesses by the mean of different interfaces, it
is so simpler to understand how a function is related to threads.
Lastly this rewrite is the base for future code consolidations and
semplifications that are easier now that we have only one thread's
access point.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Ensure threads are sleeping when leaving init_threads() and
the newly introduced put_threads_to_sleep().
Also ensure threads are not sleeping when leaving
wake_sleeping_threads().
As a side effect we now leave think() with all the threads
(but the main one) guaranteed to sleep. So when we enter
again in think(), after the opponent next move, we know
threads must be sleeping.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Wait inside wake_sleeping_threads() for the threads to be
effectively and reliably woken up.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Will be used by future patches. Also:
- Renamed Idle in AllThreadsShouldSleep
- Explicitly inited AllThreadsShouldExit and AllThreadsShouldSleep
in init_thread() instead of use an anonymous global initialization.
- Rewritten idle_loop() while condition to avoid a 'break' statement
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Patch from Richard Lloyd (slightly edited from me), following the list
of changes as described by the author:
src/Makefile:
- Added PREFIX and BINDIR for the install: rule.
- Added a "make hpux" line to the help: rule.
- Added "make test"/"make check" rule that runs the $(PGOBENCH) command.
- "make clean" now additionally removes core and bench.txt.
- Added an hpux: rule.
- Added an install: rule to mkdir $(BINDIR), copy $(EXE) to $(BINDIR) and
then strip it.
- "make strip" now ensures that $(EXE) is built first before trying to
strip it.
- Hide errors and output from the g++ command used by the .depend: rule and
then touch .depend in case g++ isn't available.
- Hide errors from the "include .depend" in case .depend doesn't exist
(e.g. directly after a "make clean").
src/book.cpp and src/book.h:
- HP-UX's aCC really didn't like the const keywords used for the
Book::file_name() definitions, so they were removed. I checked that this
didn't affect a Linux build and it was still fine.
src/misc.cpp:
- HP-UX uses <sys/pstat.h> and pstat_getdynamic() to determine the number of
CPU cores, so added conditional code for that (if pstat_getdynamic() fails,
set the number of cores to 1).
src/tt.cpp:
- <xmmintrin.h> and _mm_prefetch() seem highly specific to the Intel x86(_64)
and gcc platforms - neither exist in HP-UX, so conditionally avoid that
code in HP-UX's case. Perhaps some sort of define is needed here
such as -DHAS_MM_PREFETCH that could be #ifdef'ed for instead?
Even after these changes, it's more convenient for HP-UX users to edit the
default: rule in the Makefile to run "$(MAKE) hpux" before they build
stockfish, but that's not a big deal if they're warned about that first (the
same applies to all other builds other than the standard "$(MAKE) gcc" one).
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Compiler complains because in Book we have a d'tor but not
copy c'tor and assignement operator (warning C4511 and C4512),
note that after adding them (just declared) you now need also
default c'tor !
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is an hidden bug waiting to fire. The main problem is
that ss[ply] is overwritten by search() and qsearch() called
from IID and razoring, so that we cannot hold a pointer to a
local EvalInfo variable.
For instance if we go razoring then we overwrite the pointer
with the address of a variable local to qsearch(), when we return
from qsearch() variable goes out of scope and now ss[ply].evalInfo
holds a stale pointer !
Because we are not looking for troubles we go through the
safe route and we remove it entirely.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Gain value is multiplied by 16 to be of comparable magnitudo
of negative history, on average.
This patch shows very good results in tactical tests, but
started very bad in real games, so I have run two test matches.
After 896 games at 1+0
Mod vs Orig +187 =525 -184 +1 ELO
After 999 games at 1+0
Mod vs Orig +223 =590 -186 +13 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We can do this only when needed, if we get a cut-off
before we skip sorting entirely. This reduces sorting
time of about 20%.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In search routines we use information from previous ply
and init killers two plies ahead.
So for me it seems correct to copy 4 searchstack items
in split:
ply - 1, ply, ply + 1, ply + 2
Because
a) we do not split at root (ply == 0)
b) ply < PLY_MAX and SearchStack size is PLY_MAX_PLUS_2
there should be no risk of underflows or overflows
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With negative history we don't have anymore a
lot of zeroes to score, so just split moves in
positives and non-positives sets.
Speed up is almost zero, we cannot test speed directly
because node count changed due to reorder, but I have
verified sorting is correct. With a profiler I have
seen we gain a little in sort_moves() and lose a little
in insertion_sort(), so the net effect is almost zero,
but code is simpler.
No real change, just move reordering.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It was only used to control StopOnPonderHit variable.
Now use FailLow variable instead.
Patch has a minor effect on time management when ponder is on.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
noProblemFound condition is never true.
This was verified by running 800 games 1+0 match in 1 CPU computer.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Because H.move_ordering_score() can return negative values
some negative see moves could be searched before non-negative
see moves with negative history.
This patch restores proper ordering.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Now that history can go negative and is almost alwyas
non zero we have no more reasons to use also psqt term.
After 994 games at 1+0
Mod vs Orig +204 =597 -193 +4 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Instead of piece-from-to, in this way it is similar
to what we already do for history.
Almost no change, but seems a bit simpler in this way.
After 995 games at 1+0
Mod vs Orig +207 =596 -192 +5 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems more standard conformant. Also added a bit of
description directly from Tord.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We erroneusly added two times the same scaling function
to endgame's map.
Fix detected by valgrind becasue resulted in a memleak
of the first added scaling function.
Bug introduced by 30e8f0c9ad6a473 of 13/02/2009
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We had an overflow due to use an integer for hash size,
now we use a size_t as we should, so we can increase to
an higher limit.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Normally it's up to the GUI to check for option's limits,
but we could receive the new value directly from the user
by teminal window. So let's check the bounds anyway.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With new target 'make gcc-popcnt' it is now
possible to compile with enabled hardware POPCNT
support also with gcc. Until now was possible only
for Intel and MSVC compilers.
When this instruction is supported by CPU, for instance
on Intel i7 or i5 family, produced binary is a bit faster.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This reverts commit c43c5fe9e0.
Produces following build error after 'make clean', 'make icc' under Mandriva
with icc version 11.0
Makefile:306: .depend: No such file or directory
In file included from tt.cpp:28:
/usr/lib/gcc/i586-manbo-linux-gnu/4.3.2/include/xmmintrin.h:35:3: error: #error "SSE instruction set not enabled"
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Futility captures alone does not seem an improvment.
Perhaps is a combination of stand pat + futility that is winning,
so revert for now and continue testing starting from a standard
base until we find the correct receipe.
After 999 games at 1+0
Mod vs Orig +231 0506 -201 +10 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Under some rare cases we can have a search tree explosion
due to a perpetual check or to a very long non-capture TT
sequence.
This avoids the tree explosion not following TT moves that
are not captures or promotions when we are below the
'generate checks' depth.
Idea suggested by Richard Vida.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Not a biggie but is a reduced pruned patch that doesn't
seems to hurt, so it is welcomed ;-)
After 999 games at 1+0
Mod vs Orig +207 =601 -191 +6 ELO
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
At ply == OnePly (common case) we avoid some useless
floating point computation.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Previously input like "setoption name Use Search Log value true "
(note space at the end of the line) didn't work.
Now parse value same way as option name. This way we implicitly
left- and right-trim value.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Initializing high-level object at startup is very dangerous,
because low-level snippets are not yet initialized.
For example Position's constructor calls find_checkers() which
calls attackers_to() which depends on various global bitboard arrays
which are not yet initialized. I think we are lucky not to crash.
RootPosition.from_fen(StartPosition); is called immediately after
all initializations are made at uci_main_loop() which is the
correct behaviour
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Joona new aspiration window. Main idea is to always
research aspiration fail highs/low at the same
ply and use much smaller aspiration window than previously.
Testing result is very positive.
1CPU:
953-1149
4CPU:
545 - 656
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Avoid to take the lock two times in a tight sequence, the first
in get_next_move() and the second to update sp->moves.
Do all with one lock and so retire the now useless locked version
of get_next_move().
Also fix some theorical race due to comparison sp->bestValue < sp->beta
is done out of lock protection. Finally fix another (harmless but time
waster) race that coudl occur because thread_should_stop() is also
called outside of lock protection.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In debug run with 2 threads it happens to be following
assert after some minutes:
assert(value > -VALUE_INFINITE && value < VALUE_INFINITE);
in search(), line 1615.
I am not able to understand why, anyhow reverted for the moment.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This avoid us to forget some very needed tests now that
futility has changed in a whole big chunk we need to fine
tuning every splitted change.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This will be useful to use gains table in move
ordering along with history table.
No functional change and big code remove.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
With current search control system, I can see absolutely no
reason to classify fixed time search as infinite search.
So remove old dated hack
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Value of uip.eof() should not be trusted.
input like "go infinite searchmoves " (note space in the end of line)
causes problems.
Check the return value of (uip >> token) instead
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
In less then 1% of cases value > sp->bestValue, so avoid
an useless lock in the common case. This is the same change
already applied to sp_search().
Also SplitPoint futilityValue is not volatile because
never changes after has been assigned in split()
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
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>
It is not useful becasue it is safe to call
get_next_move() multiple times when phase == PH_STOP
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Because that's the correct meaning. Note that also the
corresponding UCI option has been renamed.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
After history accounting rewrite in 1.6, a small
tweak of history parameters seems positive.
Note that these are not to be considered the optimal
values, just a wild guess that proved good.
Finding the optimal values would require a much longer
testing time.
After 967 games at 1+0
Mod vs Orig 240 529 198 +15 ELO
Functionality Signature: 21222553
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.
