Skip to content

Commit 3cf6471

Browse files
mcostalbamcostalba
committed
Revert evaluation cache
And return on using TT as backing store for position evaluations. Tests (even on single thread) show eval cache was a regression. In multi thread result should be even worst because eval cache is a per-thread struct, while TT is shared. After 4957 games at 15"+0.05 (single thread) eval cache vs master 969 - 1093 - 2895 -9 ELO So previous reported result of +18 ELO was probably due to an issue in the testing framework (a bug in cutechess-cli) that has been fixed in the meanwhile. bench: 5386711
1 parent f78b68b commit 3cf6471

6 files changed

Lines changed: 61 additions & 57 deletions

File tree

‎src/evaluate.cpp‎

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ namespace {
244244
Score evaluate_pieces_of_color(const Position& pos, EvalInfo& ei, Score& mobility);
245245

246246
template<Color Us, bool Trace>
247-
Score evaluate_king(const Position& pos, EvalInfo& ei, int16_t margins[]);
247+
Score evaluate_king(const Position& pos, EvalInfo& ei, Value margins[]);
248248

249249
template<Color Us>
250250
Score evaluate_threats(const Position& pos, EvalInfo& ei);
@@ -364,27 +364,13 @@ Value do_evaluate(const Position& pos, Value& margin) {
364364
assert(!pos.checkers());
365365

366366
EvalInfo ei;
367+
Value margins[COLOR_NB];
367368
Score score, mobilityWhite, mobilityBlack;
368-
369-
Key key = pos.key();
370369
Thread* th = pos.this_thread();
371-
Eval::Entry* e = th->evalTable[key];
372-
373-
// If e->key matches the position's hash key, it means that we have analysed
374-
// this node before, and we can simply return the information we found the last
375-
// time instead of recomputing it.
376-
if (e->key == key)
377-
{
378-
margin = Value(e->margins[pos.side_to_move()]);
379-
return e->value;
380-
}
381-
382-
// Otherwise we overwrite current content with this node info.
383-
e->key = key;
384370

385371
// margins[] store the uncertainty estimation of position's evaluation
386372
// that typically is used by the search for pruning decisions.
387-
e->margins[WHITE] = e->margins[BLACK] = VALUE_ZERO;
373+
margins[WHITE] = margins[BLACK] = VALUE_ZERO;
388374

389375
// Initialize score by reading the incrementally updated scores included
390376
// in the position object (material + piece square tables) and adding
@@ -400,8 +386,7 @@ Value do_evaluate(const Position& pos, Value& margin) {
400386
if (ei.mi->specialized_eval_exists())
401387
{
402388
margin = VALUE_ZERO;
403-
e->value = ei.mi->evaluate(pos);
404-
return e->value;
389+
return ei.mi->evaluate(pos);
405390
}
406391

407392
// Probe the pawn hash table
@@ -420,8 +405,8 @@ Value do_evaluate(const Position& pos, Value& margin) {
420405

421406
// Evaluate kings after all other pieces because we need complete attack
422407
// information when computing the king safety evaluation.
423-
score += evaluate_king<WHITE, Trace>(pos, ei, e->margins)
424-
- evaluate_king<BLACK, Trace>(pos, ei, e->margins);
408+
score += evaluate_king<WHITE, Trace>(pos, ei, margins)
409+
- evaluate_king<BLACK, Trace>(pos, ei, margins);
425410

426411
// Evaluate tactical threats, we need full attack information including king
427412
score += evaluate_threats<WHITE>(pos, ei)
@@ -467,7 +452,7 @@ Value do_evaluate(const Position& pos, Value& margin) {
467452
sf = ScaleFactor(50);
468453
}
469454

470-
margin = Value(e->margins[pos.side_to_move()]);
455+
margin = margins[pos.side_to_move()];
471456
Value v = interpolate(score, ei.mi->game_phase(), sf);
472457

473458
// In case of tracing add all single evaluation contributions for both white and black
@@ -484,16 +469,16 @@ Value do_evaluate(const Position& pos, Value& margin) {
484469
Score b = make_score(ei.mi->space_weight() * evaluate_space<BLACK>(pos, ei), 0);
485470
trace_add(SPACE, apply_weight(w, Weights[Space]), apply_weight(b, Weights[Space]));
486471
trace_add(TOTAL, score);
487-
TraceStream << "\nUncertainty margin: White: " << to_cp(Value(e->margins[WHITE]))
488-
<< ", Black: " << to_cp(Value(e->margins[BLACK]))
472+
TraceStream << "\nUncertainty margin: White: " << to_cp(margins[WHITE])
473+
<< ", Black: " << to_cp(margins[BLACK])
489474
<< "\nScaling: " << std::noshowpos
490475
<< std::setw(6) << 100.0 * ei.mi->game_phase() / 128.0 << "% MG, "
491476
<< std::setw(6) << 100.0 * (1.0 - ei.mi->game_phase() / 128.0) << "% * "
492477
<< std::setw(6) << (100.0 * sf) / SCALE_FACTOR_NORMAL << "% EG.\n"
493478
<< "Total evaluation: " << to_cp(v);
494479
}
495480

496-
return e->value = pos.side_to_move() == WHITE ? v : -v;
481+
return pos.side_to_move() == WHITE ? v : -v;
497482
}
498483

499484

@@ -768,7 +753,7 @@ Value do_evaluate(const Position& pos, Value& margin) {
768753
// evaluate_king<>() assigns bonuses and penalties to a king of a given color
769754

770755
template<Color Us, bool Trace>
771-
Score evaluate_king(const Position& pos, EvalInfo& ei, int16_t margins[]) {
756+
Score evaluate_king(const Position& pos, EvalInfo& ei, Value margins[]) {
772757

773758
const Color Them = (Us == WHITE ? BLACK : WHITE);
774759

@@ -868,7 +853,7 @@ Value do_evaluate(const Position& pos, Value& margin) {
868853
// be very big, and so capturing a single attacking piece can therefore
869854
// result in a score change far bigger than the value of the captured piece.
870855
score -= KingDangerTable[Us == Search::RootColor][attackUnits];
871-
margins[Us] += int16_t(mg_value(KingDangerTable[Us == Search::RootColor][attackUnits]));
856+
margins[Us] += mg_value(KingDangerTable[Us == Search::RootColor][attackUnits]);
872857
}
873858

874859
if (Trace)

‎src/evaluate.h‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#if !defined(EVALUATE_H_INCLUDED)
2121
#define EVALUATE_H_INCLUDED
2222

23-
#include "misc.h"
2423
#include "types.h"
2524

2625
class Position;
@@ -31,16 +30,6 @@ extern void init();
3130
extern Value evaluate(const Position& pos, Value& margin);
3231
extern std::string trace(const Position& pos);
3332

34-
const int TableSize = 262144;
35-
36-
struct Entry {
37-
Key key;
38-
Value value;
39-
int16_t margins[2];
40-
};
41-
42-
struct Table : HashTable<Entry, TableSize> {};
43-
4433
}
4534

4635
#endif // !defined(EVALUATE_H_INCLUDED)

‎src/search.cpp‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -575,17 +575,31 @@ namespace {
575575
// Step 5. Evaluate the position statically and update parent's gain statistics
576576
if (inCheck)
577577
ss->staticEval = ss->evalMargin = eval = VALUE_NONE;
578-
else
578+
579+
else if (tte)
579580
{
580-
eval = ss->staticEval = evaluate(pos, ss->evalMargin);
581+
// Following asserts are valid only in single thread condition because
582+
// TT access is always racy and its contents cannot be trusted.
583+
assert(tte->static_value() != VALUE_NONE || Threads.size() > 1);
584+
assert(ttValue != VALUE_NONE || tte->type() == BOUND_NONE || Threads.size() > 1);
585+
586+
ss->staticEval = eval = tte->static_value();
587+
ss->evalMargin = tte->static_value_margin();
588+
589+
if (eval == VALUE_NONE || ss->evalMargin == VALUE_NONE) // Due to a race
590+
eval = ss->staticEval = evaluate(pos, ss->evalMargin);
581591

582592
// Can ttValue be used as a better position evaluation?
583-
if (tte && ttValue != VALUE_NONE)
584-
{
593+
if (ttValue != VALUE_NONE)
585594
if ( ((tte->type() & BOUND_LOWER) && ttValue > eval)
586595
|| ((tte->type() & BOUND_UPPER) && ttValue < eval))
587596
eval = ttValue;
588-
}
597+
}
598+
else
599+
{
600+
eval = ss->staticEval = evaluate(pos, ss->evalMargin);
601+
TT.store(posKey, VALUE_NONE, BOUND_NONE, DEPTH_NONE, MOVE_NONE,
602+
ss->staticEval, ss->evalMargin);
589603
}
590604

591605
// Update gain for the parent non-capture move given the static position
@@ -1041,7 +1055,8 @@ namespace {
10411055

10421056
if (bestValue >= beta) // Failed high
10431057
{
1044-
TT.store(posKey, value_to_tt(bestValue, ss->ply), BOUND_LOWER, depth, bestMove);
1058+
TT.store(posKey, value_to_tt(bestValue, ss->ply), BOUND_LOWER, depth,
1059+
bestMove, ss->staticEval, ss->evalMargin);
10451060

10461061
if (!pos.is_capture_or_promotion(bestMove) && !inCheck)
10471062
{
@@ -1066,7 +1081,7 @@ namespace {
10661081
else // Failed low or PV search
10671082
TT.store(posKey, value_to_tt(bestValue, ss->ply),
10681083
PvNode && bestMove != MOVE_NONE ? BOUND_EXACT : BOUND_UPPER,
1069-
depth, bestMove);
1084+
depth, bestMove, ss->staticEval, ss->evalMargin);
10701085

10711086
assert(bestValue > -VALUE_INFINITE && bestValue < VALUE_INFINITE);
10721087

@@ -1147,14 +1162,25 @@ namespace {
11471162
ss->staticEval = bestValue = -(ss-1)->staticEval;
11481163
ss->evalMargin = VALUE_ZERO;
11491164
}
1165+
else if (tte)
1166+
{
1167+
assert(tte->static_value() != VALUE_NONE || Threads.size() > 1);
1168+
1169+
ss->staticEval = bestValue = tte->static_value();
1170+
ss->evalMargin = tte->static_value_margin();
1171+
1172+
if (ss->staticEval == VALUE_NONE || ss->evalMargin == VALUE_NONE) // Due to a race
1173+
ss->staticEval = bestValue = evaluate(pos, ss->evalMargin);
1174+
}
11501175
else
11511176
ss->staticEval = bestValue = evaluate(pos, ss->evalMargin);
11521177

11531178
// Stand pat. Return immediately if static value is at least beta
11541179
if (bestValue >= beta)
11551180
{
11561181
if (!tte)
1157-
TT.store(pos.key(), value_to_tt(bestValue, ss->ply), BOUND_LOWER, DEPTH_NONE, MOVE_NONE);
1182+
TT.store(pos.key(), value_to_tt(bestValue, ss->ply), BOUND_LOWER,
1183+
DEPTH_NONE, MOVE_NONE, ss->staticEval, ss->evalMargin);
11581184

11591185
return bestValue;
11601186
}
@@ -1263,7 +1289,9 @@ namespace {
12631289
}
12641290
else // Fail high
12651291
{
1266-
TT.store(posKey, value_to_tt(value, ss->ply), BOUND_LOWER, ttDepth, move);
1292+
TT.store(posKey, value_to_tt(value, ss->ply), BOUND_LOWER,
1293+
ttDepth, move, ss->staticEval, ss->evalMargin);
1294+
12671295
return value;
12681296
}
12691297
}
@@ -1277,7 +1305,7 @@ namespace {
12771305

12781306
TT.store(posKey, value_to_tt(bestValue, ss->ply),
12791307
PvNode && bestValue > oldAlpha ? BOUND_EXACT : BOUND_UPPER,
1280-
ttDepth, bestMove);
1308+
ttDepth, bestMove, ss->staticEval, ss->evalMargin);
12811309

12821310
assert(bestValue > -VALUE_INFINITE && bestValue < VALUE_INFINITE);
12831311

@@ -1574,7 +1602,7 @@ void RootMove::insert_pv_in_tt(Position& pos) {
15741602
tte = TT.probe(pos.key());
15751603

15761604
if (!tte || tte->move() != pv[ply]) // Don't overwrite correct entries
1577-
TT.store(pos.key(), VALUE_NONE, BOUND_NONE, DEPTH_NONE, pv[ply]);
1605+
TT.store(pos.key(), VALUE_NONE, BOUND_NONE, DEPTH_NONE, pv[ply], VALUE_NONE, VALUE_NONE);
15781606

15791607
assert(MoveList<LEGAL>(pos).contains(pv[ply]));
15801608

‎src/thread.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
#include <vector>
2424

25-
#include "evaluate.h"
2625
#include "material.h"
2726
#include "movepick.h"
2827
#include "pawns.h"
@@ -109,7 +108,6 @@ class Thread {
109108
void wait_for_stop_or_ponderhit();
110109

111110
SplitPoint splitPoints[MAX_SPLITPOINTS_PER_THREAD];
112-
Eval::Table evalTable;
113111
Material::Table materialTable;
114112
Endgames endgames;
115113
Pawns::Table pawnsTable;

‎src/tt.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ void TranspositionTable::clear() {
8282
/// more valuable than a TTEntry t2 if t1 is from the current search and t2 is from
8383
/// a previous search, or if the depth of t1 is bigger than the depth of t2.
8484

85-
void TranspositionTable::store(const Key posKey, Value v, Bound t, Depth d, Move m) {
85+
void TranspositionTable::store(const Key posKey, Value v, Bound t, Depth d, Move m, Value statV, Value kingD) {
8686

8787
int c1, c2, c3;
8888
TTEntry *tte, *replace;
@@ -98,7 +98,7 @@ void TranspositionTable::store(const Key posKey, Value v, Bound t, Depth d, Move
9898
if (m == MOVE_NONE)
9999
m = tte->move();
100100

101-
tte->save(posKey32, v, t, d, m, generation);
101+
tte->save(posKey32, v, t, d, m, generation, statV, kingD);
102102
return;
103103
}
104104

@@ -110,7 +110,7 @@ void TranspositionTable::store(const Key posKey, Value v, Bound t, Depth d, Move
110110
if (c1 + c2 + c3 > 0)
111111
replace = tte;
112112
}
113-
replace->save(posKey32, v, t, d, m, generation);
113+
replace->save(posKey32, v, t, d, m, generation, statV, kingD);
114114
}
115115

116116

‎src/tt.h‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,16 @@
4444
class TTEntry {
4545

4646
public:
47-
void save(uint32_t k, Value v, Bound b, Depth d, Move m, int g) {
47+
void save(uint32_t k, Value v, Bound b, Depth d, Move m, int g, Value statV, Value statM) {
4848

4949
key32 = (uint32_t)k;
5050
move16 = (uint16_t)m;
5151
bound = (uint8_t)b;
5252
generation8 = (uint8_t)g;
5353
value16 = (int16_t)v;
5454
depth16 = (int16_t)d;
55+
staticValue = (int16_t)statV;
56+
staticMargin = (int16_t)statM;
5557
}
5658
void set_generation(int g) { generation8 = (uint8_t)g; }
5759

@@ -61,12 +63,14 @@ class TTEntry {
6163
Value value() const { return (Value)value16; }
6264
Bound type() const { return (Bound)bound; }
6365
int generation() const { return (int)generation8; }
66+
Value static_value() const { return (Value)staticValue; }
67+
Value static_value_margin() const { return (Value)staticMargin; }
6468

6569
private:
6670
uint32_t key32;
6771
uint16_t move16;
6872
uint8_t bound, generation8;
69-
int16_t value16, depth16;
73+
int16_t value16, depth16, staticValue, staticMargin;
7074
};
7175

7276

@@ -96,7 +100,7 @@ class TranspositionTable {
96100
~TranspositionTable();
97101
void set_size(size_t mbSize);
98102
void clear();
99-
void store(const Key posKey, Value v, Bound type, Depth d, Move m);
103+
void store(const Key posKey, Value v, Bound type, Depth d, Move m, Value statV, Value kingD);
100104
TTEntry* probe(const Key posKey) const;
101105
void new_search();
102106
TTEntry* first_entry(const Key posKey) const;

0 commit comments

Comments
 (0)