Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix incorrect mate score.
Current master 648c7ec will generate an
incorrect mate score for:

```
setoption name Hash value 8
setoption name Threads value 1
position fen 8/1p2KP2/1p4q1/1Pp5/2P5/N1Pp1k2/3P4/1N6 b - - 76 40
go depth 49
```
even though the position is a draw. Generally, SF tries to display only
proven mate scores, so this is a bug.

This was posted http://www.talkchess.com/forum3/viewtopic.php?f=2&t=72166
by Uri Blass, with the correct analysis that this must be related to the
50 moves draw rule being ignored somewhere.

Indeed, this is possible as positions and there eval are stored in the TT,
without reference to the 50mr counter. Depending on the search path followed
a position can thus be mate or draw in the TT (GHI or Graph history interaction).
Therefore, to prove mate lines, the TT content has to be used with care. Rather
than ignoring TT content in general or for mate scores (which impact search or
mate finding), it is possible to be more selective. In particular, @WOnder93
suggested to only ignore the TT if the 50mr draw ply is closer than the mate
ply. This patch implements this idea, by clamping the eval in the TT to
+-VALUE_MATED_IN_MAX_PLY. This retains the TTmove, but causes a research of
these lines (with the current 50mr counter) as needed.

This patch hardly ever affects search (as indicated by the unchanged
bench), but fixes the testcase. As the conditions are very specific,
also mate finding will almost never be less efficient (testing welcome).

It was also shown to pass STC and LTC non-regression testing, in a form
using if/then/else instead of ternary operators:

STC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 93605 W: 15346 L: 15340 D: 62919
http://tests.stockfishchess.org/tests/view/5db45bb00ebc5908127538d4

LTC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 33873 W: 7359 L: 7261 D: 19253
http://tests.stockfishchess.org/tests/view/5db4c8940ebc5902d6b146fc

closes #2370

Bench: 4362323
  • Loading branch information
vondele authored and snicolet committed Nov 12, 2019
1 parent 9f312c8 commit 5ae195e
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions src/search.cpp
Expand Up @@ -150,7 +150,7 @@ namespace {
Value qsearch(Position& pos, Stack* ss, Value alpha, Value beta, Depth depth = 0);

Value value_to_tt(Value v, int ply);
Value value_from_tt(Value v, int ply);
Value value_from_tt(Value v, int ply, int r50c);
void update_pv(Move* pv, Move move, Move* childPv);
void update_continuation_histories(Stack* ss, Piece pc, Square to, int bonus);
void update_quiet_stats(const Position& pos, Stack* ss, Move move, int bonus);
Expand Down Expand Up @@ -661,7 +661,7 @@ namespace {
excludedMove = ss->excludedMove;
posKey = pos.key() ^ Key(excludedMove << 16); // Isn't a very good hash
tte = TT.probe(posKey, ttHit);
ttValue = ttHit ? value_from_tt(tte->value(), ss->ply) : VALUE_NONE;
ttValue = ttHit ? value_from_tt(tte->value(), ss->ply, pos.rule50_count()) : VALUE_NONE;
ttMove = rootNode ? thisThread->rootMoves[thisThread->pvIdx].pv[0]
: ttHit ? tte->move() : MOVE_NONE;
ttPv = PvNode || (ttHit && tte->is_pv());
Expand Down Expand Up @@ -899,7 +899,7 @@ namespace {
search<NT>(pos, ss, alpha, beta, depth - 7, cutNode);

tte = TT.probe(posKey, ttHit);
ttValue = ttHit ? value_from_tt(tte->value(), ss->ply) : VALUE_NONE;
ttValue = ttHit ? value_from_tt(tte->value(), ss->ply, pos.rule50_count()) : VALUE_NONE;
ttMove = ttHit ? tte->move() : MOVE_NONE;
}

Expand Down Expand Up @@ -1344,7 +1344,7 @@ namespace {
// Transposition table lookup
posKey = pos.key();
tte = TT.probe(posKey, ttHit);
ttValue = ttHit ? value_from_tt(tte->value(), ss->ply) : VALUE_NONE;
ttValue = ttHit ? value_from_tt(tte->value(), ss->ply, pos.rule50_count()) : VALUE_NONE;
ttMove = ttHit ? tte->move() : MOVE_NONE;
pvHit = ttHit && tte->is_pv();

Expand Down Expand Up @@ -1530,11 +1530,11 @@ namespace {
// from the transposition table (which refers to the plies to mate/be mated
// from current position) to "plies to mate/be mated from the root".

Value value_from_tt(Value v, int ply) {
Value value_from_tt(Value v, int ply, int r50c) {

return v == VALUE_NONE ? VALUE_NONE
: v >= VALUE_MATE_IN_MAX_PLY ? v - ply
: v <= VALUE_MATED_IN_MAX_PLY ? v + ply : v;
: v >= VALUE_MATE_IN_MAX_PLY ? VALUE_MATE - v > 99 - r50c ? VALUE_MATE_IN_MAX_PLY : v - ply
: v <= VALUE_MATED_IN_MAX_PLY ? VALUE_MATE + v > 99 - r50c ? VALUE_MATED_IN_MAX_PLY : v + ply : v;
}


Expand Down

4 comments on commit 5ae195e

@svivanov72
Copy link

Choose a reason for hiding this comment

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

Maybe I'm a bit late to comment here, but what does this patch do to TB win scores? The constant VALUE_MATE_IN_MAX_PLY (= VALUE_MATE - 2 * MAX_PLY) is actually the lower bound of the TB win range. The TB win scores are between this value and VALUE_MATE - MAX_PLY. It seems that the new value_from_tt() interprets them as mates beyond 50-move rule and returns VALUE_MATE_IN_MAX_PLY for all of them. Wouldn't SF lose direction to the TB win because of this?

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

can you file an issue for this so we can investigate. Ideally with some data to show the problem is there.

@svivanov72
Copy link

Choose a reason for hiding this comment

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

Unfortunately I have no data, and I do not have syzygy TB to test. Does it make sense to file an issue or a "no functional change" patch that just changes the constant?

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

please file an issue, it will be easier to have other people engage in the discussion, and so that we don't forget.

Please sign in to comment.