From 2463f334db573727e28b095a353b664ba9e2aebb Mon Sep 17 00:00:00 2001 From: "Noah S. Elliott" Date: Thu, 23 Jan 2025 13:35:06 -0800 Subject: [PATCH 1/6] Add option to do piecewise loading of external data in spio IOManager. --- src/axom/sidre/core/Group.cpp | 34 +- src/axom/sidre/core/Group.hpp | 18 +- src/axom/sidre/spio/IOManager.cpp | 141 +++++++++ src/axom/sidre/spio/IOManager.hpp | 34 ++ src/axom/sidre/tests/spio/spio_parallel.hpp | 329 ++++++++++++++++++++ 5 files changed, 554 insertions(+), 2 deletions(-) diff --git a/src/axom/sidre/core/Group.cpp b/src/axom/sidre/core/Group.cpp index f4314dfadc..9495654fc1 100644 --- a/src/axom/sidre/core/Group.cpp +++ b/src/axom/sidre/core/Group.cpp @@ -2266,7 +2266,7 @@ bool Group::load(const hid_t& h5_id, * * Load External Data from an hdf5 file * - * Note: this ASSUMES uses the "sidre_hdf5" protocol + * Note: this ASSUMES usage of the "sidre_hdf5" protocol ************************************************************************* */ bool Group::loadExternalData(const hid_t& h5_id) @@ -2281,6 +2281,38 @@ bool Group::loadExternalData(const hid_t& h5_id) return !(getDataStore()->getConduitErrorOccurred()); } +/* + ************************************************************************* + * + * Load External Data from a path within an hdf5 file + * + * Note: this ASSUMES usage of the "sidre_hdf5" protocol + ************************************************************************* + */ +bool Group::loadExternalData(const hid_t& h5_id, const std::string& group_path) +{ + bool success; + std::string delim(1, getPathDelimiter()); + if (group_path.empty() || group_path == delim) { + // An empty or trivial path means the load should read from the start of + // the file. + success = loadExternalData(h5_id); + } + else + { + Node n; + createExternalLayout(n); + ConduitErrorSuppressor checkConduitCall(getDataStore()); + + std::string read_path("sidre/external/" + group_path); + + checkConduitCall( + [&] { conduit::relay::io::hdf5_read(h5_id, read_path, n); }); + success = !(getDataStore()->getConduitErrorOccurred()); + } + return success; +} + #endif /* AXOM_USE_HDF5 */ //////////////////////////////////////////////////////////////////////// diff --git a/src/axom/sidre/core/Group.hpp b/src/axom/sidre/core/Group.hpp index 690f48e7d4..04d57dda84 100644 --- a/src/axom/sidre/core/Group.hpp +++ b/src/axom/sidre/core/Group.hpp @@ -1650,7 +1650,7 @@ class Group std::string& name_from_file); /*! - * \brief Load data into the Group's external views from a hdf5 handle. + * \brief Load data into the Group's external views from an hdf5 handle. * * No protocol argument is needed, as this only is used with the sidre_hdf5 * protocol. Returns true (success) if no Conduit I/O error occurred since @@ -1662,6 +1662,22 @@ class Group */ bool loadExternalData(const hid_t& h5_id); + /*! + * \brief Load data into the Group's external views from a path into + * hdf5 handle + * + * No protocol argument is needed, as this only is used with the sidre_hdf5 + * protocol. Returns true (success) if no Conduit I/O error occurred since + * this Group's DataStore was created or had its error flag cleared; false, + * if an error occurred at some point. + * + * \param h5_id hdf5 handle + * \param group_path path to a group below the group pointed by the h5_id. + * This path points to the group in the hdf5 hierarchy + * that represents this Group object. + */ + bool loadExternalData(const hid_t& h5_id, const std::string& group_path); + #endif /* AXOM_USE_HDF5 */ //@} diff --git a/src/axom/sidre/spio/IOManager.cpp b/src/axom/sidre/spio/IOManager.cpp index dddf70b092..d5ade22f7e 100644 --- a/src/axom/sidre/spio/IOManager.cpp +++ b/src/axom/sidre/spio/IOManager.cpp @@ -465,6 +465,147 @@ void IOManager::loadExternalData(sidre::Group* datagroup, (void)m_baton->pass(); } +void IOManager::loadExternalData(sidre::Group* parent_group, + sidre::Group* load_group, + const std::string& root_file) +{ + int num_files = getNumFilesFromRoot(root_file); + int num_groups = getNumGroupsFromRoot(root_file); + SLIC_ASSERT(num_files > 0); + SLIC_ASSERT(num_groups > 0); + + if(m_baton) + { + if(m_baton->getNumFiles() != num_files) + { + delete m_baton; + m_baton = nullptr; + } + } + + if(!m_baton) + { + m_baton = new IOBaton(m_mpi_comm, num_files, num_groups); + } + +#ifdef AXOM_USE_HDF5 + std::string file_pattern = getHDF5FilePattern(root_file); + + std::string parent_name(parent_group->getPathName()); + std::string load_name(load_group->getPathName()); + axom::Path parent_path(parent_name); + axom::Path load_path(load_name); + std::vector parent_parts(parent_path.parts()); + std::vector load_parts(load_path.parts()); + + size_t parent_size = parent_parts.size(); + bool can_load = true; + if (load_parts.size() < parent_size) { + can_load = false; + } + + for (size_t i = 0; i < parent_size; ++i) { + if (parent_parts[i] != load_parts[i]) { + can_load = false; + } + } + + std::string subpath = load_name.substr(parent_name.size()); + char delimiter = parent_group->getPathDelimiter(); + if (!subpath.empty() && subpath[0] == delimiter) { + subpath.erase(0,1); + } + + if (!subpath.empty() && !parent_group->hasGroup(subpath)) { + can_load = false; + } + + int set_id = m_baton->wait(); + + if (can_load) { + if(num_groups <= m_comm_size) + { + if(m_my_rank < num_groups) + { + herr_t errv; + AXOM_UNUSED_VAR(errv); + + std::string hdf5_name = getFileNameForRank(file_pattern, root_file, set_id); + + hdf5_name = getSCRPath(hdf5_name); + + hid_t h5_file_id = conduit::relay::io::hdf5_open_file_for_read(hdf5_name); + SLIC_ASSERT(h5_file_id >= 0); + + std::string group_name = "datagroup"; + if(H5Lexists(h5_file_id, group_name.c_str(), 0) <= 0) + { + group_name = fmt::sprintf("datagroup_%07d", m_my_rank); + } + + hid_t h5_group_id = H5Gopen(h5_file_id, group_name.c_str(), 0); + SLIC_ASSERT(h5_group_id >= 0); + + load_group->loadExternalData(h5_group_id, subpath); + + errv = H5Gclose(h5_group_id); + SLIC_ASSERT(errv >= 0); + + errv = H5Fclose(h5_file_id); + SLIC_ASSERT(errv >= 0); + } + } + else + { + for(int input_rank = m_my_rank; input_rank < num_groups; + input_rank += m_comm_size) + { + herr_t errv; + AXOM_UNUSED_VAR(errv); + + std::string hdf5_name = + getFileNameForRank(file_pattern, root_file, input_rank); + + hdf5_name = getSCRPath(hdf5_name); + + hid_t h5_file_id = conduit::relay::io::hdf5_open_file_for_read(hdf5_name); + SLIC_ASSERT(h5_file_id >= 0); + + std::string group_name = "datagroup"; + if(H5Lexists(h5_file_id, group_name.c_str(), 0) <= 0) + { + group_name = fmt::sprintf("datagroup_%07d", input_rank); + } + + hid_t h5_group_id = H5Gopen(h5_file_id, group_name.c_str(), 0); + SLIC_ASSERT(h5_group_id >= 0); + + std::string input_name = fmt::sprintf("rank_%07d", input_rank); + + input_name = input_name + delimiter + "sidre_input" + delimiter + subpath; + + Group* one_rank_input = parent_group->getGroup(input_name); + + one_rank_input->loadExternalData(h5_group_id, subpath); + + errv = H5Gclose(h5_group_id); + SLIC_ASSERT(errv >= 0); + + errv = H5Fclose(h5_file_id); + SLIC_ASSERT(errv >= 0); + } + } + } + +#else + AXOM_UNUSED_VAR(datagroup); + SLIC_WARNING("Loading external data only only available " + << "when Axom is configured with hdf5"); +#endif /* AXOM_USE_HDF5 */ + + (void)m_baton->pass(); +} + /* ************************************************************************* * diff --git a/src/axom/sidre/spio/IOManager.hpp b/src/axom/sidre/spio/IOManager.hpp index 68af7be0a8..86620f2350 100644 --- a/src/axom/sidre/spio/IOManager.hpp +++ b/src/axom/sidre/spio/IOManager.hpp @@ -289,11 +289,45 @@ class IOManager * This currently only works if the root file was created for protocol * sidre_hdf5. * + * The call to this must follow a call to the IOManager::read with the + * same group and root file. + * + * This is intended as the third step of the three step process to load + * external data. The first step is the call to IOManager::read; the + * second step is to set valid pointers on all external views in the + * hierarchy under the passed-in group; the third step is the call to + * this method. + * * \param group Group to fill with external data from input * \param root_file root file containing input data */ void loadExternalData(sidre::Group* group, const std::string& root_file); + /*! + * \brief piecewise load of external data into a group + * + * This currently only works if the root file was created for protocol + * sidre_hdf5. + * + * This is intended as an alternative way to load external data, in a + * piecewise manner rather than loading all external data of a Sidre + * hierarchy at once. load_group is a Group somewhere in the hierarchy + * under parent_group, and only the external data for views in the + * subtree under load_group will be loaded. In the second step of the + * three step process, the calling code should set valid pointers on all + * external views in the subtree under load_group. The third step is + * to call this method. parent_group must be the same group that was + * passed to IOManager::read with the same root file. + * + * This may be called multiple times with different instances of load_group + * representing different subtrees of the hierachy under parent_group. + * + * \param parent_group Group that was passed to IOManager::read + * \param load_group Group holding views to be filled with external data + * \param root_file root file containing input data + */ + void loadExternalData(sidre::Group* parent_group, sidre::Group* load_group, const std::string& root_file); + /*! * \brief gets the number of files in the dataset from the specified root file */ diff --git a/src/axom/sidre/tests/spio/spio_parallel.hpp b/src/axom/sidre/tests/spio/spio_parallel.hpp index 974e7d5241..cdefb8ceda 100644 --- a/src/axom/sidre/tests/spio/spio_parallel.hpp +++ b/src/axom/sidre/tests/spio/spio_parallel.hpp @@ -420,6 +420,335 @@ TEST(spio_parallel, external_writeread) delete ds2; } +//------------------------------------------------------------------------------ +TEST(spio_parallel, external_partial_writeread) +{ + if(PROTOCOL != "sidre_hdf5") + { + SUCCEED() << "Loading external data in spio only currently supported " + << " for 'sidre_hdf5' protocol"; + return; + } + + int my_rank; + MPI_Comm_rank(MPI_COMM_WORLD, &my_rank); + + int num_ranks; + MPI_Comm_size(MPI_COMM_WORLD, &num_ranks); + + const int num_output = numOutputFiles(num_ranks); + + const int nvals = 10; + int orig_vals1[nvals], orig_vals2[nvals]; + for(int i = 0; i < 10; i++) + { + orig_vals1[i] = (i + 10) * (404 - my_rank - i); + orig_vals2[i] = (i + 10) * (404 - my_rank - i) + 20; + } + + /* + * Create a DataStore and give it a small hierarchy of groups and views. + * + * The views are filled with repeatable nonsense data that will vary based + * on rank + */ + DataStore* ds1 = new DataStore(); + + Group* root1 = ds1->getRoot(); + + Group* flds = root1->createGroup("testdata/fields"); + + Group* ga = flds->createGroup("a"); + Group* gb = flds->createGroup("b"); + ga->createView("vals1", axom::sidre::INT_ID, nvals, orig_vals1); + gb->createView("vals2", axom::sidre::INT_ID, nvals, orig_vals2); + + /* + * Contents of the DataStore written to files with IOManager. + */ + const int num_files = num_output; + IOManager writer(MPI_COMM_WORLD); + + std::string file_name = "out_spio_external_partial"; + + writer.write(root1, num_files, file_name, PROTOCOL); + + /* + * Create another DataStore to be the destination for input from file + */ + DataStore* ds2 = new DataStore(); + Group* root2 = ds2->getRoot(); + + /* + * Read from the files that were written above. + */ + IOManager reader(MPI_COMM_WORLD); + + reader.read(root2, file_name + ROOT_EXT); + + int restored_vals1[nvals], restored_vals2[nvals]; + for(int i = 0; i < nvals; ++i) + { + restored_vals1[i] = -1; + restored_vals2[i] = -1; + } + + Group* group_a = root2->getGroup("testdata/fields/a"); + Group* group_b = root2->getGroup("testdata/fields/b"); + + View* view1 = group_a->getView("vals1"); + view1->setExternalDataPtr(restored_vals1); + + View* view2 = group_b->getView("vals2"); + view2->setExternalDataPtr(restored_vals2); + + reader.loadExternalData(root2, group_a, file_name + ROOT_EXT); + + enum SpioTestResult + { + SPIO_TEST_SUCCESS = 0, + HIERARCHY_ERROR = 1 << 0, + EXT_ARRAY_ERROR = 1 << 1, + EXT_UNDESC_ERROR = 1 << 2 + }; + int result = SPIO_TEST_SUCCESS; + + /* + * Verify that the contents of view1 are restored. + */ + EXPECT_EQ(view1->getNumElements(), nvals); + if(view1->getNumElements() != nvals) + { + result |= EXT_ARRAY_ERROR; + } + else + { + for(int i = 0; i < nvals; ++i) + { + EXPECT_EQ(orig_vals1[i], restored_vals1[i]); + if(orig_vals1[i] != restored_vals1[i]) + { + result |= EXT_ARRAY_ERROR; + break; + } + } + } + SLIC_WARNING_IF(result & EXT_ARRAY_ERROR, + "External_array was not correctly loaded"); + + result = SPIO_TEST_SUCCESS; + + /* + * Load view within group_b + */ + reader.loadExternalData(root2, group_b, file_name + ROOT_EXT); + + /* + * Verify that the contents of view2 are restored. + */ + EXPECT_EQ(view2->getNumElements(), nvals); + if(view2->getNumElements() != nvals) + { + result |= EXT_ARRAY_ERROR; + } + else + { + for(int i = 0; i < nvals; ++i) + { + EXPECT_EQ(orig_vals2[i], restored_vals2[i]); + if(orig_vals2[i] != restored_vals2[i]) + { + result |= EXT_ARRAY_ERROR; + break; + } + } + } + SLIC_WARNING_IF(result & EXT_ARRAY_ERROR, + "External_array was not correctly loaded"); + + /* + * Create another DataStore to test another way of reading input + */ + DataStore* ds3 = new DataStore(); + Group* root3 = ds3->getRoot(); + + reader.read(root3, file_name + ROOT_EXT); + + /* + * Reset the restored_vals arrays to incorrect values. + */ + for(int i = 0; i < nvals; ++i) + { + restored_vals1[i] = -1; + restored_vals2[i] = -1; + } + + + view1 = root3->getView("testdata/fields/a/vals1"); + view1->setExternalDataPtr(restored_vals1); + + view2 = root3->getView("testdata/fields/b/vals2"); + view2->setExternalDataPtr(restored_vals2); + + /* + * This uses the API for piecewise loading of external data but calls it + * to load into the root group. When loading into the root group, it is + * unnecessary to use this API, but here we test that it works nonetheless. + */ + reader.loadExternalData(root3, root3, file_name + ROOT_EXT); + + result = SPIO_TEST_SUCCESS; + + /* + * Verify that the contents of view1 are restored. + */ + EXPECT_EQ(view1->getNumElements(), nvals); + if(view1->getNumElements() != nvals) + { + result |= EXT_ARRAY_ERROR; + } + else + { + for(int i = 0; i < nvals; ++i) + { + EXPECT_EQ(orig_vals1[i], restored_vals1[i]); + if(orig_vals1[i] != restored_vals1[i]) + { + result |= EXT_ARRAY_ERROR; + break; + } + } + } + SLIC_WARNING_IF(result & EXT_ARRAY_ERROR, + "External_array was not correctly loaded"); + + result = SPIO_TEST_SUCCESS; + + /* + * Verify that the contents of view2 are restored. + */ + EXPECT_EQ(view2->getNumElements(), nvals); + if(view2->getNumElements() != nvals) + { + result |= EXT_ARRAY_ERROR; + } + else + { + for(int i = 0; i < nvals; ++i) + { + EXPECT_EQ(orig_vals2[i], restored_vals2[i]); + if(orig_vals2[i] != restored_vals2[i]) + { + result |= EXT_ARRAY_ERROR; + break; + } + } + } + SLIC_WARNING_IF(result & EXT_ARRAY_ERROR, + "External_array was not correctly loaded"); + + /* + * Create another DataStore to test another way of reading input + */ + DataStore* ds4 = new DataStore(); + + /* + * Write the ds1 data to a different file name, starting at a subgroup, + * not at the root. + */ + file_name = "out_spio_external_partial_subgroup"; + writer.write(root1->getGroup("testdata"), num_files, file_name, PROTOCOL); + + Group* root4 = ds4->getRoot(); + Group* testdata = root4->createGroup("testdata"); + reader.read(testdata, file_name + ROOT_EXT); + + /* + * Reset the restored_vals arrays to incorrect values. + */ + for(int i = 0; i < nvals; ++i) + { + restored_vals1[i] = -1; + restored_vals2[i] = -1; + } + + group_a = root4->getGroup("testdata/fields/a"); + group_b = root4->getGroup("testdata/fields/b"); + + view1 = group_a->getView("vals1"); + view1->setExternalDataPtr(restored_vals1); + + view2 = group_b->getView("vals2"); + view2->setExternalDataPtr(restored_vals2); + + /* + * Do a piecewise loadExternalData for group_a. The first argument must + * match the Group passed in the 'reader.read()' call. + */ + reader.loadExternalData(testdata, group_a, file_name + ROOT_EXT); + + result = SPIO_TEST_SUCCESS; + + /* + * Verify that the contents of view1 are restored. + */ + EXPECT_EQ(view1->getNumElements(), nvals); + if(view1->getNumElements() != nvals) + { + result |= EXT_ARRAY_ERROR; + } + else + { + for(int i = 0; i < nvals; ++i) + { + EXPECT_EQ(orig_vals1[i], restored_vals1[i]); + if(orig_vals1[i] != restored_vals1[i]) + { + result |= EXT_ARRAY_ERROR; + break; + } + } + } + SLIC_WARNING_IF(result & EXT_ARRAY_ERROR, + "External_array was not correctly loaded"); + + result = SPIO_TEST_SUCCESS; + + /* + * Do a piecewise loadExternalData for group_b. The first argument must + * match the Group passed in the 'reader.read()' call. + */ + reader.loadExternalData(testdata, group_b, file_name + ROOT_EXT); + + /* + * Verify that the contents of view2 are restored. + */ + EXPECT_EQ(view2->getNumElements(), nvals); + if(view2->getNumElements() != nvals) + { + result |= EXT_ARRAY_ERROR; + } + else + { + for(int i = 0; i < nvals; ++i) + { + EXPECT_EQ(orig_vals2[i], restored_vals2[i]); + if(orig_vals2[i] != restored_vals2[i]) + { + result |= EXT_ARRAY_ERROR; + break; + } + } + } + SLIC_WARNING_IF(result & EXT_ARRAY_ERROR, + "External_array was not correctly loaded"); + + delete ds1; + delete ds2; + delete ds3; + delete ds4; +} + //---------------------------------------------------------------------- TEST(spio_parallel, irregular_writeread) { From 974e3dcae0d851aac587f1cb7a1af9f5d945ce93 Mon Sep 17 00:00:00 2001 From: "Noah S. Elliott" Date: Thu, 23 Jan 2025 13:38:00 -0800 Subject: [PATCH 2/6] style --- src/axom/sidre/core/Group.cpp | 6 ++-- src/axom/sidre/spio/IOManager.cpp | 31 +++++++++++++-------- src/axom/sidre/spio/IOManager.hpp | 4 ++- src/axom/sidre/tests/spio/spio_parallel.hpp | 3 +- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/axom/sidre/core/Group.cpp b/src/axom/sidre/core/Group.cpp index 9495654fc1..49be794d86 100644 --- a/src/axom/sidre/core/Group.cpp +++ b/src/axom/sidre/core/Group.cpp @@ -2293,7 +2293,8 @@ bool Group::loadExternalData(const hid_t& h5_id, const std::string& group_path) { bool success; std::string delim(1, getPathDelimiter()); - if (group_path.empty() || group_path == delim) { + if(group_path.empty() || group_path == delim) + { // An empty or trivial path means the load should read from the start of // the file. success = loadExternalData(h5_id); @@ -2306,8 +2307,7 @@ bool Group::loadExternalData(const hid_t& h5_id, const std::string& group_path) std::string read_path("sidre/external/" + group_path); - checkConduitCall( - [&] { conduit::relay::io::hdf5_read(h5_id, read_path, n); }); + checkConduitCall([&] { conduit::relay::io::hdf5_read(h5_id, read_path, n); }); success = !(getDataStore()->getConduitErrorOccurred()); } return success; diff --git a/src/axom/sidre/spio/IOManager.cpp b/src/axom/sidre/spio/IOManager.cpp index d5ade22f7e..c90ccbcc12 100644 --- a/src/axom/sidre/spio/IOManager.cpp +++ b/src/axom/sidre/spio/IOManager.cpp @@ -500,29 +500,35 @@ void IOManager::loadExternalData(sidre::Group* parent_group, size_t parent_size = parent_parts.size(); bool can_load = true; - if (load_parts.size() < parent_size) { - can_load = false; + if(load_parts.size() < parent_size) + { + can_load = false; } - for (size_t i = 0; i < parent_size; ++i) { - if (parent_parts[i] != load_parts[i]) { - can_load = false; - } + for(size_t i = 0; i < parent_size; ++i) + { + if(parent_parts[i] != load_parts[i]) + { + can_load = false; + } } std::string subpath = load_name.substr(parent_name.size()); char delimiter = parent_group->getPathDelimiter(); - if (!subpath.empty() && subpath[0] == delimiter) { - subpath.erase(0,1); + if(!subpath.empty() && subpath[0] == delimiter) + { + subpath.erase(0, 1); } - if (!subpath.empty() && !parent_group->hasGroup(subpath)) { - can_load = false; + if(!subpath.empty() && !parent_group->hasGroup(subpath)) + { + can_load = false; } int set_id = m_baton->wait(); - if (can_load) { + if(can_load) + { if(num_groups <= m_comm_size) { if(m_my_rank < num_groups) @@ -530,7 +536,8 @@ void IOManager::loadExternalData(sidre::Group* parent_group, herr_t errv; AXOM_UNUSED_VAR(errv); - std::string hdf5_name = getFileNameForRank(file_pattern, root_file, set_id); + std::string hdf5_name = + getFileNameForRank(file_pattern, root_file, set_id); hdf5_name = getSCRPath(hdf5_name); diff --git a/src/axom/sidre/spio/IOManager.hpp b/src/axom/sidre/spio/IOManager.hpp index 86620f2350..8133108176 100644 --- a/src/axom/sidre/spio/IOManager.hpp +++ b/src/axom/sidre/spio/IOManager.hpp @@ -326,7 +326,9 @@ class IOManager * \param load_group Group holding views to be filled with external data * \param root_file root file containing input data */ - void loadExternalData(sidre::Group* parent_group, sidre::Group* load_group, const std::string& root_file); + void loadExternalData(sidre::Group* parent_group, + sidre::Group* load_group, + const std::string& root_file); /*! * \brief gets the number of files in the dataset from the specified root file diff --git a/src/axom/sidre/tests/spio/spio_parallel.hpp b/src/axom/sidre/tests/spio/spio_parallel.hpp index cdefb8ceda..3d341b2ec3 100644 --- a/src/axom/sidre/tests/spio/spio_parallel.hpp +++ b/src/axom/sidre/tests/spio/spio_parallel.hpp @@ -583,7 +583,6 @@ TEST(spio_parallel, external_partial_writeread) restored_vals2[i] = -1; } - view1 = root3->getView("testdata/fields/a/vals1"); view1->setExternalDataPtr(restored_vals1); @@ -624,7 +623,7 @@ TEST(spio_parallel, external_partial_writeread) result = SPIO_TEST_SUCCESS; - /* + /* * Verify that the contents of view2 are restored. */ EXPECT_EQ(view2->getNumElements(), nvals); From 1a6b043ae5fe32674b2bde0614d6154e5f3ba3d0 Mon Sep 17 00:00:00 2001 From: "Noah S. Elliott" Date: Fri, 24 Jan 2025 10:16:10 -0800 Subject: [PATCH 3/6] Small edits to comments --- src/axom/sidre/spio/IOManager.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/axom/sidre/spio/IOManager.hpp b/src/axom/sidre/spio/IOManager.hpp index 8133108176..719817a4b9 100644 --- a/src/axom/sidre/spio/IOManager.hpp +++ b/src/axom/sidre/spio/IOManager.hpp @@ -289,7 +289,7 @@ class IOManager * This currently only works if the root file was created for protocol * sidre_hdf5. * - * The call to this must follow a call to the IOManager::read with the + * The call to this method must follow a call to the IOManager::read with the * same group and root file. * * This is intended as the third step of the three step process to load @@ -319,8 +319,9 @@ class IOManager * to call this method. parent_group must be the same group that was * passed to IOManager::read with the same root file. * - * This may be called multiple times with different instances of load_group - * representing different subtrees of the hierachy under parent_group. + * This method may be called multiple times with different instances of + * load_group representing different subtrees of the hierachy under + * parent_group. * * \param parent_group Group that was passed to IOManager::read * \param load_group Group holding views to be filled with external data From b9d5c09393138ab43b78a4353755657010ecf260 Mon Sep 17 00:00:00 2001 From: "Noah S. Elliott" Date: Fri, 24 Jan 2025 11:27:40 -0800 Subject: [PATCH 4/6] Add warning if the path between groups passed to loadExternalData cannot be found. --- src/axom/sidre/spio/IOManager.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/axom/sidre/spio/IOManager.cpp b/src/axom/sidre/spio/IOManager.cpp index 304da22b44..7ea5fbf684 100644 --- a/src/axom/sidre/spio/IOManager.cpp +++ b/src/axom/sidre/spio/IOManager.cpp @@ -603,6 +603,12 @@ void IOManager::loadExternalData(sidre::Group* parent_group, } } } + else + { + SLIC_WARNING("Path from parent group " << parent_group->getPathName() + << " to group " << load_group->getPathName() + << " was not found. No external data will be loaded."); + } #else AXOM_UNUSED_VAR(datagroup); From a1b5018a5aaed3b480f3c93bca6286e36a6dd2ec Mon Sep 17 00:00:00 2001 From: "Noah S. Elliott" Date: Fri, 24 Jan 2025 13:41:25 -0800 Subject: [PATCH 5/6] Rewording of a comment about a path argument --- src/axom/sidre/core/Group.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/axom/sidre/core/Group.hpp b/src/axom/sidre/core/Group.hpp index a4e9613629..50c9a506cc 100644 --- a/src/axom/sidre/core/Group.hpp +++ b/src/axom/sidre/core/Group.hpp @@ -1672,9 +1672,9 @@ class Group * if an error occurred at some point. * * \param h5_id hdf5 handle - * \param group_path path to a group below the group pointed by the h5_id. - * This path points to the group in the hdf5 hierarchy - * that represents this Group object. + * \param group_path Path pointing to this Group. This path must be the + * relative path from the top Group that was written + * to a file to this Group. */ bool loadExternalData(const hid_t& h5_id, const std::string& group_path); From 583025e51f7281dbfc0d3a039e4e15edcd34db85 Mon Sep 17 00:00:00 2001 From: "Noah S. Elliott" Date: Fri, 24 Jan 2025 14:15:47 -0800 Subject: [PATCH 6/6] style --- src/axom/sidre/spio/IOManager.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/axom/sidre/spio/IOManager.cpp b/src/axom/sidre/spio/IOManager.cpp index 7ea5fbf684..1449e9316c 100644 --- a/src/axom/sidre/spio/IOManager.cpp +++ b/src/axom/sidre/spio/IOManager.cpp @@ -605,9 +605,10 @@ void IOManager::loadExternalData(sidre::Group* parent_group, } else { - SLIC_WARNING("Path from parent group " << parent_group->getPathName() - << " to group " << load_group->getPathName() - << " was not found. No external data will be loaded."); + SLIC_WARNING("Path from parent group " + << parent_group->getPathName() << " to group " + << load_group->getPathName() + << " was not found. No external data will be loaded."); } #else