Cmake #440
Cmake #440
Conversation
to reader routine
and avoid using reserved stdout/stderr keywords as library interface dummy argument names (for py interoperability vi f90wrap)
get_proj(w90main, w90dat, n, site, l, m, s, istdout, istderr, ierr, comm, sqa, z, x, rad, zona) l,m,s,rad,zona must be arrays of length n site, sqa, z,x must be arrays with size (3,n) sqa, z, x, rad, zona are optional arguments returned is the full list of projections as indicated by projections block in .win file
still missing: 1. branching for serial/parallel testing 2. config setup for compiler options 3. integration to github runner 4. documentation
…nt variables (i.e. to use them)
There was a problem hiding this comment.
I would recommend https://github.com/spglib/spglib or https://github.com/stevengj/nlopt for more modern implementations
wannier90.pc.cmake
Outdated
| Includedir: ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR} | ||
| Libdir: ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} | ||
| Cflags: -I${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_INCLUDEDIR} | ||
| Libs: -L${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR} -lwannier90 |
There was a problem hiding this comment.
Please move to /cmake folder. Preferrably use wannier90.pc.in.
Use @ONLY replacements
CMakeLists.txt
Outdated
| # -Dwith_mpif08=off to disable use of f08 MPI module, to use f90 module instead | ||
| # -Dwith_mpih=on to force use of mpif.h include file | ||
|
|
||
| option(with_mpi "build MPI version" ON) |
There was a problem hiding this comment.
Namespace the options, otherwise it will not work with FetchContent
CMakeLists.txt
Outdated
| option(with_mpi "build MPI version" ON) | ||
| if (with_mpi) | ||
| option(with_mpif08 "build MPI version using f08 module" ON) | ||
| option(with_mpih "build MPI version using Fortran 77 mpif.h header" OFF) |
There was a problem hiding this comment.
Options should not be if blocked against other option, they are not properly parsed. Also why not fully remove mpih support in 2023? Legacy systems should use legacy code or upgrade
CMakeLists.txt
Outdated
| find_package(MPI REQUIRED) | ||
| include_directories(${MPI_Fortran_INCLUDE_PATH}) | ||
| message(STATUS "MPI_Fortran_INCLUDE_PATH ${MPI_Fortran_INCLUDE_PATH}") | ||
| set(LIBS ${LIBS} ${MPI_Fortran_LINK_FLAGS} ${MPI_Fortran_LIBRARIES}) |
There was a problem hiding this comment.
Use target_link_libraries() not manual set. Manual approach does not include definition and other settings that find_package provides
CMakeLists.txt
Outdated
|
|
||
| find_package(BLAS REQUIRED) # binaries require blas | ||
| if (BLAS_FOUND) | ||
| set(LIBS ${LIBS} ${BLAS_LIBRARIES}) |
There was a problem hiding this comment.
Same, use target_link_libraries
CMakeLists.txt
Outdated
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/wannier90.pc DESTINATION ${CMAKE_INSTALL_PREFIX}/lib/pkgconfig COMPONENT pkgconfig) | ||
|
|
||
| # test system | ||
| include(CTest) |
There was a problem hiding this comment.
Wrap around WANNIER90_TEST
CMakeLists.txt
Outdated
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/w90_helper_types.mod DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | ||
|
|
||
| # pkg-config | ||
| configure_file(wannier90.pc.cmake ${CMAKE_CURRENT_BINARY_DIR}/wannier90.pc) |
There was a problem hiding this comment.
Add equivalent wannier90Config.cmake files to expose to find_package
test-suite/CMakeLists.txt
Outdated
| @@ -0,0 +1,71 @@ | |||
| add_test(NAME partestw90_mpierr COMMAND ./run_tests -c partestw90_mpierr) | |||
There was a problem hiding this comment.
Wrap these in a for loop
| @@ -0,0 +1,216 @@ | |||
|
|
|||
There was a problem hiding this comment.
What is the plan with these. Maintenance of two build systems will be a nightmare, and the version is already bumped to a new major version
| @@ -0,0 +1,2 @@ | |||
| #!/bin/bash | |||
| make -j -C ../.. lib && make && ./test_library_serial.x | |||
There was a problem hiding this comment.
Why are these using plain make here?
Modern cmake
Opening a PR to review this branch and track the progress.
Please rebase at some point