Skip to content

Use the physics from https://github.com/NOAA-GSL/UGWP.git with cmake build #1316

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.

Sign up for GitHub

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

Jump to bottom
Merged
mgduda merged 1 commit into from
May 15, 2025
Merged

Use the physics from https://github.com/NOAA-GSL/UGWP.git with cmake build #1316

mgduda merged 1 commit into from
May 15, 2025

Conversation

@jim-p-w
Copy link
Contributor

@jim-p-w jim-p-w commented May 14, 2025

There is a new dependency from the physics model to https://github.com/NOAA-GSL/UGWP.git
This PR adds the necessary elements to src/core_atmosphere/CMakeLists.txt to

This was tested by building with the following commands:

cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DMPAS_DOUBLE_PRECISION=OFF ../MPAS-Model-develop |& tee cmake.log
qcmd -A nmmm0043 -q develop -- make -j4 |&tee make.log

@mgduda mgduda requested review from amstokely and mgduda May 15, 2025 15:30
@mgduda
Copy link
Contributor

mgduda commented May 15, 2025

@jim-p-w Can you take another look at the commit message? "Use the physics from https://github.com/NOAA-GSL/UGWP.git when building with cmake." doesn't seem to me like it fully explains why we're making the changes in the commit.

It's probably worth mentioning the point (hash) on the develop branch at which new files were added, and that because the CMakeLists.txt files weren't updated to account for the new code, the builds with CMake were broken. It can sometimes also be worth including the build error that the changes in the commit aim to fix.

@jim-p-w
This change updates the cmake files to incorporate NOAA's Unified For…
30b880b 
…ecast

System Unified Gravity Wave Physics suite of physics parameterizations.

git commit d2c0680  (PR MPAS-Dev#1276) introduced a new external dependency for
the physics code.  While the Makefiles were updated accordingly, the cmake
files were not.  This resulted in compile failures when building with cmake:
MPAS-Model-develop/src/core_atmosphere/physics/mpas_atmphys_finalize.F:14:6:
14 |  use cires_ugwpv1_module
|      1
Fatal Error: Cannot open module file 'cires_ugwpv1_module.mod' for reading at
(1): No such file or directory
@jim-p-w jim-p-w force-pushed the atmosphere/cmake_noaa_ugwp branch from c7e46ae to 30b880b Compare May 15, 2025 18:15
@jim-p-w
Copy link
Contributor Author

jim-p-w commented May 15, 2025

I have updated the commit message.

@mgduda
Copy link
Contributor

mgduda commented May 15, 2025

I have updated the commit message.

Thanks -- the new message looks good!

@mgduda mgduda merged commit c797ba1 into MPAS-Dev:develop May 15, 2025
@mgduda mgduda mentioned this pull request May 15, 2025
Closed
@byoung-joo
Copy link
Contributor

byoung-joo commented May 16, 2025

Ah... does this break the mpas-bundle build ?
Do I also need #1317 if I want to build the latest mpas-bundle, @jim-p-w ?

@jim-p-w
Copy link
Contributor Author

jim-p-w commented May 16, 2025

Ah... does this break the mpas-bundle build ? Do I also need #1317 if I want to build the latest mpas-bundle, @jim-p-w ?

@byoung-joo Yes the mpas-bundle build is broken. Yes you need the changes from both this PR and PR 1317. With both sets of changes the mpas_bundle build works.

@byoung-joo
Copy link
Contributor

byoung-joo commented May 16, 2025

Thank you for making this work, @jim-p-w!
I am afraid if this will happen more often. Fortunately, jedi-bundle is using v8.2.1 tag for MPAS-Model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgduda mgduda mgduda approved these changes

+1 more reviewer

@amstokely amstokely amstokely approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Atmosphere

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jim-p-w @mgduda @byoung-joo @amstokely