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

Update requirements #3375

Merged
merged 4 commits into from
Jan 3, 2020
Merged

Update requirements #3375

merged 4 commits into from
Jan 3, 2020

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented Dec 11, 2019

Increase requirements of CMake, boost, Cython and Python, as discussed in es meeting 2019-11-26.

Closes #3090, Partial fix for #3093

Depends on espressomd/docker#149

Only support boost versions starting from Ubuntu 16.04 LTS.
Tested in the cuda:9 and clang:6 docker images.
Only support versions starting from Ubuntu 16.04 LTS.
Only support versions starting from Ubuntu 18.04 LTS.
Use the new CMake features to simplify the CMake logic.
@jngrad jngrad added the CMake label Dec 11, 2019
@jngrad jngrad added this to the Espresso 4.2 milestone Dec 11, 2019
@codecov
Copy link

codecov bot commented Dec 11, 2019

Codecov Report

Merging #3375 into python will increase coverage by <1%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           python   #3375    +/-   ##
=======================================
+ Coverage      86%     86%   +<1%     
=======================================
  Files         537     538     +1     
  Lines       25282   25306    +24     
=======================================
+ Hits        21806   21827    +21     
- Misses       3476    3479     +3
Impacted Files Coverage Δ
src/core/io/writer/h5md_core.cpp 89% <ø> (ø) ⬆️
src/core/MpiCallbacks.hpp 97% <ø> (ø) ⬆️
...rc/utils/include/utils/math/triangle_functions.hpp 91% <ø> (ø) ⬆️
src/core/accumulators/Correlator.cpp 56% <ø> (ø) ⬆️
src/core/observables/ParticleBodyVelocities.hpp 100% <100%> (ø) ⬆️
src/core/observables/ParticleAngularVelocities.hpp 100% <100%> (ø) ⬆️
...core/observables/ParticleBodyAngularVelocities.hpp 100% <100%> (ø) ⬆️
src/core/domain_decomposition.cpp 94% <100%> (ø) ⬆️
src/core/observables/ComForce.hpp 100% <100%> (ø) ⬆️
src/core/domain_decomposition.hpp 100% <100%> (ø) ⬆️
... and 4 more

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 a6deb15...65e5e6b. Read the comment docs.

@@ -93,17 +93,6 @@ default:
- docker
- linux

min_boost:
Copy link
Member

Choose a reason for hiding this comment

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

why do you want to remove this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

Here's how I understood our last es meeting:

  • we don't test boost 1.55 any more
  • boost 1.58 is already tested in cuda:9.0 and clang:6.0
  • once Ubuntu 20.04 LTS is released next April, we upgrade boost requirements to 1.65 (for cuda:9.0 which requires Ubuntu 16.04, we build boost 1.65 from sources)
  • except for building boost from sources, Dockerfile-min_boost = Dockerfile-18.04, so we can transfer it from master to a new 4.1 branch

Copy link
Contributor

Choose a reason for hiding this comment

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

We could think about having one container where all the library versions are pinned to the minimal required versions, so that we don't silently stop testing this when a library version is updated in the distribution. This isn't really complicated, all major package managers let you just fix a version.

Copy link
Member Author

Choose a reason for hiding this comment

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

cuda:9.0 will essentially fulfill that role. It uses Ubuntu 16.04 and the minimal supported versions of CUDA and CMake. The version numbers of non-graphical Python packages available in the Ubuntu 16.04 repository are used as the minimal versions in requirements.txt (fb4862e). Once Ubuntu 20.04 is out and espresso drops support for CUDA9, we can use the cuda:10.1 to fulfill that role. We could rename the CI job from cuda10:maxset to ubuntu:min-dependencies for clarity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whatever you think is appropriate. I'm just saying that it is not as such guaranteed that the package versions in the distributions stay the same, which could lead to a situation where the minimal version of some package is no longer tested.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could come up with an extra check in the CI job that tests the docker image to make sure the versions of dependencies match the minimal requirements.

@RudolfWeeber
Copy link
Contributor

My understanding is that we increase minimum requirements on Boost to 1.65, once we get rid of the Ubuntu 16.04 build. That in turn depends on us being able to give up on cuda 9.
This will be pursued offline.

@fweik
Copy link
Contributor

fweik commented Jan 3, 2020

@jngrad did we decide how to deal with the insufficient cmake versions? Unfortunately, I don't remember...

@jngrad
Copy link
Member Author

jngrad commented Jan 3, 2020

@fweik in es meeting 2019-11-26 we decided to move to CMake from Ubuntu 18.04 and install it via pip in Ubuntu 16.04 docker images. This PR will pass CI in ~2 hours, when the new Ubuntu 16.04 images get deployed.

@fweik
Copy link
Contributor

fweik commented Jan 3, 2020

Ah, right. Thank you.

@jngrad jngrad marked this pull request as ready for review January 3, 2020 14:01
@jngrad jngrad added the automerge Merge with kodiak label Jan 3, 2020
@kodiakhq kodiakhq bot merged commit 7dbb510 into espressomd:python Jan 3, 2020
@jngrad
Copy link
Member Author

jngrad commented Jan 3, 2020

Looks like kodiak won't merge a PR if the last CI pipeline passed while the PR was still in draft mode. Once the PR is marked for review, simply re-run the status_success job to make kodiak happy.

@jngrad jngrad deleted the update-requirements branch January 18, 2022 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge with kodiak CMake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump required cmake version to 3.10
4 participants