From 2172907bcfa8ac7463b67c000ac3056b1ca92ec6 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Fri, 31 Mar 2023 13:46:42 -0400 Subject: [PATCH 01/17] refactor particle source import/export --- include/openmc/file_utils.h | 26 +++++ include/openmc/mcpl_interface.h | 18 +++- include/openmc/message_passing.h | 16 +++ include/openmc/shared_array.h | 6 ++ include/openmc/state_point.h | 27 ++++- src/mcpl_interface.cpp | 55 ++++------ src/message_passing.cpp | 23 ++++ src/output.cpp | 10 ++ src/simulation.cpp | 34 ++++-- src/source.cpp | 2 +- src/state_point.cpp | 128 ++++++++++------------- tests/cpp_unit_tests/CMakeLists.txt | 1 + tests/cpp_unit_tests/test_file_utils.cpp | 28 +++++ 13 files changed, 248 insertions(+), 126 deletions(-) create mode 100644 tests/cpp_unit_tests/test_file_utils.cpp diff --git a/include/openmc/file_utils.h b/include/openmc/file_utils.h index 9612896be..5604e3230 100644 --- a/include/openmc/file_utils.h +++ b/include/openmc/file_utils.h @@ -1,6 +1,8 @@ #ifndef OPENMC_FILE_UTILS_H #define OPENMC_FILE_UTILS_H +#include // any_of +#include // for isalpha #include // for ifstream #include #include @@ -33,6 +35,30 @@ inline bool file_exists(const std::string& filename) return s.good(); } +// Gets the file extension of whatever string is passed in. This is defined as +// a sequence of strictly alphanumeric characters which follow the last period, +// i.e. at least one alphabet character is present, and zero or more numbers. +// If such a sequence of characters is not found, an empty string is returned. +inline std::string get_file_extension(const std::string& filename) +{ + // check that at least one letter is present + const std::string::size_type last_period_pos = filename.find_last_of('.'); + + // no file extension + if (last_period_pos == std::string::npos) + return ""; + + const std::string ending = filename.substr(last_period_pos + 1); + + // check that at least one character is present. + const bool has_alpha = std::any_of(ending.begin(), ending.end(), + [](char x) { return static_cast(std::isalpha(x)); }); + if (has_alpha) + return ending; + else + return ""; +} + } // namespace openmc #endif // OPENMC_FILE_UTILS_H diff --git a/include/openmc/mcpl_interface.h b/include/openmc/mcpl_interface.h index 64f15c13a..ec2c7ac35 100644 --- a/include/openmc/mcpl_interface.h +++ b/include/openmc/mcpl_interface.h @@ -4,6 +4,8 @@ #include "openmc/particle_data.h" #include "openmc/vector.h" +#include + #include namespace openmc { @@ -26,11 +28,17 @@ vector mcpl_source_sites(std::string path); //! Write an MCPL source file // -//! \param[in] filename Path to MCPL file -//! \param[in] surf_source_bank Whether to use the surface source bank -void write_mcpl_source_point( - const char* filename, bool surf_source_bank = false); - +//! \param[in] filename Path to MCPL file +//! \param[in] source_bank Vector of SourceSites to write to file for this +//! MPI rank +//! \param[in] bank_indx Pointer to vector of site index ranges over all +//! MPI ranks, allowed to leave this argument alone +//! or pass a nullptr if not running in MPI mode. +//! The option of passing a nullptr has been left +//! for developers to experiment with serial code +//! before writing the fully MPI-parallel version. +void write_mcpl_source_point(const char* filename, + gsl::span source_bank, vector const& bank_index); } // namespace openmc #endif // OPENMC_MCPL_INTERFACE_H diff --git a/include/openmc/message_passing.h b/include/openmc/message_passing.h index b02d2938f..abd6f68b0 100644 --- a/include/openmc/message_passing.h +++ b/include/openmc/message_passing.h @@ -1,10 +1,14 @@ #ifndef OPENMC_MESSAGE_PASSING_H #define OPENMC_MESSAGE_PASSING_H +#include + #ifdef OPENMC_MPI #include #endif +#include "openmc/vector.h" + namespace openmc { namespace mpi { @@ -17,6 +21,18 @@ extern MPI_Datatype source_site; extern MPI_Comm intracomm; #endif +// Calculates global indices of the bank particles +// across all ranks using a parallel scan. This is used to write +// the surface source file in parallel runs. It will probably +// be used in the future for other types of bank like particles +// in flight used to kick off transient simulations. +// +// More abstractly, this just takes a number from each MPI rank, +// and returns a vector which is the exclusive parallel scan across +// all of those numbers, having a length of the number of MPI ranks +// plus one. +vector calculate_parallel_index_vector(const int64_t size); + } // namespace mpi } // namespace openmc diff --git a/include/openmc/shared_array.h b/include/openmc/shared_array.h index 6fee29b82..676481296 100644 --- a/include/openmc/shared_array.h +++ b/include/openmc/shared_array.h @@ -115,6 +115,12 @@ public: T* data() { return data_.get(); } const T* data() const { return data_.get(); } + //! Classic iterators + T* begin() { return data_.get(); } + const T* cbegin() const { return data_.get(); } + T* end() { return data_.get() + size_; } + const T* cend() const { return data_.get() + size_; } + private: //========================================================================== // Data members diff --git a/include/openmc/state_point.h b/include/openmc/state_point.h index 5ada2bf88..ca4b293eb 100644 --- a/include/openmc/state_point.h +++ b/include/openmc/state_point.h @@ -3,18 +3,39 @@ #include +#include + #include "hdf5.h" #include "openmc/capi.h" #include "openmc/particle.h" +#include "openmc/shared_array.h" #include "openmc/vector.h" namespace openmc { void load_state_point(); -vector calculate_surf_source_size(); -void write_source_point(const char* filename, bool surf_source_bank = false); -void write_source_bank(hid_t group_id, bool surf_source_bank); + +// By passing in a filename, source bank, and list of source indices +// on each MPI rank, this writes an HDF5 file which contains that +// information which can later be read in by read_source_bank +// (defined below). If you're writing code to write out a new kind +// of particle bank, this function is the one you want to use! +// +// For example, this is used to write both the surface source sites +// or fission source sites for eigenvalue continuation runs. +// +// This function ends up calling write_source_bank, and is responsible +// for opening the file to be written to and controlling whether the +// write is done in parallel (if compiled with parallel HDF5). +void write_source_point(const char* filename, gsl::span source_bank, + vector const& bank_index); + +// This appends a source bank specification to an HDF5 file +// that's already open. It is used internally by write_source_point. +void write_source_bank(hid_t group_id, gsl::span source_bank, + vector const& bank_index); + void read_source_bank( hid_t group_id, vector& sites, bool distribute); void write_tally_results_nr(hid_t file_id); diff --git a/src/mcpl_interface.cpp b/src/mcpl_interface.cpp index a38891478..b64531eed 100644 --- a/src/mcpl_interface.cpp +++ b/src/mcpl_interface.cpp @@ -2,6 +2,7 @@ #include "openmc/bank.h" #include "openmc/error.h" +#include "openmc/file_utils.h" #include "openmc/message_passing.h" #include "openmc/settings.h" #include "openmc/simulation.h" @@ -110,37 +111,18 @@ vector mcpl_source_sites(std::string path) //============================================================================== #ifdef OPENMC_MCPL -void write_mcpl_source_bank(mcpl_outfile_t file_id, bool surf_source_bank) +void write_mcpl_source_bank(mcpl_outfile_t file_id, + gsl::span source_bank, vector const& bank_index) { int64_t dims_size = settings::n_particles; int64_t count_size = simulation::work_per_rank; - // Set vectors for source bank and starting bank index of each process - vector* bank_index = &simulation::work_index; - vector* source_bank = &simulation::source_bank; - vector surf_source_index_vector; - vector surf_source_bank_vector; - - if (surf_source_bank) { - surf_source_index_vector = calculate_surf_source_size(); - dims_size = surf_source_index_vector[mpi::n_procs]; - count_size = simulation::surf_source_bank.size(); - - bank_index = &surf_source_index_vector; - - // Copy data in a SharedArray into a vector. - surf_source_bank_vector.resize(count_size); - surf_source_bank_vector.assign(simulation::surf_source_bank.data(), - simulation::surf_source_bank.data() + count_size); - source_bank = &surf_source_bank_vector; - } - if (mpi::master) { // Particles are writeen to disk from the master node only // Save source bank sites since the array is overwritten below #ifdef OPENMC_MPI - vector temp_source {source_bank->begin(), source_bank->end()}; + vector temp_source {source_bank.begin(), source_bank.end()}; #endif // loop over the other nodes and receive data - then write those. @@ -151,11 +133,11 @@ void write_mcpl_source_bank(mcpl_outfile_t file_id, bool surf_source_bank) #ifdef OPENMC_MPI if (i > 0) - MPI_Recv(source_bank->data(), count[0], mpi::source_site, i, i, + MPI_Recv(source_bank.data(), count[0], mpi::source_site, i, i, mpi::intracomm, MPI_STATUS_IGNORE); #endif // now write the source_bank data again. - for (const auto& site : *source_bank) { + for (const auto& site : source_bank) { // particle is now at the iterator // write it to the mcpl-file mcpl_particle_t p; @@ -194,11 +176,11 @@ void write_mcpl_source_bank(mcpl_outfile_t file_id, bool surf_source_bank) } #ifdef OPENMC_MPI // Restore state of source bank - std::copy(temp_source.begin(), temp_source.end(), source_bank->begin()); + std::copy(temp_source.begin(), temp_source.end(), source_bank.begin()); #endif } else { #ifdef OPENMC_MPI - MPI_Send(source_bank->data(), count_size, mpi::source_site, 0, mpi::rank, + MPI_Send(source_bank.data(), count_size, mpi::source_site, 0, mpi::rank, mpi::intracomm); #endif } @@ -207,17 +189,16 @@ void write_mcpl_source_bank(mcpl_outfile_t file_id, bool surf_source_bank) //============================================================================== -void write_mcpl_source_point(const char* filename, bool surf_source_bank) +void write_mcpl_source_point(const char* filename, + gsl::span source_bank, vector const& bank_index) { - std::string filename_; - if (filename) { - filename_ = filename; - } else { - // Determine width for zero padding - int w = std::to_string(settings::n_max_batches).size(); - - filename_ = fmt::format("{0}source.{1:0{2}}.mcpl", settings::path_output, - simulation::current_batch, w); + std::string filename_(filename); + const auto extension = get_file_extension(filename_); + if (extension == "") { + filename_.append(".mcpl"); + } else if (extension != "mcpl") { + fatal_error("write_mcpl_source_point was passed an incorrect file " + "extension. Must either pass nothing or .mcpl"); } #ifdef OPENMC_MCPL @@ -236,7 +217,7 @@ void write_mcpl_source_point(const char* filename, bool surf_source_bank) mcpl_hdr_set_srcname(file_id, line.c_str()); } - write_mcpl_source_bank(file_id, surf_source_bank); + write_mcpl_source_bank(file_id, source_bank, bank_index); if (mpi::master) { mcpl_close_outfile(file_id); diff --git a/src/message_passing.cpp b/src/message_passing.cpp index f87782083..fa9036c71 100644 --- a/src/message_passing.cpp +++ b/src/message_passing.cpp @@ -17,6 +17,29 @@ extern "C" bool openmc_master() return mpi::master; } +vector calculate_parallel_index_vector(const int64_t size) +{ + vector result; + result.reserve(n_procs + 1); + +#ifdef OPENMC_MPI + result.resize(n_procs); + vector bank_size(n_procs); + + // Populate the result with cumulative sum of the number of + // surface source banks per process + MPI_Scan(&size, bank_size.data(), 1, MPI_INT64_T, MPI_SUM, intracomm); + MPI_Allgather( + bank_size.data(), 1, MPI_INT64_T, result.data(), 1, MPI_INT64_T, intracomm); + result.insert(result.begin(), 0); +#else + result.push_back(0); + result.push_back(size); +#endif + + return result; +} + } // namespace mpi } // namespace openmc diff --git a/src/output.cpp b/src/output.cpp index ae0907d02..53f5b647f 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -360,6 +360,8 @@ void print_build_info() std::string png(n); std::string profiling(n); std::string coverage(n); + std::string mcpl(n); + std::string ncrystal(n); #ifdef PHDF5 phdf5 = y; @@ -382,6 +384,12 @@ void print_build_info() #ifdef COVERAGEBUILD coverage = y; #endif +#ifdef NCRYSTAL + ncrystal = y; +#endif +#ifdef OPENMC_MCPL + mcpl = y; +#endif // Wraps macro variables in quotes #define STRINGIFY(x) STRINGIFY2(x) @@ -396,6 +404,8 @@ void print_build_info() fmt::print("PNG support: {}\n", png); fmt::print("DAGMC support: {}\n", dagmc); fmt::print("libMesh support: {}\n", libmesh); + fmt::print("NCrystal support: {}\n", profiling); + fmt::print("MCPL support: {}\n", profiling); fmt::print("Coverage testing: {}\n", coverage); fmt::print("Profiling flags: {}\n", profiling); } diff --git a/src/simulation.cpp b/src/simulation.cpp index 7740c81d8..d46787c28 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -392,21 +392,32 @@ void finalize_batch() // Write out a separate source point if it's been specified for this batch if (contains(settings::sourcepoint_batch, simulation::current_batch) && settings::source_write && settings::source_separate) { + + // Determine width for zero padding + int w = std::to_string(settings::n_max_batches).size(); + std::string source_point_filename = fmt::format("{0}source.{1:0{2}}", + settings::path_output, simulation::current_batch, w); + gsl::span bankspan(simulation::source_bank); if (settings::source_mcpl_write) { - write_mcpl_source_point(nullptr); + write_mcpl_source_point( + source_point_filename.c_str(), bankspan, simulation::work_index); } else { - write_source_point(nullptr); + write_source_point( + source_point_filename.c_str(), bankspan, simulation::work_index); } } // Write a continously-overwritten source point if requested. if (settings::source_latest) { + + // note: correct file extension appended automatically + auto filename = settings::path_output + "source"; + gsl::span bankspan(simulation::source_bank); if (settings::source_mcpl_write) { - auto filename = settings::path_output + "source.mcpl"; - write_mcpl_source_point(filename.c_str()); + write_mcpl_source_point( + filename.c_str(), bankspan, simulation::work_index); } else { - auto filename = settings::path_output + "source.h5"; - write_source_point(filename.c_str()); + write_source_point(filename.c_str(), bankspan, simulation::work_index); } } } @@ -414,12 +425,15 @@ void finalize_batch() // Write out surface source if requested. if (settings::surf_source_write && simulation::current_batch == settings::n_batches) { + auto filename = settings::path_output + "surface_source"; + auto surf_work_index = + mpi::calculate_parallel_index_vector(simulation::surf_source_bank.size()); + gsl::span surfbankspan(simulation::surf_source_bank.begin(), + simulation::surf_source_bank.size()); if (settings::surf_mcpl_write) { - auto filename = settings::path_output + "surface_source.mcpl"; - write_mcpl_source_point(filename.c_str(), true); + write_mcpl_source_point(filename.c_str(), surfbankspan, surf_work_index); } else { - auto filename = settings::path_output + "surface_source.h5"; - write_source_point(filename.c_str(), true); + write_source_point(filename.c_str(), surfbankspan, surf_work_index); } } } diff --git a/src/source.cpp b/src/source.cpp index 5862a41e2..163c85676 100644 --- a/src/source.cpp +++ b/src/source.cpp @@ -403,7 +403,7 @@ void initialize_source() write_message("Writing out initial source...", 5); std::string filename = settings::path_output + "initial_source.h5"; hid_t file_id = file_open(filename, 'w', true); - write_source_bank(file_id, false); + write_source_bank(file_id, simulation::source_bank, simulation::work_index); file_close(file_id); } } diff --git a/src/state_point.cpp b/src/state_point.cpp index dfd3d381b..10bcd9365 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -13,6 +13,7 @@ #include "openmc/constants.h" #include "openmc/eigenvalue.h" #include "openmc/error.h" +#include "openmc/file_utils.h" #include "openmc/hdf5_interface.h" #include "openmc/mesh.h" #include "openmc/message_passing.h" @@ -34,7 +35,8 @@ extern "C" int openmc_statepoint_write(const char* filename, bool* write_source) { simulation::time_statepoint.start(); - // Set the filename + // If a nullptr is passed in, we assume that the user + // wants a default name for this, of the form like output/statepoint.20.h5 std::string filename_; if (filename) { filename_ = filename; @@ -47,6 +49,15 @@ extern "C" int openmc_statepoint_write(const char* filename, bool* write_source) simulation::current_batch, w); } + // If a file name was specified, ensure it has .h5 file extension + const auto extension = get_file_extension(filename_); + if (extension == "") { + filename_.append(".h5"); + } else if (extension != "h5") { + fatal_error("openmc_statepoint_write was passed an incorrect file " + "extension. Must either have no file extension or .h5"); + } + // Determine whether or not to write the source bank bool write_source_ = write_source ? *write_source : true; @@ -316,7 +327,7 @@ extern "C" int openmc_statepoint_write(const char* filename, bool* write_source) if (write_source_) { if (mpi::master || parallel) file_id = file_open(filename_, 'a', true); - write_source_bank(file_id, false); + write_source_bank(file_id, simulation::source_bank, simulation::work_index); if (mpi::master || parallel) file_close(file_id); } @@ -541,31 +552,8 @@ hid_t h5banktype() return banktype; } -vector calculate_surf_source_size() -{ - vector surf_source_index; - surf_source_index.reserve(mpi::n_procs + 1); - -#ifdef OPENMC_MPI - surf_source_index.resize(mpi::n_procs); - vector bank_size(mpi::n_procs); - - // Populate the surf_source_index with cumulative sum of the number of - // surface source banks per process - int64_t size = simulation::surf_source_bank.size(); - MPI_Scan(&size, bank_size.data(), 1, MPI_INT64_T, MPI_SUM, mpi::intracomm); - MPI_Allgather(bank_size.data(), 1, MPI_INT64_T, surf_source_index.data(), 1, - MPI_INT64_T, mpi::intracomm); - surf_source_index.insert(surf_source_index.begin(), 0); -#else - surf_source_index.push_back(0); - surf_source_index.push_back(simulation::surf_source_bank.size()); -#endif - - return surf_source_index; -} - -void write_source_point(const char* filename, bool surf_source_bank) +void write_source_point(const char* filename, gsl::span source_bank, + vector const& bank_index) { // When using parallel HDF5, the file is written to collectively by all // processes. With MPI-only, the file is opened and written by the master @@ -577,58 +565,59 @@ void write_source_point(const char* filename, bool surf_source_bank) bool parallel = false; #endif - std::string filename_; - if (filename) { - filename_ = filename; - } else { - // Determine width for zero padding - int w = std::to_string(settings::n_max_batches).size(); + if (!filename) + fatal_error("write_source_point filename needs a nonempty name."); - filename_ = fmt::format("{0}source.{1:0{2}}.h5", settings::path_output, - simulation::current_batch, w); + std::string filename_(filename); + const auto extension = get_file_extension(filename_); + if (extension == "") { + filename_.append(".h5"); + } else if (extension != "h5") { + fatal_error("openmc_source_point was passed an incorrect file " + "extension. Must either have no file extension or .h5"); } hid_t file_id; if (mpi::master || parallel) { - file_id = file_open(filename_, 'w', true); + file_id = file_open(filename_.c_str(), 'w', true); write_attribute(file_id, "filetype", "source"); } // Get pointer to source bank and write to file - write_source_bank(file_id, surf_source_bank); + write_source_bank(file_id, source_bank, bank_index); if (mpi::master || parallel) file_close(file_id); } -void write_source_bank(hid_t group_id, bool surf_source_bank) +void write_source_bank(hid_t group_id, gsl::span source_bank, + vector const& bank_index) { hid_t banktype = h5banktype(); // Set total and individual process dataspace sizes for source bank - int64_t dims_size = settings::n_particles; - int64_t count_size = simulation::work_per_rank; + // TODO: are these correct? + // the old code was + // + // int64_t dims_size = settings::n_particles; + // int64_t count_size = simulation::work_per_rank; + // + // but that doesn't make sense! The count on a rank is not + // necessarily equal to work_per_rank. For the case of the + // fission source bank, that's true, but for surface source + // creation, there may be a different number of particles + // on each rank. + // + // Hence, I have changed count_size to be the number + // on this rank rather than work_per_rank. Similarly, + // dims_size used to be n_particles, but for surface sources, + // we are not guaranteed to create n_particles at the surface. + // As a result, I've changed this to be the total size of + // the bank across all ranks, which maintains the correct + // behavior for fission bank outputs. - // Set vectors for source bank and starting bank index of each process - vector* bank_index = &simulation::work_index; - vector* source_bank = &simulation::source_bank; - vector surf_source_index_vector; - vector surf_source_bank_vector; - - // Reset dataspace sizes and vectors for surface source bank - if (surf_source_bank) { - surf_source_index_vector = calculate_surf_source_size(); - dims_size = surf_source_index_vector[mpi::n_procs]; - count_size = simulation::surf_source_bank.size(); - - bank_index = &surf_source_index_vector; - - // Copy data in a SharedArray into a vector. - surf_source_bank_vector.resize(count_size); - surf_source_bank_vector.assign(simulation::surf_source_bank.data(), - simulation::surf_source_bank.data() + count_size); - source_bank = &surf_source_bank_vector; - } + int64_t dims_size = bank_index.back(); + int64_t count_size = bank_index[mpi::rank + 1] - bank_index[mpi::rank]; #ifdef PHDF5 // Set size of total dataspace for all procs and rank @@ -642,7 +631,7 @@ void write_source_bank(hid_t group_id, bool surf_source_bank) hid_t memspace = H5Screate_simple(1, count, nullptr); // Select hyperslab for this dataspace - hsize_t start[] {static_cast((*bank_index)[mpi::rank])}; + hsize_t start[] {static_cast(bank_index[mpi::rank])}; H5Sselect_hyperslab(dspace, H5S_SELECT_SET, start, nullptr, count, nullptr); // Set up the property list for parallel writing @@ -650,7 +639,7 @@ void write_source_bank(hid_t group_id, bool surf_source_bank) H5Pset_dxpl_mpio(plist, H5FD_MPIO_COLLECTIVE); // Write data to file in parallel - H5Dwrite(dset, banktype, memspace, dspace, plist, source_bank->data()); + H5Dwrite(dset, banktype, memspace, dspace, plist, source_bank.data()); // Free resources H5Sclose(dspace); @@ -669,31 +658,30 @@ void write_source_bank(hid_t group_id, bool surf_source_bank) // Save source bank sites since the array is overwritten below #ifdef OPENMC_MPI - vector temp_source {source_bank->begin(), source_bank->end()}; + vector temp_source {source_bank.begin(), source_bank.end()}; #endif for (int i = 0; i < mpi::n_procs; ++i) { // Create memory space - hsize_t count[] { - static_cast((*bank_index)[i + 1] - (*bank_index)[i])}; + hsize_t count[] {static_cast(bank_index[i + 1] - bank_index[i])}; hid_t memspace = H5Screate_simple(1, count, nullptr); #ifdef OPENMC_MPI // Receive source sites from other processes if (i > 0) - MPI_Recv(source_bank->data(), count[0], mpi::source_site, i, i, + MPI_Recv(source_bank.data(), count[0], mpi::source_site, i, i, mpi::intracomm, MPI_STATUS_IGNORE); #endif // Select hyperslab for this dataspace dspace = H5Dget_space(dset); - hsize_t start[] {static_cast((*bank_index)[i])}; + hsize_t start[] {static_cast(bank_index[i])}; H5Sselect_hyperslab( dspace, H5S_SELECT_SET, start, nullptr, count, nullptr); // Write data to hyperslab H5Dwrite( - dset, banktype, memspace, dspace, H5P_DEFAULT, (*source_bank).data()); + dset, banktype, memspace, dspace, H5P_DEFAULT, source_bank.data()); H5Sclose(memspace); H5Sclose(dspace); @@ -704,11 +692,11 @@ void write_source_bank(hid_t group_id, bool surf_source_bank) #ifdef OPENMC_MPI // Restore state of source bank - std::copy(temp_source.begin(), temp_source.end(), source_bank->begin()); + std::copy(temp_source.begin(), temp_source.end(), source_bank.begin()); #endif } else { #ifdef OPENMC_MPI - MPI_Send(source_bank->data(), count_size, mpi::source_site, 0, mpi::rank, + MPI_Send(source_bank.data(), count_size, mpi::source_site, 0, mpi::rank, mpi::intracomm); #endif } diff --git a/tests/cpp_unit_tests/CMakeLists.txt b/tests/cpp_unit_tests/CMakeLists.txt index 3d48545b4..e017b8700 100644 --- a/tests/cpp_unit_tests/CMakeLists.txt +++ b/tests/cpp_unit_tests/CMakeLists.txt @@ -1,5 +1,6 @@ set(TEST_NAMES test_distribution + test_file_utils # Add additional unit test files here ) diff --git a/tests/cpp_unit_tests/test_file_utils.cpp b/tests/cpp_unit_tests/test_file_utils.cpp new file mode 100644 index 000000000..23222f639 --- /dev/null +++ b/tests/cpp_unit_tests/test_file_utils.cpp @@ -0,0 +1,28 @@ +#include "openmc/file_utils.h" +#include + +using namespace openmc; + +TEST_CASE("Test get_file_extension") +{ + REQUIRE(get_file_extension("rememberthealamo.png") == "png"); + REQUIRE(get_file_extension("statepoint.20.h5") == "h5"); + REQUIRE(get_file_extension("wEiRDNaa_ame.h4") == "h4"); + REQUIRE(get_file_extension("has_directory/asdf.20.h5") == "h5"); + REQUIRE(get_file_extension("wasssssup_lol") == ""); + REQUIRE(get_file_extension("has_directory/secret_file") == ""); +} + +TEST_CASE("Test dir_exists") +{ + // not sure how to test this when running on windows? + REQUIRE(dir_exists("/")); + + // if this exists on your system... you deserve for this test to fail + REQUIRE(!dir_exists("/asdfa/asdfasdf/asdgasodgosuihasjkgh/")); +} + +TEST_CASE("Test file_exists") +{ + // TODO make a file test it exists, delete it +} From 30ed6020092768b2dd208e5343c399c2eac13d64 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Fri, 31 Mar 2023 23:59:04 -0400 Subject: [PATCH 02/17] update documentation --- include/openmc/mcpl_interface.h | 8 +++----- include/openmc/state_point.h | 5 +++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/include/openmc/mcpl_interface.h b/include/openmc/mcpl_interface.h index ec2c7ac35..1f0c94d6d 100644 --- a/include/openmc/mcpl_interface.h +++ b/include/openmc/mcpl_interface.h @@ -32,11 +32,9 @@ vector mcpl_source_sites(std::string path); //! \param[in] source_bank Vector of SourceSites to write to file for this //! MPI rank //! \param[in] bank_indx Pointer to vector of site index ranges over all -//! MPI ranks, allowed to leave this argument alone -//! or pass a nullptr if not running in MPI mode. -//! The option of passing a nullptr has been left -//! for developers to experiment with serial code -//! before writing the fully MPI-parallel version. +//! MPI ranks. This can be computed by calling +//! calculate_parallel_index_vector on +//! source_bank.size(). void write_mcpl_source_point(const char* filename, gsl::span source_bank, vector const& bank_index); } // namespace openmc diff --git a/include/openmc/state_point.h b/include/openmc/state_point.h index ca4b293eb..db1ac2edb 100644 --- a/include/openmc/state_point.h +++ b/include/openmc/state_point.h @@ -28,6 +28,11 @@ void load_state_point(); // This function ends up calling write_source_bank, and is responsible // for opening the file to be written to and controlling whether the // write is done in parallel (if compiled with parallel HDF5). +// +// bank_index is an exclusive parallel scan of the source_bank.size() +// values on each rank, used to create global indexing. This vector +// can be created by calling calculate_parallel_index_vector on +// source_bank.size() if such a vector is not already available. void write_source_point(const char* filename, gsl::span source_bank, vector const& bank_index); From 5a8e1cc988b7d6e7204aa45675ca31316e94988e Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 1 Apr 2023 00:22:50 -0400 Subject: [PATCH 03/17] fix typo --- src/mcpl_interface.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mcpl_interface.cpp b/src/mcpl_interface.cpp index b64531eed..64d6046e3 100644 --- a/src/mcpl_interface.cpp +++ b/src/mcpl_interface.cpp @@ -128,8 +128,7 @@ void write_mcpl_source_bank(mcpl_outfile_t file_id, // loop over the other nodes and receive data - then write those. for (int i = 0; i < mpi::n_procs; ++i) { // number of particles for node node i - size_t count[] { - static_cast((*bank_index)[i + 1] - (*bank_index)[i])}; + size_t count[] {static_cast(bank_index[i + 1] - bank_index[i])}; #ifdef OPENMC_MPI if (i > 0) From 0738a359c2369d563d5a10af92802763640b793b Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Fri, 14 Apr 2023 12:38:46 -0400 Subject: [PATCH 04/17] if/else -> ternary Co-authored-by: Paul Romano --- include/openmc/file_utils.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/openmc/file_utils.h b/include/openmc/file_utils.h index 5604e3230..6a4837989 100644 --- a/include/openmc/file_utils.h +++ b/include/openmc/file_utils.h @@ -53,10 +53,7 @@ inline std::string get_file_extension(const std::string& filename) // check that at least one character is present. const bool has_alpha = std::any_of(ending.begin(), ending.end(), [](char x) { return static_cast(std::isalpha(x)); }); - if (has_alpha) - return ending; - else - return ""; + return has_alpha ? ending : ""; } } // namespace openmc From 5ca6ee06540c9ce80515af639b8a510a5b542b48 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Fri, 14 Apr 2023 12:39:09 -0400 Subject: [PATCH 05/17] use auto rather than std::string::size_type Co-authored-by: Paul Romano --- include/openmc/file_utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/openmc/file_utils.h b/include/openmc/file_utils.h index 6a4837989..786d35955 100644 --- a/include/openmc/file_utils.h +++ b/include/openmc/file_utils.h @@ -42,7 +42,7 @@ inline bool file_exists(const std::string& filename) inline std::string get_file_extension(const std::string& filename) { // check that at least one letter is present - const std::string::size_type last_period_pos = filename.find_last_of('.'); + const auto last_period_pos = filename.find_last_of('.'); // no file extension if (last_period_pos == std::string::npos) From 489eb59c99862106e0d8b4efe5ed8b32d94c276f Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Fri, 14 Apr 2023 13:00:39 -0400 Subject: [PATCH 06/17] alphabetize source files in cmakelists --- CMakeLists.txt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 866050a98..70152bb6b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -334,10 +334,10 @@ list(APPEND libopenmc_SOURCES src/bank.cpp src/boundary_condition.cpp src/bremsstrahlung.cpp - src/dagmc.cpp src/cell.cpp src/cmfd_solver.cpp src/cross_sections.cpp + src/dagmc.cpp src/distribution.cpp src/distribution_angle.cpp src/distribution_energy.cpp @@ -347,11 +347,11 @@ list(APPEND libopenmc_SOURCES src/endf.cpp src/error.cpp src/event.cpp - src/initialize.cpp src/finalize.cpp src/geometry.cpp src/geometry_aux.cpp src/hdf5_interface.cpp + src/initialize.cpp src/lattice.cpp src/material.cpp src/math_functions.cpp @@ -393,15 +393,15 @@ list(APPEND libopenmc_SOURCES src/tallies/derivative.cpp src/tallies/filter.cpp src/tallies/filter_azimuthal.cpp - src/tallies/filter_cellborn.cpp - src/tallies/filter_cellfrom.cpp src/tallies/filter_cell.cpp src/tallies/filter_cell_instance.cpp + src/tallies/filter_cellborn.cpp + src/tallies/filter_cellfrom.cpp + src/tallies/filter_collision.cpp src/tallies/filter_delayedgroup.cpp src/tallies/filter_distribcell.cpp - src/tallies/filter_energyfunc.cpp src/tallies/filter_energy.cpp - src/tallies/filter_collision.cpp + src/tallies/filter_energyfunc.cpp src/tallies/filter_legendre.cpp src/tallies/filter_material.cpp src/tallies/filter_mesh.cpp @@ -418,8 +418,8 @@ list(APPEND libopenmc_SOURCES src/tallies/tally.cpp src/tallies/tally_scoring.cpp src/tallies/trigger.cpp - src/timer.cpp src/thermal.cpp + src/timer.cpp src/track_output.cpp src/universe.cpp src/urr.cpp From e1102b6408437ea3e4ecfd2364ea0d739ef20a1f Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 11:52:36 -0400 Subject: [PATCH 07/17] move file_utils to its own source file --- CMakeLists.txt | 1 + include/openmc/file_utils.h | 40 +++----------------------------- src/file_utils.cpp | 46 +++++++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 37 deletions(-) create mode 100644 src/file_utils.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 70152bb6b..6530016fe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -347,6 +347,7 @@ list(APPEND libopenmc_SOURCES src/endf.cpp src/error.cpp src/event.cpp + src/file_utils.cpp src/finalize.cpp src/geometry.cpp src/geometry_aux.cpp diff --git a/include/openmc/file_utils.h b/include/openmc/file_utils.h index 786d35955..11b20b4aa 100644 --- a/include/openmc/file_utils.h +++ b/include/openmc/file_utils.h @@ -1,11 +1,7 @@ #ifndef OPENMC_FILE_UTILS_H #define OPENMC_FILE_UTILS_H -#include // any_of -#include // for isalpha -#include // for ifstream #include -#include namespace openmc { @@ -13,48 +9,18 @@ namespace openmc { //! Determine if a path is a directory //! \param[in] path Path to check //! \return Whether the path is a directory -inline bool dir_exists(const std::string& path) -{ - struct stat s; - if (stat(path.c_str(), &s) != 0) - return false; - - return s.st_mode & S_IFDIR; -} +bool dir_exists(const std::string& path); //! Determine if a file exists //! \param[in] filename Path to file //! \return Whether file exists -inline bool file_exists(const std::string& filename) -{ - // rule out file being a path to a directory - if (dir_exists(filename)) - return false; - - std::ifstream s {filename}; - return s.good(); -} +bool file_exists(const std::string& filename); // Gets the file extension of whatever string is passed in. This is defined as // a sequence of strictly alphanumeric characters which follow the last period, // i.e. at least one alphabet character is present, and zero or more numbers. // If such a sequence of characters is not found, an empty string is returned. -inline std::string get_file_extension(const std::string& filename) -{ - // check that at least one letter is present - const auto last_period_pos = filename.find_last_of('.'); - - // no file extension - if (last_period_pos == std::string::npos) - return ""; - - const std::string ending = filename.substr(last_period_pos + 1); - - // check that at least one character is present. - const bool has_alpha = std::any_of(ending.begin(), ending.end(), - [](char x) { return static_cast(std::isalpha(x)); }); - return has_alpha ? ending : ""; -} +std::string get_file_extension(const std::string& filename); } // namespace openmc diff --git a/src/file_utils.cpp b/src/file_utils.cpp new file mode 100644 index 000000000..15b30e3b6 --- /dev/null +++ b/src/file_utils.cpp @@ -0,0 +1,46 @@ +#include "openmc/file_utils.h" + +#include // any_of +#include // for isalpha +#include // for ifstream +#include + +namespace openmc { + +bool dir_exists(const std::string& path) +{ + struct stat s; + if (stat(path.c_str(), &s) != 0) + return false; + + return s.st_mode & S_IFDIR; +} + +bool file_exists(const std::string& filename) +{ + // rule out file being a path to a directory + if (dir_exists(filename)) + return false; + + std::ifstream s {filename}; + return s.good(); +} + +std::string get_file_extension(const std::string& filename) +{ + // check that at least one letter is present + const auto last_period_pos = filename.find_last_of('.'); + + // no file extension + if (last_period_pos == std::string::npos) + return ""; + + const std::string ending = filename.substr(last_period_pos + 1); + + // check that at least one character is present. + const bool has_alpha = std::any_of(ending.begin(), ending.end(), + [](char x) { return static_cast(std::isalpha(x)); }); + return has_alpha ? ending : ""; +} + +} // namespace openmc From f202377fb7f2b4426cdb82f4d550ebc456ab4d42 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 12:50:30 -0400 Subject: [PATCH 08/17] add west const and enforce in clang format --- .clang-format | 1 + include/openmc/state_point.h | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.clang-format b/.clang-format index 153f0e4a3..74975cc3c 100644 --- a/.clang-format +++ b/.clang-format @@ -84,6 +84,7 @@ PenaltyBreakTemplateDeclaration: 10 PenaltyExcessCharacter: 1000000 PenaltyReturnTypeOnItsOwnLine: 200 PointerAlignment: Left +QualifierAlignment: Left ReflowComments: true SortIncludes: true SortUsingDeclarations: true diff --git a/include/openmc/state_point.h b/include/openmc/state_point.h index db1ac2edb..95524f6b6 100644 --- a/include/openmc/state_point.h +++ b/include/openmc/state_point.h @@ -34,12 +34,12 @@ void load_state_point(); // can be created by calling calculate_parallel_index_vector on // source_bank.size() if such a vector is not already available. void write_source_point(const char* filename, gsl::span source_bank, - vector const& bank_index); + const vector& bank_index); // This appends a source bank specification to an HDF5 file // that's already open. It is used internally by write_source_point. void write_source_bank(hid_t group_id, gsl::span source_bank, - vector const& bank_index); + const vector& bank_index); void read_source_bank( hid_t group_id, vector& sites, bool distribute); From 62708ab3add4e1caa907ef76b8db4fbaa98eb839 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 13:01:01 -0400 Subject: [PATCH 09/17] PR comments addressed --- src/message_passing.cpp | 8 ++++---- src/output.cpp | 6 ------ tests/cpp_unit_tests/test_file_utils.cpp | 1 + 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/message_passing.cpp b/src/message_passing.cpp index fa9036c71..7a43f78f4 100644 --- a/src/message_passing.cpp +++ b/src/message_passing.cpp @@ -24,14 +24,14 @@ vector calculate_parallel_index_vector(const int64_t size) #ifdef OPENMC_MPI result.resize(n_procs); - vector bank_size(n_procs); + result[0] = 0; + vector bank_size(n_procs + 1); // Populate the result with cumulative sum of the number of // surface source banks per process MPI_Scan(&size, bank_size.data(), 1, MPI_INT64_T, MPI_SUM, intracomm); - MPI_Allgather( - bank_size.data(), 1, MPI_INT64_T, result.data(), 1, MPI_INT64_T, intracomm); - result.insert(result.begin(), 0); + MPI_Allgather(bank_size.data() + 1, 1, MPI_INT64_T, result.data(), 1, + MPI_INT64_T, intracomm); #else result.push_back(0); result.push_back(size); diff --git a/src/output.cpp b/src/output.cpp index c280677a2..04e15b82d 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -390,12 +390,6 @@ void print_build_info() #ifdef COVERAGEBUILD coverage = y; #endif -#ifdef NCRYSTAL - ncrystal = y; -#endif -#ifdef OPENMC_MCPL - mcpl = y; -#endif // Wraps macro variables in quotes #define STRINGIFY(x) STRINGIFY2(x) diff --git a/tests/cpp_unit_tests/test_file_utils.cpp b/tests/cpp_unit_tests/test_file_utils.cpp index 23222f639..69d9b5a59 100644 --- a/tests/cpp_unit_tests/test_file_utils.cpp +++ b/tests/cpp_unit_tests/test_file_utils.cpp @@ -11,6 +11,7 @@ TEST_CASE("Test get_file_extension") REQUIRE(get_file_extension("has_directory/asdf.20.h5") == "h5"); REQUIRE(get_file_extension("wasssssup_lol") == ""); REQUIRE(get_file_extension("has_directory/secret_file") == ""); + REQUIRE(get_file_extension("lovely.dir/extensionless_file") == ""); } TEST_CASE("Test dir_exists") From 4c7876beb90692989966126c46c1c0c7635d201d Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 22:26:34 -0400 Subject: [PATCH 10/17] allow directories containing periods in file extension grabber --- src/file_utils.cpp | 17 +++++++++++++++-- tests/cpp_unit_tests/test_file_utils.cpp | 5 ++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/file_utils.cpp b/src/file_utils.cpp index 15b30e3b6..8a078fe18 100644 --- a/src/file_utils.cpp +++ b/src/file_utils.cpp @@ -28,11 +28,24 @@ bool file_exists(const std::string& filename) std::string get_file_extension(const std::string& filename) { + // try our best to work on windows... +#if defined(_WIN32) || defined(_WIN64) + const char sep_char = '\\'; +#else + const char sep_char = '/'; +#endif + // check that at least one letter is present const auto last_period_pos = filename.find_last_of('.'); + const auto last_sep_pos = filename.find_last_of(sep_char); - // no file extension - if (last_period_pos == std::string::npos) + // no file extension. In the first case, we are only given + // a file name. In the second, we have been given a file path. + // If that's the case, periods are allowed in directory names, + // but have the interpretation as preceding a file extension + // after the last separator. + if (last_period_pos == std::string::npos || + (last_sep_pos < std::string::npos && last_period_pos < last_sep_pos)) return ""; const std::string ending = filename.substr(last_period_pos + 1); diff --git a/tests/cpp_unit_tests/test_file_utils.cpp b/tests/cpp_unit_tests/test_file_utils.cpp index 69d9b5a59..3b7a74346 100644 --- a/tests/cpp_unit_tests/test_file_utils.cpp +++ b/tests/cpp_unit_tests/test_file_utils.cpp @@ -12,6 +12,8 @@ TEST_CASE("Test get_file_extension") REQUIRE(get_file_extension("wasssssup_lol") == ""); REQUIRE(get_file_extension("has_directory/secret_file") == ""); REQUIRE(get_file_extension("lovely.dir/extensionless_file") == ""); + REQUIRE(get_file_extension("lovely.dir/statepoint.20.h5") == "h5"); + REQUIRE(get_file_extension("lovely.dir/asdf123.cpp") == "cpp"); } TEST_CASE("Test dir_exists") @@ -25,5 +27,6 @@ TEST_CASE("Test dir_exists") TEST_CASE("Test file_exists") { - // TODO make a file test it exists, delete it + // Note: not clear how to portably test where a file should exist. + REQUIRE(!file_exists("./should_not_exist/really_do_not_make_this_please")); } From 30e2bfb2f97d8afff83347be2059d01c49fa9d43 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 22:31:53 -0400 Subject: [PATCH 11/17] warn people rather than fatal error if specifying a weird file extension --- src/state_point.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/state_point.cpp b/src/state_point.cpp index 10bcd9365..a1aa05efb 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -54,8 +54,8 @@ extern "C" int openmc_statepoint_write(const char* filename, bool* write_source) if (extension == "") { filename_.append(".h5"); } else if (extension != "h5") { - fatal_error("openmc_statepoint_write was passed an incorrect file " - "extension. Must either have no file extension or .h5"); + warning("openmc_statepoint_write was passed a file extension differing " + "from .h5, but an hdf5 file will be written."); } // Determine whether or not to write the source bank @@ -553,7 +553,7 @@ hid_t h5banktype() } void write_source_point(const char* filename, gsl::span source_bank, - vector const& bank_index) + const vector& bank_index) { // When using parallel HDF5, the file is written to collectively by all // processes. With MPI-only, the file is opened and written by the master @@ -591,7 +591,7 @@ void write_source_point(const char* filename, gsl::span source_bank, } void write_source_bank(hid_t group_id, gsl::span source_bank, - vector const& bank_index) + const vector& bank_index) { hid_t banktype = h5banktype(); From 355b6f4e88b318d653ddba580110bae16b419ad4 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 22:34:45 -0400 Subject: [PATCH 12/17] const int by value is kinda pointless Co-authored-by: Paul Romano --- include/openmc/message_passing.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/openmc/message_passing.h b/include/openmc/message_passing.h index abd6f68b0..a1641a906 100644 --- a/include/openmc/message_passing.h +++ b/include/openmc/message_passing.h @@ -31,7 +31,7 @@ extern MPI_Comm intracomm; // and returns a vector which is the exclusive parallel scan across // all of those numbers, having a length of the number of MPI ranks // plus one. -vector calculate_parallel_index_vector(const int64_t size); +vector calculate_parallel_index_vector(int64_t size); } // namespace mpi } // namespace openmc From b52e1f106cae71c884ec3f0200dd56ccdd8f53e6 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Sat, 15 Apr 2023 22:35:14 -0400 Subject: [PATCH 13/17] const int by value is pointless Co-authored-by: Paul Romano --- src/message_passing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/message_passing.cpp b/src/message_passing.cpp index 7a43f78f4..1630e52c0 100644 --- a/src/message_passing.cpp +++ b/src/message_passing.cpp @@ -17,7 +17,7 @@ extern "C" bool openmc_master() return mpi::master; } -vector calculate_parallel_index_vector(const int64_t size) +vector calculate_parallel_index_vector(int64_t size) { vector result; result.reserve(n_procs + 1); From 2ff6c0408e863363be6fc1bb96774e804913f784 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Tue, 18 Apr 2023 17:24:36 -0400 Subject: [PATCH 14/17] remove unnecessary comment Co-authored-by: Paul Romano --- src/state_point.cpp | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/state_point.cpp b/src/state_point.cpp index a1aa05efb..b9f805ffe 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -596,26 +596,6 @@ void write_source_bank(hid_t group_id, gsl::span source_bank, hid_t banktype = h5banktype(); // Set total and individual process dataspace sizes for source bank - // TODO: are these correct? - // the old code was - // - // int64_t dims_size = settings::n_particles; - // int64_t count_size = simulation::work_per_rank; - // - // but that doesn't make sense! The count on a rank is not - // necessarily equal to work_per_rank. For the case of the - // fission source bank, that's true, but for surface source - // creation, there may be a different number of particles - // on each rank. - // - // Hence, I have changed count_size to be the number - // on this rank rather than work_per_rank. Similarly, - // dims_size used to be n_particles, but for surface sources, - // we are not guaranteed to create n_particles at the surface. - // As a result, I've changed this to be the total size of - // the bank across all ranks, which maintains the correct - // behavior for fission bank outputs. - int64_t dims_size = bank_index.back(); int64_t count_size = bank_index[mpi::rank + 1] - bank_index[mpi::rank]; From c22448061a030e26cc5261960781415c1650d6d3 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Tue, 18 Apr 2023 17:27:16 -0400 Subject: [PATCH 15/17] warn when passing incorrect file extension to source point write --- src/state_point.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/state_point.cpp b/src/state_point.cpp index b9f805ffe..eefce8fa4 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -573,8 +573,8 @@ void write_source_point(const char* filename, gsl::span source_bank, if (extension == "") { filename_.append(".h5"); } else if (extension != "h5") { - fatal_error("openmc_source_point was passed an incorrect file " - "extension. Must either have no file extension or .h5"); + warning("write_source_point was passed a file extension differing " + "from .h5, but an hdf5 file will be written."); } hid_t file_id; From abfe5ebf99ffc0ba8bb69a6b8cd4f6ec0db03722 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Fri, 21 Apr 2023 16:09:49 -0400 Subject: [PATCH 16/17] warning rather than error for weird file name passing to mcpl --- src/mcpl_interface.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mcpl_interface.cpp b/src/mcpl_interface.cpp index 64d6046e3..5c3df026c 100644 --- a/src/mcpl_interface.cpp +++ b/src/mcpl_interface.cpp @@ -112,7 +112,7 @@ vector mcpl_source_sites(std::string path) #ifdef OPENMC_MCPL void write_mcpl_source_bank(mcpl_outfile_t file_id, - gsl::span source_bank, vector const& bank_index) + gsl::span source_bank, const vector& bank_index) { int64_t dims_size = settings::n_particles; int64_t count_size = simulation::work_per_rank; @@ -189,15 +189,15 @@ void write_mcpl_source_bank(mcpl_outfile_t file_id, //============================================================================== void write_mcpl_source_point(const char* filename, - gsl::span source_bank, vector const& bank_index) + gsl::span source_bank, const vector& bank_index) { std::string filename_(filename); const auto extension = get_file_extension(filename_); if (extension == "") { filename_.append(".mcpl"); } else if (extension != "mcpl") { - fatal_error("write_mcpl_source_point was passed an incorrect file " - "extension. Must either pass nothing or .mcpl"); + warning("write_mcpl_source_point was passed a file extension differing " + "from .mcpl, but an mcpl file will be written."); } #ifdef OPENMC_MCPL From 8344431ea06fa9612694d4dcaef00e6e93f8be77 Mon Sep 17 00:00:00 2001 From: Gavin Ridley Date: Mon, 24 Apr 2023 13:19:29 -0400 Subject: [PATCH 17/17] fix my incorrect MPI code --- src/message_passing.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/message_passing.cpp b/src/message_passing.cpp index 1630e52c0..374c1aa72 100644 --- a/src/message_passing.cpp +++ b/src/message_passing.cpp @@ -20,21 +20,19 @@ extern "C" bool openmc_master() vector calculate_parallel_index_vector(int64_t size) { vector result; - result.reserve(n_procs + 1); + result.resize(n_procs + 1); + result[0] = 0; #ifdef OPENMC_MPI - result.resize(n_procs); - result[0] = 0; - vector bank_size(n_procs + 1); // Populate the result with cumulative sum of the number of // surface source banks per process - MPI_Scan(&size, bank_size.data(), 1, MPI_INT64_T, MPI_SUM, intracomm); - MPI_Allgather(bank_size.data() + 1, 1, MPI_INT64_T, result.data(), 1, - MPI_INT64_T, intracomm); + int64_t scan_total; + MPI_Scan(&size, &scan_total, 1, MPI_INT64_T, MPI_SUM, intracomm); + MPI_Allgather( + &scan_total, 1, MPI_INT64_T, result.data() + 1, 1, MPI_INT64_T, intracomm); #else - result.push_back(0); - result.push_back(size); + result[1] = size; #endif return result;