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

updated daint-ci #438

Merged
merged 7 commits into from
Mar 18, 2021
Merged

updated daint-ci #438

merged 7 commits into from
Mar 18, 2021

Conversation

hfp
Copy link
Member

@hfp hfp commented Mar 17, 2021

Account for an updated Daint-CI (CMake, etc), and counter -Werror with newer GNU Compiler (certain translation units).

( Unrelated: updated LIBXSMM. )

hfp added 3 commits March 17, 2021 17:13
* Account for updated/revised Daint-CI.
* Unrelated: updated LIBXSMM.
… certain translation units (MPI/legacy related rank-mismatches).
@alazzaro
Copy link
Member

@hfp CSCS introduced a new module for the gpu:

module load cdt-cuda

@alazzaro
Copy link
Member

Concerning the -Wno-error, we had already a discussion about that and we really want to avoid it.
The problem is introduced in GCC 10.x (however ctd-gpu will load GCC 9.x, since CUDA doesn't support GCC 10 yet).
So, could you revert your commits?
For the GCC 10 rank issue, the problem comes with some MPI (this is the case for the Cray-mpich installed on Daint). @dev-zero proposed this solution.

@hfp
Copy link
Member Author

hfp commented Mar 18, 2021

Concerning the -Wno-error, we had already a discussion about that and we really want to avoid it.

I am applying -Wno-error specifically for the MPI wrapper and one of the examples. Even with GCC 10.x, we preserve -Werror for the entire source code base, and still produce warnings for the issue in question (log output). Do you think this solution is acceptable?

The problem is introduced in GCC 10.x (however ctd-gpu will load GCC 9.x, since CUDA doesn't support GCC 10 yet).
So, could you revert your commits?

ACK, I will adjust the build scripts. I think we can nevertheless keep the above relaxation about warning-errors for those who want to use GCC 10.x (without CUDA).

For the GCC 10 rank issue, the problem comes with some MPI (this is the case for the Cray-mpich installed on Daint). @dev-zero proposed this solution.

ACK, makes sense. I remember there was also a proposal to wrap things in C. The MPI-F08 module is also supposedly type-safe, but that's another discussion (similar to a C based wrapper).

…kit". Removed workaround "module load gcc" (module "cdt-cuda" is used instead).
@hfp
Copy link
Member Author

hfp commented Mar 18, 2021

( Coverage dropped by 5% with no source change while Codecov has its own environment. )

@dev-zero
Copy link
Contributor

( Coverage dropped by 5% with no source change while Codecov has its own environment. )

Codecov is still missing at least one of the tests (hence no post here from the bot and the coverage diff)

@alazzaro
Copy link
Member

Concerning the -Wno-error, we had already a discussion about that and we really want to avoid it.

I am applying -Wno-error specifically for the MPI wrapper and one of the examples. Even with GCC 10.x, we preserve -Werror for the entire source code base, and still produce warnings for the issue in question (log output). Do you think this solution is acceptable?

This is OK, but please extend the check to only GCC >=10.x. The Daint test is to really cover all possible problems, we want to check all warnings... Then, we will make the change more automatic by adopting the @dev-zero solution...

@hfp
Copy link
Member Author

hfp commented Mar 18, 2021

This is OK, but please extend the check to only GCC >=10.x. The Daint test is to really cover all possible problems, we want to check all warnings... Then, we will make the change more automatic by adopting the @dev-zero solution...

ACK. I am just waiting for current Daint-CI to finish.

@hfp hfp merged commit 612c604 into cp2k:develop Mar 18, 2021
@hfp hfp deleted the daintci branch March 18, 2021 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants