From b4854805e60e989189256ce4b750bfb3ea6bb51d Mon Sep 17 00:00:00 2001 From: jacobdenobel Date: Thu, 5 Nov 2026 17:47:22 +0100 Subject: [PATCH 1/5] E02 step 1/2: failing tests for the FlatFile separator between meta data and properties Co-Authored-By: Claude Sonnet 5.5 --- WP_NOTES.md | 16 ++++++ tests/cpp/logger/test_flatfile.cpp | 86 ++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 WP_NOTES.md diff --git a/WP_NOTES.md b/WP_NOTES.md new file mode 100644 index 000000000..6ee5bf2bf --- /dev/null +++ b/WP_NOTES.md @@ -0,0 +1,16 @@ +# WP E02: FlatFile separator bug +Branch: wp/E02-flatfile-separator +Status: in progress + +## Done +- step 1: failing tests (meta data on: run and first property glued, e.g. "01"; meta data off passes) in tests/cpp/logger/test_flatfile.cpp + +## Next step +Step 2: fix include/ioh/logger/flatfile.hpp, changelog entry. + +## Decisions +- Test target: `test_flatfile` (cmake -DIOH_TEST_PER_FILE=ON -DCMAKE_BUILD_TYPE=Debug), build dir `build-e02` (excluded via .git/info/exclude). Run: `build-e02/tests/test_flatfile Warning ".*" 99999` (the arguments are needed). Release (NDEBUG) builds crash in SetUp (clutchlog no-op `logger()` returns nothing, a separate pre-existing issue, not fixed here). +- Submodules fmt and googletest were initialised in the worktree (`git submodule update --init`). + +## Open questions / Blocked +- none diff --git a/tests/cpp/logger/test_flatfile.cpp b/tests/cpp/logger/test_flatfile.cpp index abdd2ec1f..f3987d071 100644 --- a/tests/cpp/logger/test_flatfile.cpp +++ b/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 split_on(const std::string &s, const std::string &sep) + { + std::vector 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 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 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 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); +} From 14c8912c330c49f4eeab364cfaa6cfae1b4087af Mon Sep 17 00:00:00 2001 From: jacobdenobel Date: Thu, 5 Nov 2026 17:48:01 +0100 Subject: [PATCH 2/5] E02 step 2/2: separate the meta data from the first property in FlatFile rows Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 8 + WP_NOTES.md | 7 +- include/ioh/logger/flatfile.hpp | 456 ++++++++++++++++---------------- tests/python/test_logger.py | 7 + 4 files changed, 249 insertions(+), 229 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a85431039..c6b82dd44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,14 @@ change. The ids, names and values of the built-in problems are unchanged, apart * `Experiment.add_custom_problem` failed when the problem ids were a numpy array (e.g. a suite's `problem_ids`). * `logger.property.Property(container, attribute)` raised `SystemError` for an attribute that is not a number; it raises `TypeError`. +* `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. iohinspector's reader flags these + files. ### C++ diff --git a/WP_NOTES.md b/WP_NOTES.md index 6ee5bf2bf..672f61b93 100644 --- a/WP_NOTES.md +++ b/WP_NOTES.md @@ -1,12 +1,13 @@ # WP E02: FlatFile separator bug Branch: wp/E02-flatfile-separator -Status: in progress +Status: ready for review ## Done - step 1: failing tests (meta data on: run and first property glued, e.g. "01"; meta data off passes) in tests/cpp/logger/test_flatfile.cpp +- step 2: `cache_meta_data()` now ends with `sep_` (include/ioh/logger/flatfile.hpp); CHANGELOG.md entry under 0.4.0 / Fixed; Python test in tests/python/test_logger.py checks field counts (not run: no built ioh module here). `test_flatfile`: 4 tests pass (Debug build). ## Next step -Step 2: fix include/ioh/logger/flatfile.hpp, changelog entry. +None. Controller turns this file into the PR body. ## Decisions - Test target: `test_flatfile` (cmake -DIOH_TEST_PER_FILE=ON -DCMAKE_BUILD_TYPE=Debug), build dir `build-e02` (excluded via .git/info/exclude). Run: `build-e02/tests/test_flatfile Warning ".*" 99999` (the arguments are needed). Release (NDEBUG) builds crash in SetUp (clutchlog no-op `logger()` returns nothing, a separate pre-existing issue, not fixed here). @@ -14,3 +15,5 @@ Step 2: fix include/ioh/logger/flatfile.hpp, changelog entry. ## Open questions / Blocked - none +- Other variants checked: no meta data (empty titles) was correct; store_positions was correct (sep before x columns); header is correct. Nothing else in the repo parses FlatFile output. +- Same bug exists in all earlier versions (cache_meta_data unchanged since the logger was added). diff --git a/include/ioh/logger/flatfile.hpp b/include/ioh/logger/flatfile.hpp index 18571b3fe..06b9b845a 100644 --- a/include/ioh/logger/flatfile.hpp +++ b/include/ioh/logger/flatfile.hpp @@ -1,227 +1,229 @@ -#pragma once - -#include "ioh/common/format.hpp" -#include "ioh/problem/single.hpp" - -namespace ioh::logger -{ - /** A logger that stores some information in a single, tabular-like, file. - * - * Each line displays the problem metadata and the watched properties. - * This format displays a lot of redundant information, but is very easy to parse. - * - * @code - logger::FlatFile( - {trigger::always}, - {watch::evaluations, watch::transformed_y}, - "my_experiment.dat", - "./today/" - ); - * @endcode - * - * @ingroup Loggers - */ - class FlatFile : public Watcher - { - protected: - //! Seperator - const std::string sep_; - - //! Comment character - const std::string com_; - - //! EOL character - const std::string eol_; - - //! NAN string - const std::string nan_; - - //! Common header - const std::string common_header_; - - //! Repeat header for every run? - const bool repeat_header_; - - //! Store x positions? - const bool store_positions_; - - //! Requires header? - bool requires_header_; - - //! Log meta data? - const bool log_meta_data_; - - //! Output directory - fs::path output_directory_; - - //! Filename - std::string filename_; - - //! Output stream - std::ofstream out_; - - - - //! Current run - size_t current_run_; - - //! Current meta data - std::string current_meta_data_; - - //! Open a file - void open_stream(const std::string &filename, const fs::path &output_directory) - { - if (filename != filename_) - { - filename_ = filename; - out_.close(); - } - if (output_directory_ != output_directory) - { - output_directory_ = output_directory; - if (not exists(output_directory_)) - { - IOH_DBG(debug, "some directories do not exist in " << output_directory_ << ", try to create them") - create_directories(output_directory_); - } - out_.close(); - } - if (!out_.is_open()) - { - IOH_DBG(debug, "will output data in " << output_directory_ / filename_) - out_ = std::ofstream(output_directory_ / filename_, std::ofstream::out | std::ofstream::app); - requires_header_ = true; - } - } - - public: - /** The logger should at least track one logger::Property, or else it makes no sense to use it. - * - * @param triggers When to fire a log event. - * @param properties What to log. - * @param filename File name in which to write the logged properties. - * @param output_directory Directory in which to put the data file. - * @param separator The string separating fields. - * @param comment The string indicating a comment. - * @param no_value The string indicating that a watched property does not exists in this context. - * @param end_of_line The string to use when all fields have been written. - * @param repeat_header If true, the commented header is printed for each new run. - * @param store_positions Whether to store x positions in the logged data - * @param common_header_titles Seven strings to print in the header for the common problem meta data (property - * names are automatically printed after). - */ - FlatFile(std::vector> triggers, - std::vector> properties, const std::string &filename = "IOH.dat", - const fs::path &output_directory = fs::current_path(), const std::string &separator = "\t", - const std::string &comment = "# ", const std::string &no_value = "None", - const std::string &end_of_line = "\n", const bool repeat_header = false, - const bool store_positions = false, - const std::vector &common_header_titles = {"suite_name", "problem_name", "problem_id", - "problem_instance", "optimization_type", - "dimension", "run"}) : - Watcher(triggers, properties), - sep_(separator), com_(comment), eol_(end_of_line), nan_(no_value), - common_header_(format("{}", fmt::join(common_header_titles.begin(), common_header_titles.end(), sep_)) + - (common_header_titles.empty() ? "" : sep_)), - repeat_header_(repeat_header), store_positions_(store_positions), requires_header_(true), - log_meta_data_(!common_header_titles.empty()), output_directory_(output_directory), filename_(filename), - current_run_(0), current_meta_data_{} - { - assert(common_header_titles.empty() || common_header_titles.size() == 7); - } - - - - void attach_problem(const problem::MetaData &problem) override - { - // If this is a new problem. - if (!problem_.has_value() or problem_.value() != problem) - { - IOH_DBG(xdebug, "reset run counter") - current_run_ = 0; // Then reset the run counter. - } - else - { - IOH_DBG(xdebug, "increment run counter") - current_run_++; // Then it's a new run. - } - - Logger::attach_problem(problem); - open_stream(filename_, output_directory_); - - requires_header_ = requires_header_ or repeat_header_; - cache_meta_data(); - } - - void call(const Info &log_info) override - { - IOH_DBG(debug, "FlatFile called"); - if (requires_header_) - { - IOH_DBG(xdebug, "print header") - out_ << com_ + common_header_ + format("{}", fmt::join(properties_vector_, sep_)); - if (store_positions_) - for (size_t i = 0; i < log_info.x.size(); i++) - out_ << sep_ << "x" << i; - out_ << eol_; - requires_header_ = false; - } - - IOH_DBG(xdebug, "print problem meta data") - out_ << current_meta_data_; - - IOH_DBG(xdebug, "print watched properties") - - for (auto p = properties_vector_.begin(); p != properties_vector_.end();){ - out_ << p->get().call_to_string(log_info, nan_) << (++p != properties_vector_.end() ? sep_ : ""); - } - - if (store_positions_) - out_ << sep_ << format("{:f}", fmt::join(log_info.x, sep_)); - - out_ << eol_; - out_.flush(); - } - - //! Accessor for output directory - virtual fs::path output_directory() const { return output_directory_; } - - //! Accessor for filename - std::string filename() const { return filename_; } - - //! close data file - virtual void close() override { - if (out_.is_open()){ - IOH_DBG(debug, "close data file") - out_.close(); - } - } - - virtual ~FlatFile() - { - close(); - } - - private: - /** - * \brief Caches the meta data logged at every row. If log_meta_data is false, - * it will store an empty string. - */ - void cache_meta_data() - { - current_meta_data_.clear(); - if (log_meta_data_) - { - std::stringstream ss; - ss /* no sep */ << suite_; - ss << sep_ << problem_.value().name; - ss << sep_ << problem_.value().problem_id; - ss << sep_ << problem_.value().instance; - ss << sep_ << (problem_.value().optimization_type == common::OptimizationType::MIN ? "min" : "max"); - ss << sep_ << problem_.value().n_variables; - ss << sep_ << current_run_; - current_meta_data_ = ss.str(); - } - } - }; -} // namespace ioh::logger +#pragma once + +#include "ioh/common/format.hpp" +#include "ioh/problem/single.hpp" + +namespace ioh::logger +{ + /** A logger that stores some information in a single, tabular-like, file. + * + * Each line displays the problem metadata and the watched properties. + * This format displays a lot of redundant information, but is very easy to parse. + * + * @code + logger::FlatFile( + {trigger::always}, + {watch::evaluations, watch::transformed_y}, + "my_experiment.dat", + "./today/" + ); + * @endcode + * + * @ingroup Loggers + */ + class FlatFile : public Watcher + { + protected: + //! Seperator + const std::string sep_; + + //! Comment character + const std::string com_; + + //! EOL character + const std::string eol_; + + //! NAN string + const std::string nan_; + + //! Common header + const std::string common_header_; + + //! Repeat header for every run? + const bool repeat_header_; + + //! Store x positions? + const bool store_positions_; + + //! Requires header? + bool requires_header_; + + //! Log meta data? + const bool log_meta_data_; + + //! Output directory + fs::path output_directory_; + + //! Filename + std::string filename_; + + //! Output stream + std::ofstream out_; + + + + //! Current run + size_t current_run_; + + //! Current meta data + std::string current_meta_data_; + + //! Open a file + void open_stream(const std::string &filename, const fs::path &output_directory) + { + if (filename != filename_) + { + filename_ = filename; + out_.close(); + } + if (output_directory_ != output_directory) + { + output_directory_ = output_directory; + if (not exists(output_directory_)) + { + IOH_DBG(debug, "some directories do not exist in " << output_directory_ << ", try to create them") + create_directories(output_directory_); + } + out_.close(); + } + if (!out_.is_open()) + { + IOH_DBG(debug, "will output data in " << output_directory_ / filename_) + out_ = std::ofstream(output_directory_ / filename_, std::ofstream::out | std::ofstream::app); + requires_header_ = true; + } + } + + public: + /** The logger should at least track one logger::Property, or else it makes no sense to use it. + * + * @param triggers When to fire a log event. + * @param properties What to log. + * @param filename File name in which to write the logged properties. + * @param output_directory Directory in which to put the data file. + * @param separator The string separating fields. + * @param comment The string indicating a comment. + * @param no_value The string indicating that a watched property does not exists in this context. + * @param end_of_line The string to use when all fields have been written. + * @param repeat_header If true, the commented header is printed for each new run. + * @param store_positions Whether to store x positions in the logged data + * @param common_header_titles Seven strings to print in the header for the common problem meta data (property + * names are automatically printed after). + */ + FlatFile(std::vector> triggers, + std::vector> properties, const std::string &filename = "IOH.dat", + const fs::path &output_directory = fs::current_path(), const std::string &separator = "\t", + const std::string &comment = "# ", const std::string &no_value = "None", + const std::string &end_of_line = "\n", const bool repeat_header = false, + const bool store_positions = false, + const std::vector &common_header_titles = {"suite_name", "problem_name", "problem_id", + "problem_instance", "optimization_type", + "dimension", "run"}) : + Watcher(triggers, properties), + sep_(separator), com_(comment), eol_(end_of_line), nan_(no_value), + common_header_(format("{}", fmt::join(common_header_titles.begin(), common_header_titles.end(), sep_)) + + (common_header_titles.empty() ? "" : sep_)), + repeat_header_(repeat_header), store_positions_(store_positions), requires_header_(true), + log_meta_data_(!common_header_titles.empty()), output_directory_(output_directory), filename_(filename), + current_run_(0), current_meta_data_{} + { + assert(common_header_titles.empty() || common_header_titles.size() == 7); + } + + + + void attach_problem(const problem::MetaData &problem) override + { + // If this is a new problem. + if (!problem_.has_value() or problem_.value() != problem) + { + IOH_DBG(xdebug, "reset run counter") + current_run_ = 0; // Then reset the run counter. + } + else + { + IOH_DBG(xdebug, "increment run counter") + current_run_++; // Then it's a new run. + } + + Logger::attach_problem(problem); + open_stream(filename_, output_directory_); + + requires_header_ = requires_header_ or repeat_header_; + cache_meta_data(); + } + + void call(const Info &log_info) override + { + IOH_DBG(debug, "FlatFile called"); + if (requires_header_) + { + IOH_DBG(xdebug, "print header") + out_ << com_ + common_header_ + format("{}", fmt::join(properties_vector_, sep_)); + if (store_positions_) + for (size_t i = 0; i < log_info.x.size(); i++) + out_ << sep_ << "x" << i; + out_ << eol_; + requires_header_ = false; + } + + IOH_DBG(xdebug, "print problem meta data") + out_ << current_meta_data_; + + IOH_DBG(xdebug, "print watched properties") + + for (auto p = properties_vector_.begin(); p != properties_vector_.end();){ + out_ << p->get().call_to_string(log_info, nan_) << (++p != properties_vector_.end() ? sep_ : ""); + } + + if (store_positions_) + out_ << sep_ << format("{:f}", fmt::join(log_info.x, sep_)); + + out_ << eol_; + out_.flush(); + } + + //! Accessor for output directory + virtual fs::path output_directory() const { return output_directory_; } + + //! Accessor for filename + std::string filename() const { return filename_; } + + //! close data file + virtual void close() override { + if (out_.is_open()){ + IOH_DBG(debug, "close data file") + out_.close(); + } + } + + virtual ~FlatFile() + { + close(); + } + + private: + /** + * \brief Caches the meta data logged at every row. If log_meta_data is false, + * it will store an empty string. Otherwise, the string ends with a separator. + */ + void cache_meta_data() + { + current_meta_data_.clear(); + if (log_meta_data_) + { + std::stringstream ss; + ss /* no sep */ << suite_; + ss << sep_ << problem_.value().name; + ss << sep_ << problem_.value().problem_id; + ss << sep_ << problem_.value().instance; + 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(); + } + } + }; +} // namespace ioh::logger diff --git a/tests/python/test_logger.py b/tests/python/test_logger.py index e4be36164..a37172ad0 100644 --- a/tests/python/test_logger.py +++ b/tests/python/test_logger.py @@ -24,7 +24,14 @@ def test_flatfile(self): pr.attach_logger(l) self.assertEqual(os.path.realpath("./"), os.path.realpath(l.output_directory)) pr([0] * 5) + l.close() self.assertTrue(os.path.isfile("IOH.dat")) + 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(" "))) + os.remove("IOH.dat") def test_analyzer(self): pr = ioh.get_problem("Sphere", 1, 5) From d3d75f699cc4bb503ce3e4c3e3a27fd14499c5e8 Mon Sep 17 00:00:00 2001 From: jacobdenobel Date: Thu, 5 Nov 2026 17:48:46 +0100 Subject: [PATCH 3/5] E02: restore CRLF in flatfile.hpp, move the changelog entry to Unreleased Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 20 +- WP_NOTES.md | 3 + include/ioh/logger/flatfile.hpp | 458 ++++++++++++++++---------------- 3 files changed, 244 insertions(+), 237 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6b82dd44..7c2415b11 100644 --- a/CHANGELOG.md +++ b/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 @@ -82,14 +94,6 @@ change. The ids, names and values of the built-in problems are unchanged, apart * `Experiment.add_custom_problem` failed when the problem ids were a numpy array (e.g. a suite's `problem_ids`). * `logger.property.Property(container, attribute)` raised `SystemError` for an attribute that is not a number; it raises `TypeError`. -* `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. iohinspector's reader flags these - files. ### C++ diff --git a/WP_NOTES.md b/WP_NOTES.md index 672f61b93..fd3ccaae7 100644 --- a/WP_NOTES.md +++ b/WP_NOTES.md @@ -4,6 +4,7 @@ Status: ready for review ## Done - step 1: failing tests (meta data on: run and first property glued, e.g. "01"; meta data off passes) in tests/cpp/logger/test_flatfile.cpp +- step 3 (review): CRLF restored in flatfile.hpp, changelog entry moved to Unreleased, iohinspector sentence removed. - step 2: `cache_meta_data()` now ends with `sep_` (include/ioh/logger/flatfile.hpp); CHANGELOG.md entry under 0.4.0 / Fixed; Python test in tests/python/test_logger.py checks field counts (not run: no built ioh module here). `test_flatfile`: 4 tests pass (Debug build). ## Next step @@ -13,6 +14,8 @@ None. Controller turns this file into the PR body. - Test target: `test_flatfile` (cmake -DIOH_TEST_PER_FILE=ON -DCMAKE_BUILD_TYPE=Debug), build dir `build-e02` (excluded via .git/info/exclude). Run: `build-e02/tests/test_flatfile Warning ".*" 99999` (the arguments are needed). Release (NDEBUG) builds crash in SetUp (clutchlog no-op `logger()` returns nothing, a separate pre-existing issue, not fixed here). - Submodules fmt and googletest were initialised in the worktree (`git submodule update --init`). +- The changelog entry does not mention iohinspector; add the cross-reference ("iohinspector's reader flags these files") once that reader ships. + ## Open questions / Blocked - none - Other variants checked: no meta data (empty titles) was correct; store_positions was correct (sep before x columns); header is correct. Nothing else in the repo parses FlatFile output. diff --git a/include/ioh/logger/flatfile.hpp b/include/ioh/logger/flatfile.hpp index 06b9b845a..d520c0840 100644 --- a/include/ioh/logger/flatfile.hpp +++ b/include/ioh/logger/flatfile.hpp @@ -1,229 +1,229 @@ -#pragma once - -#include "ioh/common/format.hpp" -#include "ioh/problem/single.hpp" - -namespace ioh::logger -{ - /** A logger that stores some information in a single, tabular-like, file. - * - * Each line displays the problem metadata and the watched properties. - * This format displays a lot of redundant information, but is very easy to parse. - * - * @code - logger::FlatFile( - {trigger::always}, - {watch::evaluations, watch::transformed_y}, - "my_experiment.dat", - "./today/" - ); - * @endcode - * - * @ingroup Loggers - */ - class FlatFile : public Watcher - { - protected: - //! Seperator - const std::string sep_; - - //! Comment character - const std::string com_; - - //! EOL character - const std::string eol_; - - //! NAN string - const std::string nan_; - - //! Common header - const std::string common_header_; - - //! Repeat header for every run? - const bool repeat_header_; - - //! Store x positions? - const bool store_positions_; - - //! Requires header? - bool requires_header_; - - //! Log meta data? - const bool log_meta_data_; - - //! Output directory - fs::path output_directory_; - - //! Filename - std::string filename_; - - //! Output stream - std::ofstream out_; - - - - //! Current run - size_t current_run_; - - //! Current meta data - std::string current_meta_data_; - - //! Open a file - void open_stream(const std::string &filename, const fs::path &output_directory) - { - if (filename != filename_) - { - filename_ = filename; - out_.close(); - } - if (output_directory_ != output_directory) - { - output_directory_ = output_directory; - if (not exists(output_directory_)) - { - IOH_DBG(debug, "some directories do not exist in " << output_directory_ << ", try to create them") - create_directories(output_directory_); - } - out_.close(); - } - if (!out_.is_open()) - { - IOH_DBG(debug, "will output data in " << output_directory_ / filename_) - out_ = std::ofstream(output_directory_ / filename_, std::ofstream::out | std::ofstream::app); - requires_header_ = true; - } - } - - public: - /** The logger should at least track one logger::Property, or else it makes no sense to use it. - * - * @param triggers When to fire a log event. - * @param properties What to log. - * @param filename File name in which to write the logged properties. - * @param output_directory Directory in which to put the data file. - * @param separator The string separating fields. - * @param comment The string indicating a comment. - * @param no_value The string indicating that a watched property does not exists in this context. - * @param end_of_line The string to use when all fields have been written. - * @param repeat_header If true, the commented header is printed for each new run. - * @param store_positions Whether to store x positions in the logged data - * @param common_header_titles Seven strings to print in the header for the common problem meta data (property - * names are automatically printed after). - */ - FlatFile(std::vector> triggers, - std::vector> properties, const std::string &filename = "IOH.dat", - const fs::path &output_directory = fs::current_path(), const std::string &separator = "\t", - const std::string &comment = "# ", const std::string &no_value = "None", - const std::string &end_of_line = "\n", const bool repeat_header = false, - const bool store_positions = false, - const std::vector &common_header_titles = {"suite_name", "problem_name", "problem_id", - "problem_instance", "optimization_type", - "dimension", "run"}) : - Watcher(triggers, properties), - sep_(separator), com_(comment), eol_(end_of_line), nan_(no_value), - common_header_(format("{}", fmt::join(common_header_titles.begin(), common_header_titles.end(), sep_)) + - (common_header_titles.empty() ? "" : sep_)), - repeat_header_(repeat_header), store_positions_(store_positions), requires_header_(true), - log_meta_data_(!common_header_titles.empty()), output_directory_(output_directory), filename_(filename), - current_run_(0), current_meta_data_{} - { - assert(common_header_titles.empty() || common_header_titles.size() == 7); - } - - - - void attach_problem(const problem::MetaData &problem) override - { - // If this is a new problem. - if (!problem_.has_value() or problem_.value() != problem) - { - IOH_DBG(xdebug, "reset run counter") - current_run_ = 0; // Then reset the run counter. - } - else - { - IOH_DBG(xdebug, "increment run counter") - current_run_++; // Then it's a new run. - } - - Logger::attach_problem(problem); - open_stream(filename_, output_directory_); - - requires_header_ = requires_header_ or repeat_header_; - cache_meta_data(); - } - - void call(const Info &log_info) override - { - IOH_DBG(debug, "FlatFile called"); - if (requires_header_) - { - IOH_DBG(xdebug, "print header") - out_ << com_ + common_header_ + format("{}", fmt::join(properties_vector_, sep_)); - if (store_positions_) - for (size_t i = 0; i < log_info.x.size(); i++) - out_ << sep_ << "x" << i; - out_ << eol_; - requires_header_ = false; - } - - IOH_DBG(xdebug, "print problem meta data") - out_ << current_meta_data_; - - IOH_DBG(xdebug, "print watched properties") - - for (auto p = properties_vector_.begin(); p != properties_vector_.end();){ - out_ << p->get().call_to_string(log_info, nan_) << (++p != properties_vector_.end() ? sep_ : ""); - } - - if (store_positions_) - out_ << sep_ << format("{:f}", fmt::join(log_info.x, sep_)); - - out_ << eol_; - out_.flush(); - } - - //! Accessor for output directory - virtual fs::path output_directory() const { return output_directory_; } - - //! Accessor for filename - std::string filename() const { return filename_; } - - //! close data file - virtual void close() override { - if (out_.is_open()){ - IOH_DBG(debug, "close data file") - out_.close(); - } - } - - virtual ~FlatFile() - { - close(); - } - - private: - /** - * \brief Caches the meta data logged at every row. If log_meta_data is false, - * it will store an empty string. Otherwise, the string ends with a separator. - */ - void cache_meta_data() - { - current_meta_data_.clear(); - if (log_meta_data_) - { - std::stringstream ss; - ss /* no sep */ << suite_; - ss << sep_ << problem_.value().name; - ss << sep_ << problem_.value().problem_id; - ss << sep_ << problem_.value().instance; - 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(); - } - } - }; -} // namespace ioh::logger +#pragma once + +#include "ioh/common/format.hpp" +#include "ioh/problem/single.hpp" + +namespace ioh::logger +{ + /** A logger that stores some information in a single, tabular-like, file. + * + * Each line displays the problem metadata and the watched properties. + * This format displays a lot of redundant information, but is very easy to parse. + * + * @code + logger::FlatFile( + {trigger::always}, + {watch::evaluations, watch::transformed_y}, + "my_experiment.dat", + "./today/" + ); + * @endcode + * + * @ingroup Loggers + */ + class FlatFile : public Watcher + { + protected: + //! Seperator + const std::string sep_; + + //! Comment character + const std::string com_; + + //! EOL character + const std::string eol_; + + //! NAN string + const std::string nan_; + + //! Common header + const std::string common_header_; + + //! Repeat header for every run? + const bool repeat_header_; + + //! Store x positions? + const bool store_positions_; + + //! Requires header? + bool requires_header_; + + //! Log meta data? + const bool log_meta_data_; + + //! Output directory + fs::path output_directory_; + + //! Filename + std::string filename_; + + //! Output stream + std::ofstream out_; + + + + //! Current run + size_t current_run_; + + //! Current meta data + std::string current_meta_data_; + + //! Open a file + void open_stream(const std::string &filename, const fs::path &output_directory) + { + if (filename != filename_) + { + filename_ = filename; + out_.close(); + } + if (output_directory_ != output_directory) + { + output_directory_ = output_directory; + if (not exists(output_directory_)) + { + IOH_DBG(debug, "some directories do not exist in " << output_directory_ << ", try to create them") + create_directories(output_directory_); + } + out_.close(); + } + if (!out_.is_open()) + { + IOH_DBG(debug, "will output data in " << output_directory_ / filename_) + out_ = std::ofstream(output_directory_ / filename_, std::ofstream::out | std::ofstream::app); + requires_header_ = true; + } + } + + public: + /** The logger should at least track one logger::Property, or else it makes no sense to use it. + * + * @param triggers When to fire a log event. + * @param properties What to log. + * @param filename File name in which to write the logged properties. + * @param output_directory Directory in which to put the data file. + * @param separator The string separating fields. + * @param comment The string indicating a comment. + * @param no_value The string indicating that a watched property does not exists in this context. + * @param end_of_line The string to use when all fields have been written. + * @param repeat_header If true, the commented header is printed for each new run. + * @param store_positions Whether to store x positions in the logged data + * @param common_header_titles Seven strings to print in the header for the common problem meta data (property + * names are automatically printed after). + */ + FlatFile(std::vector> triggers, + std::vector> properties, const std::string &filename = "IOH.dat", + const fs::path &output_directory = fs::current_path(), const std::string &separator = "\t", + const std::string &comment = "# ", const std::string &no_value = "None", + const std::string &end_of_line = "\n", const bool repeat_header = false, + const bool store_positions = false, + const std::vector &common_header_titles = {"suite_name", "problem_name", "problem_id", + "problem_instance", "optimization_type", + "dimension", "run"}) : + Watcher(triggers, properties), + sep_(separator), com_(comment), eol_(end_of_line), nan_(no_value), + common_header_(format("{}", fmt::join(common_header_titles.begin(), common_header_titles.end(), sep_)) + + (common_header_titles.empty() ? "" : sep_)), + repeat_header_(repeat_header), store_positions_(store_positions), requires_header_(true), + log_meta_data_(!common_header_titles.empty()), output_directory_(output_directory), filename_(filename), + current_run_(0), current_meta_data_{} + { + assert(common_header_titles.empty() || common_header_titles.size() == 7); + } + + + + void attach_problem(const problem::MetaData &problem) override + { + // If this is a new problem. + if (!problem_.has_value() or problem_.value() != problem) + { + IOH_DBG(xdebug, "reset run counter") + current_run_ = 0; // Then reset the run counter. + } + else + { + IOH_DBG(xdebug, "increment run counter") + current_run_++; // Then it's a new run. + } + + Logger::attach_problem(problem); + open_stream(filename_, output_directory_); + + requires_header_ = requires_header_ or repeat_header_; + cache_meta_data(); + } + + void call(const Info &log_info) override + { + IOH_DBG(debug, "FlatFile called"); + if (requires_header_) + { + IOH_DBG(xdebug, "print header") + out_ << com_ + common_header_ + format("{}", fmt::join(properties_vector_, sep_)); + if (store_positions_) + for (size_t i = 0; i < log_info.x.size(); i++) + out_ << sep_ << "x" << i; + out_ << eol_; + requires_header_ = false; + } + + IOH_DBG(xdebug, "print problem meta data") + out_ << current_meta_data_; + + IOH_DBG(xdebug, "print watched properties") + + for (auto p = properties_vector_.begin(); p != properties_vector_.end();){ + out_ << p->get().call_to_string(log_info, nan_) << (++p != properties_vector_.end() ? sep_ : ""); + } + + if (store_positions_) + out_ << sep_ << format("{:f}", fmt::join(log_info.x, sep_)); + + out_ << eol_; + out_.flush(); + } + + //! Accessor for output directory + virtual fs::path output_directory() const { return output_directory_; } + + //! Accessor for filename + std::string filename() const { return filename_; } + + //! close data file + virtual void close() override { + if (out_.is_open()){ + IOH_DBG(debug, "close data file") + out_.close(); + } + } + + virtual ~FlatFile() + { + close(); + } + + private: + /** + * \brief Caches the meta data logged at every row. If log_meta_data is false, + * it will store an empty string. Otherwise, the string ends with a separator. + */ + void cache_meta_data() + { + current_meta_data_.clear(); + if (log_meta_data_) + { + std::stringstream ss; + ss /* no sep */ << suite_; + ss << sep_ << problem_.value().name; + ss << sep_ << problem_.value().problem_id; + ss << sep_ << problem_.value().instance; + 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(); + } + } + }; +} // namespace ioh::logger From 549b20b3754524550e4a27fbcd93a16cf56f6a2a Mon Sep 17 00:00:00 2001 From: jacobdenobel Date: Fri, 6 Nov 2026 11:56:32 +0100 Subject: [PATCH 4/5] E02: remove WP_NOTES.md, ready for review Co-Authored-By: Claude Opus 5.5 --- WP_NOTES.md | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 WP_NOTES.md diff --git a/WP_NOTES.md b/WP_NOTES.md deleted file mode 100644 index fd3ccaae7..000000000 --- a/WP_NOTES.md +++ /dev/null @@ -1,22 +0,0 @@ -# WP E02: FlatFile separator bug -Branch: wp/E02-flatfile-separator -Status: ready for review - -## Done -- step 1: failing tests (meta data on: run and first property glued, e.g. "01"; meta data off passes) in tests/cpp/logger/test_flatfile.cpp -- step 3 (review): CRLF restored in flatfile.hpp, changelog entry moved to Unreleased, iohinspector sentence removed. -- step 2: `cache_meta_data()` now ends with `sep_` (include/ioh/logger/flatfile.hpp); CHANGELOG.md entry under 0.4.0 / Fixed; Python test in tests/python/test_logger.py checks field counts (not run: no built ioh module here). `test_flatfile`: 4 tests pass (Debug build). - -## Next step -None. Controller turns this file into the PR body. - -## Decisions -- Test target: `test_flatfile` (cmake -DIOH_TEST_PER_FILE=ON -DCMAKE_BUILD_TYPE=Debug), build dir `build-e02` (excluded via .git/info/exclude). Run: `build-e02/tests/test_flatfile Warning ".*" 99999` (the arguments are needed). Release (NDEBUG) builds crash in SetUp (clutchlog no-op `logger()` returns nothing, a separate pre-existing issue, not fixed here). -- Submodules fmt and googletest were initialised in the worktree (`git submodule update --init`). - -- The changelog entry does not mention iohinspector; add the cross-reference ("iohinspector's reader flags these files") once that reader ships. - -## Open questions / Blocked -- none -- Other variants checked: no meta data (empty titles) was correct; store_positions was correct (sep before x columns); header is correct. Nothing else in the repo parses FlatFile output. -- Same bug exists in all earlier versions (cache_meta_data unchanged since the logger was added). From 4e3e4df366feedabbcf652189d3f5b1aa9a4349e Mon Sep 17 00:00:00 2001 From: jacobdenobel Date: Fri, 6 Nov 2026 12:18:40 +0100 Subject: [PATCH 5/5] E02: Python FlatFile test reads the flushed file instead of calling close Co-Authored-By: Claude Sonnet 5.5 --- tests/python/test_logger.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/python/test_logger.py b/tests/python/test_logger.py index a37172ad0..57ba20249 100644 --- a/tests/python/test_logger.py +++ b/tests/python/test_logger.py @@ -24,14 +24,13 @@ def test_flatfile(self): pr.attach_logger(l) self.assertEqual(os.path.realpath("./"), os.path.realpath(l.output_directory)) pr([0] * 5) - l.close() 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(" "))) - os.remove("IOH.dat") def test_analyzer(self): pr = ioh.get_problem("Sphere", 1, 5)