SebWouters / CheMPS2

CheMPS2: a spin-adapted implementation of DMRG for ab initio quantum chemistry
GNU General Public License v2.0
70 stars 34 forks source link

CheMPS2Config.cmake's EXACT dependency on @TargetHDF5_VERSION@ is problematic #82

Closed mbanck closed 2 years ago

mbanck commented 2 years ago

It looks like chemps2 encodes the exact version of HDF5 during build (1.10.6) for the chemps2 version in Debian unstable.

However, Debian has moved on to HDF5 1.10.7 since, and now package that try to find chemps2 won't (from Psi4's cmake run):

-- HDF5 detected.
CMake Error at /usr/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find TargetHDF5: Found unsuitable version "1.10.7", but required
  is exact version "1.10.6" (found Found HDF5:
  /usr/lib/x86_64-linux-gnu/hdf5/serial/libhdf5.so;/usr/lib/x86_64-linux-gnu/libcrypto.so;/usr/lib/x86_64-linux-gnu/libcurl.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.10.7))
Call Stack (most recent call first):
  /usr/share/cmake-3.22/Modules/FindPackageHandleStandardArgs.cmake:592 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake/CheMPS2/FindTargetHDF5.cmake:88 (find_package_handle_standard_args)
  /usr/share/cmake-3.22/Modules/CMakeFindDependencyMacro.cmake:47 (find_package)
  /usr/share/cmake/CheMPS2/CheMPS2Config.cmake:175 (find_dependency)
  external/upstream/chemps2/CMakeLists.txt:2 (find_package)
SebWouters commented 2 years ago

I agree. The exact versions are coded in a somewhat overengineered CMake/CheMPS2Config.cmake.in (L151) and CMake/FindTargetHDF5.cmake :flushed:.

@loriab : How much of these files is still needed for psi4 building? Would it be an option to remove the overrides for finding hdf5 and lapack altogether given cmake's evolution; to just use the cmake-provided find functions; and to pass the relevant paths command-line when needed on configuration? This would be my preference.

loriab commented 2 years ago

Doesn't one usually need to pin to three levels for HDF5 version (https://github.com/AnacondaRecipes/hdf5-feedstock/blob/master/recipe/meta.yaml#L28-L31 https://github.com/conda-forge/hdf5-feedstock/blob/master/recipe/meta.yaml#L92)? Wouldn't the CheMPS2 Debian package have needed to be rebuilt when Debian updated to 1.10.7? Sorry for my confusion — I'm not too familiar with non-conda packaging.

Alas, the tgt::hdf5 and tgt::lapack are alive and well in the psi4 build system. They enable psi to pre-detect hdf5 and lapack (usually with cmake's built-in FindProject modules) with the user able to pass in all the hint/specification/flavor variables to the Find routine. Then psi can make sure the same hdf5 gets used by chemps2, ambit, etc. without forwarding the tens of hint variables to each of the ExternalProjects. I haven't investigated lately whether there are any new modern cmake ways to save and pass targets.

mbanck commented 2 years ago

On the other hand, I don't know anything about conda...

HDF5 is a shared library, so unless it breaks API, packages linking to it should work also for future versions. The binary library package (libhdf5-103-1 where 103-1 is apparently the API version) did not change between 1.10.6 and 1.10.7 so they should be compatible - otherwise, the package name should have changed from libhd5-103-1 to libdhd5-104-1 or something, breaking all dependencies and inducing a rebuild/library transition.

So no, unless there was an undeclared API break, chemps2 did not require a rebuild between 1.10.6 and 1.10.7.

SebWouters commented 2 years ago

Sorry for the delay. Does fdfb63445772bc664ddef2d1d59344503f4566b4 solve the issue?

loriab commented 2 years ago

Here's an analysis of symbols consistency in the hdf5 history, https://abi-laboratory.pro/index.php?view=timeline&l=hdf5 . Looks like early in the CheMPS2 timeline, hdf5 wasn't too careful about consistency between M.m.p versions. Hence this comment https://github.com/AnacondaRecipes/hdf5-feedstock/blob/master/recipe/meta.yaml#L28-L31 and conda playing it self by pinning to M.m.p . More recent hdf5 seems better behaved for pinning to M.m and takes advantage of SAMEMINORVERSION that CMake added in 3.10 https://github.com/HDFGroup/hdf5/blob/516d9677109b81b79ad1754246f4f147f9d984a3/CMakeInstallation.cmake#L108 . So fdfb634 is fine with me. Thanks for the change.