From 6c7c0a6816cf8940402810255440b7c99b0141d1 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Tue, 5 Jan 2021 22:23:19 -0600 Subject: [PATCH] Addressing initial review comments from @paulromano. --- include/openmc/dagmc.h | 2 +- include/openmc/mesh.h | 24 ++++++++------ openmc/lib/__init__.py | 4 +-- src/dagmc.cpp | 4 +-- src/finalize.cpp | 2 +- src/initialize.cpp | 9 ++--- src/mesh.cpp | 66 ++++++++++++++++++++++++------------- src/state_point.cpp | 6 ++-- src/tallies/filter_mesh.cpp | 2 -- 9 files changed, 72 insertions(+), 47 deletions(-) diff --git a/include/openmc/dagmc.h b/include/openmc/dagmc.h index 859a229f0a..9f409d8730 100644 --- a/include/openmc/dagmc.h +++ b/include/openmc/dagmc.h @@ -3,7 +3,7 @@ #define OPENMC_DAGMC_H namespace openmc { -extern "C" const bool dagmc_enabled; +extern "C" const bool DAGMC_ENABLED; } #ifdef DAGMC diff --git a/include/openmc/mesh.h b/include/openmc/mesh.h index 5bdb5353fc..e57f09e857 100644 --- a/include/openmc/mesh.h +++ b/include/openmc/mesh.h @@ -40,7 +40,7 @@ namespace openmc { // Global variables //============================================================================== -extern "C" const bool libmesh_enabled; +extern "C" const bool LIBMESH_ENABLED; class Mesh; @@ -54,7 +54,7 @@ extern std::vector> meshes; #ifdef LIBMESH namespace settings { // used when creating new libMesh::Mesh instances -extern std::unique_ptr LMI; +extern std::unique_ptr libmesh_init; } #endif @@ -95,6 +95,9 @@ public: //! Get the number of mesh cell surfaces. virtual int n_surface_bins() const = 0; + //! Set the mesh ID + void set_id(int32_t id=-1); + //! Write mesh data to an HDF5 group // //! \param[in] group HDF5 group @@ -285,12 +288,6 @@ public: UnstructuredMesh(const std::string& filename); // Methods -private: - - //! Setup method for the mesh. Builds data structures, - //! sets up element mapping, creates bounding boxes, etc. - virtual void initialize() = 0; - public: //! Add a variable to the mesh instance @@ -332,6 +329,13 @@ public: void to_hdf5(hid_t group) const override; +private: + + //! Setup method for the mesh. Builds data structures, + //! sets up element mapping, creates bounding boxes, etc. + virtual void initialize() = 0; + +public: // Data members bool output_ {true}; //!< Write tallies onto the unstructured mesh at the end of a run std::string filename_; //!< Path to unstructured mesh file @@ -534,10 +538,10 @@ private: // Data members std::unique_ptr m_; //!< pointer to the libMesh mesh instance - std::vector> PL_; //!< per-thread point locators + std::vector> pl_; //!< per-thread point locators std::unique_ptr equation_systems_; //!< pointer to the equation systems of the mesh std::string eq_system_name_; //!< name of the equation system holding OpenMC results - std::map variable_map_; //!< mapping of variable names (tally scores) to libMesh variable numbers + std::unordered_map variable_map_; //!< mapping of variable names (tally scores) to libMesh variable numbers libMesh::BoundingBox bbox_; //!< bounding box of the mesh libMesh::Elem* first_element_; //!< pointer to the first element in the mesh (maybe should be a key?) }; diff --git a/openmc/lib/__init__.py b/openmc/lib/__init__.py index 9a72403ff4..82dc92ba48 100644 --- a/openmc/lib/__init__.py +++ b/openmc/lib/__init__.py @@ -40,13 +40,13 @@ else: def _dagmc_enabled(): - return c_bool.in_dll(_dll, "dagmc_enabled").value + return c_bool.in_dll(_dll, "DAGMC_ENABLED").value def _coord_levels(): return c_int.in_dll(_dll, "n_coord_levels").value def _libmesh_enabled(): - return c_bool.in_dll(_dll, "libmesh_enabled").value + return c_bool.in_dll(_dll, "LIBMESH_ENABLED").value from .error import * from .core import * diff --git a/src/dagmc.cpp b/src/dagmc.cpp index 58d83f54c9..5939f3f400 100644 --- a/src/dagmc.cpp +++ b/src/dagmc.cpp @@ -26,9 +26,9 @@ namespace openmc { #ifdef DAGMC -const bool dagmc_enabled = true; +const bool DAGMC_ENABLED = true; #else -const bool dagmc_enabled = false; +const bool DAGMC_ENABLED = false; #endif } diff --git a/src/finalize.cpp b/src/finalize.cpp index 3afd9e1861..45dd0fc7cc 100644 --- a/src/finalize.cpp +++ b/src/finalize.cpp @@ -138,7 +138,7 @@ int openmc_finalize() free_memory(); #ifdef LIBMESH - settings::LMI.reset(); + settings::libmesh_init.reset(); #endif // Free all MPI types diff --git a/src/initialize.cpp b/src/initialize.cpp index c3c7411ab9..c480392cba 100644 --- a/src/initialize.cpp +++ b/src/initialize.cpp @@ -68,16 +68,17 @@ int openmc_init(int argc, char* argv[], const void* intracomm) #endif // initialize libMesh if it hasn't been initialized already -// (if initialized externally, the LMI object needs to be provided also) -if (!settings::LMI && !libMesh::initialized()) { +// (if initialized externally, the libmesh_init object needs to be provided also) +if (!settings::libmesh_init && !libMesh::initialized()) +{ #ifdef OPENMC_MPI // pass command line args, empty MPI communicator, and number of threads. // Because libMesh was not initialized, we assume that OpenMC is the primary // application and that its main MPI comm should be used. - settings::LMI = std::make_unique(argc, argv, comm, n_threads); + settings::libmesh_init = std::make_unique(argc, argv, comm, n_threads); #else // pass command line args, empty MPI communicator, and number of threads - settings::LMI = std::make_unique(argc, argv, 0, n_threads); + settings::libmesh_init = std::make_unique(argc, argv, 0, n_threads); #endif } diff --git a/src/mesh.cpp b/src/mesh.cpp index 4cc7efc1cf..06f8701519 100644 --- a/src/mesh.cpp +++ b/src/mesh.cpp @@ -40,9 +40,9 @@ namespace openmc { //============================================================================== #ifdef LIBMESH -const bool libmesh_enabled = true; +const bool LIBMESH_ENABLED = true; #else -const bool libmesh_enabled = false; +const bool LIBMESH_ENABLED = false; #endif @@ -55,7 +55,7 @@ std::vector> meshes; #ifdef LIBMESH namespace settings { -std::unique_ptr LMI; +std::unique_ptr libmesh_init; } #endif @@ -103,6 +103,35 @@ Mesh::Mesh(pugi::xml_node node) } } +void +Mesh::set_id(int32_t id) { + Expects(id >=0 || id == C_NONE); + + // Clear entry in mesh map in case one was already assigned + if (id_ != C_NONE) { + model::mesh_map.erase(id_); + id_ = C_NONE; + } + + // Ensure no other mesh has the same ID + if (model::mesh_map.find(id) != model::mesh_map.end()) { + throw std::runtime_error{fmt::format("Two meshes have the same ID: {}", id)}; + } + + // If no ID is specified, auto-assign the next ID in the sequence + if (id == C_NONE) { + id = 0; + for (const auto& m : model::meshes) { + id = std::max(id, m->id_); + } + ++id; + } + + // Update ID and entry in the mesh map + id_ = id; + model::mesh_map[id] = model::meshes.size() - 1; +} + //============================================================================== // Structured Mesh implementation //============================================================================== @@ -1388,12 +1417,9 @@ extern "C" int openmc_add_unstructured_mesh(const char filename[], return OPENMC_E_INVALID_ARGUMENT; } - int mesh_id = 0; - for (const auto& m : model::meshes) { mesh_id = std::max(m->id_, mesh_id); } - mesh_id += 1; - - model::meshes.back()->id_ = mesh_id + 1; - *id = mesh_id; + // auto-assign new ID + model::meshes.back()->set_id(-1); + *id = model::meshes.back()->id_; return 0; } @@ -2124,11 +2150,11 @@ LibMesh::LibMesh(const std::string& filename) void LibMesh::initialize() { - if (!settings::LMI) { + if (!settings::libmesh_init) { fatal_error("Attempting to use an unstructured mesh without a libMesh::LibMeshInit object."); } - m_ = std::make_unique(settings::LMI->comm(), n_dimension_); + m_ = std::make_unique(settings::libmesh_init->comm(), n_dimension_); m_->read(filename_); m_->prepare_for_use(); @@ -2152,9 +2178,9 @@ void LibMesh::initialize() #endif for (int i = 0; i < n_threads; i++) { - PL_.emplace_back(m_->sub_point_locator()); - PL_.back()->set_contains_point_tol(FP_COINCIDENT); - PL_.back()->enable_out_of_mesh_mode(); + pl_.emplace_back(m_->sub_point_locator()); + pl_.back()->set_contains_point_tol(FP_COINCIDENT); + pl_.back()->enable_out_of_mesh_mode(); } // store first element in the mesh to use as an offset for bin indices @@ -2188,7 +2214,7 @@ int LibMesh::n_surface_bins() const // if this is a boundary element, sure to count boundary faces // twice for (int j = 0; j < e.n_sides(); j++) { - if (e.neighbor_ptr(j) == NULL) { n_bins++; } + if (e.neighbor_ptr(j) == nullptr) { n_bins++; } } } return n_bins; @@ -2289,14 +2315,10 @@ LibMesh::get_bin(Position r) const int thread_num = 0; #endif - const auto& point_locator = PL_.at(thread_num); + const auto& point_locator = pl_.at(thread_num); - auto e = (*point_locator)(p); - if (!e) { - return -1; - } else { - return get_bin_from_element(e); - } + const auto elem_ptr = (*point_locator)(p); + return elem_ptr ? get_bin_from_element(elem_ptr) : -1; } int diff --git a/src/state_point.cpp b/src/state_point.cpp index 33c682a1e9..8e1c6e55d9 100644 --- a/src/state_point.cpp +++ b/src/state_point.cpp @@ -806,11 +806,11 @@ void write_unstructured_mesh_results() { int n_realizations = tally->n_realizations_; for (int i_score = 0; i_score < tally->scores_.size(); i_score++) { - for (int i_nuc = 0; i_nuc < tally->nuclides_.size(); i_nuc++) { + for (auto nuc_idx : tally->nuclides_) { // generate a name for the value std::string nuclide_name = "total"; // start with total by default - if (tally->nuclides_[i_nuc] > -1) { - nuclide_name = data::nuclides[tally->nuclides_[i_nuc]]->name_; + if (nuc_idx > -1) { + nuclide_name = data::nuclides[nuc_idx]->name_; } std::string score_name = tally->score_name(i_score); diff --git a/src/tallies/filter_mesh.cpp b/src/tallies/filter_mesh.cpp index eb9275bd44..7897173868 100644 --- a/src/tallies/filter_mesh.cpp +++ b/src/tallies/filter_mesh.cpp @@ -43,8 +43,6 @@ const } else { model::meshes[mesh_]->bins_crossed(p, match.bins_, match.weights_); } - - double total = std::accumulate(match.weights_.begin(), match.weights_.end(), 0.0); } void