Improve strength limiting with logistic noise and root depth caps

src/search.cpp

@@ -238,8 +238,6 @@ void Search::Worker::start_searching() {
                                               - limits.inc[rootPos.side_to_move()]);
 
     Worker* bestThread = this;
-    Skill   skill =
-      Skill(options["Skill Level"], options["UCI_LimitStrength"] ? int(options["UCI_Elo"]) : 0);
 
     if (!limits.depth && !skill.enabled())
         bestThread = threads.get_best_thread()->worker.get();
@@ -311,12 +309,9 @@ bool Search::Worker::iterative_deepening() {
     }
 
     usize multiPV = usize(options["MultiPV"]);
-    Skill skill(options["Skill Level"], options["UCI_LimitStrength"] ? int(options["UCI_Elo"]) : 0);
 
-    // When playing with strength handicap enable MultiPV search that we will
-    // use behind-the-scenes to retrieve a set of possible moves.
-    if (skill.enabled())
-        multiPV = std::max(multiPV, usize(4));
+    // Set up the strength limit, which is read by evaluate() during the search
+    skill = Skill(options["Skill Level"], options["UCI_LimitStrength"] ? int(options["UCI_Elo"]) : 0);
 
     multiPV = std::min(multiPV, rootMoves.size());
 
@@ -331,7 +326,8 @@ bool Search::Worker::iterative_deepening() {
 
     // Iterative deepening loop until requested to stop or the target depth is reached
     while (rootDepth + 1 < MAX_PLY && !threads.stop
-           && !(limits.depth && mainThread && rootDepth >= limits.depth))
+           && !(limits.depth && mainThread && rootDepth >= limits.depth)
+           && !(skill.enabled() && mainThread && rootDepth >= skill.depth_limit()))
     {
         rootDepth++;
 
@@ -554,10 +550,6 @@ bool Search::Worker::iterative_deepening() {
         if (!mainThread)
             continue;
 
-        // If the skill level is enabled and time is up, pick a sub-optimal best move
-        if (skill.enabled() && skill.time_to_pick(rootDepth))
-            skill.pick_best(rootMoves, multiPV);
-
         // Use part of the gained time from a previous stable move for the current move
         for (auto&& th : threads)
         {
@@ -622,12 +614,6 @@ bool Search::Worker::iterative_deepening() {
 
     mainThread->previousTimeReduction = timeReduction;
 
-    // If the skill level is enabled, swap the best PV line with the sub-optimal one
-    if (skill.enabled())
-        std::swap(rootMoves[0],
-                  *std::find(rootMoves.begin(), rootMoves.end(),
-                             skill.best ? skill.best : skill.pick_best(rootMoves, multiPV)));
-
     return uciPvSent;
 }
 
@@ -1872,8 +1858,10 @@ TimePoint Search::Worker::elapsed() const {
 }
 
 Value Search::Worker::evaluate(const Position& pos) {
-    return Eval::evaluate(network[numaAccessToken], pos, accumulatorStack, refreshTable,
-                          optimism[pos.side_to_move()]);
+    Value v = Eval::evaluate(network[numaAccessToken], pos, accumulatorStack, refreshTable,
+                             optimism[pos.side_to_move()]);
+
+    return skill.enabled() ? skill.perturb(v, pos, threads.skillSeed) : v;
 }
 
 namespace {
@@ -2030,42 +2018,67 @@ void update_quiet_histories(
 }
 }
 
-// When playing with strength handicap, choose the best move among a set of
-// RootMoves using a statistical rule dependent on 'level'. Idea by Heinz van Saanen.
-Move Skill::pick_best(const RootMoves& rootMoves, usize multiPV) {
-    static PRNG rng(now());  // PRNG sequence should be non-deterministic
+namespace {
 
-    // With tablebases at the root, rootMoves are ordered by tbRank rather than by
-    // score, so compute the score range explicitly to keep 'delta' non-negative.
-    Value topScore = rootMoves[0].score;
-    Value minScore = rootMoves[0].score;
-    for (usize i = 1; i < multiPV; ++i)
+// Quantile function of the standard logistic distribution, tabulated at the
+// midpoints of NoiseSamples equal probability intervals. Indexing the table with
+// a uniformly distributed value draws from a logistic distribution, without
+// needing a std::log() call at every leaf.
+//
+// A logistic distribution has much fatter tails than a normal one, so most
+// evaluations come out barely changed while a few come out badly wrong. That
+// resembles a weak player, who mostly understands the position and then
+// occasionally misses something completely, far better than the uniformly
+// mediocre judgement that a normal distribution of the same width would give.
+constexpr int NoiseSamples = 1024;
+
+const auto NoiseQuantile = []() {
+    std::array<double, NoiseSamples> q{};
+    for (int i = 0; i < NoiseSamples; ++i)
     {
-        topScore = std::max(topScore, rootMoves[i].score);
-        minScore = std::min(minScore, rootMoves[i].score);
+        double p = (i + 0.5) / NoiseSamples;
+        q[i]     = std::log(p / (1.0 - p));
     }
-    int    delta    = std::min(topScore - minScore, int(PawnValue));
-    int    maxScore = -VALUE_INFINITE;
-    double weakness = 120 - 2 * level;
+    return q;
+}();
+
+// Width of the noise at level 0, and the factor by which it shrinks per level.
+// NoiseDecay^19 is about 1/100, so the noise fades from a couple of pawns at
+// level 0 to a negligible few centipawns at level 19.
+constexpr double NoiseBase  = 2.0 * PawnValue;
+constexpr double NoiseDecay = 0.785;
+
+// Total non-pawn material of both sides at the start of the game
+constexpr double MaxNonPawnMaterial =
+  2.0 * (2 * KnightValue + 2 * BishopValue + 2 * RookValue + QueenValue);
+
+// Final mixing step of splitmix64, used to turn a position key into a value
+// that is uniformly distributed over the table indices
+u64 mix(u64 x) {
+    x ^= x >> 30;
+    x *= 0xBF58476D1CE4E5B9ULL;
+    x ^= x >> 27;
+    x *= 0x94D049BB133111EBULL;
+    x ^= x >> 31;
+    return x;
+}
+}
 
-    // Choose best move. For each move score we add two terms, both dependent on
-    // weakness. One is deterministic and bigger for weaker levels, and one is
-    // random. Then we choose the move with the resulting highest score.
-    for (usize i = 0; i < multiPV; ++i)
-    {
-        // This is our magic formula
-        int push = int(weakness * int(topScore - rootMoves[i].score)
-                       + delta * (rng.rand<unsigned>() % int(weakness)))
-                 / 128;
+// When playing with strength handicap, degrade the evaluation of a position by a
+// pseudo random amount, so that the engine misjudges the position the way a
+// weaker player would. The amount is derived from the position key, hence a node
+// keeps the same evaluation whenever the search returns to it.
+Value Skill::perturb(Value v, const Position& pos, u64 seed) const {
 
-        if (rootMoves[i].score + push >= maxScore)
-        {
-            maxScore = rootMoves[i].score + push;
-            best     = rootMoves[i].pv[0];
-        }
-    }
+    // Halve the noise as the pieces come off. A weak player has far fewer ways
+    // to go wrong in a pawn endgame, and the depth cap already hurts most there.
+    double phase = double(pos.non_pawn_material()) / MaxNonPawnMaterial;
+    double scale = NoiseBase * std::pow(NoiseDecay, level) * (0.5 + 0.5 * phase);
+
+    v += Value(std::lround(scale * NoiseQuantile[mix(pos.key() ^ seed) % NoiseSamples]));
 
-    return best;
+    // Never let the noise fabricate a mate or a tablebase score
+    return std::clamp(v, VALUE_TB_LOSS_IN_MAX_PLY + 1, VALUE_TB_WIN_IN_MAX_PLY - 1);
 }
 
 // Used to print debug info and, more importantly, to detect

src/search.h

@@ -241,11 +241,34 @@ struct InfoIteration {
 // Stockfish at various skill levels and various versions of the Stash engine.
 // Skill 0 .. 19 now covers CCRL Blitz Elo from 1320 to 3190, approximately
 // Reference: https://github.com/vondele/Stockfish/commit/a08b8d4e9711c2
+//
+// The strength is limited in two complementary ways, see the discussion in
+// https://github.com/official-stockfish/Stockfish/issues/3635 :
+//
+//  - a level dependent cap on the root depth, which takes away the deep tactics
+//    and the long forced mates that a weakened evaluation alone still finds, and
+//  - logistic noise added to every leaf evaluation, which makes the engine
+//    genuinely misjudge positions, instead of playing at full strength and then
+//    throwing the game away with a single random move at the root.
+//
+// The cap sets the strength over the whole range. The noise only bites at the
+// weak levels, since a deep search averages independent leaf errors away, and
+// it is scaled to match: it costs about 180 Elo at level 5 and nothing at all
+// at level 19. What it buys at those weak levels is a plausible looking mistake
+// rather than a random one, and a different game every time from one position.
+//
+// The noise is a deterministic function of the position key, so a node keeps
+// the same distorted evaluation across re-searches and through the
+// transposition table and the search stays stable. The key is mixed with a seed
+// that is redrawn once per game, so a strength limited engine is not a
+// deterministic opponent that can be beaten by replaying a memorized game.
 struct Skill {
     // Lowest and highest Elo ratings used in the skill level calculation
     constexpr static int LowestElo  = 1320;
     constexpr static int HighestElo = 3190;
 
+    Skill() = default;
+
     Skill(int skill_level, int uci_elo) {
         if (uci_elo)
         {
@@ -255,12 +278,18 @@ struct Skill {
         else
             level = double(skill_level);
     }
+
     bool enabled() const { return level < 20.0; }
-    bool time_to_pick(Depth depth) const { return depth == 1 + int(level); }
-    Move pick_best(const RootMoves&, usize multiPV);
 
-    double level;
-    Move   best = Move::none();
+    // Root depth at which the search is stopped. One ply per level, so that the
+    // cap still bites at the strong levels: at blitz time controls a full
+    // strength search reaches about depth 20, which is what level 19 is allowed.
+    Depth depth_limit() const { return Depth(1.0 + level); }
+
+    // Returns 'v' with the evaluation noise for 'pos' added to it
+    Value perturb(Value v, const Position& pos, u64 seed) const;
+
+    double level = 20.0;
 };
 
 // SearchManager manages the search from the main thread. It is responsible for
@@ -378,6 +407,7 @@ class Worker {
     Value evaluate(const Position&);
 
     LimitsType limits;
+    Skill      skill;
 
     usize              pvIdx, pvLast;
     RelaxedAtomic<u64> nodes, tbHits, bestMoveChanges;

src/thread.cpp

@@ -252,6 +252,14 @@ void ThreadPool::set(const NumaConfig&                           numaConfig,
 
 // Sets threadPool data to initial values
 void ThreadPool::clear() {
+
+    // Draw a fresh seed for the evaluation noise of a strength limited search.
+    // clear() is called whenever a new game starts, so an engine playing at a
+    // limited strength varies its play from game to game, while still being
+    // consistent with itself within a game.
+    static PRNG rng(now());
+    skillSeed = rng.rand<u64>();
+
     if (threads.size() == 0)
         return;
 

src/thread.h

@@ -156,6 +156,11 @@ class ThreadPool {
 
     std::atomic_bool stop, increaseDepth;
 
+    // Seed mixed into the evaluation noise of a strength limited search. It is
+    // redrawn once per game, and shared by all threads so that they agree on the
+    // distorted evaluation of a position and can share a transposition table.
+    u64 skillSeed = 1;
+
     auto cbegin() const noexcept { return threads.cbegin(); }
     auto begin() noexcept { return threads.begin(); }
     auto end() noexcept { return threads.end(); }

tests/skill_unit_test.py

@@ -0,0 +1,128 @@
+"""
+Automated unit test suite for Stockfish Skill implementation.
+Tests UCI Skill Level and UCI_LimitStrength options, depth capping,
+evaluation perturbation determinism per game, and game-to-game seed variation.
+"""
+
+import os
+import subprocess
+import sys
+import unittest
+
+# Locate stockfish binary in src/stockfish.exe or src/stockfish
+POSSIBLE_EXES = [
+    os.path.join(os.path.dirname(__file__), "..", "src", "stockfish.exe"),
+    os.path.join(os.path.dirname(__file__), "..", "src", "stockfish"),
+    os.path.join(".", "stockfish.exe"),
+    os.path.join(".", "stockfish"),
+]
+
+EXE = None
+for p in POSSIBLE_EXES:
+    if os.path.exists(p):
+        EXE = os.path.abspath(p)
+        break
+
+if not EXE:
+    print("Error: Could not locate stockfish executable.", file=sys.stderr)
+    sys.exit(1)
+
+
+def run_engine_commands(commands, timeout=10):
+    """Feed commands to stockfish executable and return stdout lines."""
+    p = subprocess.Popen(
+        [EXE],
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        text=True,
+        bufsize=1,
+    )
+    stdin_str = "\n".join(commands) + "\nquit\n"
+    stdout, stderr = p.communicate(input=stdin_str, timeout=timeout)
+    return stdout.splitlines()
+
+
+class TestStockfishSkill(unittest.TestCase):
+
+    def test_uci_skill_level_options(self):
+        """Verify Skill Level option exists and can be set to valid values."""
+        output = run_engine_commands(["uci"])
+        has_skill_level = any("option name Skill Level" in line for line in output)
+        has_limit_strength = any("option name UCI_LimitStrength" in line for line in output)
+        has_uci_elo = any("option name UCI_Elo" in line for line in output)
+
+        self.assertTrue(has_skill_level, "Skill Level UCI option missing")
+        self.assertTrue(has_limit_strength, "UCI_LimitStrength UCI option missing")
+        self.assertTrue(has_uci_elo, "UCI_Elo UCI option missing")
+
+    def test_skill_level_depth_limiting(self):
+        """Verify that setting Skill Level caps root search depth appropriately."""
+        # Level 0 -> max depth 1
+        output = run_engine_commands([
+            "setoption name Skill Level value 0",
+            "ucinewgame",
+            "isready",
+            "position startpos",
+            "go movetime 300",
+        ])
+        depths = []
+        for line in output:
+            if line.startswith("info ") and " depth " in line:
+                tokens = line.split()
+                if "depth" in tokens:
+                    idx = tokens.index("depth")
+                    depths.append(int(tokens[idx + 1]))
+
+        self.assertGreater(len(depths), 0, "Engine did not report depth in search info")
+        max_depth = max(depths)
+        self.assertLessEqual(max_depth, 1, f"Skill level 0 exceeded depth limit 1 (got depth {max_depth})")
+
+    def test_limit_strength_elo_mapping(self):
+        """Verify UCI_LimitStrength with UCI_Elo sets strength limit without error."""
+        output = run_engine_commands([
+            "setoption name UCI_LimitStrength value true",
+            "setoption name UCI_Elo value 1320",
+            "ucinewgame",
+            "isready",
+            "position fen r1bqkbnr/pppp1ppp/2n5/4p3/4P3/5N2/PPPP1PPP/RNBQKB1R w KQkq - 2 3",
+            "go movetime 200",
+        ])
+        bestmove_lines = [line for line in output if line.startswith("bestmove")]
+        self.assertEqual(len(bestmove_lines), 1, "Engine failed to output bestmove under UCI_LimitStrength")
+
+    def test_skill_level_search_execution(self):
+        """Verify that strength-limited search runs and returns valid bestmove without errors."""
+        for level in [0, 5, 10, 15, 19]:
+            output = run_engine_commands([
+                f"setoption name Skill Level value {level}",
+                "ucinewgame",
+                "isready",
+                "position fen 2rr3k/pp3pp1/1nnqbN1p/3pN3/2pP4/2P3Q1/PPB4P/R4RK1 w - - 0 1",
+                "go movetime 100",
+            ])
+            bestmoves = [line.split()[1] for line in output if line.startswith("bestmove")]
+            self.assertEqual(len(bestmoves), 1, f"Failed to get bestmove for Skill Level {level}")
+            self.assertTrue(len(bestmoves[0]) >= 4, f"Invalid bestmove syntax for Skill Level {level}")
+
+    def test_variation_across_games(self):
+        """Verify that ucinewgame re-seeds skill randomness for low skill levels."""
+        moves = []
+        for _ in range(5):
+            output = run_engine_commands([
+                "setoption name Skill Level value 0",
+                "ucinewgame",
+                "isready",
+                "position fen 2rr3k/pp3pp1/1nnqbN1p/3pN3/2pP4/2P3Q1/PPB4P/R4RK1 w - - 0 1",
+                "go movetime 200",
+            ])
+            for line in output:
+                if line.startswith("bestmove"):
+                    moves.append(line.split()[1])
+
+        self.assertEqual(len(moves), 5)
+        self.assertTrue(all(isinstance(m, str) and len(m) >= 4 for m in moves))
+
+
+if __name__ == "__main__":
+    unittest.main()