Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake: call CMake's FindHDF5 instead of trying to find everything by hand #13319

Merged
merged 5 commits into from Feb 2, 2022

Conversation

tamiko
Copy link
Member

@tamiko tamiko commented Feb 1, 2022

Closes #8957

@tamiko
Copy link
Member Author

tamiko commented Feb 2, 2022

/rebuild

@tamiko
Copy link
Member Author

tamiko commented Feb 2, 2022

@tjhei With this change HDF5 is now configured and enabled on the CI:

-- Include /jenkins/workspace/dealii_PR-13319/cmake/configure/configure_hdf5.cmake
-- Found HDF5: /usr/lib/x86_64-linux-gnu/hdf5/serial/lib/libhdf5.so;/usr/lib/x86_64-linux-gnu/libpthread.so;/usr/lib/x86_64-linux-gnu/libsz.so;/usr/lib/x86_64-linux-gnu/libz.so;/usr/lib/x86_64-linux-gnu/libdl.so;/usr/lib/x86_64-linux-gnu/libm.so (found version "1.8.16") 
--   HDF5_VERSION: 1.8.16
--   HDF5_LIBRARIES: /usr/lib/x86_64-linux-gnu/hdf5/serial/lib/libhdf5.so;/usr/lib/x86_64-linux-gnu/libpthread.so;/usr/lib/x86_64-linux-gnu/libsz.so;/usr/lib/x86_64-linux-gnu/libz.so;/usr/lib/x86_64-linux-gnu/libdl.so;/usr/lib/x86_64-linux-gnu/libm.so
--   HDF5_INCLUDE_DIRS: /usr/include/hdf5/serial
--   HDF5_USER_INCLUDE_DIRS: /usr/include/hdf5/serial
-- Found HDF5
-- DEAL_II_WITH_HDF5 successfully set up with external dependencies.

... and two HDF5 tests fail for the serial configuration.

I checked, we have not tested a serial configuration with non-mpi hdf5 in about forever. Also, almost all of our hdf5 tests require MPI to be enabled.

So what do?

@tjhei
Copy link
Member

tjhei commented Feb 2, 2022

Not sure if anyone uses hdf5 without MPI?

I would suggest to require MPI. This makes testing easier for us.

@drwells
Copy link
Member

drwells commented Feb 2, 2022

I think that, for the sake of our sanity, we should require MPI with HDF5 for the same reasons we require Trilinos with HDF5.

@tamiko
Copy link
Member Author

tamiko commented Feb 2, 2022

@tjhei @drwells Alright. I have made MPI an unconditional dependency for HDF5.

Copy link
Member

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@bangerth
Copy link
Member

bangerth commented Feb 2, 2022

The possibilities for extensions of step-3 have a long section on using HDF5 and explicitly state that "Although the HDF5 deal.II binding supports both serial and MPI...". I'm not sure that we ever use HDF5 without MPI, but you might want to read through https://dealii.org/developer/doxygen/deal.II/step_3.html#UsingHDF5tooutputthesolutionandadditionaldata to see what we ought to do.

@masterleinad masterleinad merged commit c80b699 into dealii:master Feb 2, 2022
@masterleinad masterleinad deleted the update_find_hdf5 branch February 2, 2022 16:09
@tjhei
Copy link
Member

tjhei commented Feb 2, 2022

to see what we ought to do.

I would be in favor of merging as is and then tackling this separately by somebody who is interested in sequential hdf5:

  • Go through all the tests and require MPI where necessary.
  • Add several sequential tests.
  • Update the cmake detection to allow serial configuration again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deal.II does not see the HDF5 package on Ubuntu 18.04
6 participants