Disallow same option being added twice

Now exits during startup. ``` ./stockfish Stockfish dev-20250202-243c7c6a by the Stockfish developers (see AUTHORS file) x1,5,0,10,0.5,0.0020 Option: "x1" was already added! ``` i.e. prevents and helps debug this case ```cpp int x1 = 5; TUNE(x1); TUNE(x1); ``` No functional change
official-pikafish · Feb 5, 2025 · 8654039 · 8654039
1 parent 15e406a
commit 8654039
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 76 deletions.
diff --git a/src/engine.cpp b/src/engine.cpp
@@ -51,40 +51,46 @@ Engine::Engine(std::optional<std::string> path) :
     network(numaContext, NN::Network({EvalFileDefaultName, "None", ""})) {
     pos.set(StartFEN, &states->back());
 
-    options["Debug Log File"] << Option("", [](const Option& o) {
-        start_logger(o);
-        return std::nullopt;
-    });
-
-    options["NumaPolicy"] << Option("auto", [this](const Option& o) {
-        set_numa_config_from_option(o);
-        return numa_config_information_as_string() + "\n"
-             + thread_allocation_information_as_string();
-    });
-
-    options["Threads"] << Option(1, 1, 1024, [this](const Option&) {
-        resize_threads();
-        return thread_allocation_information_as_string();
-    });
-
-    options["Hash"] << Option(16, 1, MaxHashMB, [this](const Option& o) {
-        set_tt_size(o);
-        return std::nullopt;
-    });
-
-    options["Clear Hash"] << Option([this](const Option&) {
-        search_clear();
-        return std::nullopt;
-    });
-    options["Ponder"] << Option(false);
-    options["MultiPV"] << Option(1, 1, MAX_MOVES);
-    options["Move Overhead"] << Option(10, 0, 5000);
-    options["nodestime"] << Option(0, 0, 10000);
-    options["UCI_ShowWDL"] << Option(false);
-    options["EvalFile"] << Option(EvalFileDefaultName, [this](const Option& o) {
-        load_network(o);
-        return std::nullopt;
-    });
+    options.add("Debug Log File", Option("", [](const Option& o) {
+                    start_logger(o);
+                    return std::nullopt;
+                }));
+
+    options.add("NumaPolicy", Option("auto", [this](const Option& o) {
+                    set_numa_config_from_option(o);
+                    return numa_config_information_as_string() + "\n"
+                         + thread_allocation_information_as_string();
+                }));
+
+    options.add("Threads", Option(1, 1, 1024, [this](const Option&) {
+                    resize_threads();
+                    return thread_allocation_information_as_string();
+                }));
+
+    options.add("Hash", Option(16, 1, MaxHashMB, [this](const Option& o) {
+                    set_tt_size(o);
+                    return std::nullopt;
+                }));
+
+    options.add("Clear Hash", Option([this](const Option&) {
+                    search_clear();
+                    return std::nullopt;
+                }));
+
+    options.add("Ponder", Option(false));
+
+    options.add("MultiPV", Option(1, 1, MAX_MOVES));
+
+    options.add("Move Overhead", Option(10, 0, 5000));
+
+    options.add("nodestime", Option(0, 0, 10000));
+
+    options.add("UCI_ShowWDL", Option(false));
+
+    options.add("EvalFile", Option(EvalFileDefaultName, [this](const Option& o) {
+                    load_network(o);
+                    return std::nullopt;
+                }));
 
     load_network(options["EvalFile"]);
     resize_threads();

diff --git a/src/search.cpp b/src/search.cpp
@@ -780,8 +780,8 @@ Value Search::Worker::search(
     // Step 9. Internal iterative reductions
     // For PV nodes without a ttMove as well as for deep enough cutNodes, we decrease depth.
     // (* Scaler) Especially if they make IIR more aggressive.
-    if (((PvNode || cutNode) && depth >= 7 - 4 * PvNode) && !ttData.move)
-        depth -= 2;
+    if (((PvNode || cutNode) && depth >= 7 - 3 * PvNode) && !ttData.move)
+        depth--;
 
     // Step 10. ProbCut
     // If we have a good enough capture and a reduced search
@@ -938,7 +938,7 @@ Value Search::Worker::search(
                 if (!givesCheck && lmrDepth < 18 && !ss->inCheck)
                 {
                     Value futilityValue = ss->staticEval + 332 + 371 * lmrDepth
-                                        + PieceValue[capturedPiece] + captHist / 5;
+                                        + PieceValue[capturedPiece] + 100 * captHist / 500;
                     if (futilityValue <= alpha)
                         continue;
                 }
@@ -959,7 +959,7 @@ Value Search::Worker::search(
                 if (history < -3190 * depth)
                     continue;
 
-                history += 2 * thisThread->mainHistory[us][move.from_to()];
+                history += 64 * thisThread->mainHistory[us][move.from_to()] / 32;
 
                 lmrDepth += history / 3718;
 
@@ -1011,10 +1011,11 @@ Value Search::Worker::search(
 
                 if (value < singularBeta)
                 {
-                    int corrValAdj   = std::abs(correctionValue) / 262144;
-                    int doubleMargin = 249 * PvNode - 194 * !ttCapture - corrValAdj;
+                    int corrValAdj1  = std::abs(correctionValue) / 265083;
+                    int corrValAdj2  = std::abs(correctionValue) / 253680;
+                    int doubleMargin = 267 * PvNode - 181 * !ttCapture - corrValAdj1;
                     int tripleMargin =
-                      94 + 287 * PvNode - 249 * !ttCapture + 99 * ss->ttPv - corrValAdj;
+                      96 + 282 * PvNode - 250 * !ttCapture + 103 * ss->ttPv - corrValAdj2;
 
                     extension = 1 + (value < singularBeta - doubleMargin)
                               + (value < singularBeta - tripleMargin);
@@ -1047,13 +1048,6 @@ Value Search::Worker::search(
                 else if (cutNode)
                     extension = -2;
             }
-
-            // Extension for capturing the previous moved piece
-            else if (PvNode && move.to_sq() == prevSq
-                     && thisThread->captureHistory[movedPiece][move.to_sq()]
-                                                  [type_of(pos.piece_on(move.to_sq()))]
-                          > 5255)
-                extension = 1;
         }
 
         // Step 15. Make the move
@@ -1103,7 +1097,7 @@ Value Search::Worker::search(
 
         if (capture)
             ss->statScore =
-              7 * int(PieceValue[pos.captured_piece()])
+              700 * int(PieceValue[pos.captured_piece()]) / 100
               + thisThread->captureHistory[movedPiece][move.to_sq()][type_of(pos.captured_piece())]
               - 5000;
         else
@@ -1312,10 +1306,10 @@ Value Search::Worker::search(
     // Bonus for prior countermove that caused the fail low
     else if (!priorCapture && prevSq != SQ_NONE)
     {
-        int bonusScale = (195 * (depth > 6) + 161 * ((ss - 1)->moveCount > 11)
-                          + 82 * (!ss->inCheck && bestValue <= ss->staticEval - 157)
-                          + 179 * (!(ss - 1)->inCheck && bestValue <= -(ss - 1)->staticEval - 99)
-                          + 85 * ((ss - 1)->isTTMove) + std::min(-(ss - 1)->statScore / 79, 234));
+        int bonusScale = (184 * (depth > 6) + 80 * !allNode + 152 * ((ss - 1)->moveCount > 11)
+                          + 77 * (!ss->inCheck && bestValue <= ss->staticEval - 157)
+                          + 169 * (!(ss - 1)->inCheck && bestValue <= -(ss - 1)->staticEval - 99)
+                          + 80 * ((ss - 1)->isTTMove) + std::min(-(ss - 1)->statScore / 79, 234));
 
         bonusScale = std::max(bonusScale, 0);
 
@@ -1750,7 +1744,7 @@ void update_all_stats(const Position&      pos,
 // at ply -1, -2, -3, -4, and -6 with current move.
 void update_continuation_histories(Stack* ss, Piece pc, Square to, int bonus) {
     static constexpr std::array<ConthistBonus, 6> conthist_bonuses = {
-      {{1, 1025}, {2, 621}, {3, 325}, {4, 512}, {5, 122}, {6, 534}}};
+      {{1, 1029}, {2, 656}, {3, 326}, {4, 536}, {5, 120}, {6, 537}}};
 
     for (const auto [i, weight] : conthist_bonuses)
     {

diff --git a/src/tune.cpp b/src/tune.cpp
@@ -55,7 +55,7 @@ void Tune::make_option(OptionsMap* opts, const string& n, int v, const SetRange&
     if (TuneResults.count(n))
         v = TuneResults[n];
 
-    (*opts)[n] << Option(v, r(v).first, r(v).second, on_tune);
+    opts->add(n, Option(v, r(v).first, r(v).second, on_tune));
     LastOption = &((*opts)[n]);
 
     // Print formatted parameters, ready to be copy-pasted in Fishtest

diff --git a/src/ucioption.cpp b/src/ucioption.cpp
@@ -21,6 +21,7 @@
 #include <algorithm>
 #include <cassert>
 #include <cctype>
+#include <cstdlib>
 #include <iostream>
 #include <sstream>
 #include <utility>
@@ -57,17 +58,31 @@ void OptionsMap::setoption(std::istringstream& is) {
         sync_cout << "No such option: " << name << sync_endl;
 }
 
-Option OptionsMap::operator[](const std::string& name) const {
+const Option& OptionsMap::operator[](const std::string& name) const {
     auto it = options_map.find(name);
-    return it != options_map.end() ? it->second : Option(this);
+    assert(it != options_map.end());
+    return it->second;
 }
 
-Option& OptionsMap::operator[](const std::string& name) {
+// Inits options and assigns idx in the correct printing order
+void OptionsMap::add(const std::string& name, const Option& option) {
     if (!options_map.count(name))
-        options_map[name] = Option(this);
-    return options_map[name];
+    {
+        static size_t insert_order = 0;
+
+        options_map[name] = option;
+
+        options_map[name].parent = this;
+        options_map[name].idx    = insert_order++;
+    }
+    else
+    {
+        std::cerr << "Option \"" << name << "\" was already added!" << std::endl;
+        std::exit(EXIT_FAILURE);
+    }
 }
 
+
 std::size_t OptionsMap::count(const std::string& name) const { return options_map.count(name); }
 
 Option::Option(const OptionsMap* map) :
@@ -130,18 +145,6 @@ bool Option::operator==(const char* s) const {
 bool Option::operator!=(const char* s) const { return !(*this == s); }
 
 
-// Inits options and assigns idx in the correct printing order
-void Option::operator<<(const Option& o) {
-
-    static size_t insert_order = 0;
-
-    auto p = this->parent;
-    *this  = o;
-
-    this->parent = p;
-    idx          = insert_order++;
-}
-
 // Updates currentValue and triggers on_change() action. It's up to
 // the GUI to check for option's limits, but we could receive the new value
 // from the user by console window, so let's check the bounds anyway.
@@ -160,7 +163,7 @@ Option& Option::operator=(const std::string& v) {
         std::string        token;
         std::istringstream ss(defaultValue);
         while (ss >> token)
-            comboMap[token] << Option();
+            comboMap.add(token, Option());
         if (!comboMap.count(v) || v == "var")
             return *this;
     }

diff --git a/src/ucioption.h b/src/ucioption.h
@@ -54,12 +54,13 @@ class Option {
 
     friend std::ostream& operator<<(std::ostream&, const OptionsMap&);
 
+    int operator<<(const Option&) = delete;
+
    private:
     friend class OptionsMap;
     friend class Engine;
     friend class Tune;
 
-    void operator<<(const Option&);
 
     std::string       defaultValue, currentValue, type;
     int               min, max;
@@ -82,8 +83,9 @@ class OptionsMap {
 
     void setoption(std::istringstream&);
 
-    Option  operator[](const std::string&) const;
-    Option& operator[](const std::string&);
+    const Option& operator[](const std::string&) const;
+
+    void add(const std::string&, const Option& option);
 
     std::size_t count(const std::string&) const;