Skip to content

Commit

Permalink
Join all capture init stages in MovePicker
Browse files Browse the repository at this point in the history 
Passed STC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 16789 W: 3685 L: 3554 D: 9550
http://tests.stockfishchess.org/tests/view/5a91a8bb0ebc590297cc875b

Passed LTC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 21293 W: 3527 L: 3407 D: 14359
http://tests.stockfishchess.org/tests/view/5a920a730ebc590297cc87ba

No functional change
  • Loading branch information
@vondele @snicolet
vondele authored and snicolet committed Feb 25, 2018
1 parent 16b31bb commit 8dd6875
Showing 1 changed file with 7 additions and 22 deletions.
29 changes: 7 additions & 22 deletions src/movepick.cpp
View file
Expand Up @@ -27,7 +27,7 @@ namespace {
enum Stages {
MAIN_SEARCH, CAPTURES_INIT, GOOD_CAPTURES, KILLERS, COUNTERMOVE, QUIET_INIT, QUIET, BAD_CAPTURES,
EVASION, EVASIONS_INIT, ALL_EVASIONS,
PROBCUT, PROBCUT_INIT, PROBCUT_CAPTURES,
PROBCUT, PROBCUT_CAPTURES_INIT, PROBCUT_CAPTURES,
QSEARCH, QCAPTURES_INIT, QCAPTURES, QCHECKS, QSEARCH_RECAPTURES, QRECAPTURES
};

Expand Down Expand Up @@ -161,11 +161,16 @@ Move MovePicker::next_move(bool skipQuiets) {
return ttMove;

case CAPTURES_INIT:
case PROBCUT_CAPTURES_INIT:
case QCAPTURES_INIT:
case QSEARCH_RECAPTURES:
endBadCaptures = cur = moves;
endMoves = generate<CAPTURES>(pos, cur);
score<CAPTURES>();
++stage;
/* fallthrough */

// Rebranch at the top of the switch via a recursive call
return next_move(skipQuiets);

case GOOD_CAPTURES:
while (cur < endMoves)
Expand Down Expand Up @@ -257,13 +262,6 @@ Move MovePicker::next_move(bool skipQuiets) {
}
break;

case PROBCUT_INIT:
cur = moves;
endMoves = generate<CAPTURES>(pos, cur);
score<CAPTURES>();
++stage;
/* fallthrough */

case PROBCUT_CAPTURES:
while (cur < endMoves)
{
Expand All @@ -274,13 +272,6 @@ Move MovePicker::next_move(bool skipQuiets) {
}
break;

case QCAPTURES_INIT:
cur = moves;
endMoves = generate<CAPTURES>(pos, cur);
score<CAPTURES>();
++stage;
/* fallthrough */

case QCAPTURES:
while (cur < endMoves)
{
Expand All @@ -304,12 +295,6 @@ Move MovePicker::next_move(bool skipQuiets) {
}
break;

case QSEARCH_RECAPTURES:
cur = moves;
endMoves = generate<CAPTURES>(pos, cur);
++stage;
/* fallthrough */

case QRECAPTURES:
while (cur < endMoves)
{
Expand Down

4 comments on commit 8dd6875

@snicolet
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, we now have this code in the capture init stages:

  case CAPTURES_INIT:
  case PROBCUT_CAPTURES_INIT:
  case QCAPTURES_INIT:
  case QSEARCH_RECAPTURES:
      endBadCaptures = cur = moves;
      endMoves = generate<CAPTURES>(pos, cur);
      score<CAPTURES>();
      ++stage;

I suddenly wonder if it is safe to change endBadCaptures again in stage PROBCUT_CAPTURES_INIT.
Could it be the cause of some recent crashes observed in the framework?

Should it be:

  case CAPTURES_INIT:
      endBadCaptures = moves;
  case PROBCUT_CAPTURES_INIT:
  case QCAPTURES_INIT:
  case QSEARCH_RECAPTURES:
      cur = moves;
      endMoves = generate<CAPTURES>(pos, cur);
      score<CAPTURES>();
      ++stage;

@vondele @mcostalba @syzygy1
Opinions?

I have never really liked the trick with the juggling of pointers for endBadCaptures and putting it back at the top, if we could rewrite it in a more pedestrian way it would be an occasion :-)

@vondele
Copy link
Member Author

Choose a reason for hiding this comment

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

@snicolet I don't think it is a problem. The assignment is not needed, but I wanted to treat all 4 stages the same.

Concerning the crashes, I'm running some more analysis of current master (40000 games match with debugging settings etc), but will only be able to investigate results tonight.

@snicolet
Copy link
Member

Choose a reason for hiding this comment

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

You are right, endBadCaptures is only used in the MovePicker for normal search, never for probcut, so it shouldn't matter.

Thanks for having a look at the crashes, keep us updated if you find anything :)

@protonspring
Copy link

@protonspring protonspring commented on 8dd6875 Feb 28, 2018 •
edited

Choose a reason for hiding this comment

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

(edited) The only crashes I've seen in movepicker is when cur is not set to moves before looking at moves.
Looks like it is set properly in the PROBCUT_INIT stage.

Please sign in to comment.