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

Refactor CMake package inclusion #3574

Merged
merged 15 commits into from
Mar 12, 2020

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Mar 10, 2020

Description of changes:

  • move logic to import packages from CMakeLists.txt to dedicated helper files cmake/Find<package>.cmake for find_package()
  • enforce the Cython version requested in CMakeLists.txt
  • CMake now fails if WITH_CUDA is set to true but no CUDA-capable compiler is found
  • CMake now fails if WITH_CLANG_TIDY is set to true but Clang-tidy is not found or its version doesn't match the Clang compiler version
  • drop deprecated FindCUDA in favor of native CUDA support in CMake 3.10 (required for WIP: Stokesian Dynamics #3445)
  • add partial support for ROCm 3.1 (closes Partial ROCm 3.1 support #3571, required for Support for ROCm 3.1.0 docker#156)

jngrad and others added 14 commits March 10, 2020 20:41
The feature is not longer required, and actually has compatibility
issues with CUDA 10.1 (e.g. `CUDA_cublas_device_LIBRARY-NOTFOUND`).
From the CMake 3.10 documentation:
["The FindCUDA module has been superseded by first-class support
for the CUDA language in CMake. It is no longer necessary to use
this module or call `find_package(CUDA)`. This module now exists
only for compatibility with projects that have not been ported."](
https://cmake.org/cmake/help/v3.10/module/FindCUDA.html)
Co-authored-by: Jean-Noël Grad <jgrad@icp.uni-stuttgart.de>
The Cython version is already written to the output of CMake 3.10+.
The CMake check ignored the Cython version passed as argument to
the `find_package()` function.
The NumPy version is already written to the output of CMake 3.10+.
Break Python/Cython cyclic dependency and re-use values from the
first call to `find_package(PythonInterp 3.5 REQUIRED)` instead
of calling the function again without the correct version.
@jngrad jngrad added this to the Espresso 4.1.3 milestone Mar 10, 2020
@codecov
Copy link

codecov bot commented Mar 10, 2020

Codecov Report

Merging #3574 into python will decrease coverage by <1%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #3574   +/-   ##
======================================
- Coverage      88%     88%   -1%     
======================================
  Files         524     524           
  Lines       23599   23599           
======================================
- Hits        20776   20775    -1     
- Misses       2823    2824    +1
Impacted Files Coverage Δ
src/core/electrostatics_magnetostatics/p3m.cpp 85% <0%> (-1%) ⬇️
src/core/particle_data.cpp 96% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be99119...378ecbb. Read the comment docs.

@jngrad jngrad changed the title WIP: Refactor CMake package inclusion Refactor CMake package inclusion Mar 11, 2020
@jngrad jngrad requested a review from KaiSzuttor March 11, 2020 11:40
@jngrad
Copy link
Member Author

jngrad commented Mar 11, 2020

@KaiSzuttor here is a draft of the next PR where we split the CUDA compiler detection mechanism in separate files: jngrad/espresso:refactor-cmake-with_gpu

Copy link
Member

@KaiSzuttor KaiSzuttor left a comment

Choose a reason for hiding this comment

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

that improves readability a lot! thanks. Could you please look at my comments, maybe we can get rid of some global flags.

cmake/FindCUDACompiler.cmake Show resolved Hide resolved
maintainer/CI/build_cmake.sh Show resolved Hide resolved
@KaiSzuttor
Copy link
Member

@KaiSzuttor here is a draft of the next PR where we split the CUDA compiler detection mechanism in separate files: jngrad/espresso:refactor-cmake-with_gpu

just open a draft PR, LGTM

@KaiSzuttor KaiSzuttor added the automerge Merge with kodiak label Mar 12, 2020
@kodiakhq kodiakhq bot merged commit f258a3a into espressomd:python Mar 12, 2020
@jngrad jngrad deleted the refactor-cmake-find_package branch January 18, 2022 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants