-
Notifications
You must be signed in to change notification settings - Fork 157
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 github workflow #805
Conversation
b2e1223
to
4ef2eac
Compare
26e2e37
to
a7d09cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good to me. Just out of curiosity what's the motivation for going from gcc 11 to 12?
If I remember correctly gcc 11 had some issues with mpich. |
I see this warning: Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20: actions/checkout@v3, actions/cache@v3, actions/upload-artifact@v3. For more information see: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/. here: https://github.com/NOAA-EMC/fv3atm/actions/runs/8375122977 |
All of those actions are currently at version 4.x, so changing |
This PR will probably create conflicts with #752. @AlexanderRichert-NOAA I do not know which PR we'll be merging first but feel free to combine the changes from this PR with your 'Add unit testing' PR. |
@DusanJovic-NOAA I'll take a look, thanks |
@AlexanderRichert-NOAA did you have a chance to look into potential conflicts in your #752 PR? Or should we merge this PR now as it is? |
Yes, thanks. I don't foresee any problem with merging this first. |
Unfortunately workflow is failing, this time with:
Previous run from this branch finished successfully: https://github.com/NOAA-EMC/fv3atm/actions/runs/8375122977 |
I tried to reproduce the build failure in the github workflow by building spack stack on my desktop computer but I couldn't. It's a different OS (Red Hat vs. Ubuntu), slightly different compiler and mpi versions. Building the spack stack failed first while building the ip library. In CI ip@develop is specified which needs LAPACK, which I do not have installed. Model currently uses ip/4.3.0 not ip/develop. I also noticed that the FMS version compiled in the workflow stack is 2023.04 while the model currently uses 2023.02.01. The versions of ESMF are also different 8.4.3 in CI vs. 8.5.0 that model currently needs. It's strange that in CI some libraries are newer that what model currently uses while some others are older. We should still try to use the same versions everywhere, just for consistency. However, none of these explains why compilation fails in the github workflow. |
How recently was the cache rebuilt? That's the only thing I can think of... Can you try printing the value of that variable that it's complaining about? Or if nothing else, maybe |
When I look at the fms-config.cmake in the install tree on my desktop, which should be either identical or very similar to the one in the github cache, I see:
Variable
I do not understand how this file (fms-config.cmake) exists but it's parent does not exist. |
I added
which is different than in the equivalent fms-config.cmake file on hera, for example:
That first line should set I do not know why this is different in workflow cache than on Hera. Could it be FMS issue, or spack issue, or cmake issue, I'm not sure. |
Looks like this is an issue with the specific version of cmake (3.29.1) currently used by github workflow: https://gitlab.kitware.com/cmake/cmake/-/issues/25827 |
Hm that's unfortunate, good find though. Unless there's another version of cmake available in the runners, like under /usr/local somewhere, then maybe the way to go is to build it through Spack, which takes a few minutes, but at least it'll get cached. I think that would only require removing cmake from the |
Or we can just wait for github to update their VMs with the latest version, which I hope they'll do soon and we can then just remove the step that installs cmake from a tar file. Three weeks ago this worked just fine, which means in the meantime version 3.29.1 has been released and github picked it. We should also add status message to our CMakeLists.txt to print the version of cmake used, which is useful in situations like this. |
Merged via #811 |
Description
Issue(s) addressed
Testing
How were these changes tested? GitHub CI
What compilers / HPCs was it tested with?
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
Have the ufs-weather-model regression test been run? On what platform?
Dependencies
N/A