From b6276ea9e4ca82592230a8f3910f05fbb442fb9f Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:10:26 -0400 Subject: [PATCH 01/14] Reduce number of string concats with fmt::format --- src/mgxs_interface.cpp | 13 ++++++------- src/particle.cpp | 10 +++++----- src/plot.cpp | 2 +- src/simulation.cpp | 4 +++- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/mgxs_interface.cpp b/src/mgxs_interface.cpp index 936eeb31f..7ad07d0c9 100644 --- a/src/mgxs_interface.cpp +++ b/src/mgxs_interface.cpp @@ -3,6 +3,8 @@ #include #include +#include + #include "openmc/cell.h" #include "openmc/cross_sections.h" #include "openmc/container_util.h" @@ -57,8 +59,7 @@ void MgxsInterface::init() // Check if MGXS Library exists if (!file_exists(cross_sections_path_)) { // Could not find MGXS Library file - fatal_error("Cross sections HDF5 file '" + cross_sections_path_ + - "' does not exist."); + fatal_error(fmt::format("Cross sections HDF5 file '{}' does not exist!", cross_sections_path_)); } write_message("Loading cross section data...", 5); @@ -98,15 +99,14 @@ void MgxsInterface::add_mgxs(hid_t file_id, const std::string& name, const std::vector& temperature) { - write_message("Loading " + std::string(name) + " data...", 6); + write_message(fmt::format("Loading {} data...", name), 6); // Check to make sure cross section set exists in the library hid_t xs_grp; if (object_exists(file_id, name.c_str())) { xs_grp = open_group(file_id, name.c_str()); } else { - fatal_error("Data for " + std::string(name) + " does not exist in " - + "provided MGXS Library"); + fatal_error(fmt::format("Data for {} does not exist in provided MGXS Library", name)); } nuclides_.emplace_back(xs_grp, temperature, num_energy_groups_, @@ -187,8 +187,7 @@ void MgxsInterface::read_header(const std::string& path_cross_sections) // Check if MGXS Library exists if (!file_exists(cross_sections_path_)) { // Could not find MGXS Library file - fatal_error("Cross sections HDF5 file '" + cross_sections_path_ + - "' does not exist."); + fatal_error(fmt::format("Cross section HDF5 file '{}' does not exist", cross_sections_path_)); } write_message("Reading cross sections HDF5 file...", 5); diff --git a/src/particle.cpp b/src/particle.cpp index ae75d91b4..3916eacba 100644 --- a/src/particle.cpp +++ b/src/particle.cpp @@ -410,7 +410,7 @@ Particle::cross_surface() // TODO: off-by-one const auto& surf {model::surfaces[i_surface - 1].get()}; if (settings::verbosity >= 10 || trace_) { - write_message(" Crossing surface " + std::to_string(surf->id_)); + write_message(fmt::format(" Crossing surface {}", surf->id_)); } if (surf->bc_ == Surface::BoundaryType::VACUUM && (settings::run_mode != RunMode::PLOTTING)) { @@ -437,7 +437,7 @@ Particle::cross_surface() // Display message if (settings::verbosity >= 10 || trace_) { - write_message(" Leaked out of surface " + std::to_string(surf->id_)); + write_message(fmt::format(" Leaked out of surface {}", surf->id_)); } return; @@ -502,7 +502,7 @@ Particle::cross_surface() // Diagnostic message if (settings::verbosity >= 10 || trace_) { - write_message(" Reflected from surface " + std::to_string(surf->id_)); + write_message(fmt::format(" Reflected from surface {}", surf->id_)); } return; @@ -556,8 +556,8 @@ Particle::cross_surface() // Diagnostic message if (settings::verbosity >= 10 || trace_) { - write_message(" Hit periodic boundary on surface " + - std::to_string(surf->id_)); + write_message(fmt::format(" Hit periodic boundary on surface {}", + surf->id_)); } return; } diff --git a/src/plot.cpp b/src/plot.cpp index e2fe5abde..267810712 100644 --- a/src/plot.cpp +++ b/src/plot.cpp @@ -114,7 +114,7 @@ void read_plots_xml() // Check if plots.xml exists std::string filename = settings::path_input + "plots.xml"; if (!file_exists(filename)) { - fatal_error("Plots XML file '" + filename + "' does not exist!"); + fatal_error(fmt::format("Plots XML file '{}' does not exist!", filename)); } write_message("Reading plot XML file...", 5); diff --git a/src/simulation.cpp b/src/simulation.cpp index 1b39c1329..92cee64cd 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -33,6 +33,8 @@ #include #endif +#include + #include #include #include @@ -500,7 +502,7 @@ void initialize_history(Particle& p, int64_t index_source) // Display message if high verbosity or trace is on if (settings::verbosity >= 9 || p.trace_) { - write_message("Simulating Particle " + std::to_string(p.id_)); + write_message(fmt::format("Simulating Particle {}", p.id_)); } // Add paricle's starting weight to count for normalizing tallies later From cf763e2477720df7847efc45d4c914a0efbbfcf5 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:10:59 -0400 Subject: [PATCH 02/14] Add templated write_message for formatting messages This new function lives inside include/openmc/error.h and checks if a message will be written by checking the current verbosity level prior to formatting the message. This works very similar to python logging modules and the spdlog project, where the call to the logging function accepts a formattable message string and the format arguments separately, e.g. write_message(1, "Writing data for statepoint {}", point) The function accepts any number and type of argument after the format message and passes directly to fmt::format and in-turn to the non-templated write_message. While this later function does similar checking with the level and MPI rank, the real goal is the output function in src/error.cpp which is not currently exposed through the error header file Thanks to @paulromano for feedback and suggestions --- include/openmc/error.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/include/openmc/error.h b/include/openmc/error.h index 32ed36c8b..4c4efe6a7 100644 --- a/include/openmc/error.h +++ b/include/openmc/error.h @@ -5,7 +5,10 @@ #include #include +#include + #include "openmc/capi.h" +#include "openmc/settings.h" #ifdef __GNUC__ #define UNREACHABLE() __builtin_unreachable() @@ -63,6 +66,21 @@ void write_message(const std::stringstream& message, int level) write_message(message.str(), level); } +template +void write_message( + int level, const std::string& message, const Params&... fmt_args) +{ + if (settings::verbosity >= level) { + write_message(fmt::format(message, fmt_args...)); + } +} + +template +void write_message(const std::string& message, const Params&... fmt_args) +{ + write_message(fmt::format(message, fmt_args...)); +} + #ifdef OPENMC_MPI extern "C" void abort_mpi(int code); #endif From 2113c4c06b67389f40eb54d4c57eb09769135896 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:50 -0400 Subject: [PATCH 03/14] Apply templated write_message to cross_sections.cpp --- src/cross_sections.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/cross_sections.cpp b/src/cross_sections.cpp index c06a11924..b82a690e6 100644 --- a/src/cross_sections.cpp +++ b/src/cross_sections.cpp @@ -228,7 +228,7 @@ read_ce_cross_sections(const std::vector>& nuc_temps, int idx = data::library_map[key]; std::string& filename = data::libraries[idx].path_; - write_message("Reading " + name + " from " + filename, 6); + write_message(6, "Reading {} from {}", name, filename); // Open file and make sure version matches hid_t file_id = file_open(filename, 'r'); @@ -256,10 +256,8 @@ read_ce_cross_sections(const std::vector>& nuc_temps, } // Show minimum/maximum temperature - write_message("Minimum neutron data temperature: " + - std::to_string(data::temperature_min) + " K", 4); - write_message("Maximum neutron data temperature: " + - std::to_string(data::temperature_max) + " K", 4); + write_message(4, "Minimum neutron data temperature: {} K", data::temperature_min); + write_message(4, "Maximum neutron data temperature: {} K", data::temperature_max); // If the user wants multipole, make sure we found a multipole library. if (settings::temperature_multipole) { From 1ae1d9dc8f24380d54f4f3e2b8bbf9a60d3343d3 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:51 -0400 Subject: [PATCH 04/14] Apply templated write_message to mesh.cpp --- src/mesh.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesh.cpp b/src/mesh.cpp index 4954ca373..82bacdcf5 100644 --- a/src/mesh.cpp +++ b/src/mesh.cpp @@ -2051,7 +2051,7 @@ void UnstructuredMesh::write(std::string base_filename) const { // add extension to the base name auto filename = base_filename + ".vtk"; - write_message("Writing unstructured mesh " + filename + "...", 5); + write_message(5, "Writing unstructured mesh {}...", filename); filename = settings::path_output + filename; // write the tetrahedral elements of the mesh only From 4feb40364696a02853ae783959f0507fbe3308ec Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:51 -0400 Subject: [PATCH 05/14] Apply templated write_message to nuclide.cpp --- src/nuclide.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nuclide.cpp b/src/nuclide.cpp index 59e5f6e8c..0a0e40057 100644 --- a/src/nuclide.cpp +++ b/src/nuclide.cpp @@ -944,7 +944,7 @@ extern "C" int openmc_load_nuclide(const char* name, const double* temps, int n) // Get filename for library containing nuclide int idx = it->second; const auto& filename = data::libraries[idx].path_; - write_message("Reading " + std::string{name} + " from " + filename, 6); + write_message(6, "Reading {} from {}", std::string{name}, filename); // Open file and make sure version is sufficient hid_t file_id = file_open(filename, 'r'); @@ -977,7 +977,7 @@ extern "C" int openmc_load_nuclide(const char* name, const double* temps, int n) int idx = it->second; const auto& filename = data::libraries[idx].path_; - write_message("Reading " + element + " from " + filename, 6); + write_message(6, "Reading {} from {} ", element, filename); // Open file and make sure version is sufficient hid_t file_id = file_open(filename, 'r'); From a6c9f173a4bde3b5d296e9a0d37e293260e60fab Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:51 -0400 Subject: [PATCH 06/14] Apply templated write_message to particle_restart.cpp --- src/particle_restart.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/particle_restart.cpp b/src/particle_restart.cpp index b0f739bd0..be9eda77f 100644 --- a/src/particle_restart.cpp +++ b/src/particle_restart.cpp @@ -24,8 +24,7 @@ namespace openmc { void read_particle_restart(Particle& p, RunMode& previous_run_mode) { // Write meessage - write_message("Loading particle restart file " + - settings::path_particle_restart + "...", 5); + write_message(5, "Loading particle restart file {}", settings::path_particle_restart); // Open file hid_t file_id = file_open(settings::path_particle_restart, 'r'); From 697f8c77eb4ddae0982e2c9957ee299d6b2b36b6 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:51 -0400 Subject: [PATCH 07/14] Apply templated write_message to plot.cpp --- src/plot.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plot.cpp b/src/plot.cpp index 267810712..283207a42 100644 --- a/src/plot.cpp +++ b/src/plot.cpp @@ -94,8 +94,7 @@ extern "C" int openmc_plot_geometry() { for (auto pl : model::plots) { - write_message(fmt::format("Processing plot {}: {}...", - pl.id_, pl.path_plot_), 5); + write_message(5, "Processing plot {}: {}...", pl.id_, pl.path_plot_); if (PlotType::slice == pl.type_) { // create 2D image From 9bb219af8eb12cbb128a78870e54c2bfd7b81697 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:52 -0400 Subject: [PATCH 08/14] Apply templated write_message to simulation.cpp --- src/simulation.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/simulation.cpp b/src/simulation.cpp index 92cee64cd..def9cee5d 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -307,8 +307,7 @@ void initialize_batch() ++simulation::current_batch; if (settings::run_mode == RunMode::FIXED_SOURCE) { - int b = simulation::current_batch; - write_message("Simulating batch " + std::to_string(b), 6); + write_message(6, "Simulating batch {}", simulation::current_batch); } // Reset total starting particle weight used for normalizing tallies @@ -612,9 +611,8 @@ void initialize_data() double max_E = nuc->grid_[0].energy.back(); int neutron = static_cast(Particle::Type::neutron); if (max_E == data::energy_max[neutron]) { - write_message("Maximum neutron transport energy: " + - std::to_string(data::energy_max[neutron]) + " eV for " + - nuc->name_, 7); + write_message(7, "Maximum neutron transport energy: {} eV for {}", + data::energy_max[neutron], nuc->name_); if (mpi::master && data::energy_max[neutron] < 20.0e6) { warning("Maximum neutron energy is below 20 MeV. This may bias " "the results."); From bd76dbd9a2b58b70f8ca4b611ee26aeb2abe1b2c Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:52 -0400 Subject: [PATCH 09/14] Apply templated write_message to source.cpp --- src/source.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/source.cpp b/src/source.cpp index 3f31541db..7e9b16859 100644 --- a/src/source.cpp +++ b/src/source.cpp @@ -263,8 +263,7 @@ void initialize_source() // Read the source from a binary file instead of sampling from some // assumed source distribution - write_message(fmt::format("Reading source file from {}...", - settings::path_source), 6); + write_message(6, "Reading source file from {}...", settings::path_source); // Open the binary file hid_t file_id = file_open(settings::path_source, 'r', true); @@ -285,8 +284,7 @@ void initialize_source() file_close(file_id); } else if (!settings::path_source_library.empty()) { - write_message(fmt::format("Sampling from library source {}...", - settings::path_source), 6); + write_message(6, "Sampling library source {}...", settings::path_source); fill_source_bank_custom_source(); From 67e8e0ab77f33cae94b927f70b4562c596700291 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:52 -0400 Subject: [PATCH 10/14] Apply templated write_message to tallies/trigger.cpp --- src/tallies/trigger.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tallies/trigger.cpp b/src/tallies/trigger.cpp index cc8fff1d7..56f5c4c56 100644 --- a/src/tallies/trigger.cpp +++ b/src/tallies/trigger.cpp @@ -164,8 +164,7 @@ check_triggers() // If all the triggers are satisfied, alert the user and return. if (std::max(keff_ratio, tally_ratio) <= 1.) { simulation::satisfy_triggers = true; - write_message("Triggers satisfied for batch " - + std::to_string(current_batch), 7); + write_message(7, "Triggers satisfied for batch {}", current_batch); return; } From 27e102066604e14db4ababa38c773e51398942ab Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:52 -0400 Subject: [PATCH 11/14] Apply templated write_message to volume_calc.cpp --- src/volume_calc.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/volume_calc.cpp b/src/volume_calc.cpp index d1b14e329..96ef55234 100644 --- a/src/volume_calc.cpp +++ b/src/volume_calc.cpp @@ -468,9 +468,7 @@ int openmc_calculate_volumes() { time_volume.start(); for (int i = 0; i < model::volume_calcs.size(); ++i) { - if (mpi::master) { - write_message(fmt::format("Running volume calculation {}...", i + 1), 4); - } + write_message(4, "Running volume calculation {}", i+1); // Run volume calculation const auto& vol_calc {model::volume_calcs[i]}; @@ -488,8 +486,8 @@ int openmc_calculate_volumes() { // Display domain volumes for (int j = 0; j < vol_calc.domain_ids_.size(); j++) { - write_message(fmt::format("{}{}: {} +/- {} cm^3", domain_type, - vol_calc.domain_ids_[j], results[j].volume[0], results[j].volume[1]), 4); + write_message(4, "{}{}: {} +/- {} cm^3", domain_type, + vol_calc.domain_ids_[j], results[j].volume[0], results[j].volume[1]); } // Write volumes to HDF5 file @@ -502,9 +500,7 @@ int openmc_calculate_volumes() { // Show elapsed time time_volume.stop(); - if (mpi::master) { - write_message(fmt::format("Elapsed time: {} s", time_volume.elapsed()), 6); - } + write_message(6, "Elapsed time: {} s", time_volume.elapsed()); return 0; } From b441fe1c29c7f5966d44f5c321afaaec7ca1b27f Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Sun, 2 Aug 2020 22:11:53 -0400 Subject: [PATCH 12/14] Apply templated write_message to wmp.cpp --- src/wmp.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wmp.cpp b/src/wmp.cpp index 96e1d5641..44f7eb51a 100644 --- a/src/wmp.cpp +++ b/src/wmp.cpp @@ -5,6 +5,7 @@ #include "openmc/hdf5_interface.h" #include "openmc/math_functions.h" #include "openmc/nuclide.h" +#include "openmc/error.h" // for writing messages #include @@ -227,7 +228,7 @@ void read_multipole_data(int i_nuclide) std::string& filename = data::libraries[idx].path_; // Display message - write_message("Reading " + nuc->name_ + " WMP data from " + filename, 6); + write_message(6, "Reading {} WMP data from {}", nuc->name_, filename); // Open file and make sure version is sufficient hid_t file = file_open(filename, 'r'); From 4de893043eac2402309648a1e03b76e5b7755b88 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 6 Aug 2020 07:01:10 -0400 Subject: [PATCH 13/14] Apply templated write_message to mgxs_interface.cpp --- src/mgxs_interface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mgxs_interface.cpp b/src/mgxs_interface.cpp index 7ad07d0c9..31294e3c9 100644 --- a/src/mgxs_interface.cpp +++ b/src/mgxs_interface.cpp @@ -99,7 +99,7 @@ void MgxsInterface::add_mgxs(hid_t file_id, const std::string& name, const std::vector& temperature) { - write_message(fmt::format("Loading {} data...", name), 6); + write_message(5, "Loading {} data...", name); // Check to make sure cross section set exists in the library hid_t xs_grp; From f1b7cd79382675f83dfa5738d47d85f92c5a4259 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Thu, 6 Aug 2020 08:19:03 -0400 Subject: [PATCH 14/14] Use templated write_message in a few more places From review suggestions Co-authored-by: Paul Romano --- src/nuclide.cpp | 2 +- src/particle.cpp | 9 ++++----- src/simulation.cpp | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/nuclide.cpp b/src/nuclide.cpp index 0a0e40057..59c631167 100644 --- a/src/nuclide.cpp +++ b/src/nuclide.cpp @@ -944,7 +944,7 @@ extern "C" int openmc_load_nuclide(const char* name, const double* temps, int n) // Get filename for library containing nuclide int idx = it->second; const auto& filename = data::libraries[idx].path_; - write_message(6, "Reading {} from {}", std::string{name}, filename); + write_message(6, "Reading {} from {}", name, filename); // Open file and make sure version is sufficient hid_t file_id = file_open(filename, 'r'); diff --git a/src/particle.cpp b/src/particle.cpp index 3916eacba..a983b6600 100644 --- a/src/particle.cpp +++ b/src/particle.cpp @@ -410,7 +410,7 @@ Particle::cross_surface() // TODO: off-by-one const auto& surf {model::surfaces[i_surface - 1].get()}; if (settings::verbosity >= 10 || trace_) { - write_message(fmt::format(" Crossing surface {}", surf->id_)); + write_message(" Crossing surface {}", surf->id_); } if (surf->bc_ == Surface::BoundaryType::VACUUM && (settings::run_mode != RunMode::PLOTTING)) { @@ -437,7 +437,7 @@ Particle::cross_surface() // Display message if (settings::verbosity >= 10 || trace_) { - write_message(fmt::format(" Leaked out of surface {}", surf->id_)); + write_message(" Leaked out of surface {}", surf->id_); } return; @@ -502,7 +502,7 @@ Particle::cross_surface() // Diagnostic message if (settings::verbosity >= 10 || trace_) { - write_message(fmt::format(" Reflected from surface {}", surf->id_)); + write_message(" Reflected from surface {}", surf->id_); } return; @@ -556,8 +556,7 @@ Particle::cross_surface() // Diagnostic message if (settings::verbosity >= 10 || trace_) { - write_message(fmt::format(" Hit periodic boundary on surface {}", - surf->id_)); + write_message(" Hit periodic boundary on surface {}", surf->id_); } return; } diff --git a/src/simulation.cpp b/src/simulation.cpp index def9cee5d..c8115ae37 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -501,7 +501,7 @@ void initialize_history(Particle& p, int64_t index_source) // Display message if high verbosity or trace is on if (settings::verbosity >= 9 || p.trace_) { - write_message(fmt::format("Simulating Particle {}", p.id_)); + write_message("Simulating Particle {}", p.id_); } // Add paricle's starting weight to count for normalizing tallies later