Stockfish 1.7 - nullmove blunder

[mcostalba]

Does this mean that a bugfix will be coming shortly? Just wondering if I should hold off testing.

It's possible that there will be a bugfix release soon, but the difference in strength between 1.7 and the hypothetical 1.7.1 will be too small to measure. We'll almost certainly recommend that they're rated as a single engine on the various rating lists.

Yes. And also could be wise do not rush a fix release (if nothing very serious appears) but wait a bit to collect all the bug reports and post a fix release only once.

[mcostalba]

Black is here easy winning, but Stockfish 1.7 is blind!

Hi,

this is a candidate patch that should fix the problem. We are regression testing the patch to see if we have a performance issue.

It would be very welcomed if you (or someone else), could apply the patch to 1.7 and verify that the zugzwang positions are now correctly detected and scored.

Thanks in advance for everybody that will test the patch.

Code: Select all

Date: Sat, 10 Apr 2010 11:56:51 +0100
Subject: [PATCH] Candidate 'null blunder' fix

---
 src/search.cpp |   20 ++++++++++++--------
 src/tt.h       |    8 ++++----
 src/value.h    |   11 +++++++----
 3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/src/search.cpp b/src/search.cpp
index fd5687e..3f84364 100644
--- a/src/search.cpp
+++ b/src/search.cpp
@@ -294,7 +294,7 @@ namespace {
   Depth extension(const Position&, Move, bool, bool, bool, bool, bool, bool*);
   bool ok_to_do_nullmove(const Position& pos);
   bool ok_to_prune(const Position& pos, Move m, Move threat);
-  bool ok_to_use_TT(const TTEntry* tte, Depth depth, Value beta, int ply);
+  bool ok_to_use_TT(const TTEntry* tte, Depth depth, Value beta, int ply, bool allowNullmove);
   Value refine_eval(const TTEntry* tte, Value defaultEval, int ply);
   void update_history(const Position& pos, Move move, Depth depth, Move movesSearched[], int moveCount);
   void update_killers(Move m, SearchStack& ss);
@@ -1299,7 +1299,7 @@ namespace {
     tte = TT.retrieve(posKey);
     ttMove = (tte ? tte->move() : MOVE_NONE);
 
-    if (tte && ok_to_use_TT(tte, depth, beta, ply))
+    if (tte && ok_to_use_TT(tte, depth, beta, ply, allowNullmove))
     {
         ss[ply].currentMove = ttMove; // Can be MOVE_NONE
         return value_from_tt(tte->value(), ply);
@@ -1388,7 +1388,7 @@ namespace {
             {
                 assert(value_to_tt(nullValue, ply) == nullValue);
 
-                TT.store(posKey, nullValue, VALUE_TYPE_LOWER, depth, MOVE_NONE);
+                TT.store(posKey, nullValue, VALUE_TYPE_NS_LO, depth, MOVE_NONE);
                 return nullValue;
             }
         } else {
@@ -1625,7 +1625,7 @@ namespace {
     tte = TT.retrieve(pos.get_key());
     ttMove = (tte ? tte->move() : MOVE_NONE);
 
-    if (!pvNode && tte && ok_to_use_TT(tte, depth, beta, ply))
+    if (!pvNode && tte && ok_to_use_TT(tte, depth, beta, ply, true))
     {
         assert(tte->type() != VALUE_TYPE_EVAL);
 
@@ -2306,14 +2306,18 @@ namespace {
   }
 
 
-  // ok_to_use_TT() returns true if a transposition table score
-  // can be used at a given point in search.
+  // ok_to_use_TT() returns true if a transposition table score can be used at a
+  // given point in search. To avoid zugzwang issues TT cutoffs at the root node
+  // of a null move verification search are not allowed if the TT value was found
+  // by a null search, this is implemented testing allowNullmove and TT entry type.
 
-  bool ok_to_use_TT(const TTEntry* tte, Depth depth, Value beta, int ply) {
+  bool ok_to_use_TT(const TTEntry* tte, Depth depth, Value beta, int ply, bool allowNullmove) {
 
     Value v = value_from_tt(tte->value(), ply);
 
-    return   (   tte->depth() >= depth
+    return   (allowNullmove || !(tte->type() & VALUE_TYPE_NULL))
+
+          && (   tte->depth() >= depth
               || v >= Max(value_mate_in(PLY_MAX), beta)
               || v < Min(value_mated_in(PLY_MAX), beta))
 
diff --git a/src/tt.h b/src/tt.h
index 272e752..021019b 100644
--- a/src/tt.h
+++ b/src/tt.h
@@ -46,8 +46,8 @@
 /// the 32 bits of the data field are so defined
 ///
 /// bit  0-16: move
-/// bit 17-19: not used
-/// bit 20-22: value type
+/// bit 17-18: not used
+/// bit 19-22: value type
 /// bit 23-31: generation
 
 class TTEntry {
@@ -55,14 +55,14 @@ class TTEntry {
 public:
   TTEntry() {}
   TTEntry(uint32_t k, Value v, ValueType t, Depth d, Move m, int generation)
-        : key_ (k), data((m & 0x1FFFF) | (t << 20) | (generation << 23)),
+        : key_ (k), data((m & 0x1FFFF) | (t << 19) | (generation << 23)),
           value_(int16_t(v)), depth_(int16_t(d)) {}
 
   uint32_t key() const { return key_; }
   Depth depth() const { return Depth(depth_); }
   Move move() const { return Move(data & 0x1FFFF); }
   Value value() const { return Value(value_); }
-  ValueType type() const { return ValueType((data >> 20) & 7); }
+  ValueType type() const { return ValueType((data >> 19) & 0xF); }
   int generation() const { return (data >> 23); }
 
 private:
diff --git a/src/value.h b/src/value.h
index 62d9458..4b62027 100644
--- a/src/value.h
+++ b/src/value.h
@@ -33,13 +33,16 @@
 ////
 
 enum ValueType {
-  VALUE_TYPE_NONE = 0,
+  VALUE_TYPE_NONE =  0,
   VALUE_TYPE_UPPER = 1,  // Upper bound
   VALUE_TYPE_LOWER = 2,  // Lower bound
   VALUE_TYPE_EXACT = 3,  // Exact score
-  VALUE_TYPE_EVAL  = 4,  // Evaluation cache
-  VALUE_TYPE_EV_UP = 5,  // Evaluation cache for upper bound
-  VALUE_TYPE_EV_LO = 6   // Evaluation cache for lower bound
+  VALUE_TYPE_EVAL  = 4,  // Static evaluation value
+  VALUE_TYPE_NULL  = 8,  // Null search value
+
+  VALUE_TYPE_EV_UP = VALUE_TYPE_EVAL | VALUE_TYPE_UPPER,
+  VALUE_TYPE_EV_LO = VALUE_TYPE_EVAL | VALUE_TYPE_LOWER,
+  VALUE_TYPE_NS_LO = VALUE_TYPE_NULL | VALUE_TYPE_LOWER,
 };
 
 
-- 
1.6.5.1.1367.gcd48