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

Add HDF5 H5Z-ZFP support in CMake #2753

Merged
merged 4 commits into from
May 11, 2022
Merged

Add HDF5 H5Z-ZFP support in CMake #2753

merged 4 commits into from
May 11, 2022

Conversation

jrood-nrel
Copy link
Contributor

@jrood-nrel jrood-nrel commented May 5, 2022

Summary

This adds HDF5 ZFP lossy compression support to plot files within CMake.

Additional background

I've tested this as much as I could on my own. I'm finding that the variable in CMake HDF5_IS_PARALLEL may not get populated correctly (at least with CMake 3.23.1 on MacOS), which appears to be a bug in CMake. I think this also depends on whether HDF5 was installed using Autotools or CMake as the installation is totally different between both. This is unfortunate because we want the logic for matching HDF5 with or without MPI to AMReX with or without MPI, but CMake can fail in this logic because it reports parallel HDF5 wrong.

Closes #2696

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

@WeiqunZhang WeiqunZhang requested review from ax3l and etpalmer63 May 6, 2022 20:24
@ax3l ax3l added the install label May 9, 2022
@ax3l
Copy link
Member

ax3l commented May 9, 2022

I've tested this as much as I could on my own. I'm finding that the variable in CMake HDF5_IS_PARALLEL may not get populated correctly (at least with CMake 3.23.1 on MacOS), which appears to be a bug in CMake. I think this also depends on whether HDF5 was installed using Autotools or CMake as the installation is totally different between both

Uff, yes HDF5 is just about to fully modernize their CMake infrastructure and fix all bug reports: https://github.com/HDFGroup/hdf5/issues?q=is%3Aissue+is%3Aopen+cmake

On Unix systems, use Autotools still until this process is complete.

Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

That looks great, thank you!

Please add the new option in the user-facing docs in Docs/sphinx_documentation/source/BuildingAMReX.rst as well :)

Copy link
Contributor

@etpalmer63 etpalmer63 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution!

@WeiqunZhang WeiqunZhang merged commit 6264e81 into AMReX-Codes:development May 11, 2022
kngott pushed a commit to kngott/amrex that referenced this pull request May 19, 2022
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.

HDF5 ZFP support in CMake
4 participants