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

Capgen in SCM: Fix to allow for scheme subcycling. #633

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

dustinswales
Copy link
Collaborator

@dustinswales dustinswales commented Jan 29, 2025

Overview
This PR contains changes to fix scheme subcycing in Capgen and extends the var_compatability_test to exercise subcycling.
UPDATE: Added bugfix for suite-part list ordering.

Description
Create local group variable for subcycle indexing.
Fix bug (self.loop -> self._loop) in the Subcycle write phase, and in ccpp_datafile.

User interface changes?: No

Fixes: #632
Fixes: #634

Testing:
Added to var_compatibility_test to exercise feature.

This PR contains changes in #631

climbfuji and others added 11 commits December 9, 2024 14:25
…20241205

Update develop from main 2024/12/05
…ts (NCAR#617)

**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 NCAR#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
Added test using a DDT host object to pass information
Fix problems so that test passes
Improve formatting for readability

User interface changes?: No

Fixes: NCAR#589 

Testing:
  test removed: None
  unit tests: Pass
  system tests: Pass, added DDT host object test
  manual testing: Ran doctests, examined generated code for system tests

---------

Co-authored-by: Steve Goldhaber <[email protected]>
Co-authored-by: Dom Heinzeller <[email protected]>
PR to address issue with variable intents *across* groups.

**Description**
- Current behavior for variables across groups: the intent for the first
group is used as the "truth".
- If variable "foo" is intent "out" in Group A and intent "in" in Group
B, "foo" will be added to the variable dictionary used by the host as
intent "out", which then the host assumes means that the framework will
handle it
- Updated behavior for variables across groups: adjust the intent of the
existing variable to "inout" if a conflict arises across groups (or
suites)

User interface changes?: No

Testing:
  test removed: N/A
unit tests: Updated capgen & ddt test - variable intents across suites
also affected; PASS
  system tests: all PASS
  manual testing: Ran in CAM-SIMA
Fixes a bug where a ddt used in only one phase (thereby not promoted to
the group level) does not get the necessary "use" statement added at the
subroutine level in the suite cap.
@dustinswales dustinswales changed the title Fix to allow for scheme subcycling. Capgen in SCM: Fix to allow for scheme subcycling. Jan 29, 2025
scripts/metavar.py Outdated Show resolved Hide resolved
scripts/metavar.py Outdated Show resolved Hide resolved
if gen_unique:
new_lname = self.new_internal_variable_name(prefix=lname)
newvar = newvar.clone(new_lname)
# Local_name needs to be the local_name for the new
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am very confused by this comment. Should the first Local_name be lname?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All instances of lname are replaced, there is no logic to keep the first instance. So for three instances of the same lname, we have lname1, lname2, and lname3, not lname, lname1, lname2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, this is part of #631 and does not impact the sub-cycling isssue

test/var_compatibility_test/var_compatibility_suite.xml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suite-part list is wrong Sub-cycling not working properly
4 participants