E02: FlatFile separator bug

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## Unreleased
+
+### Fixed
+
+* `FlatFile` data rows had no separator between the last meta data column (`run`) and the first logged property, so
+  the two were glued together: run 1 and evaluations 123 were written as `1123`. The header row was correct, so an
+  affected file has data rows with one field fewer than its header (for example 8 instead of 9 with the default
+  header and two properties), and the `run` column holds the run number followed by the first property's value. Files
+  written with no meta data (empty `common_header_titles`) were not affected. Files written by earlier versions stay
+  wrong: the run number and the first value cannot be told apart reliably when the value starts with a digit. Log them
+  again with this version, or recover the run number from the order of the rows.
+
 ## 0.4.0
 
 The first release since 0.3.22. Parts of the Python and C++ APIs changed: the

include/ioh/logger/flatfile.hpp

@@ -205,7 +205,7 @@ namespace ioh::logger
     private:
         /**
          * \brief Caches the meta data logged at every row. If log_meta_data is false,
-         * it will store an empty string.
+         * it will store an empty string. Otherwise, the string ends with a separator.
          */
         void cache_meta_data()
         {
@@ -220,6 +220,8 @@ namespace ioh::logger
                 ss << sep_ << (problem_.value().optimization_type == common::OptimizationType::MIN ? "min" : "max");
                 ss << sep_ << problem_.value().n_variables;
                 ss << sep_ << current_run_;
+                // The header (common_header_) ends with a separator too: the first property follows directly.
+                ss << sep_;
                 current_meta_data_ = ss.str();
             }
         }

tests/cpp/logger/test_flatfile.cpp

@@ -35,3 +35,89 @@ TEST_F(BaseTest, logger_flatfile)
     fs::remove("./IOH.dat");
     EXPECT_TRUE(!fs::exists("./IOH.dat"));
 }
+
+namespace
+{
+    //! Split a string on a separator (an empty field is kept)
+    std::vector<std::string> split_on(const std::string &s, const std::string &sep)
+    {
+        std::vector<std::string> fields;
+        size_t start = 0, pos;
+        while ((pos = s.find(sep, start)) != std::string::npos)
+        {
+            fields.push_back(s.substr(start, pos - start));
+            start = pos + sep.size();
+        }
+        fields.push_back(s.substr(start));
+        return fields;
+    }
+
+    //! Log 3 evaluations of a 2D sphere with the given logger arguments and return the lines of the file
+    std::vector<std::string> flatfile_lines(const std::string &sep, const bool meta, const bool positions)
+    {
+        const std::string name = "ioh_flatfile_separator_test.dat";
+        fs::remove(name);
+        {
+            auto problem = problem::bbob::Sphere(1, 2);
+            trigger::Always always;
+            watch::Evaluations evaluations;
+            watch::TransformedY transformed_y;
+            std::vector<std::string> titles = {"suite_name",      "problem_name", "problem_id", "problem_instance",
+                                               "optimization_type", "dimension",    "run"};
+            if (!meta)
+                titles.clear();
+            logger::FlatFile logger({always}, {evaluations, transformed_y}, name, ".", sep, "# ", "None", "\n", false,
+                                    positions, titles);
+            problem.attach_logger(logger);
+            for (int s = 0; s < 3; ++s)
+                problem({0.1 * s, 0.2});
+        }
+        std::ifstream in(name);
+        std::vector<std::string> lines;
+        for (std::string line; std::getline(in, line);)
+            lines.push_back(line);
+        in.close();
+        fs::remove(name);
+        return lines;
+    }
+
+    //! Check that every data row has as many fields as the header, and that the fields hold what they should
+    void check_flatfile(const std::string &sep, const bool meta, const bool positions)
+    {
+        const auto lines = flatfile_lines(sep, meta, positions);
+        ASSERT_EQ(lines.size(), 4u) << "sep '" << sep << "'";
+        ASSERT_EQ(lines[0].substr(0, 2), "# ");
+        const auto header = split_on(lines[0].substr(2), sep);
+        const size_t n_fields = (meta ? 7 : 0) + 2 + (positions ? 2 : 0);
+        EXPECT_EQ(header.size(), n_fields) << lines[0];
+        for (size_t i = 1; i < lines.size(); ++i)
+        {
+            const auto row = split_on(lines[i], sep);
+            ASSERT_EQ(row.size(), header.size()) << "row: " << lines[i];
+            const size_t first = meta ? 7 : 0;
+            if (meta)
+            {
+                EXPECT_EQ(row[5], "2") << lines[i];     // dimension
+                EXPECT_EQ(row[6], "0") << lines[i];     // run
+            }
+            EXPECT_EQ(row[first], std::to_string(i)) << lines[i]; // evaluations
+        }
+    }
+} // namespace
+
+TEST_F(BaseTest, logger_flatfile_separator_with_meta_data)
+{
+    for (const std::string sep : {"\t", ",", " ; "})
+        check_flatfile(sep, true, false);
+}
+
+TEST_F(BaseTest, logger_flatfile_separator_with_positions)
+{
+    check_flatfile("\t", true, true);
+}
+
+TEST_F(BaseTest, logger_flatfile_separator_without_meta_data)
+{
+    check_flatfile("\t", false, false);
+    check_flatfile(",", false, true);
+}

tests/python/test_logger.py

@@ -25,6 +25,12 @@ def test_flatfile(self):
         self.assertEqual(os.path.realpath("./"), os.path.realpath(l.output_directory))
         pr([0] * 5)
         self.assertTrue(os.path.isfile("IOH.dat"))
+        # FlatFile has no close in Python, but it flushes after every row, so the file can be read while it is open
+        header, *rows = open("IOH.dat").read().splitlines()
+        self.assertTrue(rows)
+        # every data row has as many fields as the header (the run and the first property are separated)
+        for row in rows:
+            self.assertEqual(len(row.split(" ")), len(header[2:].split(" ")))
 
     def test_analyzer(self):
         pr = ioh.get_problem("Sphere", 1, 5)