search

LLNL / H5Z-ZFP

A registered ZFP compression plugin for HDF5
Other
50 stars 22 forks source link

CMake: (fix) MPI parallel HDF5 1.14.0 #97

Closed jwsblokland closed 1 year ago

jwsblokland commented 1 year ago
jwsblokland commented 1 year ago

This pull request solves issue #98.

@markcmiller86 Apparently, the Ubuntu test fails because certain url does not longer exist. I am not sure how and if I can fix it.

jwsblokland commented 1 year ago

@markcmiller86 I was able to figure it out the Ubuntu problem. Now everything works.

markcmiller86 commented 1 year ago

This pull request solves issue #98.

@markcmiller86 Apparently, the Ubuntu test fails because certain url does not longer exist. I am not sure how and if I can fix it.

I've got that fixed in #96 but haven't merged that yet and don't know if I actually will. You can go grab the fix there though.

markcmiller86 commented 1 year ago

Has this been tested agains older HDF5 versions? I can't recall..what would happen if HDF5_IS_PARALLEL is not defined? I assume it would treat it as a no answer instead of failing CMake but wanted to check for sure.

jwsblokland commented 1 year ago

I tested both HDF5 1.12.1 and 1.14.0 and both of them work. Actually, HDF5 version 1.12.1 does not require MPI::MPI_C target. The current implementation always to try to find MPI::MPI_C target for parallel HDF5 regardless of the version number. If desired a more restrictive if-statement based on HDF5 version number can be implemented but this is going to be tricky because I assume that this CMake change of MPI::MPI_C has been backported to older versions of HDF5 which makes the more restrictive if-statement rather complicated. Furthermore, it requires a careful investigation which versions contains this CMake change.

Currently, the CMake build configuration of H5Z-ZFP requires CMake 3.9 as a minimum version. For this minimum version 3.9 the CMake variable HDF5_IS_PARALLEL is defined (see https://cmake.org/cmake/help/v3.9/module/FindHDF5.html). The same holds for MPI::MPI_C (see https://cmake.org/cmake/help/v3.9/module/FindMPI.htm).

Furthermore, I also checked the case if HDF5_IS_PARALLEL is not defined. In that case the variable will be treated as FALSE and CMake will not fail if the variable is undefined.

brtnfld commented 1 year ago

For reference, see https://github.com/HDFGroup/hdf5/issues/2404, https://github.com/HDFGroup/hdf5/pull/2400

jwsblokland commented 1 year ago

Reading the discussion https://github.com/HDFGroup/hdf5/issues/2404 and https://github.com/HDFGroup/hdf5/pull/2400, mentioned by @brtnfld we may want to specialize the implemented solution by only applying it for HDF5 1.14.0. Of course assuming that the bug only occurs in 1.14.0 and that implemented solution in PR https://github.com/HDFGroup/hdf5/pull/2400 will be part of HDF5 1.14.1.