Transition CCPP framework from ccpp-prebuild to ccpp-capgen v1 - #695
Transition CCPP framework from ccpp-prebuild to ccpp-capgen v1#695climbfuji wants to merge 36 commits into
Conversation
…dim-names to capgen call
There was a problem hiding this comment.
Ignore this, for my development only
Changes to get the SCM tests to run: * clm_lake was included in a SDF, but not used. In the Cap, there is a (new?) check to see if the active conditions are met before calling the scheme. This was causing a runtime error. CLM Lake returns if not turned on, so having this new runtime check beforehand triggered the failure before entering the scheme. I removed the unused CLM Lake from the SDF and it worked fine. * Remove GFS_v16_debug SDFs from test list. (I see these were removed from the CCPP SCM?) * Add snippet to ccpp/CMakeLists to build only requested SDFs (i.e., -DCCPP_SUITES). * Uncomment and test 32-bit and Intel oneAPI in CMakeLists.
…; fix metadata-fortran inconsistencies, fix GitHub actions take 1
|
@climbfuji Looks like most of the tests are B4B, the only exception are the GNU release (optimized) tests. Could this be from compilation flag changes (e.g., Adding -fPIC)? The differences appear to be rounding errors for the GFS based tests, whereas the HRRR_gf and WoFs tests show non-negligible differences. For example: Full set of difference plots are stored as artifacts here |
Thanks for doing this comparison. The NEPTUNE runs are also not b4b identical. I think this is predominantly because the caps look different and the compiler optimizes the code differently. But for the two suites you mentioned, what do they have in common that the other suites don't have? I am certain that the much stricter validator and code generator helped identify and fix a few actual bugs. |
|
@dustinswales @climbfuji GNU release RTs for the SCM have been changing results basically every PR. It seems like something is just not reproducible or otherwise fishy with the environment. I wouldn't base any decisions on whether GNU release tests are b4b or not. |
@grantfirl Thanks. I thought it was an odd result. In any event... I've seen enough to warrant trying capgen-ng in the UFS. Any objections @climbfuji? |
|
No objections at all, please go ahead. I can also help out if you share a working branch and/or a checkout on Ursa, for example. I expect we'll have to fiddle a bit with the physics called from the dycore ... |
|
@dustinswales @grantfirl Looks like we'll have a clean pass through all CI tests in a few minutes. |
|
@dustinswales @grantfirl I think this is good now. We've got the tendencies and C3 updates in this branch (updated today from main), and the tests all pass. |
@climbfuji Thanks for these bug fixes. |
climbfuji
changed the title
50fa6f8 to
fa33ff6
Compare
There was a problem hiding this comment.
As part of my review I submitted the PR climbfuji#6 . It has the following changes that I noticed while reviewing and testing this PR. If there are any issues just let me know, thanks for this work!
DESCRIPTION OF REVIEW PR CHANGES:
- CMakeLists.txt
- End of line characters were
^M, indicating mixed Windows and Linux carriage returns, randos2unixon the file - another commit reduces Intel compile time optimization from default
-O3to-O2and removes-qoverride-limits. These two changes reduce ifx-j4compile time from 93 minutes to 2 min 8 sec.
- End of line characters were
- suites that were removed also needed to be removed from
suite_info.py - changed macros in
scm.F90andscm_time_integration.F90to parameters. I though the macro additions came from a coding agent but if there is a reason to keep the macros I can remove them!
Thanks for these updates. No coding agent ever touched the SCM code for this PR. I personally prefer macros over defining variables that are never used, but it's a personal preference and I don't mind parameters either. |
Ok, good to know! The coding agent I've been using had started using them more, so I guessed incorrectly. I'll change the PR back to the original macros, I can see the logic in using them for variables that aren't used |
* Removing Intel -qoverride-limits flag and setting release build to use -O2 optimization, instead of the default -O3, decrease the -j4 compile time from 94 minutes to 2 * ran dos2unix on this file to remove ^M end-of-line characters * Suite info cleanup, suite has been removed
scrasmussen
left a comment
scrasmussen
left a comment
There was a problem hiding this comment.
Everything building and looking good to me!
4 participants
Description
This PR updates the CCPP framework code generator from ccpp-prebuild to ccpp-capgen v1. The large update of the ccpp-framework code also requires changes in ccpp-physics and ccpp-scm.
The most important changes in CCPP-SCM are:
scm/binviacmake ../...type = controlmetadata tables.ccpp_*in the SCM host model code).Dependencies
Testing