Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Don't double check for move legality 
In case of a RootNode or a SpNode move has
been already checked for legality so we can
skip a redundant check.

Spotted by Frank Genot.

No functional change.
  • Loading branch information
@mcostalba
mcostalba committed Nov 27, 2012
1 parent 5af8179 commit 4502917
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/search.cpp
View file
Expand Up @@ -893,7 +893,7 @@ namespace {
}

// Check for legality only before to do the move
if (!pos.pl_move_is_legal(move, ci.pinned))
if (!RootNode && !SpNode && !pos.pl_move_is_legal(move, ci.pinned))
{
moveCount--;
continue;
Expand Down

5 comments on commit 4502917

@lucasart
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello Marco,

I was curious to see what the FutilityMargins[][] of SF looked like, so I fired up a spreadsheet and replicated your formula:

// Init futility margins array
for (d = 1; d < 16; d++) for (mc = 0; mc < 64; mc++)
FutilityMargins[d][mc] = Value(112 * int(log(double(d * d) / 2) / log(2.0) + 1.001) - 8 * mc + 45);

It gives negative numbers when

  • d = 1, mc >= 6
  • d = 2, mc >= 34
  • d = 3, mc >= 48
  • d = 4 or d = 5, mc >= 62

I suspect fixing this bug won't have a big impact, but it's still worth doing for the sake of cleanlyness. Perhaps you apply a floor like:

FutilityMargins[d][mc] = std::max(0, ...);

And possibly floor by some epsilon > 0, rather than by zero.

@lucasart
Copy link
Contributor

Choose a reason for hiding this comment

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

Come to think of it, I find your formula quite strange. You are basically using an additive form like
(i) f(d,mc) = g(d) + h(mc)
(ii) h() is linear

I think it would make more sense to consider something like
(i) f(d,mc) = g(d)h(mc)
(ii) h() is a move count rescaling basically:

  • h() is a double in the interval [0,1]
  • h(0)=1, so that g(d) is really the margin at depth d for mc=0

How about, for example:
f(d,mc) = g(d).sqrt[(128-d)/d]

where g(d) is your unmodified
112 * int(log(double(d * d) / 2) / log(2.0) + 1.001)

Anyway, it's probably best to experiment in a simple spreadsheet with the formula and see how it looks, before spending time testing it.

@zamar
Copy link
Contributor

@zamar zamar commented on 4502917 Nov 28, 2012

Choose a reason for hiding this comment

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

It's not a bug. Negative futility pruning values at depth = 1 are okay. Move count based pruning is going to hit us hard anyway razoring everything where moveCount > 4. Negative futility pruning values only mean that side to move needs to have very good position to avoid prunings.

Of course I don't claim that current formula is optimal, but there is no bug

@lucasart
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, understood.

I'm surprised at how aggressive the futility pruning + move count pruning of SF is. And given the way it's been developped (systematic testing), it must be right, or at least better than not doing it.

Out of curiosity, how much elo do you reckon futility pruning (including move count pruning) is worth in Stockfish ?

@mcostalba
Copy link
Owner Author

@mcostalba mcostalba commented on 4502917 Nov 28, 2012 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.