gromacs +cp2k: build in CI #40494

haampie · 2023-10-12T18:05:20Z

Since there were some issues with gromacs, lets build it in CI, together
with cp2k, which is an important and currently untested package in
Spack.

Overview of changes:

Require x86_64: in libxsmm s.t. dbcsr will default to ordinary BLAS on aarch64 / ppc64;
Fix compatibility issues with dbcsr and mpich@4.1: -- needs mpi_f08;
Bump C standard from C99 to C11 for newer versions of cp2k, for either build system
Add a patch for cp2k build_system=cmake that fixes installed config files
Remove dbcsr from gromacs+cp2k; it's not a direct dep, it's a dep of cp2k

haampie · 2023-10-13T07:34:30Z

@mtaillefumier can you have a look at the CP2K changes?

Do you know from what point CP2K switched from the C99 standard to C11?

Also I quickly glanced over the cmake build system for CP2K, is it missing set(CMAKE_C_STANDARD 11)?

Lastly, CP2K also seems to have this issue with mpich 4.1. Any workaround? Or should I put a hard upperbound on mpich 4.0?

haampie · 2023-10-13T07:42:01Z

@dev-zero / @alazzaro the ~mpi_f08 variant default in dbcsr is actually holding back mpich to version 4.0. Is it necessary to have it as a variant? Alternatively the cmake define could be set by default if mpich >= 4.1?

mtaillefumier · 2023-10-13T07:42:19Z

we never switched to c11 or above. cp2k compiles fine with gnu17 (not fully c17 compliant) and should compile with c11 standard as well.
What MPI issue are you referring to because mpich is not particularly user friendly with fortran codes ?

haampie · 2023-10-13T07:47:26Z

we never switched to c11 or above.

cp2k/cp2k@77977db

In this commit it went from

const int D = DECIMAL_DIG; // In C11 we could use DBL_DECIMAL_DIG.

to

const int D = DBL_DECIMAL_DIG;

which is in CP2K v2023.2, so pressumably you went from C99 -> C11 during some release? It looks like the toolchain fails to set the relevant -std=c99/c11 flags. Apparently somebody ran into that earlier cause we explicitly set -std=c99 in the spack package, probably cause cp2k failed to compile with compilers that defaulted to c89.

mtaillefumier · 2023-10-13T07:47:47Z

I do not see any potential issue setting the C standard to c11 though.

mtaillefumier · 2023-10-13T07:52:21Z

that's the problem when two build systems are present. We always miss something in the process. So the only way to fix it is to enforce c11 in the spack recipe and the cmake build system. Actually the cmake build system will default to gnu17 for gcc 12 and 13. @haampie do you want me to do it ?

mtaillefumier · 2023-10-13T07:53:50Z

Note it will require a PR in cp2k as well for the cmake build and we must have a workaround for the recipe (-DCMAKE_C_STANDARD) crossing our fingers that the standard defined for each target does not overwrite this.

haampie · 2023-10-13T07:56:14Z

I've created a PR cp2k/cp2k#3038.

I'd much rather use the CMake build system to be honest, exactly cause CMake knows how to set the language related flags, and the other build system (toolchain?) doesn't seem to care about it at all.

mtaillefumier · 2023-10-13T07:57:44Z

I've created a PR cp2k/cp2k#3038.

I'd much rather use the CMake build system to be honest, exactly cause CMake knows how to set the language related flags, and the other build system (toolchain?) doesn't seem to care about it at all.

I agree with you. But the toolchain is not going to disappear anytime soon. This problem is also related to this issue
#40372 where c99 is explicitly defined but the build still fails. I really suspect an issue with the compiler here more than anything else.

I wanted to enforce cmake use in spack with 2023.2 but my proposal was controversial. master should enforce cmake otherwise we will always have these problems.

haampie · 2023-10-13T08:07:44Z

where c99 is explicitly defined but the build still fails. I really suspect an issue with the compiler here more than anything else.

No, see https://en.cppreference.com/w/c/types/limits#Limits_of_floating_point_types for reference, -std=c99 fails correctly, CP2K is using C11.

mtaillefumier · 2023-10-13T08:09:04Z

where c99 is explicitly defined but the build still fails. I really suspect an issue with the compiler here more than anything else.

No, see https://en.cppreference.com/w/c/types/limits#Limits_of_floating_point_types for reference, -std=c99 fails correctly, CP2K is using C11.

good to know thanks. That's why it is failing std=c99.

mtaillefumier · 2023-10-13T08:09:13Z

We have to fix the recipe.

haampie · 2023-10-13T08:10:39Z

I wanted to enforce cmake use in spack with 2023.2

For my understanding, it looks like the CMake build of CP2K uses DBCSR as a separate package, whereas the toolchain is using the internal copy of DBCSR, or not?

I'd much rather use DBCSR as a separate package, cause otherwise we have to duplicate all conflicts and compat bounds from the DBCSR package into CP2K. (Right now: mpich 4.1 compatibility and fixes to make it work).

We have to fix the recipe.

Yeah, that's done in this PR ;)

haampie · 2023-10-13T08:12:59Z

var/spack/repos/builtin/packages/libxsmm/package.py

@@ -82,6 +82,9 @@ class Libxsmm(MakefilePackage):
    # (<https://github.com/spack/spack/pull/21671#issuecomment-779882282>).
    depends_on("binutils+ld+gas@2.33:", type="build", when="@:1.17")

+    # Intel Architecture or compatible CPU required
+    requires("target=x86_64:")


@hfp is this OK? I wanna turn the configure/build time error into a constraint, s.t. packages like dbcsr automatically use ordinary blas instead of libxsmm on arm / powerpc

mtaillefumier · 2023-10-13T08:13:46Z

I wanted to enforce cmake use in spack with 2023.2

For my understanding, it looks like the CMake build of CP2K uses DBCSR as a separate package, whereas the toolchain is using the internal copy of DBCSR, or not?

I'd much rather use DBCSR as a separate package, cause otherwise we have to duplicate all conflicts and compat bounds from the DBCSR package into CP2K. (Right now: mpich 4.1 compatibility and fixes to make it work).

We have to fix the recipe.

Yeah, that's done in this PR ;)

You are right. DBCSR is an external package. I only added the option to build dbcsr with cp2k but i do not like the idea from the first place. That's also why the this option is not available with the spack recipe. If people build cp2k with spack and cmake then dbcsr will be considered external dependency. I am not going to change this what ever users want it or not.

mtaillefumier · 2023-10-13T08:16:14Z

PR cp2k/cp2k#3039 for cp2k. Will solve issue #40372 at the same time.

mtaillefumier · 2023-10-13T08:24:59Z

NB : dbcsr dependence is only activated when the build system is cmake.

haampie · 2023-10-13T08:37:32Z

Ok yeah. Whatever the default build system is, we can always decide to build with cmake in our CI 🙃 I think that's much better, given the dbcsr + mpich 4.1 issues... duplicating all dbcsr constraints in cp2k sounds like a bad idea.

Just pushed that, so we test gromacs +cp2k ^cp2k build_system=cmake

haampie · 2023-10-13T09:08:59Z

I've opened #40505 about going forward with cp2k build_system=cmake.

haampie · 2023-10-13T10:29:13Z

gromacs cannot locate cp2k through pkg-config when using build_system=cmake @mtaillefumier, does it generate a different pkg config file from the toolchain?

mtaillefumier · 2023-10-13T11:43:52Z

gromacs cannot locate cp2k through pkg-config when using build_system=cmake @mtaillefumier, does it generate a different pkg config file from the toolchain?

I am sure I generate the pkgconfig file. I suspect I do not install it. One more fix

mtaillefumier · 2023-10-13T11:44:50Z

You can also use find_package(cp2k). these files are installed

mtaillefumier

nothing to say. Approved.

haampie · 2023-10-19T18:22:11Z

gromacs+cp2k builds fine for x86 / arm / ppc, so should be good to go

RMeli · 2023-10-20T06:13:49Z

var/spack/repos/builtin/packages/cp2k/package.py

+        # C standard was omitted in the first release with CMake build system.
+        # https://github.com/cp2k/cp2k/pull/3039
+        if spec.satisfies("@2023.2"):
+            args.append(self.define("CMAKE_C_STANDARD", "11"))


Is this still useful given the patch?

RMeli · 2023-10-20T06:14:02Z

var/spack/repos/builtin/packages/cp2k/package.py

+            # Use of DBL_DECIMAL_DIG
+            cflags.append(self.compiler.c11_flag)


Is this still useful given the patch?

This one should stay, it's for the other build system

RMeli · 2023-10-20T06:19:42Z

var/spack/repos/builtin/packages/cp2k/package.py

Given the CMake patch introduced here for @2023.2, I think the conflict with +mpi_f08 introduced in #40574 can be relaxed to include @2023.2 as well? (I think the changes introduced in #40574 are not currently in this PR.)

haampie · 2023-10-20T08:35:11Z

var/spack/repos/builtin/packages/cp2k/package.py

@@ -277,11 +277,10 @@ class Cp2k(MakefilePackage, CudaPackage, CMakePackage, ROCmPackage):

    with when("build_system=cmake"):
        depends_on("dbcsr")
-        depends_on("dbcsr@2.6:", when="@2023.2:")
+        depends_on("dbcsr@2.6:")


this when condition was implied by build_system=cmake already

* gromacs +cp2k: build in CI * libxsmm: x86 only * attempt to fix dbcsr + new mpich * use c11 standard * gromacs: does not depend on dbcsr * cp2k: build with cmake in CI, s.t. dbcsr is a separate package * cp2k: cmake patches for config files and C/C++ std * cp2k: remove unnecessary constraints due to patch

spackbot-app bot added core PR affects Spack core functionality gitlab Issues related to gitlab integration conflicts dependencies update-package labels Oct 12, 2023

haampie force-pushed the ci/add-gromacs-cp2k branch from 534f336 to 8a67bda Compare October 12, 2023 19:43

haampie mentioned this pull request Oct 13, 2023

Error with mpich 4.1: Type mismatch in argument 'baseptr' at (1); passed TYPE(c_ptr) to INTEGER(4) cp2k/dbcsr#661

Closed

haampie mentioned this pull request Oct 13, 2023

Installation issue: cp2k #40372

Closed

4 tasks

haampie commented Oct 13, 2023

View reviewed changes

msimberg requested a review from mtaillefumier October 19, 2023 14:49

mtaillefumier approved these changes Oct 19, 2023

View reviewed changes

RMeli reviewed Oct 20, 2023

View reviewed changes

haampie added 5 commits October 20, 2023 10:16

gromacs +cp2k: build in CI

ac8aac6

libxsmm: x86 only

ad25b7e

attempt to fix dbcsr + new mpich

a0899bc

use c11 standard

25c49de

gromacs: does not depend on dbcsr

1a17f22

haampie force-pushed the ci/add-gromacs-cp2k branch 2 times, most recently from 85cb6c0 to 0713eb1 Compare October 20, 2023 08:21

haampie and others added 2 commits October 20, 2023 10:23

cp2k: build with cmake in CI, s.t. dbcsr is a separate package

f73ac90

cp2k: cmake patches for config files and C/C++ std

409ef1f

haampie force-pushed the ci/add-gromacs-cp2k branch from 0713eb1 to d4f3cd8 Compare October 20, 2023 08:23

haampie commented Oct 20, 2023

View reviewed changes

cp2k: remove unnecessary constraints due to patch

ea70a1d

haampie force-pushed the ci/add-gromacs-cp2k branch from d4f3cd8 to ea70a1d Compare October 20, 2023 08:35

haampie requested a review from RMeli October 20, 2023 12:26

RMeli approved these changes Oct 20, 2023

View reviewed changes

msimberg approved these changes Oct 20, 2023

View reviewed changes

msimberg merged commit 4bade7e into spack:develop Oct 20, 2023
34 checks passed

sebastiangrimberg mentioned this pull request Nov 16, 2023

Installation issue: libxsmm #41121

Closed

4 tasks

haampie deleted the ci/add-gromacs-cp2k branch November 21, 2023 08:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gromacs +cp2k: build in CI #40494

gromacs +cp2k: build in CI #40494

haampie commented Oct 12, 2023 •

edited

Loading

haampie commented Oct 13, 2023 •

edited

Loading

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023 •

edited

Loading

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023 •

edited

Loading

haampie Oct 13, 2023 •

edited

Loading

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023 •

edited

Loading

haampie commented Oct 13, 2023

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier left a comment •

edited

Loading

haampie commented Oct 19, 2023

RMeli Oct 20, 2023

RMeli Oct 20, 2023

haampie Oct 20, 2023

RMeli Oct 20, 2023

haampie Oct 20, 2023

		# Use of DBL_DECIMAL_DIG
		cflags.append(self.compiler.c11_flag)

gromacs +cp2k: build in CI #40494

gromacs +cp2k: build in CI #40494

Conversation

haampie commented Oct 12, 2023 • edited Loading

haampie commented Oct 13, 2023 • edited Loading

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023 • edited Loading

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023 • edited Loading

haampie Oct 13, 2023 • edited Loading

Choose a reason for hiding this comment

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

haampie commented Oct 13, 2023 • edited Loading

haampie commented Oct 13, 2023

haampie commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier commented Oct 13, 2023

mtaillefumier left a comment • edited Loading

Choose a reason for hiding this comment

haampie commented Oct 19, 2023

RMeli Oct 20, 2023

Choose a reason for hiding this comment

RMeli Oct 20, 2023

Choose a reason for hiding this comment

haampie Oct 20, 2023

Choose a reason for hiding this comment

RMeli Oct 20, 2023

Choose a reason for hiding this comment

haampie Oct 20, 2023

Choose a reason for hiding this comment

haampie commented Oct 12, 2023 •

edited

Loading

haampie commented Oct 13, 2023 •

edited

Loading

mtaillefumier commented Oct 13, 2023 •

edited

Loading

haampie commented Oct 13, 2023 •

edited

Loading

haampie Oct 13, 2023 •

edited

Loading

haampie commented Oct 13, 2023 •

edited

Loading

mtaillefumier left a comment •

edited

Loading