CMake development

[stenczelt]

New build system with CMake and minor changes in the code related to it.

[gregorydavidmartinez]

Hello,

I'm currently implementing this cmake build into my friend's project and I found what might be a bug and another quality of life issue. As for the "might be a bug", on line 71 of the root CMakeList.txt there's the line:

set(target_lib_dir ${PROJECT_BINARY_DIR}/lib)
set(target_bin_dir ${PROJECT_BINARY_DIR}/bin)

But this will install the libraries in the build directory, not the root directory (you already had a "lib" folder there, so I guess that's where you want it?). Did you mean to do:

set(target_lib_dir ${PROJECT_SOURCE_DIR}/lib)
set(target_bin_dir ${PROJECT_SOURCE_DIR}/bin)

?

This next issue is more of a quality of life issue. When doing a make install it installs all the targets. But, I only want one target (libchord.a). But, if the LIBRARY_OUTPUT_DIRECTORY, ARCHIVE_OUTPUT_DIRECTORY, or RUNTIME_OUTPUT_DIRECTORY property tags were used in the appropriate set_target_properties for the libraries, then there would be no need to run "make install at all" (since I'd know where the libraries are located). In other words add the line

ARCHIVE_OUTPUT_DIRECTORY ${target_lib_dir}

(and change set_property to set_target_properties) to line 15 and

LIBRARY_OUTPUT_DIRECTORY ${target_lib_dir}

to line 23 in src/polychord/CMakeLists.txt so that they look like:

set_target_properties(${PROJECT_NAME} 
                      PROPERTIES  
                      LINKER_LANGUAGE Fortran
                      ARCHIVE_OUTPUT_DIRECTORY ${target_lib_dir})

and

set_target_properties(${PROJECT_NAME}_shared
                      PROPERTIES
                      OUTPUT_NAME ${PROJECT_NAME}
                      CLEAN_DIRECT_OUTPUT 1
                      LINKER_LANGUAGE Fortran
                      LIBRARY_OUTPUT_DIRECTORY ${target_lib_dir})

Also, the same can be done to the targets in likelihoods/examples/CMakeLists.txt on line 17 and adding a "set_target_properties" on line 22. And using the properties tag RUNTIME_OUTPUT_DIRECTORY on line 26. Also by doing this, you don't need to install them locally via "make install" and leave "make install" for the python installation.

Thanks :).

[gregorydavidmartinez]

Hello,

So we have mac users who wish to compile and link PolyChord in a project of ours. Unfortunately, it's common for Mac users use Apple Clang for C/C++ and GNU for Fortran. But in the current cmake build this will fail because of lines 15-17 in CMakeLists.txt:

if (NOT "${CMAKE_Fortran_COMPILER_ID}" MATCHES "${CMAKE_CXX_COMPILER_ID}")
    message(FATAL_ERROR "You need to use the same vendor for your C++ and Fortran compiler")
endif ()

Granted, these lines are needed since on lines 21 through 51 the Fortran and C++ flags are set at the same time. I was wondering if one of two things could happen:

a) Separate the processing of the Fortran and C++ flags (lines 21-51) so to allow different Fortran and C++ compilers (thus, no need for lines 15-17). Or ...

b) Change lines 15-17 to

if (NOT "${CMAKE_Fortran_COMPILER_ID}" MATCHES "${CMAKE_CXX_COMPILER_ID}" 
    AND NOT ("${CMAKE_Fortran_COMPILER_ID}" MATCHES "GNU" AND
             "${CMAKE_CXX_COMPILER_ID}" MATCHES "AppleClang"))
    message(FATAL_ERROR "You need to use the same vendor for your C++ and Fortran compiler")
endif ()

to allow for the combination of Apple Clang and gfortran?

Thanks again :).

[williamjameshandley]

Hi @gregorydavidmartinez, re the clang suggestions above, have you checked that this creates a viable build on your system? I have tried for many years to introduce clang as a partner into the delicate dance that is the python-c++-fortran PolyChordLite system, but OSX does often seem to make changes that render any fix temporary. Currently both @stenczelt and myself lack access to an OSX system.

[gregorydavidmartinez]

Yes, clang+gfortran makes a viable build (for libchord.a). I wrote a patch that we apply when pulled and we can use polychord (with MPI) in our code. But we haven't tried using the PolyChord python interface since we have our own python interface. Is there a script you'd like us to test?

…

On Thu, Sep 9, 2021 at 11:07 PM Will Handley ***@***.***> wrote: Hi @gregorydavidmartinez <https://github.com/gregorydavidmartinez>, re the clang suggestions above, have you checked that this creates a viable build on your system? I have tried for many years to introduce clang as a partner into the delicate dance that is the python-c++-fortran PolyChordLite system, but OSX does often seem to make changes that render any fix temporary. Currently both @stenczelt <https://github.com/stenczelt> and myself lack access to an OSX system. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#76 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATJM72T2XSJ5WYLJQJYUWLUBGOC3ANCNFSM5DUDD7EA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[gregorydavidmartinez]

A follow up to @williamjameshandley response. I dusted off my old macbook and painfully updated it. I was able to compile pyPolyChord and run the run_pypolychord.py script -- without mpi (there's something screwy going on with homebrew's mpich and I don't have the patience to tract down the problem). So, it seems the apple clang + gfortran seems to be viable (modulo mpi, but I think that problem is with me rather than pypolychord). But I did find a potential problem with the cmake system with regards to Macs + Conda:

In short, in some Mac Conda builds the python executable is statically linked to its libraries. So, if you dynamically link your Python module to the python libraries, then when Python loads the extension module the python libraries will also load. This will cause Python to complain that duplicate Python symbols were loaded onto the link map. We have quite a few Mac users in my group and this problem came up -- a lot. The solution to this is to compile the extension modules without linking to the python libraries and tell the Mac linker not to throw errors for unresolved symbols at compile time (unlike Macs, the Linux linker is smart enough to know not to do this). This will cause the dynamic linker to correctly resolve the unlinked symbols when the extension module is loaded. So perhaps maybe changing lines 96-105 in CMakeLists.txt to something like

    add_library(_pypolychord SHARED pypolychord/_pypolychord.cpp $<TARGET_OBJECTS:objlib_chord>)
    if(APPLE)
        set_target_properties(_pypolychord
                PROPERTIES
                OUTPUT_NAME _pypolychord
                PREFIX ""
                SUFFIX "${python_so_suffix}"
                LINK_FLAGS "-undefined dynamic_lookup"
                LINKER_LANGUAGE Fortran
                )
    else(APPLE)
        set_target_properties(_pypolychord
                PROPERTIES
                OUTPUT_NAME _pypolychord
                PREFIX ""
                SUFFIX "${python_so_suffix}"
                LINKER_LANGUAGE Fortran
                )
    endif(APPLE)

commenting out or deleting line 104

    # dependencies and includes
    # target_link_libraries(_pypolychord Python3::Python Python3::NumPy)

and changing line 105 to include the Python and Numpy headers

    target_include_directories(_pypolychord PUBLIC src/polychord ${Python3_INCLUDE_DIRS} ${Python3_NumPy_INCLUDE_DIRS}) # PolyChord headers for python extensions

?

Also, another Quality of Life issue. There's currently no instructions to do a (cmake) manual build, which would be useful to people who need to pass cmake arguments (to give hints for library locations, to do debugging, and so on). In addition, if you do a manual build, the work flow could be kind of weird (like needing to move libraries manually):

mkdir build; cd build
cmake -DPython3_EXECUTABLE=`which python3` -DMPI=OFF ..
make
mv _pypolychord.cpython-39-darwin.so ../
cd ..; python3 run_pypolychord.py

In the above work flow, it's hard to know that _pypolychord.{stuff}.so needs to be moved to the root directory at first glance. Perhaps, it'll be nicer if _pypolychord.{stuff}.so was built directly into the root directory by adding the LIBRARY_OUTPUT_DIRECTORY ${PROJECT_SOURCE_DIR} tag to the set_target_properties for the _pypolychord library and changing line 108 to:

    set(target_python_so ${PROJECT_SOURCE_DIR}/_pypolychord${python_so_suffix})

?

And one last comment. For some odd reason, I had to change line 167 in src/polychord/mpi_utils.F90 to

            if(feedback >= normal_fb) write(*,*) '("PolyChord: MPI is already initilised, not initialising, and will not finalize")'

to keep my mpi-pypolychord builds from segfaulting. I'm not that well versed in Fortran, so I don't if this is needed, or it's just some weird peculiarity of my system.

[stenczelt]

I have implemented some of these changes. The mixed build on MacOS should indeed work, I have allowed it with your snippet @gregorydavidmartinez.

Regarding the destination of the libraries and the executables, we should be careful with using the source root, since one could have multiple builds in separate dirs and use those. For example, I had an ubuntu-GNU, ubuntu-Intel, macOS build locally for a long time testing all three. btw I am on a Mac so I can test that build as well and I am using docker for the rest

[williamjameshandley]

@stenczelt -- any idea why the new workflow isn't triggering? Is there anything I need to do on this side for the github action?

[stenczelt]

@stenczelt -- any idea why the new workflow isn't triggering? Is there anything I need to do on this side for the github action?

Hmmm, let's see. I maybe needs a rebase or manual trigger in the actions tab. Let me investigate.

[stenczelt]

I have rebased now, I think it needed my fork's master branch to be updated as well

[AdamOrmondroyd]

Would it be possible for install instructions for macOS to be added to the readme?

I also noticed that the polychord version in CMakeLists.txt line 121 is 1.18.2. Should this be 1.21.0?

[williamjameshandley]

OK nearly there:

• the CMakeLists.txt should either read the python version number from the README or the __init__.py, or add a check that the version number is obeyed in the CI version number check.
• reinstate the OSX CI build
• Add installation instructions to the README so that OSX users can install manually.

In an ideal world, the pip install . would activate the CMake, but I'm not sure how easy that is.

Any OSX volunteers?

[AdamOrmondroyd]

• the CMakeLists.txt should either read the python version number from the README or the __init__.py, or add a check that the version number is obeyed in the CI version number check.
  ...
  Any OSX volunteers?

The original setup.py reads the version from src/polychord/feedback.f90, which I've copied across to the cmake template (and works).

Ngl this feels like a hack with the version number also in __init__.py

[appetrosyan]

In an ideal world, the pip install . would activate the CMake, but I'm not sure how easy that is.

Any OSX volunteers?

This might avoid problems like the #120.

[stenczelt]

I am on a Mac, and have both an Intel & ARM one that I can test this on if needed.