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

Build MPI and serial executables in separate build trees #320

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

SeanBryan51
Copy link
Collaborator

@SeanBryan51 SeanBryan51 commented Nov 13, 2024

Benchcab currently fails to install the executables for CABLE-LSM/CABLE#469 due to a change in the structure of the CMakeLists.txt file - namely a single executable is now only built per build tree where as before both serial and MPI executables were built under the same build tree. Whilst this was convenient, it breaks the CMake model of one build configuration per build tree (see PR for more details) and will be hard to maintain with MPI development going forward.

This change adds support for the changes to CMakeLists.txt as part of CABLE-LSM/CABLE#469 by generating two out of source builds for serial and MPI applications instead of one. This change is also backwards compatible for CABLE commits prior to the changes to CMakeLists.txt.

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 68.01%. Comparing base (8e0d58f) to head (e7bb7bf).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/benchcab/model.py 14.28% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #320      +/-   ##
==========================================
- Coverage   68.28%   68.01%   -0.27%     
==========================================
  Files          21       21              
  Lines        1157     1160       +3     
==========================================
- Hits          790      789       -1     
- Misses        367      371       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SeanBryan51 SeanBryan51 force-pushed the build-mpi-and-serial-separately branch from 67f8158 to 15077ff Compare November 13, 2024 04:16
@SeanBryan51
Copy link
Collaborator Author

Integration tests passing.

@SeanBryan51
Copy link
Collaborator Author

SeanBryan51 commented Nov 13, 2024

A local benchcab install of this branch successfully runs with CABLE branch introduce_mpi_group and the current CABLE main branch.

@SeanBryan51 SeanBryan51 marked this pull request as ready for review November 13, 2024 04:48
@SeanBryan51 SeanBryan51 requested review from a team and bschroeter and removed request for a team November 13, 2024 05:03
@SeanBryan51 SeanBryan51 force-pushed the build-mpi-and-serial-separately branch from 15077ff to e7bb7bf Compare November 13, 2024 05:42
Copy link
Collaborator

@bschroeter bschroeter left a comment

Choose a reason for hiding this comment

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

LGTM

@SeanBryan51 SeanBryan51 merged commit aa5d6c0 into main Nov 18, 2024
2 of 4 checks passed
@SeanBryan51 SeanBryan51 deleted the build-mpi-and-serial-separately branch November 18, 2024 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants