Make label normalization mandatory and collision-safe, and retime it for players and strategies (#965)

Make NormalizeGameLabels run unconditionally in every reader and remove
the normalize_labels / p_normalizeLabels parameter throughout the read
path: ReadEfgFile, ReadNfgFile, ReadGbtFile, ReadAggFile, ReadBaggFile,
ReadGame, and the pygambit read_* wrappers.

Author: d-kad

src/games/file.cc

@@ -23,6 +23,7 @@
 #include <iostream>
 #include <fstream>
 #include <map>
+#include <set>
 #include <algorithm>
 
 #include "gambit.h"
@@ -318,6 +319,53 @@ class TableFileGame {
   }
 };
 
+/// Normalizes labels in place so the resulting set is distinct and nonempty:
+/// empty labels are given a suffix and repeated labels are de-duplicated by
+/// appending "_n", choosing the next n not already present in the scope.
+/// `p_get(element)` reads an element's label and `p_set(element, label)`
+/// writes it, so this works both on a container of game objects (via GetLabel/SetLabel)
+/// and on a container of raw label strings (read/write the string directly).
+template <class Container, class Getter, class Setter>
+void NormalizeLabels(Container &&p_container, Getter p_get, Setter p_set)
+{
+  // NOLINTBEGIN(misc-const-correctness)
+  std::map<std::string, std::size_t> counts;
+  std::set<std::string> used;
+  // NOLINTEND(misc-const-correctness)
+  for (auto &&element : p_container) {
+    counts[p_get(element)] += 1;
+    used.insert(p_get(element));
+  }
+  // NOLINTBEGIN(misc-const-correctness)
+  std::map<std::string, std::size_t> visited;
+  // NOLINTEND(misc-const-correctness)
+  for (auto &&element : p_container) {
+    const auto label = p_get(element);
+    // A special case: If only one label is the empty string we still want to
+    // convert it to "_1"
+    if (counts[label] == 1 && label != "") {
+      continue;
+    }
+    // Generate the next "label_n" that is not already used in this scope, so
+    // that e.g. {"x", "x", "x_1"} does not renumber to a duplicate "x_1".
+    std::string candidate;
+    do {
+      const auto index = ++visited[label];
+      candidate = label + "_" + std::to_string(index);
+    } while (used.count(candidate) > 0);
+    used.insert(candidate);
+    p_set(element, candidate);
+  }
+}
+
+/// Normalizes a list of raw label strings.
+template <class Container> void NormalizeLabelStrings(Container &p_labels)
+{
+  NormalizeLabels(
+      p_labels, [](const std::string &s) { return s; },
+      [](std::string &s, const std::string &v) { s = v; });
+}
+
 void ReadPlayers(GameFileLexer &p_state, TableFileGame &p_data)
 {
   p_state.ExpectNextToken(TOKEN_LBRACE, "'{'");
@@ -472,10 +520,19 @@ class TreeData {
 void ReadPlayers(GameFileLexer &p_state, Game &p_game, TreeData &p_treeData)
 {
   p_state.ExpectNextToken(TOKEN_LBRACE, "'{'");
+  // Buffer the raw player labels so they can be normalized (made unique and nonempty)
+  // before the player objects are created.
+  // NOLINTBEGIN(misc-const-correctness)
+  std::vector<std::string> player_labels;
+  // NOLINTEND(misc-const-correctness)
   while (p_state.GetNextToken() == TOKEN_TEXT) {
-    p_game->NewPlayer()->SetLabel(p_state.GetLastText());
+    player_labels.push_back(p_state.GetLastText());
   }
   p_state.ExpectCurrentToken(TOKEN_RBRACE, "'}'");
+  NormalizeLabelStrings(player_labels);
+  for (const auto &label : player_labels) {
+    p_game->NewPlayer()->SetLabel(label);
+  }
 }
 
 void CheckOutcomeDefinition(const GameFileLexer &p_state, int p_outcomeId,
@@ -800,48 +857,27 @@ Game GameXMLSavefile::GetGame() const
   throw InvalidFileException("No game representation found in document");
 }
 
-template <class C> void NormalizeLabels(C &&p_container)
-{
-  // NOLINTBEGIN(misc-const-correctness)
-  std::map<std::string, std::size_t> counts;
-  // NOLINTEND(misc-const-correctness)
-  for (const auto &element : p_container) {
-    counts[element->GetLabel()] += 1;
-  }
-  // NOLINTBEGIN(misc-const-correctness)
-  std::map<std::string, std::size_t> visited;
-  // NOLINTEND(misc-const-correctness)
-  for (auto element : p_container) {
-    const auto label = element->GetLabel();
-    // A special case: If only one label is the empty string we still want to
-    // convert it to "_1"
-    if (counts[label] == 1 && label != "") {
-      continue;
-    }
-    const auto index = ++visited[label];
-    element->SetLabel(label + "_" + std::to_string(index));
-  }
-}
-
 void NormalizeGameLabels(const Game &p_game)
 {
-  NormalizeLabels(p_game->GetPlayers());
-  NormalizeLabels(p_game->GetOutcomes());
+  const auto get_label = [](const auto &e) { return e->GetLabel(); };
+  const auto set_label = [](const auto &e, const std::string &s) { e->SetLabel(s); };
+  NormalizeLabels(p_game->GetPlayers(), get_label, set_label);
+  NormalizeLabels(p_game->GetOutcomes(), get_label, set_label);
   if (p_game->IsTree()) {
     for (const auto &player : p_game->GetPlayersWithChance()) {
       for (const auto &infoset : player->GetInfosets()) {
-        NormalizeLabels(infoset->GetActions());
+        NormalizeLabels(infoset->GetActions(), get_label, set_label);
       }
     }
   }
   else {
     for (const auto &player : p_game->GetPlayers()) {
-      NormalizeLabels(player->GetStrategies());
+      NormalizeLabels(player->GetStrategies(), get_label, set_label);
     }
   }
 }
 
-Game ReadEfgFile(std::istream &p_stream, bool p_normalizeLabels /* = false */)
+Game ReadEfgFile(std::istream &p_stream)
 {
   GameFileLexer parser(p_stream);
 
@@ -869,64 +905,73 @@ Game ReadEfgFile(std::istream &p_stream, bool p_normalizeLabels /* = false */)
     parser.GetNextToken();
   }
   ParseNode(parser, game, game->GetRoot(), treeData);
-  if (p_normalizeLabels) {
-    NormalizeGameLabels(game);
-  }
+  NormalizeGameLabels(game);
   return game;
 }
 
-Game ReadNfgFile(std::istream &p_stream, bool p_normalizeLabels /* = false */)
+Game ReadNfgFile(std::istream &p_stream)
 {
   GameFileLexer parser(p_stream);
   TableFileGame data;
   ParseNfgHeader(parser, data);
-  auto game = BuildNfg(parser, data);
-  if (p_normalizeLabels) {
-    NormalizeGameLabels(game);
+  // Normalize player and strategy labels on the raw lists before the game is
+  // built, so labels are unique and nonempty at construction.
+  for (auto &player : data.m_players) {
+    NormalizeLabelStrings(player.m_strategies);
   }
+  {
+    // NOLINTBEGIN(misc-const-correctness)
+    std::vector<std::string> player_labels;
+    // NOLINTEND(misc-const-correctness)
+    for (const auto &player : data.m_players) {
+      player_labels.push_back(player.m_name);
+    }
+    NormalizeLabelStrings(player_labels);
+    auto label_it = player_labels.begin();
+    for (auto &player : data.m_players) {
+      player.m_name = *label_it;
+      ++label_it;
+    }
+  }
+  auto game = BuildNfg(parser, data);
+  NormalizeGameLabels(game);
   return game;
 }
 
-Game ReadGbtFile(std::istream &p_stream, bool p_normalizeLabels /* = false */)
+Game ReadGbtFile(std::istream &p_stream)
 {
   std::stringstream buffer;
   buffer << p_stream.rdbuf();
   auto game = GameXMLSavefile(buffer.str()).GetGame();
-  if (p_normalizeLabels) {
-    NormalizeGameLabels(game);
-  }
+  NormalizeGameLabels(game);
   return game;
 }
 
-Game ReadAggFile(std::istream &p_stream, bool p_normalizeLabels /* = false */)
+Game ReadAggFile(std::istream &p_stream)
 {
   try {
     auto game = std::make_shared<GameAGGRep>(agg::AGG::makeAGG(p_stream));
-    if (p_normalizeLabels) {
-      NormalizeGameLabels(game);
-    }
+    NormalizeGameLabels(game);
     return game;
   }
   catch (std::runtime_error &ex) {
     throw InvalidFileException(ex.what());
   }
 }
 
-Game ReadBaggFile(std::istream &p_stream, bool p_normalizeLabels /* = false */)
+Game ReadBaggFile(std::istream &p_stream)
 {
   try {
     auto game = std::make_shared<GameBAGGRep>(agg::BAGG::makeBAGG(p_stream));
-    if (p_normalizeLabels) {
-      NormalizeGameLabels(game);
-    }
+    NormalizeGameLabels(game);
     return game;
   }
   catch (std::runtime_error &ex) {
     throw InvalidFileException(ex.what());
   }
 }
 
-Game ReadGame(std::istream &p_file, bool p_normalizeLabels /* = false */)
+Game ReadGame(std::istream &p_file)
 {
   std::stringstream buffer;
   buffer << p_file.rdbuf();
@@ -947,10 +992,10 @@ Game ReadGame(std::istream &p_file, bool p_normalizeLabels /* = false */)
     }
     buffer.seekg(0, std::ios::beg);
     if (parser.GetLastText() == "NFG") {
-      return ReadNfgFile(buffer, p_normalizeLabels);
+      return ReadNfgFile(buffer);
     }
     if (parser.GetLastText() == "EFG") {
-      return ReadEfgFile(buffer, p_normalizeLabels);
+      return ReadEfgFile(buffer);
     }
     if (parser.GetLastText() == "#AGG") {
       return ReadAggFile(buffer);

src/games/game.h

@@ -1395,38 +1395,32 @@ Game NewTable(const std::vector<int> &p_dim, bool p_sparseOutcomes = false);
 /// @brief Reads a game representation in .efg format
 ///
 /// @param[in] p_stream An input stream, positioned at the start of the text in .efg format
-/// @param[in] p_normalizeLabels Require element labels to be nonempty and unique within
-///                              their scope
 /// @return A handle to the game representation constructed
 /// @throw InvalidFileException If the stream does not contain a valid serialisation
 ///                             of a game in .efg format.
 /// @sa Game::WriteEfgFile, ReadNfgFile, ReadAggFile, ReadBaggFile
-Game ReadEfgFile(std::istream &p_stream, bool p_normalizeLabels = false);
+Game ReadEfgFile(std::istream &p_stream);
 
 /// @brief Reads a game representation in .nfg format
 /// @param[in] p_stream An input stream, positioned at the start of the text in .nfg format
-/// @param[in] p_normalizeLabels Require element labels to be nonempty and unique within
-///                              their scope
 /// @return A handle to the game representation constructed
 /// @throw InvalidFileException If the stream does not contain a valid serialisation
 ///                             of a game in .nfg format.
 /// @sa Game::WriteNfgFile, ReadEfgFile, ReadAggFile, ReadBaggFile
-Game ReadNfgFile(std::istream &p_stream, bool p_normalizeLabels = false);
+Game ReadNfgFile(std::istream &p_stream);
 
 /// @brief Reads a game representation from a graphical interface XML saveflie
 /// @param[in] p_stream An input stream, positioned at the start of the text
-/// @param[in] p_normalizeLabels Require element labels to be nonempty and unique within
-///                              their scope
 /// @return A handle to the game representation constructed
 /// @throw InvalidFileException If the stream does not contain a valid serialisation
 ///                             of a game in an XML savefile
 /// @sa ReadEfgFile, ReadNfgFile, ReadAggFile, ReadBaggFile
-Game ReadGbtFile(std::istream &p_stream, bool p_normalizeLabels = false);
+Game ReadGbtFile(std::istream &p_stream);
 
 /// @brief Reads a game from the input stream, attempting to autodetect file format
 /// @deprecated Deprecated in favour of the various ReadXXXGame functions.
 /// @sa ReadEfgFile, ReadNfgFile, ReadGbtFile, ReadAggFile, ReadBaggFile
-Game ReadGame(std::istream &p_stream, bool p_normalizeLabels = false);
+Game ReadGame(std::istream &p_stream);
 
 /// @brief Generate a distribution over a simplex restricted to rational numbers of given
 /// denominator

src/games/gameagg.h

@@ -113,15 +113,7 @@ class GameAGGRep : public GameRep {
 /// @return A handle to the game representation constructed
 /// @throw InvalidFileException If the stream does not contain a valid serialisation
 ///                             of a game in .agg format.
-inline Game ReadAggFile(std::istream &p_stream)
-{
-  try {
-    return std::make_shared<GameAGGRep>(agg::AGG::makeAGG(p_stream));
-  }
-  catch (std::runtime_error &ex) {
-    throw InvalidFileException(ex.what());
-  }
-}
+Game ReadAggFile(std::istream &p_stream);
 
 } // namespace Gambit
 

src/games/gamebagg.h

@@ -121,15 +121,7 @@ class GameBAGGRep : public GameRep {
 /// @return A handle to the game representation constructed
 /// @throw InvalidFileException If the stream does not contain a valid serialisation
 ///                             of a game in .bagg format.
-inline Game ReadBaggFile(std::istream &in)
-{
-  try {
-    return std::make_shared<GameBAGGRep>(agg::BAGG::makeBAGG(in));
-  }
-  catch (std::runtime_error &ex) {
-    throw InvalidFileException(ex.what());
-  }
-}
+Game ReadBaggFile(std::istream &in);
 
 } // end namespace Gambit
 

src/pygambit/gambit.pxd

@@ -488,11 +488,11 @@ cdef extern from "games/layout.h":
 
 
 cdef extern from "util.h":
-    c_Game ParseGbtGame(string, bint) except +IOError
-    c_Game ParseEfgGame(string, bint) except +IOError
-    c_Game ParseNfgGame(string, bint) except +IOError
-    c_Game ParseAggGame(string, bint) except +IOError
-    c_Game ParseBaggGame(string, bint) except +IOError
+    c_Game ParseGbtGame(string) except +IOError
+    c_Game ParseEfgGame(string) except +IOError
+    c_Game ParseNfgGame(string) except +IOError
+    c_Game ParseAggGame(string) except +IOError
+    c_Game ParseBaggGame(string) except +IOError
     string WriteEfgFile(c_Game)
     string WriteNfgFile(c_Game)
     string WriteNfgFileSupport(c_StrategySupportProfile) except +IOError