BREAKING: restructure evalable api/formatting

the new layout is less likely to cause problems with dangling references

src/evalable.cpp

@@ -6,35 +6,50 @@
 
 namespace loadl {
 
-evalable::evalable(std::string name, std::vector<std::string> used_observables, func fun)
-    : name_{std::move(name)}, used_observables_{std::move(used_observables)}, fun_{std::move(fun)} {
+evaluator::evaluator(results &res) : res_{res} {}
+
+void evaluator::evaluate(const std::string &name, const std::vector<std::string> &used_observables,
+                         func fun) {
 	// evalable names also should be valid HDF5 paths
 	if(not measurements::observable_name_is_legal(name)) {
 		throw std::runtime_error{
 		    fmt::format("illegal evalable name '{}': must not contain . or /", name)};
 	}
+
+	observable_result o = jackknife(name, used_observables, fun);
+
+	// don’t include empty results
+	if(o.rebinning_bin_count > 0) {
+		evalable_results_.emplace_back(o);
+	}
 }
 
-const std::string &evalable::name() const {
-	return name_;
+void evaluator::append_results() {
+	for(auto &eval : evalable_results_) {
+		res_.observables.emplace(eval.name, eval);
+	}
 }
 
-void evalable::jackknife(const results &res, observable_result &obs_res) const {
+observable_result evaluator::jackknife(const std::string &name,
+                                       const std::vector<std::string> &used_observables,
+                                       func fun) const {
+	observable_result obs_res;
+
 	std::vector<observable_result> observables;
-	observables.reserve(used_observables_.size());
+	observables.reserve(used_observables.size());
 
-	obs_res.name = name_;
+	obs_res.name = name;
 
 	size_t bin_count = -1; // maximal value
-	for(const auto &obs_name : used_observables_) {
-		if(res.observables.count(obs_name) <= 0) {
+	for(const auto &obs_name : used_observables) {
+		if(res_.observables.count(obs_name) <= 0) {
 			std::cerr << fmt::format(
 			    "Warning: evalable '{}': used observable '{}' not found in Monte Carlo results. "
 			    "Skipping...\n",
-			    name_, obs_name);
-			return;
+			    name, obs_name);
+			return obs_res;
 		}
-		const auto &obs = res.observables.at(obs_name);
+		const auto &obs = res_.observables.at(obs_name);
 
 		if(obs.rebinning_bin_count < bin_count) {
 			bin_count = obs.rebinning_bin_count;
@@ -42,11 +57,11 @@ void evalable::jackknife(const results &res, observable_result &obs_res) const {
 
 		observables.emplace_back(obs);
 	}
-	obs_res.rebinning_bin_count = bin_count;
 
 	if(bin_count == 0) {
-		return;
+		return obs_res;
 	}
+	obs_res.rebinning_bin_count = bin_count;
 
 	std::vector<std::vector<double>> jacked_means(observables.size());
 	std::vector<std::vector<double>> sums(observables.size());
@@ -77,14 +92,14 @@ void evalable::jackknife(const results &res, observable_result &obs_res) const {
 			}
 		}
 
-		std::vector<double> jacked_eval = fun_(jacked_means);
+		std::vector<double> jacked_eval = fun(jacked_means);
 		if(jacked_eval_mean.empty())
 			jacked_eval_mean.resize(jacked_eval.size(), 0);
 		if(jacked_eval_mean.size() != jacked_eval.size()) {
 			throw std::runtime_error(
 			    fmt::format("evalable '{}': evalables must not change their dimensions depending "
 			                "on the input",
-			                name_));
+			                name));
 		}
 
 		for(size_t i = 0; i < jacked_eval.size(); i++) {
@@ -104,7 +119,7 @@ void evalable::jackknife(const results &res, observable_result &obs_res) const {
 		}
 	}
 
-	std::vector<double> complete_eval = fun_(jacked_means);
+	std::vector<double> complete_eval = fun(jacked_means);
 	assert(complete_eval.size() == jacked_eval_mean.size());
 
 	// calculate bias-corrected jackknife estimator
@@ -127,7 +142,7 @@ void evalable::jackknife(const results &res, observable_result &obs_res) const {
 			}
 		}
 
-		std::vector<double> jacked_eval = fun_(jacked_means);
+		std::vector<double> jacked_eval = fun(jacked_means);
 		if(obs_res.error.empty()) {
 			obs_res.error.resize(jacked_eval.size(), 0);
 		}
@@ -139,5 +154,7 @@ void evalable::jackknife(const results &res, observable_result &obs_res) const {
 	for(size_t i = 0; i < obs_res.error.size(); i++) {
 		obs_res.error[i] = sqrt((bin_count - 1) * obs_res.error[i] / bin_count);
 	}
+
+	return obs_res;
 }
 }

src/evalable.h

@@ -7,7 +7,7 @@
 
 namespace loadl {
 
-class evalable {
+class evaluator {
 public:
 	// Internally all observables are vectors, so you need a function
 	//
@@ -19,13 +19,19 @@ public:
 	typedef std::function<std::vector<double>(const std::vector<std::vector<double>> &observables)>
 	    func;
 
-	evalable(std::string name, std::vector<std::string> used_observables, func fun);
-	const std::string &name() const;
-	void jackknife(const results &res, observable_result &out) const;
+	evaluator(results &res);
+	void evaluate(const std::string &name, const std::vector<std::string> &used_observables,
+	              func fun);
+
+	// appends the evalable results to the other observables in res
+	void append_results();
 
 private:
-	const std::string name_;
-	const std::vector<std::string> used_observables_;
-	const func fun_;
+	std::vector<observable_result> evalable_results_;
+	results &res_;
+
+	observable_result jackknife(const std::string &name,
+	                            const std::vector<std::string> &used_observables, func fun) const;
 };
+
 }

src/jobinfo.cpp

 #include "jobinfo.h"
+#include "mc.h"
 #include "merger.h"
 #include <ctime>
 #include <dirent.h>
@@ -148,11 +149,17 @@ void jobinfo::concatenate_results() {
 	cat_results << "]\n";
 }
 
-void jobinfo::merge_task(int task_id, const std::vector<evalable> &evalables) {
+void jobinfo::merge_task(int task_id, const mc_factory &mccreator) {
+	std::unique_ptr<mc> sys{mccreator(jobfile["tasks"][task_names[task_id]])};
+
 	std::vector<std::string> meas_files = list_run_files(taskdir(task_id), "meas\\.h5");
 	size_t rebinning_bin_length = jobfile["jobconfig"].get<size_t>("merge_rebin_length", 0);
 	size_t sample_skip = jobfile["jobconfig"].get<size_t>("merge_sample_skip", 0);
-	results results = merge(meas_files, evalables, rebinning_bin_length, sample_skip);
+	results results = merge(meas_files, rebinning_bin_length, sample_skip);
+
+	evaluator eval{results};
+	sys->register_evalables(eval);
+	eval.append_results();
 
 	std::string result_filename = fmt::format("{}/results.json", taskdir(task_id));
 	const std::string &task_name = task_names.at(task_id);

src/jobinfo.h

@@ -2,6 +2,7 @@
 
 #include "evalable.h"
 #include "iodump.h"
+#include "mc.h"
 #include "parser.h"
 #include <string>
 #include <vector>
@@ -26,7 +27,7 @@ struct jobinfo {
 	static std::vector<std::string> list_run_files(const std::string &taskdir,
 	                                               const std::string &file_ending);
 	int read_dump_progress(int task_id) const;
-	void merge_task(int task_id, const std::vector<evalable> &evalables);
+	void merge_task(int task_id, const mc_factory &mccreator);
 	void concatenate_results();
 	void log(const std::string &message);
 };

src/loadleveller.h

@@ -9,10 +9,7 @@ namespace loadl {
 
 inline int merge_only(jobinfo job, const mc_factory &mccreator, int, char **) {
 	for(size_t task_id = 0; task_id < job.task_names.size(); task_id++) {
-		std::vector<evalable> evalables;
-		std::unique_ptr<mc> sys{mccreator(job.jobfile["tasks"][job.task_names[task_id]])};
-		sys->register_evalables(evalables);
-		job.merge_task(task_id, evalables);
+		job.merge_task(task_id, mccreator);
 
 		std::cout << fmt::format("-- {} merged\n", job.taskdir(task_id));
 	}

src/mc.h

@@ -42,7 +42,7 @@ public:
 
 	size_t sweep() const;
 
-	virtual void register_evalables(std::vector<evalable> &evalables) = 0;
+	virtual void register_evalables(evaluator &evalables) = 0;
 	virtual void write_output(const std::string &filename);
 
 	// these functions do a little more, like taking care of the

src/measurements.cpp

@@ -3,9 +3,7 @@
 #include <mpi.h>
 namespace loadl {
 
-measurements::measurements(size_t default_bin_size)
-	: default_bin_size_{default_bin_size} {
-}
+measurements::measurements(size_t default_bin_size) : default_bin_size_{default_bin_size} {}
 
 bool measurements::observable_name_is_legal(const std::string &obs_name) {
 	if(obs_name.find('/') != obs_name.npos) {
@@ -24,8 +22,7 @@ void measurements::register_observable(const std::string &name, size_t bin_size)
 	}
 
 	if(observables_.count(name) > 0) {
-		throw std::runtime_error(
-			fmt::format("Observable '{}' already exists.", name));
+		throw std::runtime_error(fmt::format("Observable '{}' already exists.", name));
 	}
 
 	observables_.emplace(name, observable{name, bin_size, 0});
@@ -39,7 +36,8 @@ void measurements::checkpoint_write(const iodump::group &dump_file) {
 
 void measurements::checkpoint_read(const iodump::group &dump_file) {
 	for(const auto &obs_name : dump_file) {
-		observables_.emplace(obs_name, observable::checkpoint_read(obs_name, dump_file.open_group(obs_name)));
+		observables_.emplace(obs_name,
+		                     observable::checkpoint_read(obs_name, dump_file.open_group(obs_name)));
 	}
 }
 

src/measurements.h

@@ -42,8 +42,8 @@ private:
 	const size_t default_bin_size_{1};
 
 	template<class T>
-	size_t value_length(const T& val) {
-		if constexpr (std::is_arithmetic_v<T>) {
+	size_t value_length(const T &val) {
+		if constexpr(std::is_arithmetic_v<T>) {
 			return 1;
 		} else {
 			return val.size();

src/merger.cpp

 #include "merger.h"
-#include "evalable.h"
 #include "iodump.h"
 #include "mc.h"
 #include "measurements.h"
@@ -10,25 +9,8 @@
 
 namespace loadl {
 
-static void evaluate_evalables(results &res, const std::vector<evalable> &evalables) {
-	std::vector<observable_result> evalable_results;
-	for(auto &eval : evalables) {
-		observable_result o;
-		eval.jackknife(res, o);
-
-		// don’t include empty results
-		if(o.rebinning_bin_count > 0) {
-			evalable_results.emplace_back(o);
-		}
-	}
-
-	for(auto &eval : evalable_results) {
-		res.observables.emplace(eval.name, eval);
-	}
-}
-
-results merge(const std::vector<std::string> &filenames, const std::vector<evalable> &evalables,
-              size_t rebinning_bin_length, size_t sample_skip) {
+results merge(const std::vector<std::string> &filenames, size_t rebinning_bin_length,
+              size_t sample_skip) {
 	results res;
 
 	// This thing reads the complete time series of an observable which will
@@ -214,8 +196,6 @@ results merge(const std::vector<std::string> &filenames, const std::vector<evala
 		}
 	}
 
-	evaluate_evalables(res, evalables);
-
 	return res;
 }
 }

src/merger.h

@@ -6,6 +6,6 @@
 namespace loadl {
 
 // if rebinning_bin_length is 0, cbrt(total_sample_count) is used as default.
-results merge(const std::vector<std::string> &filenames, const std::vector<evalable> &evalables,
-              size_t rebinning_bin_length = 0, size_t skip = 0);
+results merge(const std::vector<std::string> &filenames, size_t rebinning_bin_length = 0,
+              size_t skip = 0);
 }

src/observable.cpp

@@ -50,7 +50,7 @@ observable observable::checkpoint_read(const std::string &name, const iodump::gr
 	size_t vector_length, bin_length;
 	d.read("vector_length", vector_length);
 	d.read("bin_length", bin_length);
-	
+
 	observable obs{name, bin_length, vector_length};
 	d.read("current_bin_filling", obs.current_bin_filling_);
 	d.read("samples", obs.samples_);

src/observable.h

@@ -14,11 +14,12 @@ public:
 
 	const std::string &name() const;
 
-	template<class T, std::enable_if_t<std::is_arithmetic_v<std::remove_reference_t<T>>> * = nullptr>
+	template<class T,
+	         std::enable_if_t<std::is_arithmetic_v<std::remove_reference_t<T>>> * = nullptr>
 	void add(T val);
 
 	template<class T>
-	auto add(const T& val) -> decltype(val[0], void());
+	auto add(const T &val) -> decltype(val[0], void());
 
 	void checkpoint_write(const iodump::group &dump_file) const;
 
@@ -30,6 +31,7 @@ public:
 	// switch copy with target rank.
 	// useful for parallel tempering mode
 	void mpi_sendrecv(int target_rank);
+
 private:
 	static const size_t initial_bin_length = 1000;
 
@@ -44,24 +46,24 @@ private:
 
 template<class T, std::enable_if_t<std::is_arithmetic_v<std::remove_reference_t<T>>> * = nullptr>
 void observable::add(T val) {
-	add(std::array<T,1>{val});
+	add(std::array<T, 1>{val});
 }
 
-
 template<class T>
 auto observable::add(const T &val) -> decltype(val[0], void()) {
 	if(val.size() == 0) {
 		throw std::runtime_error("observable::add: tried to add zero-length value.");
 	}
-		
+
 	if(vector_length_ != val.size()) {
 		if(vector_length_ != 0) {
-			throw std::runtime_error{
-			    fmt::format("observable::add: added vector has inconsistent size ({}). Observable was "
-			                "initialized with vector length ({})",
-			                val.size(), vector_length_)};
+			throw std::runtime_error{fmt::format(
+			    "observable::add: added vector has inconsistent size ({}). Observable was "
+			    "initialized with vector length ({})",
+			    val.size(), vector_length_)};
 		} else {
-			// when the variable is manually registered, it can happen that the vector length was not yet set.
+			// when the variable is manually registered, it can happen that the vector length was
+			// not yet set.
 			vector_length_ = val.size();
 			assert(samples_.size() == 0);
 			samples_.resize(vector_length_);

src/runner.cpp

@@ -262,8 +262,6 @@ void runner_slave::merge_measurements() {
 	std::string unique_filename = job_.taskdir(task_id_);
 	sys_->write_output(unique_filename);
 
-	std::vector<evalable> evalables;
-	sys_->register_evalables(evalables);
-	job_.merge_task(task_id_, evalables);
+	job_.merge_task(task_id_, mccreator_);
 }
 }

src/runner_pt.cpp

@@ -41,12 +41,12 @@ pt_chain_run::pt_chain_run(const pt_chain &chain, int run_id) : id{chain.id}, ru
 pt_chain_run pt_chain_run::checkpoint_read(const pt_chain &chain, const iodump::group &g) {
 	pt_chain_run run;
 	g.read("id", run.id);
-	assert(chain.id == run.id); 
+	assert(chain.id == run.id);
 	g.read("run_id", run.run_id);
 	uint8_t swap_odd;
 	g.read("swap_odd", swap_odd);
 	run.swap_odd = swap_odd;
-	
+
 	size_t size = chain.params.size();
 	run.weight_ratios.resize(size, -1);
 	run.switch_partners.resize(size);
@@ -262,7 +262,7 @@ void runner_pt_master::checkpoint_read() {
 			int id = std::stoi(name);
 			pt_chains_.at(id).checkpoint_read(pt_chains.open_group(name));
 		}
-		
+
 		auto pt_chain_runs = g.open_group("pt_chain_runs");
 		for(std::string name : pt_chain_runs) {
 			int id;
@@ -787,15 +787,7 @@ void runner_pt_slave::merge_measurements() {
 	std::string unique_filename = job_.taskdir(task_id_);
 	sys_->write_output(unique_filename);
 
-	std::vector<evalable> evalables;
-	if(job_.jobfile["jobconfig"].defined("pt_parameter_optimization")) {
-		if(rank_ == 1) {
-			job_.log("Running in parameter optimization mode. No evalables are calculated.");
-		}
-	} else {
-		sys_->register_evalables(evalables);
-	}
-	job_.merge_task(task_id_, evalables);
+	job_.merge_task(task_id_, mccreator_);
 }
 
 }

src/runner_single.cpp

@@ -97,10 +97,7 @@ void runner_single::merge_measurements() {
 	std::string unique_filename = job_.taskdir(task_id_);
 	sys_->write_output(unique_filename);
 
-	std::vector<evalable> evalables;
-	sys_->register_evalables(evalables);
-
 	job_.log(fmt::format("merging {}", job_.taskdir(task_id_)));
-	job_.merge_task(task_id_, evalables);
+	job_.merge_task(task_id_, mccreator_);
 }
 }

test/silly_mc/main.cpp

-#include "silly_mc.h"
 #include "loadleveller/loadleveller.h"
+#include "silly_mc.h"
 
 int main(int argc, char *argv[]) {
 	loadl::run<silly_mc>(argc, argv);

test/silly_mc/silly_mc.cpp

 #include "silly_mc.h"
 #include <valarray>
 
-silly_mc::silly_mc(const loadl::parser &p) : loadl::mc(p) {
-}
+silly_mc::silly_mc(const loadl::parser &p) : loadl::mc(p) {}
 
 void silly_mc::do_update() {
 	idx_++;
 }
 
 void silly_mc::do_measurement() {
-	std::vector<double> silly = {1.*idx_, 1.*(3-idx_%5)*idx_};
+	std::vector<double> silly = {1. * idx_, 1. * (3 - idx_ % 5) * idx_};
 	measure.add("MagicNumber", silly);
-	measure.add("MagicNumber2", idx_*idx_);
-	std::valarray<double> silly2 = {1.*idx_, 1.*(3-idx_%5)*idx_};
+	measure.add("MagicNumber2", idx_ * idx_);
+	std::valarray<double> silly2 = {1. * idx_, 1. * (3 - idx_ % 5) * idx_};
 	measure.add("MagicNumberValarray", silly2);
 }
 
 void silly_mc::init() {
-	idx_=0;
+	idx_ = 0;
 
 	measure.register_observable("MagicNumberValarray", 10);
-
 }
 
 void silly_mc::checkpoint_write(const loadl::iodump::group &d) {
@@ -31,14 +29,12 @@ void silly_mc::checkpoint_read(const loadl::iodump::group &d) {
 	d.read("idx", idx_);
 }
 
-void silly_mc::register_evalables(std::vector<loadl::evalable> &evalables) {
-	evalables.push_back(
-	    loadl::evalable{"AntimagicNumber",
-	                    {"MagicNumber", "MagicNumber2"},
-	                    [](const std::vector<std::vector<double>> &obs) -> std::vector<double> {
-		                    double mag = obs[0][0];
-		                    double mag2 = obs[1][0];
+void silly_mc::register_evalables(loadl::evaluator &eval) {
+	eval.evaluate("AntimagicNumber", {"MagicNumber", "MagicNumber2"},
+	              [](const std::vector<std::vector<double>> &obs) {
+		              double mag = obs[0][0];
+		              double mag2 = obs[1][0];
 
-		                    return {mag * mag / mag2};
-	                    }});
+		              return std::vector{mag * mag / mag2};
+	              });
 }

test/silly_mc/silly_mc.h

@@ -7,6 +7,7 @@
 class silly_mc : public loadl::mc {
 private:
 	uint64_t idx_;
+
 public:
 	void init();
 	void do_update();
@@ -14,7 +15,7 @@ public:
 	void checkpoint_write(const loadl::iodump::group &out);
 	void checkpoint_read(const loadl::iodump::group &in);
 
-	void register_evalables(std::vector<loadl::evalable> &evalables);
+	void register_evalables(loadl::evaluator &eval);
 
 	silly_mc(const loadl::parser &p);
 };