Skip to content

Commit

Permalink
Code style in Razoring and ProbCut
Browse files Browse the repository at this point in the history 
No functional change.
  • Loading branch information
@joergoster @snicolet
joergoster authored and snicolet committed Mar 6, 2018
1 parent 3192b09 commit 43682d0
Showing 1 changed file with 15 additions and 13 deletions.
28 changes: 15 additions & 13 deletions src/search.cpp
View file
Expand Up @@ -682,20 +682,21 @@ namespace {
goto moves_loop;

// Step 7. Razoring (skipped when in check)
if ( !PvNode
&& depth <= ONE_PLY)
if ( !PvNode
&& depth <= 2 * ONE_PLY)
{
if (eval + RazorMargin1 <= alpha)
if ( depth == ONE_PLY
&& eval + RazorMargin1 <= alpha)
return qsearch<NonPV, false>(pos, ss, alpha, alpha+1);
}
else if ( !PvNode
&& depth <= 2 * ONE_PLY
&& eval + RazorMargin2 <= alpha)
{
Value ralpha = alpha - RazorMargin2;
Value v = qsearch<NonPV, false>(pos, ss, ralpha, ralpha+1);
if (v <= ralpha)
return v;

else if (eval + RazorMargin2 <= alpha)
{
Value ralpha = alpha - RazorMargin2;
Value v = qsearch<NonPV, false>(pos, ss, ralpha, ralpha+1);

if (v <= ralpha)
return v;
}
}

// Step 8. Futility pruning: child node (skipped when in check)
Expand Down Expand Up @@ -776,7 +777,7 @@ namespace {
// Perform a preliminary search at depth 1 to verify that the move holds.
// We will only do this search if the depth is not 5, thus avoiding two
// searches at depth 1 in a row.
if (depth != 5 * ONE_PLY)
if (depth > 5 * ONE_PLY)
value = -search<NonPV>(pos, ss+1, -rbeta, -rbeta+1, ONE_PLY, !cutNode, true);

// If the first search was skipped or was performed and held, perform
Expand All @@ -785,6 +786,7 @@ namespace {
value = -search<NonPV>(pos, ss+1, -rbeta, -rbeta+1, depth - 4 * ONE_PLY, !cutNode, false);

pos.undo_move(move);

if (value >= rbeta)
return value;
}
Expand Down

12 comments on commit 43682d0

@jerrydonaldwatson
Copy link

Choose a reason for hiding this comment

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

I notice that now we get a (bogus) "uninitialized" warning for value in line 785, at least with mingw on Windows.

Obviously we never use value uninitialized, however are we ok with having this warning?

@Hanamuke
Copy link

Choose a reason for hiding this comment

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

Is it bogus ? I can't see where it is initialized. I could be wrong that's a big file.

@Hanamuke
Copy link

Choose a reason for hiding this comment

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

Before it was if(depth !=5) ... if(depth==5) so it was ok but now it is if(depth>5) if(depth ==5 ) so you could reach || value>= rbeta without having initialized it in the previous if, if depth < 5.

@Hanamuke
Copy link

Choose a reason for hiding this comment

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

besides, the comment "// If the first search was skipped or was performed and held, perform
// the regular search." is invalid too, it should be if(depth >5) ... if(depth <=5) if we want to use inequalities

@jerrydonaldwatson
Copy link

Choose a reason for hiding this comment

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

@Hanamuke
ProbCut is only done for depth >= 5 * ONE_PLY, which is why we never use the value uninitialized as it stands.

@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.

No warning here. It also passed both checks, AppVeyor and Travis CI.

@jerrydonaldwatson
Copy link

Choose a reason for hiding this comment

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

@joergoster
Are you on Linux? Because I only get a warning on Windows. Note that there is a thread in the forum about this compiler warning too.

@Hanamuke
Copy link

Choose a reason for hiding this comment

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

There is a warning in the appveyor and travis builds, i checked. But i can't blame gcc to not notice it is initialized on all paths. Changing if(depth==5) to if(depth<=5) would probably fix the warning, but it's no big deal really.

@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.

@jerrydonaldwatson Yes, Linux only.

@Hanamuke
Copy link

Choose a reason for hiding this comment

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

gcc 7.0 seem to produce a warning too, in the travis CI build

@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.

@snicolet Do we want to suppress this silly warning and revert the change in the ProbCut code?

@snicolet
Copy link
Member

Choose a reason for hiding this comment

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

Warning fixed in d42e633, thanks.

Please sign in to comment.