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

Minimal cmake build (GCC + generic FFT) #20

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

tlestang
Copy link

@tlestang tlestang commented Feb 25, 2022

Stripped down version of the build currently in place in the hack_doconcurrent branch. It contains only the ingredients necessary to build and install with GCC and the generic FFT module. This aims to have a clean, minimal build supporting unit tests with pFUnit.

I removed the call to include_directories(SYSTEM ${MPI_INCLUDE_PATH}) (see a9cecd2). It seemed redundant with linking with MPI::MPI_Fortran in both subdirectories. Moreover, if the issue is irrelevant with CMake > 3.10 as the messages suggest, then we're good with the cmake_minimum_required being 3.12?

The targets' COMPILE_OPTIONS is set instead of defining CMAKE_Fortran_flags. If I remember correcly, CMAKE_*_FLAGS variables are meant to be defined when calling cmake to add flags on top of the list (docs). Using COMPILE_OPTIONS allows to define a list of flags instead of a single string. CMake deals with escaping if needed.

A separate list of command line options is defined for both xcompact3d
and the decomp2d library. Because they are currently the same, the
list could technically be defined one for decomp2d with the PUBLIC
attribute. However, it is likely that these 2 lists will diverge in
the future. Or at least it's helpful to allow it.
@tlestang
Copy link
Author

@rfj82982 tagging you here since it looks like you wrote most of the CMake in the hack_doconcurrent branch. :)

RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to care about installation location at the moment?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really but what i did was for the full code that is why i had all this. I gave a quick look and seems ok to me. I have just to do a pull and check that everything compiles.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also add this flag if gcc >= 10 -fallow-argument-mismatch. This is what i have at this line of the CMake

set(CMAKE_Fortran_FLAGS "-cpp -funroll-loops -floop-optimize -g -Warray-bounds -fcray-pointer -fbacktrace -ffree-line-length-none -fallow-argument-mismatch")

Without this the code do not compile with new gcc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The (long term) better solution would be to move to the mpi_f08 interfaces which have proper types, however tooling support is worse than rubbish so it's a non-starter currently

Copy link
Author

@tlestang tlestang Feb 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on Debian 11 with GCC 10.2

$ cmake ..
-- The Fortran compiler identification is GNU 10.2.1
-- Detecting Fortran compiler ABI info
-- Detecting Fortran compiler ABI info - done
-- Check for working Fortran compiler: /usr/bin/f95 - skipped
-- Checking whether /usr/bin/f95 supports Fortran 90
-- Checking whether /usr/bin/f95 supports Fortran 90 - yes
-- Found MPI_Fortran: /usr/lib/x86_64-linux-gnu/libmpi_usempif08.so (found version "3.1") 
-- Found MPI: TRUE (found version "3.1")  
-- MPI_Fortran_COMPILER found: /usr/bin/mpif90
-- Configuring done
-- Generating done
-- Build files have been written to: /home/thibault/scratch/repos/x3div/build

The branch compiles and runs.

Without this the code do not compile with new gcc

Are you talking about code introduced in other branches? I'm not exactly clear on what -fallow-argument-mismatch is about, but happy to include it here if necessary.

Copy link
Collaborator

@rfj82982 rfj82982 Feb 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the error start from gcc 10.3 onwards. We had this issue on several clusters that run a relatively recent gcc. Below the esample from my MacOS with gcc 12.0, but it is the same on openSUSE with gcc11.4/5

rfj82982@ceg-rolfo-2 x3div % export FC=mpif90-mpich-mp
rfj82982@ceg-rolfo-2 build_minimal % cmake ../
-- The Fortran compiler identification is GNU 12.0.0
-- Checking whether Fortran compiler has -isysroot
-- Checking whether Fortran compiler has -isysroot - no
-- Checking whether Fortran compiler supports OSX deployment target flag
-- Checking whether Fortran compiler supports OSX deployment target flag - yes
-- Detecting Fortran compiler ABI info
-- Detecting Fortran compiler ABI info - done
-- Check for working Fortran compiler: /opt/local/bin/mpif90-mpich-mp - skipped
-- Checking whether /opt/local/bin/mpif90-mpich-mp supports Fortran 90
-- Checking whether /opt/local/bin/mpif90-mpich-mp supports Fortran 90 - yes
-- Found MPI_Fortran: /opt/local/bin/mpif90-mpich-mp (found version "3.1") 
-- Found MPI: TRUE (found version "3.1")  
-- MPI_Fortran_COMPILER found: /opt/local/bin/mpif90-mpich-mp
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/rfj82982/GIT_projects/x3div/build_minimal
rfj82982@ceg-rolfo-2 build_minimal % make 
Scanning dependencies of target decomp2d
[  6%] Building Fortran object decomp2d/CMakeFiles/decomp2d.dir/decomp_2d.f90.o
/Users/rfj82982/GIT_projects/x3div/decomp2d/halo_common.inc:167:15-15:

  167 | call MPI_IRECV(out(xs,ys,zs), 1, halo12, &
      |               1
Error: Type mismatch between actual argument at (1) and actual argument at (2) (REAL(4)/COMPLEX(4)).
/Users/rfj82982/GIT_projects/x3div/decomp2d/halo_common.inc:171:15:

  171 | call MPI_IRECV(out(xs,ye-level+1,zs), 1, halo12, &
      |               1
Error: Type mismatch between actual argument at (1) and actual argument at (2) (REAL(4)/COMPLEX(4)).
/Users/rfj82982/GIT_projects/x3div/decomp2d/halo_common.inc:175:16-16:
.....

This is to prevent the error on type mismatch on all MPI calls

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks for the clarification!

@rfj82982
Copy link
Collaborator

Hi Thibo, in my repo the default installation was inside build. I think you revert to /usr/local but people might not have right to write as in my case before. I really think the make install should put the files uder bulild/opt or something similar as i did before.

rfj82982@ceg-rolfo-2 build_minimal % make install 
[ 25%] Built target decomp2d
[100%] Built target xcompact3d
Install the project...
-- Install configuration: ""
-- Installing: /usr/local/lib/libdecomp2d.a
CMake Error at decomp2d/cmake_install.cmake:41 (file):
  file INSTALL cannot copy file
  "/Users/rfj82982/GIT_projects/x3div/build_minimal/decomp2d/libdecomp2d.a"
  to "/usr/local/lib/libdecomp2d.a": Permission denied.
Call Stack (most recent call first):
  cmake_install.cmake:42 (include)

@pbartholomew08
Copy link
Member

yeah I agree with @rfj82982, e.g. on an HPC machine you need to install somewhere under user's control

@tlestang
Copy link
Author

Thanks both - I forgot to port the part where the install dir was defined. Hopefully this behaves as expected now, that is executable is installed as build/bin/xcompact3d.

@tlestang tlestang mentioned this pull request Mar 4, 2022
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