Fixes build and tests when not using `hdf5` #1504

kennyweiss · 2025-02-19T04:02:31Z

Summary

This PR is a bugfix
It fixes the build and tests in configurations without hdf5
I also discovered and fixed an unknown dependency for scr on zlib

I noted in #1480 that Axom supports configurations without hdf5. We don't currently have CI for this, and when I configured the code without hdf5, I found several tests that did not work. This branch fixes them.

Since non-hdf5 configs require a conduit without hdf5 (i.e. a new library set), I'm not sure that we need a CI that tests this -- it might be enough to just manually run the code and fix it every so often. Perhaps we should have a manually run github-action that does this, similar to our manual Windows TPL build?

Details:

I configured the code with a host-config generated by running the following uberenv command:

> ./scripts/uberenv/uberenv.py --spec "%[email protected]+devtools~hdf5+c2c+mfem+scr+profiling  ^conduit~hdf5"

src/cmake/thirdparty/SetupAxomThirdParty.cmake

Arlie-Capps

Thank you for this fix, @kennyweiss .

BradWhitlock

It looks good to me.

Specifically, `sidre::Group::loadExternalData` is only implemented for the `sidre_hdf5` protocol.

This is apparently necessary, but was masked by hdf5's dependency on zlib. It was masked by build axom with scr, but without hdf5.

This was required for an Axom config with `scr` but without `hdf5` since it didn't get the `zlib` that `hdf5` normally sets up.

…ailable * `Load(<cycle>)` is currently hard-coded for the `sidre_hdf5` protocol, so add a SLIC_ERROR to give an indication of why this won't currently work * `Save()` had an MPI hang for non-`sidre_hdf5` protocols related to using spio to `write()` on only the root rank. Since `write` is a collective operation, we need to use `Group::save()` instead.

… available

kennyweiss · 2025-03-05T18:01:32Z

Note: My original spack config didn't include mfem and there were some additional cases that needed to be handled when we have mfem but not hdf5. I fixed these including an MPI hang in how we were using spio from sidre::MFEMSidreDataCollection for protocols other than sidre_hdf5.

kennyweiss · 2025-03-05T18:04:10Z

src/axom/sidre/core/MFEMSidreDataCollection.cpp

@@ -1220,7 +1220,7 @@ void MFEMSidreDataCollection::Save(const std::string& filename,
      // Root file support only available in hdf5.
      else
      {
-        writer.write(blueprint_indicies_grp, 1, file_path + ".root", protocol);
+        blueprint_indicies_grp->save(file_path + ".root", protocol);


@nselliott -- This call to writer.write() from only the root rank was hanging since writer.write() is collective.
Switching it to directly use Group::save() resolved the issue.

kennyweiss · 2025-03-05T18:05:44Z

src/axom/sidre/spio/IOManager.cpp

+      << axom::fmt::format("\n\tfile_base: '{}'", file_base)
+      << axom::fmt::format("\n\ttest_file_base: '{}'", test_file_base));


@nselliott -- I updated this log statement to show the filenames

kennyweiss · 2025-03-05T18:07:14Z

src/cmake/axom-config.cmake.in

+  # zlib
+  if(AXOM_USE_ZLIB)
+    set(AXOM_ZLIB_DIR  "@ZLIB_DIR@")
+    if(NOT ZLIB_DIR) 
+      set(ZLIB_DIR ${AXOM_ZLIB_DIR}) 
+    endif()
+    if(ZLIB_DIR)
+        set(ZLIB_ROOT ${ZLIB_DIR})
+    endif()
+    find_package(ZLIB REQUIRED)
+  endif()


@white238 -- thanks for your suggestion about importing zlib. I adapted the conduit example (thanks @cyrush!) and verified that we can now run the using-with-blt example w/ Axom's exports.

kennyweiss · 2025-03-05T18:10:21Z

src/axom/sidre/core/MFEMSidreDataCollection.hpp

  virtual void Load(int cycle_ = 0)
  {
+  #ifndef AXOM_USE_HDF5
+    SLIC_ERROR(
+      "MFEMSidreDataCollection::Load(<cycle>) is only implemented for the "
+      "'sidre_hdf5' protocol");
+  #endif


It was somewhat surprising that the MFEMSidreDataCollection::load(<cycle>) didn't support protocols other than sidre_hdf5. (Although the doxygen already noted this).

I added this error message so it's easier for people to see why this doesn't work. I don't think it would be too difficult to make this work with other protocols, but I also am not aware of any need/desire for this at the moment, so I'm planning to leave this as is.

kennyweiss added App Integration Issues related to integration with applications Testing Issues related to testing Axom Build system Issues related to Axom's build system maintenance Issues related to code maintenance labels Feb 19, 2025

kennyweiss requested review from white238, cyrush, rhornung67, nselliott, publixsubfan, ltaylor16, gunney1, bmhan12 and Arlie-Capps February 19, 2025 04:02

kennyweiss self-assigned this Feb 19, 2025

kennyweiss commented Feb 19, 2025

View reviewed changes

src/cmake/thirdparty/SetupAxomThirdParty.cmake Show resolved Hide resolved

kennyweiss mentioned this pull request Feb 19, 2025

Feature/sina/hdf5 output support #1480

Open

kennyweiss changed the title ~~Fixes build and test when not using hdf5~~ Fixes build and tests when not using hdf5 Feb 19, 2025

Arlie-Capps approved these changes Feb 20, 2025

View reviewed changes

BradWhitlock approved these changes Feb 21, 2025

View reviewed changes

kennyweiss added 8 commits March 4, 2025 17:32

Fixes sidre tests that implicitly depend on hdf5

38e1408

Specifically, `sidre::Group::loadExternalData` is only implemented for the `sidre_hdf5` protocol.

Filter out hdf5 tests from quest

9110278

Updates mailmap

93f51ec

Adds zlib as a dependency for scr

729e393

This is apparently necessary, but was masked by hdf5's dependency on zlib. It was masked by build axom with scr, but without hdf5.

Disable a spio scr test that requires hdf5 in configurations w/o hdf5

49a65a8

Adds ZLIB to Axom's CMake exports, when necessary

0537038

This was required for an Axom config with `scr` but without `hdf5` since it didn't get the `zlib` that `hdf5` normally sets up.

Fixes a misc. warning in primal

773f8d9

Misc: Reduces verbosity of a log message in quest's PointInCell query

0acdfca

kennyweiss force-pushed the bugfix/kweiss/optional-hdf5 branch from f8a659c to 0acdfca Compare March 5, 2025 02:34

kennyweiss added 2 commits March 5, 2025 09:39

Improves a log message in spio

5a517e2

Filters out some tests that are not expected to work when hdf5 is not…

3c5014b

… available

kennyweiss commented Mar 5, 2025

View reviewed changes

kennyweiss merged commit 0b734d6 into develop Mar 5, 2025
13 checks passed

kennyweiss deleted the bugfix/kweiss/optional-hdf5 branch March 5, 2025 20:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes build and tests when not using `hdf5` #1504

Fixes build and tests when not using `hdf5` #1504

kennyweiss commented Feb 19, 2025 •

edited

Loading

Arlie-Capps left a comment

BradWhitlock left a comment

kennyweiss commented Mar 5, 2025

kennyweiss Mar 5, 2025

kennyweiss Mar 5, 2025

kennyweiss Mar 5, 2025

kennyweiss Mar 5, 2025 •

edited

Loading

		<< axom::fmt::format("\n\tfile_base: '{}'", file_base)
		<< axom::fmt::format("\n\ttest_file_base: '{}'", test_file_base));

Fixes build and tests when not using hdf5 #1504

Fixes build and tests when not using hdf5 #1504

Conversation

kennyweiss commented Feb 19, 2025 • edited Loading

Summary

Details:

Arlie-Capps left a comment

Choose a reason for hiding this comment

BradWhitlock left a comment

Choose a reason for hiding this comment

kennyweiss commented Mar 5, 2025

kennyweiss Mar 5, 2025

Choose a reason for hiding this comment

kennyweiss Mar 5, 2025

Choose a reason for hiding this comment

kennyweiss Mar 5, 2025

Choose a reason for hiding this comment

kennyweiss Mar 5, 2025 • edited Loading

Choose a reason for hiding this comment

Fixes build and tests when not using `hdf5` #1504

Fixes build and tests when not using `hdf5` #1504

kennyweiss commented Feb 19, 2025 •

edited

Loading

kennyweiss Mar 5, 2025 •

edited

Loading