add support for CP2K QM/MM in GROMACS easyblock #2750

ostueker · 2022-06-17T19:29:53Z

This PR implements building GROMACS with CP2K QM/MM enabled.

As such a version of GROMACS needs to be compiled statically, which doesn't generate any libraries, the sanity_check needs to be adjusted to not expect any libs in such a case.

i.e. Don't look for libs when they are not exected to be built.

Auto-detection if CP2K is loaded as a depencency. Custom parameter can force behavior: * `cp2k = True` will raise an exception if CP2K is missing * `cp2k = False` will ignore CP2K (in case it has been pulled in by a different dependency. This requires GROMACS to be built statically (w/o libs).

ostueker · 2022-06-20T13:25:42Z

I found an issue when building this for AVX512. Will push some changes to this soon.

Because CP2K is built with MPI, the (Open)MPI libs would need to be linked to non-MPI Gromacs binaries, which interfers with the threaded MPI impletation.

ostueker · 2022-07-20T19:49:24Z

Resolved merge-conflict caused by changes in #2749
This PR is ready for review and this easyblock is already in use at ComputeCanada

Micket

In order to merge any easyblock changes, we need easyconfigs to test with first.

Micket · 2022-09-21T17:22:24Z

easybuild/easyblocks/g/gromacs.py

+                "$(pkg-config --libs-only-l libcp2k)"
+            ]
+            if get_software_root('Libint'):
+                # for some reason libint2 is not discovered by pkg-config:
+                cp2k_linker_flags.append('-lint2')
+            self.cfg.update('configopts', '-DCP2K_LINKER_FLAGS="%s"' % " ".join(cp2k_linker_flags))


This made me a bit curious. It's not like we are doing something strange to Libint here. It should work if we have pkg-config/pkgconf as a dependency.

[ohmanm@vera2 ~]$ ml Libint/2.6.0-iimpi-2021a-lmax-6-cp2k pkg-config/0.29.2-GCCcore-10.3.0 [ohmanm@vera2 ~]$ pkg-config --libs-only-l libint2 -lint2

For CP2K/8.2 on the other hand, there doesn't seem to be any pkgconfig file installed, in fact, no libcp2k at all, so i guess this also depends on some updated CP2K easyblock that also builds a library?

Also, since pkg-config is explicitly called here, perhaps the logic should check that pkg-config is added explicitly as a build-dep.

Thanks for the feedback. Indeed CP2K needs to be build with library = True.

On our system we have pkg-config in our Gentoo layer and therefore we don't specify that as a dependency.

I'll prepare a PR with the needed easyconfigs as best as I can over the coming days.
There will be some guesswork with the dependencies as our easybuild installation is a bit different in certain ways.

ostueker · 2022-10-05T15:15:44Z

I've opened easybuilders/easybuild-easyconfigs#16362 with the missing easyconfigs to test this patch.

I'll add the check for pkg-config shortly.

ostueker · 2022-10-05T20:02:15Z

Thanks @Micket,

I've added some checks that CP2K has been compiled with library = True and that pkg-config was used.
It also checks that pkg-config is has been loaded (e.g. as a build-dep).

Easyconfigs to test this enhancement are in: easybuilders/easybuild-easyconfigs#16362

akesandgren · 2022-10-24T08:45:11Z

easybuild/easyblocks/g/gromacs.py

+            self.log.info("Building with CP2K QM/MM.")
+            self.cfg['build_shared_libs'] = False
+            self.libext = 'a'
+            cp2k_version = get_software_version('CP2K')


Move this up above the first use of CP2K version

done in 6b1f77d

akesandgren · 2022-10-24T08:46:17Z

easybuild/easyblocks/g/gromacs.py

+            if LooseVersion(self.version) < LooseVersion('2022'):
+                msg = 'CP2K support is only available for GROMACS 2022 and newer.'
+                raise EasyBuildError(msg)
+            elif LooseVersion(get_software_version('CP2K')) < LooseVersion('8.1'):


This can be a simple "if", then one can put the

cp2k_version = get_software_version('CP2K')

before this line.

done in 6b1f77d

akesandgren · 2022-10-24T08:56:59Z

easybuild/easyblocks/g/gromacs.py

+            cp2k_linker_flags = [
+                # Need MPI linker flags b/c libcp2k.a is compiled with mpifort.
+                # These are for OpenMPI (mpifort --showme).
+                "-lmpi_usempif08 -lmpi_usempi_ignore_tkr -lmpi_mpifh",
+                "-L%s/lib/exts/dbcsr" % cp2k_root,
+                # get depenencies for libcp2k.a:
+                "$(pkg-config --libs-only-l libcp2k)"
+            ]


This is not good. We must not depend on it being built with OpenMPI. To do it this way it MUST first check that OpenMPI is part of the toolchain.
And the pkgconfig for libcp2k should probably include the MPI libs it depends on which "should" then make all this redundant if the CMake for GROMACS does things correctly.

Unfortunately CP2K's pkgconfig doesn't contain the MPI libs. That's why I had to hack them in manually.

$ pkg-config --libs-only-l libcp2k -lcp2k -ldbcsr -lm -lpthread -lstdc++ -lxcf03 -lxc -lxsmm -lxsmmf -lopenblas -lgfortran -lfftw3_omp -lfftw3 -lpthread -lopenblas -lgfortran -lpthread -lscalapack -lopenblas -lgfortran

Should I rework it like this?

cp2k_linker_flags = [] # Need MPI linker flags b/c libcp2k.a is compiled with mpifort. # These are for OpenMPI (mpifort --showme). if get_software_root('OpenMPI'): cp2k_linker_flags.append("-lmpi_usempif08 -lmpi_usempi_ignore_tkr -lmpi_mpifh") elif get_software_root('IntelMPI'): # append Intel MPI libs for fortran here: cp2k_linker_flags.append("...") cp2k_linker_flags += [ "-L%s/lib/exts/dbcsr" % cp2k_root, # get dependencies for libcp2k.a: "$(pkg-config --libs-only-l libcp2k)" ]

In 2bacbb3 the easyblock is now checking for the cases of Open MPI and Intel MPI to add the fortran-MPI libraries.
Other MPI implementations could be added if needed.

akesandgren · 2022-12-01T12:21:22Z

easybuild/easyblocks/g/gromacs.py

+
+        if cp2k_root:
+            if LooseVersion(self.version) < LooseVersion('2022'):
+                msg = 'CP2K support is only available for GROMACS 2022 and newer.'


s/for GROMACS/in GROMACS/

akesandgren · 2022-12-01T12:24:21Z

easybuild/easyblocks/g/gromacs.py

+                msg = 'CP2K needs to be compiled with "library = True".'
+                raise EasyBuildError(msg)
+            if not os.path.exists(os.path.join(cp2k_root, 'lib', 'pkgconfig', 'libcp2k.pc')):
+                msg = "pkg-config is required as a build-dependency for CP2K"


we can either use pkg-config or the newer replacement pkgconf, please change msgs and the test for pkg-config software_root below to accommodate for this.

And you should not really test if cp2k has libcp2k.pc like this. Use the pkg-config tool to check instead since it can actually be installed somewhere else than under lib/pkgconfig.

And I'm not sure we should even bother checking for it in the gromacs easyblock, the GROMACS CMake scripts should handle this and bail out if it's missing...

akesandgren · 2022-12-01T12:34:31Z

easybuild/easyblocks/g/gromacs.py

+            self.cfg.update('configopts', "-DGMX_CP2K=ON")
+            # Ensure that the GROMACS log files report that CP2K was enabled and which version was used.
+            self.cfg.update('configopts', "-DGMX_VERSION_STRING_OF_FORK=CP2K-{:}".format(cp2k_version))
+            self.cfg.update('configopts', "-DCP2K_DIR=%s/lib64" % cp2k_root)


lib64.... -> lib and please use os.path.join for this instead of hardcoding "/"

akesandgren · 2022-12-01T12:35:59Z

easybuild/easyblocks/g/gromacs.py

+            elif get_software_root('IntelMPI'):
+                # for Intel MPI (mpiifort -show)
+                cp2k_linker_flags.append("-lmpifort")


Needs a else: with an error msg that tells the user the there is currently no support for other MPI versions, like MPICH or others...

akesandgren · 2022-12-01T12:36:41Z

easybuild/easyblocks/g/gromacs.py

+                # for Intel MPI (mpiifort -show)
+                cp2k_linker_flags.append("-lmpifort")
+            cp2k_linker_flags += [
+                "-L%s/lib/exts/dbcsr" % cp2k_root,


Use os.path.join instead of hardcoding "/"

akesandgren · 2022-12-01T12:40:36Z

easybuild/easyblocks/g/gromacs.py

@@ -310,7 +380,7 @@ def configure_step(self):
                              mpiexec_path, self.cfg.get('mpiexec_numproc_flag'),
                              mpi_numprocs)

-            if LooseVersion(self.version) >= LooseVersion('2019'):
+            elif LooseVersion(self.version) >= LooseVersion('2019') and self.cfg['build_shared_libs']:


I'm fairly sure you can't change this to an elif without breaking things for the non-cp2k builds

akesandgren · 2022-12-01T12:43:59Z

easybuild/easyblocks/g/gromacs.py

@@ -556,6 +626,12 @@ def get_lib_subdir(self):
                        lib_subdir = os.path.dirname(libpaths[0])[len(self.installdir) + 1:]
                        self.log.info("Found lib subdirectory that contains %s: %s", libname, lib_subdir)
                        break
+                if not self.cfg['build_shared_libs']:
+                    msg = "Found lib subdirectory: %s but it doesn't contain: %s.\n"


Remove the ":" to make reference correct

ostueker added 3 commits June 16, 2022 19:22

GROMACS: fix sanity_check when built statically

7c732b0

i.e. Don't look for libs when they are not exected to be built.

GROMACS: simplify CP2K detection

dc10256

ostueker marked this pull request as draft June 20, 2022 13:24

ostueker closed this Jun 20, 2022

ostueker reopened this Jun 20, 2022

boegel added the enhancement label Jun 21, 2022

boegel added this to the 4.x milestone Jun 21, 2022

GROMACS: Gromacs with CP2K needs to be mpi_only

6bbc73b

Because CP2K is built with MPI, the (Open)MPI libs would need to be linked to non-MPI Gromacs binaries, which interfers with the threaded MPI impletation.

ostueker marked this pull request as ready for review June 22, 2022 18:47

ostueker changed the title ~~GROMACS with CP2K QM/MM~~ support CP2K QM/MM in GROMACS easyblock Jun 27, 2022

Merge branch 'develop' into gromacs_cp2k

ffd1858

Micket reviewed Sep 21, 2022

View reviewed changes

ostueker mentioned this pull request Oct 5, 2022

configs to support CP2K QM/MM in GROMACS easybuilders/easybuild-easyconfigs#16362

Closed

GROMACS: add some checks to CP2K QM/MM support

e53d672

ostueker requested a review from Micket October 5, 2022 20:31

boegel changed the title ~~support CP2K QM/MM in GROMACS easyblock~~ add support for CP2K QM/MM in GROMACS easyblock Oct 12, 2022

akesandgren requested changes Oct 24, 2022

View reviewed changes

ostueker added 2 commits October 25, 2022 13:53

GROMACS: check CP2K version only once

6b1f77d

GROMACS: improve cp2k_linker_flags logic

2bacbb3

ostueker requested review from akesandgren and removed request for Micket October 25, 2022 16:32

akesandgren requested changes Dec 1, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for CP2K QM/MM in GROMACS easyblock #2750

add support for CP2K QM/MM in GROMACS easyblock #2750

ostueker commented Jun 17, 2022

ostueker commented Jun 20, 2022

ostueker commented Jul 20, 2022

Micket left a comment

Micket Sep 21, 2022

ostueker Sep 21, 2022

ostueker commented Oct 5, 2022 •

edited

ostueker commented Oct 5, 2022

akesandgren Oct 24, 2022

ostueker Oct 25, 2022

akesandgren Oct 24, 2022

ostueker Oct 25, 2022

akesandgren Oct 24, 2022

ostueker Oct 25, 2022

ostueker Oct 25, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

akesandgren Dec 1, 2022

add support for CP2K QM/MM in GROMACS easyblock #2750

Are you sure you want to change the base?

add support for CP2K QM/MM in GROMACS easyblock #2750

Conversation

ostueker commented Jun 17, 2022

ostueker commented Jun 20, 2022

ostueker commented Jul 20, 2022

Micket left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ostueker commented Oct 5, 2022 • edited

ostueker commented Oct 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ostueker commented Oct 5, 2022 •

edited