search

CABLE-LSM / benchcab

Tool for evaluation of CABLE land surface model
https://benchcab.readthedocs.io/en/latest/
Apache License 2.0
2 stars 4 forks source link

Update to CMake build #278

Closed SeanBryan51 closed 7 months ago

SeanBryan51 commented 7 months ago

The CABLE build system was transitioned from Makefile to CMake: see https://github.com/CABLE-LSM/CABLE/pull/216, https://github.com/CABLE-LSM/CABLE/pull/200. This change adds support for building CABLE via CMake in benchcab so that CABLE versions which branch from the HEAD of main can be tested.

Closes #258

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 38.88889% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 71.22%. Comparing base (59910ed) to head (8d41929). Report is 1 commits behind head on main.

Files Patch % Lines
src/benchcab/model.py 25.00% 9 Missing :warning:
src/benchcab/benchcab.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #278 +/- ## ========================================== - Coverage 72.81% 71.22% -1.59% ========================================== Files 18 18 Lines 982 966 -16 ========================================== - Hits 715 688 -27 - Misses 267 278 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

SeanBryan51 commented 7 months ago

I've decided not to add unit tests for Model.build() as the functionality is environment dependent and would involve a lot of unpleasant mocking. The integration test shows that the model builds successfully.

ccarouge commented 7 months ago

Should we remove support for the old build systems at the same time? It isn't strictly necessary for this PR but wouldn't it make things nicer for us? Bringing the old build systems to CMake might not take that much time now that we have a working example to use in main.

SeanBryan51 commented 7 months ago

@ccarouge if you are referring to the custom build via the build_script key, I think I'd prefer to leave that as is for now as I would like to push a release which includes the changes in this PR. Deprecating that functionality would be major change and should require bumping the major version - we can probably save this for when spack comes around?