-
Notifications
You must be signed in to change notification settings - Fork 64
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
ccpp_prebuild: ccpp stub and various bugfixes #436
ccpp_prebuild: ccpp stub and various bugfixes #436
Conversation
…ll and cmake include files for the static API
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.
One suggested and a couple of changes (unless I am misremembering that ccpp_error_flag
got changed to ccpp_error_code
).
…f required variables
…pp_framework_stub_merged_20220202
@grantfirl @gold2718 @mkavulich This is now ready for review. It's up to date with the latest changes in NCAR main, and all tests for this repository pass. The RT testing with the ufs-weather-model can only happen after @uturuncoglu updates his corresponding ufs-weather-model, CMEPS, fv3atm, ccpp-physics branches and points to the head of this branch. But these tests passed previously and no changes were made since then that would suggest problems. @mkavulich When this gets merged, the CCPP technical documentation needs to be updated to reflect the @ligiabernardet I suggest these changes to go in after the CCPP v6 release. |
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.
Looks good now.
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.
Still learning how the whole system works, but I don't see any problems, just a couple minor comments.
@climbfuji i am plaining to update my fork in a couple of days and make all the draft PRs active to start formal process of review of other linked PRs. I have already run all the regression tests with the current version and all of them was passing at that point but I'll test again after updating the UFS fork and also run ORT test with the newly added RT for exchange grid. More update will come soon. |
@uturuncoglu Thanks for letting me know. I will update my branch with the latest changes in the authoritative repo/branch and send you the new hash. |
@climbfuji i wonder if you find a change to update the branch? |
…pp_framework_stub_merged_20220202
@uturuncoglu Done - new hash is 5fe0dc7. |
@climbfuji thanks, that is great. |
@climbfuji the model is updated and I activate all the PRs for official review. All the RT tests are passing on Cheyenne. |
@climbfuji @grantfirl this PR is ready to merge. The top level UFS PR regression tests in different platforms passed without any issue. Both CCPP physics and framework PRs need to be merged before FV3 one. BTW, CMEPS PR is already merged. |
I am more than glad to see this getting merged, thanks for your hard work @uturuncoglu ! |
@uturuncoglu PR merged. New ccpp-framework hash for NCAR main is 167313e, please update your submodule pointer in fv3atm. |
@climbfuji thanks. I'll update the UFS model soon. Thanks for your great help. It was impossible without you. |
A stub ccpp-physics package is added in directory
stub
that currently works withccpp_prebuild.py
. See #435 for some background information.The stub is so basic and generic that it can be adapted for
capgen
if required, or deleted. The recipe forcapgen.py
's hello world example will make this an easy task.If desired, the necessary build files for
capgen
can be added in the future, although there will have to be some logic in thestub/CMakeLists.txt
file that switches betweenprebuild
andcapgen
. This might actually be useful as a cookbook recipe for transitioning fromprebuild
tocapgen
.There are no updates in this PR that require changes in
capgen.py
.The stub also helped to identify a few bugs and areas that needed cleanup in
ccpp_prebuild.py
, mostly because the stub didn't require some of the variables thatccpp_prebuild.py
'smkstatic.py
expects to be there.Note. Some of these bug fixes are required for ongoing work of adding the ccpp-framework and ccpp-physics to the CMEPS mediator (exchange grid). In particular the addition of a
--namespace
option to allow for multipleccpp_static_api_NAMESPACE.F90
in coupled model runs, where multiple components call into CCPP.User interface changes?: Yes - one item in the
ccpp_prebuild_config.py
file is renamed fromstatic_api_srcfile
tostatic_api_sourcefile
.Fixes #435
Fixes #394
Testing:
stub/README.md
), this also serves as a kind of test. No other tests added or removed.capgen
tests pass (cd test && ./run_test.sh
andcd test && ./run_doctest.sh
).ccpp_prebuild
tests pass (cd tests && PYTHONPATH=/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-ufuk-exchange-grid-20220416/FV3/ccpp/framework/scripts:/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-ufuk-exchange-grid-20220416/FV3/ccpp/framework/scripts/parse_tools:$PYTHONPATH python test_metadata_parser.py
andcd tests && PYTHONPATH=/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-ufuk-exchange-grid-20220416/FV3/ccpp/framework/scripts:/Users/dom.heinzeller/scratch/ufs-weather-model/ufs-weather-model-ufuk-exchange-grid-20220416/FV3/ccpp/framework/scripts/parse_tools:$PYTHONPATH python test_mkstatic.py
).stub/README.md
.