From: Fabian Fichter <ianfab@users.noreply.github.com>
Date: Wed, 5 Feb 2020 21:11:44 +0000 (+0100)
Subject: Implement variant configuration checker
X-Git-Url: http://winboard.nl/cgi-bin?a=commitdiff_plain;h=21ccc8701947ba677af0dc056a9befe50cd9ffd9;p=fairystockfish.git

Implement variant configuration checker

Usage: `./stockfish check variants.ini`

Closes #62.
---

diff --git a/src/parser.cpp b/src/parser.cpp
index 0d0ffb2..a64a1e7 100644
--- a/src/parser.cpp
+++ b/src/parser.cpp
@@ -25,20 +25,22 @@
 
 namespace {
 
-    template <typename T> void set(const std::string& value, T& target)
+    template <typename T> bool set(const std::string& value, T& target)
     {
         std::stringstream ss(value);
         ss >> target;
+        return !ss.fail();
     }
 
-    template <> void set(const std::string& value, Rank& target) {
+    template <> bool set(const std::string& value, Rank& target) {
         std::stringstream ss(value);
         int i;
         ss >> i;
         target = Rank(i - 1);
+        return !ss.fail() && target >= RANK_1 && target <= RANK_MAX;
     }
 
-    template <> void set(const std::string& value, File& target) {
+    template <> bool set(const std::string& value, File& target) {
         std::stringstream ss(value);
         if (isdigit(ss.peek()))
         {
@@ -52,46 +54,68 @@ namespace {
             ss >> c;
             target = File(c - 'a');
         }
+        return !ss.fail() && target >= FILE_A && target <= FILE_MAX;
     }
 
-    template <> void set(const std::string& value, std::string& target) {
+    template <> bool set(const std::string& value, std::string& target) {
         target = value;
+        return true;
     }
 
-    template <> void set(const std::string& value, bool& target) {
+    template <> bool set(const std::string& value, bool& target) {
         target = value == "true";
+        return value == "true" || value == "false";
     }
 
-    template <> void set(const std::string& value, Value& target) {
+    template <> bool set(const std::string& value, Value& target) {
         target =  value == "win"  ? VALUE_MATE
                 : value == "loss" ? -VALUE_MATE
                 : VALUE_DRAW;
+        return value == "win" || value == "loss" || value == "draw";
     }
 
-    template <> void set(const std::string& value, CountingRule& target) {
+    template <> bool set(const std::string& value, CountingRule& target) {
         target =  value == "makruk"  ? MAKRUK_COUNTING
                 : value == "asean" ? ASEAN_COUNTING
                 : NO_COUNTING;
+        return value == "makruk" || value == "asean" || value == "";
     }
 
-    template <> void set(const std::string& value, Bitboard& target) {
+    template <> bool set(const std::string& value, Bitboard& target) {
         char file;
         int rank;
         std::stringstream ss(value);
         target = 0;
-        while (ss >> file && ss >> rank)
+        while (!ss.eof() && ss >> file && ss >> rank)
             target |= file == '*' ? rank_bb(Rank(rank - 1)) : square_bb(make_square(File(tolower(file) - 'a'), Rank(rank - 1)));
+        return !ss.fail();
     }
 
 } // namespace
 
-template <class T> void VariantParser::parse_attribute(const std::string& key, T& target) {
+template <bool DoCheck>
+template <class T> void VariantParser<DoCheck>::parse_attribute(const std::string& key, T& target) {
     const auto& it = config.find(key);
     if (it != config.end())
-        set(it->second, target);
+    {
+        bool valid = set(it->second, target);
+        if (DoCheck && !valid)
+        {
+            std::string typeName =  std::is_same<T, int>() ? "int"
+                                  : std::is_same<T, Rank>() ? "Rank"
+                                  : std::is_same<T, File>() ? "File"
+                                  : std::is_same<T, bool>() ? "bool"
+                                  : std::is_same<T, Value>() ? "Value"
+                                  : std::is_same<T, CountingRule>() ? "CountingRule"
+                                  : std::is_same<T, Bitboard>() ? "Bitboard"
+                                  : typeid(T).name();
+            std::cerr << key << " - Invalid value " << it->second << " for type " << typeName << std::endl;
+        }
+    }
 }
 
-void VariantParser::parse_attribute(const std::string& key, PieceType& target, std::string pieceToChar) {
+template <bool DoCheck>
+void VariantParser<DoCheck>::parse_attribute(const std::string& key, PieceType& target, std::string pieceToChar) {
     const auto& it = config.find(key);
     if (it != config.end())
     {
@@ -100,17 +124,21 @@ void VariantParser::parse_attribute(const std::string& key, PieceType& target, s
         std::stringstream ss(it->second);
         if (ss >> token && (idx = pieceToChar.find(toupper(token))) != std::string::npos)
             target = PieceType(idx);
+        else if (DoCheck)
+            std::cerr << key << " - Invalid piece type: " << token << std::endl;
     }
 }
 
-Variant* VariantParser::parse() {
+template <bool DoCheck>
+Variant* VariantParser<DoCheck>::parse() {
     Variant* v = new Variant();
     v->reset_pieces();
     v->promotionPieceTypes = {};
     return parse(v);
 }
 
-Variant* VariantParser::parse(Variant* v) {
+template <bool DoCheck>
+Variant* VariantParser<DoCheck>::parse(Variant* v) {
     // piece types
     for (const auto& pieceInfo : pieceMap)
     {
@@ -120,7 +148,11 @@ Variant* VariantParser::parse(Variant* v) {
             if (isalpha(keyValue->second.at(0)))
                 v->add_piece(pieceInfo.first, keyValue->second.at(0));
             else
+            {
+                if (DoCheck && keyValue->second.at(0) != '-')
+                    std::cerr << pieceInfo.second->name << " - Invalid letter: " << keyValue->second.at(0) << std::endl;
                 v->remove_piece(pieceInfo.first);
+            }
         }
     }
     parse_attribute("variantTemplate", v->variantTemplate);
@@ -142,6 +174,8 @@ Variant* VariantParser::parse(Variant* v) {
         std::stringstream ss(it_prom->second);
         while (ss >> token && ((idx = v->pieceToChar.find(toupper(token))) != std::string::npos))
             v->promotionPieceTypes.insert(PieceType(idx));
+        if (DoCheck && idx == std::string::npos && token != '-')
+            std::cerr << "promotionPieceTypes - Invalid piece type: " << token << std::endl;
     }
     parse_attribute("sittuyinPromotion", v->sittuyinPromotion);
     // promotion limit
@@ -149,20 +183,27 @@ Variant* VariantParser::parse(Variant* v) {
     if (it_prom_limit != config.end())
     {
         char token;
-        size_t idx;
+        size_t idx = 0;
         std::stringstream ss(it_prom_limit->second);
-        while (ss >> token && (idx = v->pieceToChar.find(toupper(token))) != std::string::npos && ss >> token && ss >> v->promotionLimit[idx]) {}
+        while (!ss.eof() && ss >> token && (idx = v->pieceToChar.find(toupper(token))) != std::string::npos
+                         && ss >> token && ss >> v->promotionLimit[idx]) {}
+        if (DoCheck && idx == std::string::npos)
+            std::cerr << "promotionLimit - Invalid piece type: " << token << std::endl;
+        else if (DoCheck && !ss.eof())
+            std::cerr << "promotionLimit - Invalid piece count for type: " << v->pieceToChar[idx] << std::endl;
     }
     // promoted piece types
     const auto& it_prom_pt = config.find("promotedPieceType");
     if (it_prom_pt != config.end())
     {
         char token;
-        size_t idx, idx2;
+        size_t idx = 0, idx2 = 0;
         std::stringstream ss(it_prom_pt->second);
         while (   ss >> token && (idx = v->pieceToChar.find(toupper(token))) != std::string::npos && ss >> token
                && ss >> token && (idx2 = (token == '-' ? 0 : v->pieceToChar.find(toupper(token)))) != std::string::npos)
             v->promotedPieceType[idx] = PieceType(idx2);
+        if (DoCheck && (idx == std::string::npos || idx2 == std::string::npos))
+            std::cerr << "promotedPieceType - Invalid piece type: " << token << std::endl;
     }
     parse_attribute("piecePromotionOnCapture", v->piecePromotionOnCapture);
     parse_attribute("mandatoryPawnPromotion", v->mandatoryPawnPromotion);
@@ -180,6 +221,7 @@ Variant* VariantParser::parse(Variant* v) {
     parse_attribute("castlingRookPiece", v->castlingRookPiece, v->pieceToChar);
     parse_attribute("kingType", v->kingType, v->pieceToChar);
     parse_attribute("checking", v->checking);
+    parse_attribute("dropChecks", v->dropChecks);
     parse_attribute("mustCapture", v->mustCapture);
     parse_attribute("mustDrop", v->mustDrop);
     parse_attribute("pieceDrops", v->pieceDrops);
@@ -221,8 +263,10 @@ Variant* VariantParser::parse(Variant* v) {
         char token;
         size_t idx;
         std::stringstream ss(it_ext->second);
-        while (ss >> token && ((idx = v->pieceToChar.find(toupper(token))) != std::string::npos || token == '*'))
-            v->extinctionPieceTypes.insert(PieceType(token == '*' ? 0 : idx));
+        while (ss >> token && (idx = token == '*' ? size_t(ALL_PIECES) : v->pieceToChar.find(toupper(token))) != std::string::npos)
+            v->extinctionPieceTypes.insert(PieceType(idx));
+        if (DoCheck && idx == std::string::npos)
+            std::cerr << "extinctionPieceTypes - Invalid piece type: " << token << std::endl;
     }
     parse_attribute("flagPiece", v->flagPiece, v->pieceToChar);
     parse_attribute("whiteFlag", v->whiteFlag);
@@ -231,5 +275,58 @@ Variant* VariantParser::parse(Variant* v) {
     parse_attribute("checkCounting", v->checkCounting);
     parse_attribute("connectN", v->connectN);
     parse_attribute("countingRule", v->countingRule);
+    // Report invalid options
+    if (DoCheck)
+    {
+        const std::set<std::string>& parsedKeys = config.get_comsumed_keys();
+        for (const auto& it : config)
+            if (parsedKeys.find(it.first) == parsedKeys.end())
+                std::cerr << "Invalid option: " << it.first << std::endl;
+    }
+    // Check consistency
+    if (DoCheck)
+    {
+        // startFen
+        std::string fenBoard = v->startFen.substr(0, v->startFen.find(' '));
+        std::stringstream ss(fenBoard);
+        char token;
+        ss >> std::noskipws;
+
+        while (ss >> token)
+        {
+            if (token == '+')
+            {
+                ss >> token;
+                size_t idx = v->pieceToChar.find(toupper(token));
+                if (idx == std::string::npos || !v->promotedPieceType[idx])
+                    std::cerr << "startFen - Invalid piece type: +" << token << std::endl;
+            }
+            else if (isalpha(token) && v->pieceToChar.find(toupper(token)) == std::string::npos)
+                std::cerr << "startFen - Invalid piece type: " << token << std::endl;
+        }
+
+        // pieceToCharTable
+        if (v->pieceToCharTable != "-")
+        {
+            ss = std::stringstream(v->pieceToCharTable);
+            while (ss >> token)
+                if (isalpha(token) && v->pieceToChar.find(toupper(token)) == std::string::npos)
+                    std::cerr << "pieceToCharTable - Invalid piece type: " << token << std::endl;
+            for (PieceType pt : v->pieceTypes)
+            {
+                char ptl = tolower(v->pieceToChar[pt]);
+                if (v->pieceToCharTable.find(ptl) == std::string::npos && fenBoard.find(ptl) != std::string::npos)
+                    std::cerr << "pieceToCharTable - Missing piece type: " << ptl << std::endl;
+                char ptu = toupper(v->pieceToChar[pt]);
+                if (v->pieceToCharTable.find(ptu) == std::string::npos && fenBoard.find(ptu) != std::string::npos)
+                    std::cerr << "pieceToCharTable - Missing piece type: " << ptu << std::endl;
+            }
+        }
+    }
     return v;
 }
+
+template Variant* VariantParser<true>::parse();
+template Variant* VariantParser<false>::parse();
+template Variant* VariantParser<true>::parse(Variant* v);
+template Variant* VariantParser<false>::parse(Variant* v);
diff --git a/src/parser.h b/src/parser.h
index 2e6c234..a5eca49 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -29,10 +29,17 @@ public:
         constexpr bool PrintOptions = false; // print config options?
         if (PrintOptions)
             std::cout << s << std::endl;
+        consumedKeys.insert(s);
         return std::map<std::string, std::string>::find(s);
     }
+    const std::set<std::string>& get_comsumed_keys() {
+        return consumedKeys;
+    }
+private:
+    std::set<std::string> consumedKeys = {};
 };
 
+template <bool DoCheck>
 class VariantParser {
 public:
     VariantParser(const Config& c) : config (c) {};
diff --git a/src/uci.cpp b/src/uci.cpp
index c55a08f..49afa05 100644
--- a/src/uci.cpp
+++ b/src/uci.cpp
@@ -211,6 +211,16 @@ namespace {
         Options["VariantPath"] = token;
   }
 
+  // check() is called when engine receives the "check" command.
+  // The function reads variant configuration files and validates them.
+
+  void check(istringstream& is) {
+
+    string token;
+    while (is >> token)
+        variants.parse<true>(token);
+  }
+
 } // namespace
 
 
@@ -291,6 +301,7 @@ void UCI::loop(int argc, char* argv[]) {
       else if (token == "eval")     sync_cout << Eval::trace(pos) << sync_endl;
       else if (token == "compiler") sync_cout << compiler_info() << sync_endl;
       else if (token == "load")     { load(is); argc = 1; } // continue reading stdin
+      else if (token == "check")    check(is);
       else
           sync_cout << "Unknown command: " << cmd << sync_endl;
 
diff --git a/src/ucioption.cpp b/src/ucioption.cpp
index e29b062..e8d9124 100644
--- a/src/ucioption.cpp
+++ b/src/ucioption.cpp
@@ -56,7 +56,7 @@ void on_hash_size(const Option& o) { TT.resize(o); }
 void on_logger(const Option& o) { start_logger(o); }
 void on_threads(const Option& o) { Threads.set(o); }
 void on_tb_path(const Option& o) { Tablebases::init(o); }
-void on_variant_path(const Option& o) { variants.parse(o); Options["UCI_Variant"].set_combo(variants.get_keys()); }
+void on_variant_path(const Option& o) { variants.parse<false>(o); Options["UCI_Variant"].set_combo(variants.get_keys()); }
 void on_variant_change(const Option &o) {
     const Variant* v = variants.find(o)->second;
     PSQT::init(v);
diff --git a/src/variant.cpp b/src/variant.cpp
index f5e39d0..df7f1dc 100644
--- a/src/variant.cpp
+++ b/src/variant.cpp
@@ -901,6 +901,7 @@ void VariantMap::init() {
 
 /// VariantMap::parse reads variants from an INI-style configuration file.
 
+template <bool DoCheck>
 void VariantMap::parse(std::string path) {
     if (path.empty() || path == "<empty>")
         return;
@@ -913,6 +914,7 @@ void VariantMap::parse(std::string path) {
     std::string variant, variant_template, key, value, input;
     while (file.peek() != '[' && std::getline(file, input)) {}
 
+    std::vector<std::string> varsToErase = {};
     while (file.get() && std::getline(std::getline(file, variant, ']'), input))
     {
         // Extract variant template, if specified
@@ -935,17 +937,34 @@ void VariantMap::parse(std::string path) {
             std::cerr << "Variant template '" << variant_template << "' does not exist." << std::endl;
         else
         {
-            Variant* v = !variant_template.empty() ? VariantParser(attribs).parse(new Variant(*variants.find(variant_template)->second))
-                                                   : VariantParser(attribs).parse();
+            if (DoCheck)
+                std::cerr << "Parsing variant: " << variant << std::endl;
+            Variant* v = !variant_template.empty() ? VariantParser<DoCheck>(attribs).parse(new Variant(*variants.find(variant_template)->second))
+                                                   : VariantParser<DoCheck>(attribs).parse();
             if (v->maxFile <= FILE_MAX && v->maxRank <= RANK_MAX)
+            {
                 add(variant, v);
+                // In order to allow inheritance, we need to temporarily add configured variants
+                // even when only checking them, but we remove them later after parsing is finished.
+                if (DoCheck)
+                    varsToErase.push_back(variant);
+            }
             else
                 delete v;
         }
     }
     file.close();
+    // Clean up temporary variants
+    for (std::string tempVar : varsToErase)
+    {
+        delete variants[tempVar];
+        variants.erase(tempVar);
+    }
 }
 
+template void VariantMap::parse<true>(std::string path);
+template void VariantMap::parse<false>(std::string path);
+
 void VariantMap::add(std::string s, const Variant* v) {
   insert(std::pair<std::string, const Variant*>(s, v));
 }
diff --git a/src/variant.h b/src/variant.h
index 6464d3f..e31b654 100644
--- a/src/variant.h
+++ b/src/variant.h
@@ -136,7 +136,7 @@ struct Variant {
 class VariantMap : public std::map<std::string, const Variant*> {
 public:
   void init();
-  void parse(std::string path);
+  template <bool DoCheck> void parse(std::string path);
   void clear_all();
   std::vector<std::string> get_keys();
 
diff --git a/src/variants.ini b/src/variants.ini
index c904288..059176b 100644
--- a/src/variants.ini
+++ b/src/variants.ini
@@ -20,6 +20,8 @@
 ### Usage:
 # Add "load" and the file path to the SF call (e.g., "./stockfish load variants.ini")
 # or set the UCI option "VariantPath" to the path of this file in order to load it.
+# In order to validate the configuration without actually loading the variants
+# run "./stockfish check variants.ini", which reports potential config errors.
 
 ################################################
 ### Variant configuration:
@@ -282,7 +284,7 @@ nMoveRule = 0
 
 [shogun:crazyhouse]
 variantTemplate = shogi
-pieceToCharTable = PNBR.D.....++++.+Kpnbr.d.....++++.+k
+pieceToCharTable = PNBR.F.....++++.+Kpnbr.f.....++++.+k
 pocketSize = 8
 startFen = rnb+fkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNB+FKBNR w KQkq - 0 1
 commoner = c