Skip to content
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

Merge two workarounds for ccpp-prebuild for handling optional arguments #617

Merged

Conversation

climbfuji
Copy link
Collaborator

@climbfuji climbfuji commented Dec 9, 2024

This PR affects ccpp-prebuild only. It can be merged into develop (or main), but it must come to main as soon as possible for use in UFS/SCM.

This PR adds workarounds for handling optional arguments the right way (finally!) in scripts/ccpp_prebuild.py and scripts/mkcap.py. This update is already in use in NEPTUNE and is required for @dustinswales' work to update/revert the optional arguments in ccpp-physics in the UFS and the SCM.

The workaround for ccpp-prebuild allows us to treat only those arguments as optional that are truly optional for a CCPP scheme. In the past, any argument that was conditionally allocated by any of the host models had to be declared as optional, even if it was required by the physics.

User interface changes?: Yes and No. This can be merged without making any changes (it won't break the previous functionality where any conditionally allocated variable had to be declared as optional in the physics). But it will allow to declare many CCPP physics variables as non-optional if they aren't really optional.

This finally resolves #566 (by making ccpp-prebuild behave the same as capgen, which is the correct way to handle optional arguments).

Testing:
test removed: none
unit tests: all pass
system tests: all pass
manual testing: implemented and tested thoroughly in NEPTUNE

…y due to ccpp-physics not being updated at the same time as ccpp-framework (optional argument changes are missing)
@climbfuji climbfuji self-assigned this Dec 9, 2024
@climbfuji climbfuji added enhancement ccpp_prebuild bugs, requests, etc. that involve ccpp_prebuild labels Dec 9, 2024
@mkavulich
Copy link
Collaborator

@climbfuji Is the intention of merging directly to main that this branch should be tested by all parties prior to merge? If so I can run the SCM and UFS RTs (unless @dustinswales has already done so?) I don't know if @peverwhee's approval means that the tests have been run on the SIMA side, but since this is a prebuild-only change then those tests are theoretically unnecessary (though it doesn't give me a great taste in my mouth to skip testing).

Maybe this was discussed at Thursday's meeting, let me know if I'm out of the loop.

@climbfuji
Copy link
Collaborator Author

@climbfuji Is the intention of merging directly to main that this branch should be tested by all parties prior to merge? If so I can run the SCM and UFS RTs (unless @dustinswales has already done so?) I don't know if @peverwhee's approval means that the tests have been run on the SIMA side, but since this is a prebuild-only change then those tests are theoretically unnecessary (though it doesn't give me a great taste in my mouth to skip testing).

Maybe this was discussed at Thursday's meeting, let me know if I'm out of the loop.

Let's merge this into develop and then bring it to main when @dustinswales needs it, if that is ok with you.

Copy link
Collaborator

@mkavulich mkavulich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@climbfuji Sounds good, I'll add my approval for posterity and merge the PR.

@mkavulich mkavulich merged commit 0345a66 into NCAR:develop Dec 9, 2024
19 checks passed
@climbfuji climbfuji deleted the feature/prebuild_optarg_for_ncar_dev branch December 9, 2024 22:30
@climbfuji
Copy link
Collaborator Author

Thanks everyone for merging this so quickly! @dustinswales I'll leave it up to you to create a PR from develop to main when you need this particular update for prebuild for the UFS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ccpp_prebuild bugs, requests, etc. that involve ccpp_prebuild enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complete implementation of optional arguments / active variables in capgen
3 participants