search

NCAR / ccpp-framework

Common Community Physics Package (CCPP)
http://www.dtcenter.org/community-code/common-community-physics-package-ccpp/
Other
26 stars 62 forks source link

Add missing MPI target in src/CMakeLists.txt #556

Closed climbfuji closed 2 months ago

climbfuji commented 2 months ago

Add missing MPI target in src/CMakeLists.txt

Not sure why this is not causing any errors currently because it should be there! The line that gets replaced is old and should have been removed in the past (neither LIBS nor CMAKE_DL_LIBS is defined).

User interface changes?: No

Fixes: [Github issue #s] n/a

Testing: test removed: n/a unit tests: n/a system tests: n/a manual testing: on my macOS with ufs-weather-model

climbfuji commented 2 months ago

Is the line needed at all? If you are using an MPI compiler (or compiler wrapper), I thought the MPI libraries were always linked in by default. Still, I can't see how this could hurt.

It is needed, because we are NOT using mpicc etc as default compilers. As I understand, the cleaner way is to use the compiler gcc etc as default and only use mpicc etc when needed. Every cmake target should define exactly what it needs, including mpi. This way you avoid overlinking.

climbfuji commented 2 months ago

@grantfirl @dustinswales Can we add this to one of the next ufs-w-m PRs?

dustinswales commented 2 months ago

@climbfuji I don't see any reason not to. I will leave it to @grantfirl @Qingfu-Liu @mkavulich, who sit in on the UFS application code mgmt. meeting

grantfirl commented 2 months ago

This will get merged with https://github.com/NOAA-EMC/fv3atm/pull/816.

mkavulich commented 2 months ago

Combined with #555 for testing