Skip to content
Snippets Groups Projects

WIP: Feature/use cmake

Closed Berenger Bramas requested to merge feature/use-cmake into master

To add cmake

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Author Maintainer

    Currently, it is not working. I can run cmake without issue on my PC with CXX=mpic++ cmake .. -DHDF5_INCLUDE_PATH=/usr/include/hdf5/openmpi/ -DHDF5_LIB_PATH=/usr/lib/x86_64-linux-gnu/hdf5/openmpi/ but I cannot compile yet.

    You can have a look, and even update if you want. Inside the cmake dir you will find files for the three dependencies for example the fftw package is not correctly defined as the omp version should be added).

    When cmake config is over, you will find BFPSConfig.cmake BFPSLibraryDepends.cmake files that contain all the information that an external application should know to use BFPS, this can be adapted, as I reused here some stuff from another project.

    I will try to improve this next week.

  • For me compiling doesn't work either, it can't find "mpi.h". I will also try to see if I can fix this.

  • added 1 commit

    Compare with previous version

  • added 1 commit

    Compare with previous version

  • Author Maintainer

    I forgot to mention to use MPI we have the choice between two strategies. The first one is to specify to CMake what compiler must be used (this is what I did with the CXX variable in the command I shown, but it can also be done inside the CMake file, and one can imagine to use mpic++ except if CXX is set). The second method is to call find_package(MPI REQUIRED), which will call mpic++ and ask for the lib/include information, and then use this information to link in the CMake (this is what you do).

    We have to carefuly choice the solution we want (and I do not know which one is better). The thing is that, I usually compile with mpic++, it means that I let it manages all the dependencies and stuff related to MPI, and I agree to use mpic++ as if it was a compiler (even so it is just an alias to the real compiler) And I suppose it is safe to use the find_package method too, but here it means we want to manage the dependencies explicitely. The only drawback of this method is when more than one MPI compiler/alias is available, we do not know which one will be selected by CMake.

    We can do as you prefer but I just wanted to pointed out before continuing (and, again, I am fine with both methods!).

  • I prefer that we use find_package(MPI REQUIRED) and find_package(HDF5 REQUIRED). recent versions of cmake look for the environment variable <package>_ROOT, and use it to choose between versions --- this is how I got it to chooose the correct HDF5 on my desktop, for instance (where the system has it's own unusable HDF5 installation that I can't get rid of). On LRZ and MPCDF machines loading a module will automatically create <package>_ROOT environment variables, and I assume that will be enough for cmake. Otherwise we can specify in README.rst that people need to export <package>_ROOT in their .bashrc --- this is for instance how I made the FFTW configuration work as well.

    I would really like that we keep this as simple as possible.

  • Question: to keep things consistent with the results of find_package, would it be ok if I rename <package>_INCLUDE_DIRS to <package>_INCLUDE_PATH?

    Comment: I don't think I made this clear, but the SZIP dependency is sort of fuzzy. It certainly comes from HDF5, and I only ever needed it when using the icc toolchain on LRZ. I actually had to turn it off again with the if statement in order to get the code to compile on my machines in the latest commit.

    Edited by Cristian Lalescu
  • I'd recommend to rely on the mpi compiler wrappers, and if there is a choice, use cmake with MPICXX=...

    schoene Gruesse - best regards Markus Rampp

  • Author Maintainer

    @clalescu, include_path is ok too. For the szip we could make the path optional then.

    @mjr, I agree that looks perfect.

  • So we prioritize environment variables MPICXX, and <package>_LIB_DIR plus <package>_INC_DIR for FFTW and HDF5, and we say this in the README. Otherwise cmake should try standard fallback options.

    @bbramas, as of commit 2e75ad06 compilation should work, in the sense that the various .cpp files are consistent with each other and the headers. I guess you will finalize the cmake config such that it installs library and headers, and then let me know so that I can figure out the Python package installation and then test everything.

  • added 1 commit

    • 4f1e39a7 - Update cmake -- remove previous system and use find instead (including find...

    Compare with previous version

  • Author Maintainer

    I pushed the new cmake files. It seems to work well.

    • it uses find package to locate the files, but warns if HDF5_ROOT and FFTW_DIR are undefined
    • it uses the find fftw from morse project (since it is LGPL we can use it, until cmake provides it)
    • it provides the install machinery to move the header along with the lib
    • it exports cmake config into different files

    Please, keep in mind that we might need to update the cmake config files (that is update the cmake/BFPSConfig.cmake.in to put all the info you will need to use BFPS from another cmake project).

  • I think there's a problem (possibly my fault, but I can no longer find the place where I did this). The BFPS_INCLUDE_DIRECTORIES from BFPSconfig.cmake lists folders from my bfps repository as well as the correct installation path of the headers. I assume this will be problematic when I start making changes to the repository that are not reflected in the installed library.

  • I can confirm compilation and installation works on my desktop and my laptop. And the BFPS_INCLUDE_DIRECTORIES issue is also present on both machines.

  • Author Maintainer

    Sorry I did not get what was the issue?

  • The file BFSConfig.cmake defines BFPS_INCLUDE_DIRECTORIES, which lists a bunch of stuff and at the end it lists the locations of headers in the repository folder. I.e. /home/chichi/repos/bfps/bfps/cpp etc. I think this happens because of lines 94-98 of CMakeLists.txt.

  • added 1 commit

    • 103238aa - Update to remove source dir from list of include paths

    Compare with previous version

  • Author Maintainer

    Ok, So I updated and I use the list of include paths before including the source dir. However, there is still something wrong (I am on it) because the installed headers are without hierarchy.

  • added 1 commit

    • 23351906 - Keep hierarchy in installed headers

    Compare with previous version

  • Author Maintainer

    Now the installed headers should keep the original hierarchy.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading