Better naming and document some endgame functions

In particular the generic scaling functions.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>

src/endgame.cpp

@@ -367,7 +367,7 @@ Value EvaluationFunction<KNNK>::apply(const Position&) {
 /// returned. If not, the return value is SCALE_FACTOR_NONE, i.e. no scaling
 /// will be used.
 template<>
-ScaleFactor ScalingFunction<KBPK>::apply(const Position& pos) {
+ScaleFactor ScalingFunction<KBPsK>::apply(const Position& pos) {
 
   assert(pos.non_pawn_material(strongerSide) == BishopValueMidgame);
   assert(pos.piece_count(strongerSide, BISHOP) == 1);
@@ -393,7 +393,6 @@ ScaleFactor ScalingFunction<KBPK>::apply(const Position& pos) {
           // The bishop has the wrong color, and the defending king is on the
           // file of the pawn(s) or the neighboring file. Find the rank of the
           // frontmost pawn.
-
           Rank rank;
           if (strongerSide == WHITE)
           {
@@ -422,7 +421,7 @@ ScaleFactor ScalingFunction<KBPK>::apply(const Position& pos) {
 /// It tests for fortress draws with a rook on the third rank defended by
 /// a pawn.
 template<>
-ScaleFactor ScalingFunction<KQKRP>::apply(const Position& pos) {
+ScaleFactor ScalingFunction<KQKRPs>::apply(const Position& pos) {
 
   assert(pos.non_pawn_material(strongerSide) == QueenValueMidgame);
   assert(pos.piece_count(strongerSide, QUEEN) == 1);

src/endgame.h

@@ -49,8 +49,8 @@ enum EndgameType {
     KmmKm, // K and two minors vs K and one or two minors
 
     // Scaling functions
-    KBPK,    // KBP vs K
+    KBPsK,   // KB+pawns vs K
-    KQKRP,   // KQ vs KRP
+    KQKRPs,  // KQ vs KR+pawns
     KRPKR,   // KRP vs KR
     KRPPKRP, // KRPP vs KRP
     KPsK,    // King and pawns vs king

src/material.cpp

@@ -30,6 +30,7 @@
 
 using namespace std;
 
+
 ////
 //// Local definitions
 ////
@@ -49,14 +50,17 @@ namespace {
   { 0, 0, 0, 0, 0, 0 }, { -5, 0, 0, 0, 0, 0 }, { -33, 23, 0, 0, 0, 0 },
   { 17, 25, -3, 0, 0, 0 }, { 10, -2, -19, -67, 0, 0 }, { 69, 64, -41, 116, 137, 0 } };
 
-  // Unmapped endgame evaluation and scaling functions, these
+  // Named endgame evaluation and scaling functions, these
   // are accessed direcly and not through the function maps.
   EvaluationFunction<KmmKm> EvaluateKmmKm(WHITE);
   EvaluationFunction<KXK>   EvaluateKXK(WHITE), EvaluateKKX(BLACK);
-  ScalingFunction<KBPK>     ScaleKBPK(WHITE),   ScaleKKBP(BLACK);
+  ScalingFunction<KBPsK>    ScaleKBPsK(WHITE),  ScaleKKBPs(BLACK);
-  ScalingFunction<KQKRP>    ScaleKQKRP(WHITE),  ScaleKRPKQ(BLACK);
+  ScalingFunction<KQKRPs>   ScaleKQKRPs(WHITE), ScaleKRPsKQ(BLACK);
   ScalingFunction<KPsK>     ScaleKPsK(WHITE),   ScaleKKPs(BLACK);
   ScalingFunction<KPKP>     ScaleKPKPw(WHITE),  ScaleKPKPb(BLACK);
+
+  typedef EndgameEvaluationFunctionBase EF;
+  typedef EndgameScalingFunctionBase SF;
 }
 
 
@@ -64,11 +68,10 @@ namespace {
 //// Classes
 ////
 
-typedef EndgameEvaluationFunctionBase EF;
+/// EndgameFunctions class stores endgame evaluation and scaling functions
-typedef EndgameScalingFunctionBase SF;
+/// in two std::map. Because STL library is not guaranteed to be thread
-
+/// safe even for read access, the maps, although with identical content,
-/// See header for a class description. It is declared here to avoid
+/// are replicated for each thread. This is faster then using locks.
-/// to include <map> in the header file.
 
 class EndgameFunctions {
 public:
@@ -82,10 +85,10 @@ private:
   static Key buildKey(const string& keyCode);
   static const string swapColors(const string& keyCode);
 
-  // Here we store two maps, one for evaluate and one for scaling
+  // Here we store two maps, for evaluate and scaling functions
   pair<map<Key, EF*>, map<Key, SF*> > maps;
 
-  // Maps accessing functions for const and non-const references
+  // Maps accessing functions returning const and non-const references
   template<typename T> const map<Key, T*>& get() const { return maps.first; }
   template<typename T> map<Key, T*>& get() { return maps.first; }
 };
@@ -103,25 +106,22 @@ EndgameFunctions::get<SF>() { return maps.second; }
 //// Functions
 ////
 
-
+/// MaterialInfoTable c'tor and d'tor, called once by each thread
-/// Constructor for the MaterialInfoTable class
 
 MaterialInfoTable::MaterialInfoTable(unsigned int numOfEntries) {
 
   size = numOfEntries;
   entries = new MaterialInfo[size];
   funcs = new EndgameFunctions();
+
   if (!entries || !funcs)
   {
-      cerr << "Failed to allocate " << (numOfEntries * sizeof(MaterialInfo))
+      cerr << "Failed to allocate " << numOfEntries * sizeof(MaterialInfo)
            << " bytes for material hash table." << endl;
       Application::exit_with_failure();
   }
 }
 
-
-/// Destructor for the MaterialInfoTable class
-
 MaterialInfoTable::~MaterialInfoTable() {
 
   delete funcs;
@@ -175,8 +175,8 @@ MaterialInfo* MaterialInfoTable::get_material_info(const Position& pos) {
            && pos.rooks() == EmptyBoardBB
            && pos.queens() == EmptyBoardBB)
   {
-      // Minor piece endgame with at least one minor piece per side,
+      // Minor piece endgame with at least one minor piece per side and
-      // and no pawns.
+      // no pawns. Note that the case KmmK is already handled by KXK.
       assert(pos.knights(WHITE) | pos.bishops(WHITE));
       assert(pos.knights(BLACK) | pos.bishops(BLACK));
 
@@ -203,29 +203,32 @@ MaterialInfo* MaterialInfoTable::get_material_info(const Position& pos) {
       return mi;
   }
 
+  // Generic scaling functions that refer to more then one material
+  // distribution. Should be probed after the specialized ones.
+  // Note that these ones don't return after setting the function.
   if (   pos.non_pawn_material(WHITE) == BishopValueMidgame
       && pos.piece_count(WHITE, BISHOP) == 1
       && pos.piece_count(WHITE, PAWN) >= 1)
-      mi->scalingFunction[WHITE] = &ScaleKBPK;
+      mi->scalingFunction[WHITE] = &ScaleKBPsK;
 
   if (   pos.non_pawn_material(BLACK) == BishopValueMidgame
       && pos.piece_count(BLACK, BISHOP) == 1
       && pos.piece_count(BLACK, PAWN) >= 1)
-      mi->scalingFunction[BLACK] = &ScaleKKBP;
+      mi->scalingFunction[BLACK] = &ScaleKKBPs;
 
   if (   pos.piece_count(WHITE, PAWN) == 0
       && pos.non_pawn_material(WHITE) == QueenValueMidgame
       && pos.piece_count(WHITE, QUEEN) == 1
       && pos.piece_count(BLACK, ROOK) == 1
       && pos.piece_count(BLACK, PAWN) >= 1)
-      mi->scalingFunction[WHITE] = &ScaleKQKRP;
+      mi->scalingFunction[WHITE] = &ScaleKQKRPs;
 
   else if (   pos.piece_count(BLACK, PAWN) == 0
            && pos.non_pawn_material(BLACK) == QueenValueMidgame
            && pos.piece_count(BLACK, QUEEN) == 1
            && pos.piece_count(WHITE, ROOK) == 1
            && pos.piece_count(WHITE, PAWN) >= 1)
-      mi->scalingFunction[BLACK] = &ScaleKRPKQ;
+      mi->scalingFunction[BLACK] = &ScaleKRPsKQ;
 
   if (pos.non_pawn_material(WHITE) + pos.non_pawn_material(BLACK) == Value(0))
   {
@@ -241,6 +244,8 @@ MaterialInfo* MaterialInfoTable::get_material_info(const Position& pos) {
       }
       else if (pos.piece_count(WHITE, PAWN) == 1 && pos.piece_count(BLACK, PAWN) == 1)
       {
+          // This is a special case because we set scaling functions
+          // for both colors instead of only one.
           mi->scalingFunction[WHITE] = &ScaleKPKPw;
           mi->scalingFunction[BLACK] = &ScaleKPKPb;
       }
@@ -298,11 +303,12 @@ MaterialInfo* MaterialInfoTable::get_material_info(const Position& pos) {
     if (pieceCount[c][ROOK] >= 1)
         matValue -= sign * ((pieceCount[c][ROOK] - 1) * RedundantRookPenalty + pieceCount[c][QUEEN] * RedundantQueenPenalty);
 
+    them = opposite_color(c);
+
     // Second-degree polynomial material imbalance by Tord Romstad
     //
     // We use NO_PIECE_TYPE as a place holder for the bishop pair "extended piece",
     // this allow us to be more flexible in defining bishop pair bonuses.
-    them = opposite_color(c);
     for (int pt1 = NO_PIECE_TYPE; pt1 <= QUEEN; pt1++)
     {
         int c1 = sign * pieceCount[c][pt1];
@@ -318,18 +324,12 @@ MaterialInfo* MaterialInfoTable::get_material_info(const Position& pos) {
         }
     }
   }
-
   mi->value = int16_t(matValue / 16);
   return mi;
 }
 
 
-/// EndgameFunctions member definitions. This class is used to store the maps
+/// EndgameFunctions member definitions.
-/// of end game and scaling functions that MaterialInfoTable will query for
-/// each key. The maps are constant and are populated only at construction,
-/// but are per-thread instead of globals to avoid expensive locks needed
-/// because std::map is not guaranteed to be thread-safe even if accessed
-/// only for a lookup.
 
 EndgameFunctions::EndgameFunctions() {
 
@@ -368,8 +368,8 @@ Key EndgameFunctions::buildKey(const string& keyCode) {
     stringstream s;
     bool upcase = false;
 
-    // Build up a fen substring with the given pieces, note
+    // Build up a fen string with the given pieces, note that
-    // that the fen string could be of an illegal position.
+    // the fen string could be of an illegal position.
     for (size_t i = 0; i < keyCode.length(); i++)
     {
         if (keyCode[i] == 'K')

src/material.h

@@ -68,19 +68,11 @@ private:
   int spaceWeight;
 };
 
-
-/// EndgameFunctions class stores the endgame evaluation functions std::map.
-/// Because STL library is not thread safe even for read access, the maps,
-/// although with identical content, are replicated for each thread. This
-/// is faster then using locks with an unique set of global maps.
-
-class EndgameFunctions;
-
-
 /// The MaterialInfoTable class represents a pawn hash table. It is basically
 /// just an array of MaterialInfo objects and a few methods for accessing these
 /// objects. The most important method is get_material_info, which looks up a
 /// position in the table and returns a pointer to a MaterialInfo object.
+class EndgameFunctions;
 
 class MaterialInfoTable {