From 15ec1ba4828ea549cf58867c75b68fcfbd4f24c9 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Wed, 10 Jan 2024 16:07:45 -0600 Subject: [PATCH 01/10] Ensure IPC is in the back of the volumes vector in DAGMC --- src/dagmc/DagMC.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/dagmc/DagMC.cpp b/src/dagmc/DagMC.cpp index c4e0494e4..e13a79e60 100644 --- a/src/dagmc/DagMC.cpp +++ b/src/dagmc/DagMC.cpp @@ -922,6 +922,26 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { // (iter++) thereafter. *(iter++) = 0; std::copy(vols.begin(), vols.end(), iter); + + // Ensure the implicit complement volume is placed at the back of this vector + // Many codes iterate over the DAGMC volumes by index and all explicit volumes should + // be checked before the implicit complement + EntityHandle implicit_complement {0}; + rval = geom_tool()->get_implicit_complement(implicit_complement); + if (rval != MB_SUCCESS) { + logger.message("Could not get the implicit complement"); + return rval; + } + auto it = std::find(vol_handles().begin(), vol_handles().end(), implicit_complement); + if (it != vol_handles().end()) { + vol_handles().erase(it); + } + else { + logger.message("Could not find the implicit complement in the volume handles vector"); + return MB_FAILURE; + } + vol_handles().push_back(implicit_complement); + idx = 1; for (Range::iterator rit = vols.begin(); rit != vols.end(); ++rit) entIndices[*rit - setOffset] = idx++; From 728764fb95d6bdcdc0df4b270ad7f47052a7648a Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 11 Jan 2024 01:24:27 -0600 Subject: [PATCH 02/10] Fix indices --- src/dagmc/DagMC.cpp | 20 +++++++++----------- src/dagmc/DagMC.hpp | 11 +++++------ 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/dagmc/DagMC.cpp b/src/dagmc/DagMC.cpp index e13a79e60..49944dad8 100644 --- a/src/dagmc/DagMC.cpp +++ b/src/dagmc/DagMC.cpp @@ -894,12 +894,6 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { logger.message("Volumes or Surfaces not found"); return MB_ENTITY_NOT_FOUND; } - setOffset = std::min(*surfs.begin(), *vols.begin()); - // surf/vol offsets are just first handles - EntityHandle tmp_offset = std::max(surfs.back(), vols.back()); - - // set size - entIndices.resize(tmp_offset - setOffset + 1); // store surf/vol handles lists (surf/vol by index) and // index by handle lists @@ -911,8 +905,10 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { *(iter++) = 0; std::copy(surfs.begin(), surfs.end(), iter); int idx = 1; - for (Range::iterator rit = surfs.begin(); rit != surfs.end(); ++rit) - entIndices[*rit - setOffset] = idx++; + for (auto surf_handle : surf_handles()) { + if (surf_handle == 0) continue; + entIndices[surf_handle] = idx++; + } vol_handles().resize(vols.size() + 1); iter = vol_handles().begin(); @@ -928,7 +924,7 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { // be checked before the implicit complement EntityHandle implicit_complement {0}; rval = geom_tool()->get_implicit_complement(implicit_complement); - if (rval != MB_SUCCESS) { + if (rval != MB_SUCCESS || implicit_complement == 0) { logger.message("Could not get the implicit complement"); return rval; } @@ -943,8 +939,10 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { vol_handles().push_back(implicit_complement); idx = 1; - for (Range::iterator rit = vols.begin(); rit != vols.end(); ++rit) - entIndices[*rit - setOffset] = idx++; + for (auto vol_handle : vol_handles()) { + if (vol_handle == 0) continue; + entIndices[vol_handle] = idx++; + } // get group handles Range groups; diff --git a/src/dagmc/DagMC.hpp b/src/dagmc/DagMC.hpp index c4f28f2bc..d39853ea9 100644 --- a/src/dagmc/DagMC.hpp +++ b/src/dagmc/DagMC.hpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -499,10 +500,8 @@ class DagMC { private: /** store some lists indexed by handle */ std::vector entHandles[5]; - /** lowest-valued handle among entity sets representing surfs and vols */ - EntityHandle setOffset; - /** entity index (contiguous 1-N indices); indexed like rootSets */ - std::vector entIndices; + /** surface and volume mapping from EntitiyHandle to DAGMC index */ + std::unordered_map entIndices; /** corresponding geometric entities; also indexed like rootSets */ std::vector geomEntities; @@ -571,8 +570,8 @@ inline EntityHandle DagMC::entity_by_index(int dimension, int index) const { } inline int DagMC::index_by_handle(EntityHandle handle) const { - assert(handle - setOffset < entIndices.size()); - return entIndices[handle - setOffset]; + assert(entIndices.count(handle) > 0); + return entIndices.at(handle); } inline unsigned int DagMC::num_entities(int dimension) const { From 0ac87d8939d8da8739fd6ec4df02109886ae56b1 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 11 Jan 2024 12:04:39 -0600 Subject: [PATCH 03/10] Update changelog --- doc/CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/CHANGELOG.rst b/doc/CHANGELOG.rst index 5e83cab43..0648283d1 100644 --- a/doc/CHANGELOG.rst +++ b/doc/CHANGELOG.rst @@ -10,6 +10,7 @@ Next version **Changed:** * Update hdf5 to v1.14.3 from v1.10.4 (#931 #933) + * Ensure implicit complement handle is placed at the back of DAGMC volume indices (#935) v3.2.3 ==================== From 928c606458a0546e789aa17eeb77dff658a380f9 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 11 Jan 2024 15:33:11 -0600 Subject: [PATCH 04/10] Adjust logic for setting implicit complement in entHandles --- src/dagmc/DagMC.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/dagmc/DagMC.cpp b/src/dagmc/DagMC.cpp index 49944dad8..8803f0bf3 100644 --- a/src/dagmc/DagMC.cpp +++ b/src/dagmc/DagMC.cpp @@ -924,19 +924,18 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { // be checked before the implicit complement EntityHandle implicit_complement {0}; rval = geom_tool()->get_implicit_complement(implicit_complement); - if (rval != MB_SUCCESS || implicit_complement == 0) { - logger.message("Could not get the implicit complement"); - return rval; - } - auto it = std::find(vol_handles().begin(), vol_handles().end(), implicit_complement); - if (it != vol_handles().end()) { - vol_handles().erase(it); - } - else { - logger.message("Could not find the implicit complement in the volume handles vector"); - return MB_FAILURE; + if (rval == MB_SUCCESS && implicit_complement != 0) { + auto it = std::find(vol_handles().begin(), vol_handles().end(), implicit_complement); + if (it != vol_handles().end()) { + vol_handles().erase(it); + } + else { + logger.message("Could not find the implicit complement in the volume handles vector"); + return MB_FAILURE; + } + // insert the implicit complement at the end of the vector + vol_handles().push_back(implicit_complement); } - vol_handles().push_back(implicit_complement); idx = 1; for (auto vol_handle : vol_handles()) { From c0179420273478fb4c0f26db0b064a1596ce48b4 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 11 Jan 2024 15:47:54 -0600 Subject: [PATCH 05/10] Applying style guide --- src/dagmc/DagMC.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/dagmc/DagMC.cpp b/src/dagmc/DagMC.cpp index 8803f0bf3..84fc1fcda 100644 --- a/src/dagmc/DagMC.cpp +++ b/src/dagmc/DagMC.cpp @@ -920,17 +920,19 @@ ErrorCode DagMC::build_indices(Range& surfs, Range& vols) { std::copy(vols.begin(), vols.end(), iter); // Ensure the implicit complement volume is placed at the back of this vector - // Many codes iterate over the DAGMC volumes by index and all explicit volumes should - // be checked before the implicit complement - EntityHandle implicit_complement {0}; + // Many codes iterate over the DAGMC volumes by index and all explicit + // volumes should be checked before the implicit complement + EntityHandle implicit_complement{0}; rval = geom_tool()->get_implicit_complement(implicit_complement); if (rval == MB_SUCCESS && implicit_complement != 0) { - auto it = std::find(vol_handles().begin(), vol_handles().end(), implicit_complement); + auto it = std::find(vol_handles().begin(), vol_handles().end(), + implicit_complement); if (it != vol_handles().end()) { vol_handles().erase(it); - } - else { - logger.message("Could not find the implicit complement in the volume handles vector"); + } else { + logger.message( + "Could not find the implicit complement in the volume handles " + "vector"); return MB_FAILURE; } // insert the implicit complement at the end of the vector From fa711b4618e94eced902f89caa9d0064ccd40d81 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 11 Jan 2024 16:04:44 -0600 Subject: [PATCH 06/10] Applying style guide to DAGMC header as well --- src/dagmc/DagMC.hpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/dagmc/DagMC.hpp b/src/dagmc/DagMC.hpp index d39853ea9..49e40f86b 100644 --- a/src/dagmc/DagMC.hpp +++ b/src/dagmc/DagMC.hpp @@ -4,7 +4,6 @@ #include #include -#include #include #include #include @@ -78,12 +77,10 @@ class DagMC { double overlap_tolerance = 0., double numerical_precision = .001, int verbosity = 1); // Deprecated Constructor - [ - [deprecated("Replaced by DagMC(std::shared_ptr mb_impl, ... " - ")")]] DagMC(Interface* mb_impl, - double overlap_tolerance = 0., - double numerical_precision = .001, - int verbosity = 1); + [[deprecated( + "Replaced by DagMC(std::shared_ptr mb_impl, ... " + ")")]] DagMC(Interface* mb_impl, double overlap_tolerance = 0., + double numerical_precision = .001, int verbosity = 1); // Destructor ~DagMC(); From 06240def7a436836e893e0b43c2c75b08b92d5eb Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 11 Jan 2024 16:24:05 -0600 Subject: [PATCH 07/10] Fixing style guide application geeze --- src/dagmc/DagMC.hpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/dagmc/DagMC.hpp b/src/dagmc/DagMC.hpp index 49e40f86b..47c10842a 100644 --- a/src/dagmc/DagMC.hpp +++ b/src/dagmc/DagMC.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "DagMCVersion.hpp" @@ -77,10 +78,12 @@ class DagMC { double overlap_tolerance = 0., double numerical_precision = .001, int verbosity = 1); // Deprecated Constructor - [[deprecated( - "Replaced by DagMC(std::shared_ptr mb_impl, ... " - ")")]] DagMC(Interface* mb_impl, double overlap_tolerance = 0., - double numerical_precision = .001, int verbosity = 1); + [ + [deprecated("Replaced by DagMC(std::shared_ptr mb_impl, ... " + ")")]] DagMC(Interface* mb_impl, + double overlap_tolerance = 0., + double numerical_precision = .001, + int verbosity = 1); // Destructor ~DagMC(); From 00d7681e59e756553ab233fc2720bf57d06d8175 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 1 Feb 2024 16:52:43 -0600 Subject: [PATCH 08/10] Adding test for the implicit complement index in the DAGMC indices --- src/dagmc/tests/CMakeLists.txt | 1 + src/dagmc/tests/dagmc_ipc_index_test.cpp | 62 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/dagmc/tests/dagmc_ipc_index_test.cpp diff --git a/src/dagmc/tests/CMakeLists.txt b/src/dagmc/tests/CMakeLists.txt index b5417eb6b..347c24c76 100644 --- a/src/dagmc/tests/CMakeLists.txt +++ b/src/dagmc/tests/CMakeLists.txt @@ -12,6 +12,7 @@ dagmc_install_test(dagmc_pointinvol_test cpp) dagmc_install_test(dagmc_rayfire_test cpp) dagmc_install_test(dagmc_simple_test cpp) dagmc_install_test(dagmc_graveyard_test cpp) +dagmc_install_test(dagmc_ipc_index_test cpp) dagmc_install_test_file(test_dagmc.h5m) dagmc_install_test_file(test_dagmc_impl.h5m) diff --git a/src/dagmc/tests/dagmc_ipc_index_test.cpp b/src/dagmc/tests/dagmc_ipc_index_test.cpp new file mode 100644 index 000000000..f7489fffb --- /dev/null +++ b/src/dagmc/tests/dagmc_ipc_index_test.cpp @@ -0,0 +1,62 @@ +#include + +#include +#include +#include + +#include "DagMC.hpp" +#include "moab/Core.hpp" +#include "moab/Interface.hpp" + +static std::string simple_file = "test_dagmc.h5m"; + +class DagmcIPCPositionTest : public ::testing::Test { + protected: + virtual void SetUp() {} + virtual void TearDown() { + if (std::filesystem::exists("tmp.h5m")) { + std::filesystem::remove("tmp.h5m"); + } + } +}; + +using namespace moab; + + +TEST_F(DagmcIPCPositionTest, dagmc_implicit_complement_position_test) { + // create a DAGMC instance + std::unique_ptr dagmc = std::make_unique(); + + // load the geometry test file + ErrorCode rval = dagmc->load_file(simple_file.c_str()); + + // add a bunch of meshsets to this file + EntityHandle tmp; + for (int i = 0; i < 1E6; i++) { + rval = dagmc->moab_instance()->create_meshset(MESHSET_SET, tmp); + ASSERT_EQ(rval, MB_SUCCESS); + } + + dagmc->moab_instance()->write_file("tmp.h5m"); + + // create a second DAGMC instance nd read the new file + std::unique_ptr dagmc2 = std::make_unique(); + + dagmc2->load_file("tmp.h5m"); + dagmc2->geom_tool()->find_geomsets(); + dagmc2->setup_impl_compl(); + dagmc2->setup_indices(); + + EntityHandle ipc = 0; + rval = dagmc2->geom_tool()->get_implicit_complement(ipc); + ASSERT_EQ(rval, MB_SUCCESS); + ASSERT_NE(ipc, 0); + + int num_vols = dagmc2->num_entities(3); + // 3 volumes in the original model, plus the IPC + ASSERT_EQ(num_vols, 4); + + // Reminder: DAGMC indieces are 1-based + std::cout << "IPC index: " << dagmc2->index_by_handle(ipc) << std::endl; + ASSERT_EQ(dagmc2->index_by_handle(ipc), 4); +} \ No newline at end of file From c50a1c98370229a237cdfad7b9d46f30666d1602 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 1 Feb 2024 16:58:52 -0600 Subject: [PATCH 09/10] Adding some test description --- src/dagmc/tests/dagmc_ipc_index_test.cpp | 26 +++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/dagmc/tests/dagmc_ipc_index_test.cpp b/src/dagmc/tests/dagmc_ipc_index_test.cpp index f7489fffb..c315a1628 100644 --- a/src/dagmc/tests/dagmc_ipc_index_test.cpp +++ b/src/dagmc/tests/dagmc_ipc_index_test.cpp @@ -22,7 +22,14 @@ class DagmcIPCPositionTest : public ::testing::Test { using namespace moab; - +// This test exists to ensure that the IPC is correctly positioned in the model +// See GitHub issue #934 for more information +// The test loads a known, working DAGMC file, adds a bunch of meshsets to it, +// and then writes a temporary file. The temporary file is then read back in +// and the IPC is checked to ensure it is in the correct position. The condision +// being tested is only triggered during a file read when EntityHandle sequences +// are generated and accessed internally and cannot be reproduced +// using the external-facing MOAB API. TEST_F(DagmcIPCPositionTest, dagmc_implicit_complement_position_test) { // create a DAGMC instance std::unique_ptr dagmc = std::make_unique(); @@ -30,32 +37,41 @@ TEST_F(DagmcIPCPositionTest, dagmc_implicit_complement_position_test) { // load the geometry test file ErrorCode rval = dagmc->load_file(simple_file.c_str()); - // add a bunch of meshsets to this file + // add one million mesh sets to this file to ensure the threshold of + // meshsets in a file is surpassed to trigger this condition + // NOTE: far fewer meshsets are needed to trigger this condition, but + // one million was chosen to be conservative should the internal + // allocation size in MOAB change again in the future EntityHandle tmp; for (int i = 0; i < 1E6; i++) { rval = dagmc->moab_instance()->create_meshset(MESHSET_SET, tmp); ASSERT_EQ(rval, MB_SUCCESS); } + // write a temprary file dagmc->moab_instance()->write_file("tmp.h5m"); - // create a second DAGMC instance nd read the new file + // create a second DAGMC instance and read the temporary file std::unique_ptr dagmc2 = std::make_unique(); - dagmc2->load_file("tmp.h5m"); + + // perform necessary DAGMC setup dagmc2->geom_tool()->find_geomsets(); dagmc2->setup_impl_compl(); dagmc2->setup_indices(); + // get the implicit complement handle, it should exist after the explicit call + // for its creation above EntityHandle ipc = 0; rval = dagmc2->geom_tool()->get_implicit_complement(ipc); ASSERT_EQ(rval, MB_SUCCESS); ASSERT_NE(ipc, 0); + // there are 3 volumes in the original model and we've added the implicit complement int num_vols = dagmc2->num_entities(3); - // 3 volumes in the original model, plus the IPC ASSERT_EQ(num_vols, 4); + // Make sure the IPC handle index is highest // Reminder: DAGMC indieces are 1-based std::cout << "IPC index: " << dagmc2->index_by_handle(ipc) << std::endl; ASSERT_EQ(dagmc2->index_by_handle(ipc), 4); From c9f062459e71a1199428217a2da8b402a09daa10 Mon Sep 17 00:00:00 2001 From: Patrick Shriwise Date: Thu, 1 Feb 2024 17:00:59 -0600 Subject: [PATCH 10/10] Style guide compliance --- src/dagmc/tests/dagmc_ipc_index_test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/dagmc/tests/dagmc_ipc_index_test.cpp b/src/dagmc/tests/dagmc_ipc_index_test.cpp index c315a1628..fad35ca58 100644 --- a/src/dagmc/tests/dagmc_ipc_index_test.cpp +++ b/src/dagmc/tests/dagmc_ipc_index_test.cpp @@ -1,8 +1,8 @@ #include +#include #include #include -#include #include "DagMC.hpp" #include "moab/Core.hpp" @@ -67,7 +67,8 @@ TEST_F(DagmcIPCPositionTest, dagmc_implicit_complement_position_test) { ASSERT_EQ(rval, MB_SUCCESS); ASSERT_NE(ipc, 0); - // there are 3 volumes in the original model and we've added the implicit complement + // there are 3 volumes in the original model and we've added the implicit + // complement int num_vols = dagmc2->num_entities(3); ASSERT_EQ(num_vols, 4);