Skip to content

Commit

Permalink
Fix LazySMP when searching to a fixed depth.
Browse files Browse the repository at this point in the history 
Currently, helper threads will only search up to the
specified depth limit. Now let them search until the
main thread has finished the specified depth.

On the other hand, we don't want to pick a thread with
a higher search depth.

This may be considered cheating. ;-)

No functional change.
  • Loading branch information
@joergoster @mcostalba
joergoster authored and mcostalba committed May 1, 2016
1 parent 2dd24dc commit dc0030d
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/search.cpp
View file
Expand Up @@ -350,6 +350,7 @@ void MainThread::search() {
Thread* bestThread = this;
if ( !this->easyMovePlayed
&& Options["MultiPV"] == 1
&& !Limits.depth
&& !Skill(Options["Skill Level"]).enabled()
&& rootMoves[0].pv[0] != MOVE_NONE)
{
Expand Down Expand Up @@ -411,7 +412,7 @@ void Thread::search() {
multiPV = std::min(multiPV, rootMoves.size());

// Iterative deepening loop until requested to stop or the target depth is reached.
while (++rootDepth < DEPTH_MAX && !Signals.stop && (!Limits.depth || rootDepth <= Limits.depth))
while (++rootDepth < DEPTH_MAX && !Signals.stop && (!Limits.depth || Threads.main()->rootDepth <= Limits.depth))
{
// Set up the new depths for the helper threads skipping on average every
// 2nd ply (using a half-density matrix).
Expand Down Expand Up @@ -1006,7 +1007,7 @@ namespace {
Depth r = reduction<PvNode>(improving, depth, moveCount);
Value hValue = thisThread->history[pos.piece_on(to_sq(move))][to_sq(move)];
Value cmhValue = cmh[pos.piece_on(to_sq(move))][to_sq(move)];

const CounterMoveStats* fm = (ss - 2)->counterMoves;
const CounterMoveStats* fm2 = (ss - 4)->counterMoves;
Value fmValue = (fm ? (*fm)[pos.piece_on(to_sq(move))][to_sq(move)] : VALUE_ZERO);
Expand Down

6 comments on commit dc0030d

@lucasart
Copy link

@lucasart lucasart commented on dc0030d May 1, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcostalba: How is this a "fix" ? There was no bug to begin with. If the user asks to limit the search depth, SF must obey, and I don't see why helper threads should be exempted.

@mcostalba
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lucasart this is a bit a philosophical case. For instance, when SF is asked for a limited depth search, should we disable singular extension if it goes beyond depth? Should we disable all qsearch above the limit depth? Well, we don't.

Indeed helper threads with lazy SMP scheme are more similar to the "extension" case; they live mainly to prefill the TT for the main thread to search faster. It is different from the classical YBWC concept in which each slave threads searches a disjointed sub-tree and the best move is peeked out of the best sub-tree. In case of YBWC this patch would probably be not ok, but lazy SMP uses the threads in a very different way: it is only the main thread that returns the best move (note that @joergoster disabled the final "best move loop" in case of depth limit).

@mstembera
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option is to keep the helper threads running as long as the main thread runs but just keep looping them at the Limits.depth and not allow them deeper. That way the main thread can't pull any results from the TT that are based on a deeper search than requested. If you like this approach let me know. I'm happy to attempt the patch.

@joergoster
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mstembera But why should we do this? When we are searching with time limit we don't restrict the helper threads either, and all time management decisions are handled by the main thread. This is how SMP in SF is now working. Now we do the same when searching with depth limit. Quite logical imho.
And if somebody wants to measure Stockfish's SMP performance with fixed depth testing, he now gets 100% and not only 70 or 80%. :-)

@mstembera
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joergoster
In order to keep the result from being polluted by searches deeper than what was requested. As Marco points out it is a bit philosophical and I agree with him. I'm simply pointing out we have another option is all.

@mcostalba
Copy link

@mcostalba mcostalba commented on dc0030d May 2, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.