NCAR / ccpp-framework

Common Community Physics Package (CCPP)
http://www.dtcenter.org/community-code/common-community-physics-package-ccpp/
Other
26 stars 62 forks source link

Add optional attribute to Capgen #529

Closed dustinswales closed 4 months ago

dustinswales commented 5 months ago

Description

This PR adds support to use the fortran optional attribute at the scheme level in Capgen. This functionality does not replace the active attribute, but rather builds on it.

Previously, conditionally allocated fields needed by a scheme were required to be accompanied with the allocation logic defined by the host (e.g the "active" condition). With this change, the scheme can internally query the "presence" instead of carrying the host model logic "down into the scheme".

The following modifications were made:

Fixes:

Addresses #527 (new test for optional variable) and partially #526 (support for optional attribute in Capgen)

Testing: test removed: unit tests: system tests: The var_compatibility_test was expanded, built on top of #525, to test this feature. manual testing:

dustinswales commented 5 months ago

@gold2718 @peverwhee @nusbaume I think that this does almost all that we want it to do. Feel free to take a gander. There's one piece that I'm not sure about? What needs to be done to add support to pass chunks of contiguous arrays? @climbfuji

climbfuji commented 5 months ago

@gold2718 @peverwhee @nusbaume I think that this does almost all that we want it to do. Feel free to take a gander. There's one piece that I'm not sure about? What needs to be done to add support to pass chunks of contiguous arrays? @climbfuji

In the caps, we need to declare a pointer for each variable with an active attribute that either points to a valid chunk of the contiguous array if the variable is active (based on the active condition), or is passed as a null pointer if the active-attribute condition is evaluated as .false.

dustinswales commented 5 months ago

@gold2718 @DomHeinzeller I'm close to having this working, just one smallish hold up.

Do either of you two know how to change the local_name of a dummy argument in a call_list? I need to go from, call sub(var=var_local) => call sub(var=var_local_ptr)

dustinswales commented 5 months ago

@DomHeinzeller @gold2718 @peverwhee @nusbaume This one is ready for review I still need to chase down a failing unit test, but the system tests are working (I probably changed a function call and broke an interface in the unit tests)

dustinswales commented 5 months ago

Snippets of auto-generated caps: Screenshot 2024-02-08 at 11 43 54 AM

Screenshot 2024-02-08 at 11 44 45 AM

climbfuji commented 4 months ago

@gold2718 Would you like to review the PR or should we go ahead and merge? Thanks!

climbfuji commented 4 months ago

We need to merge this so that I can bring feature/capgen down to main. That's needed, because ccpp_prebuild uses the capgen metadata parser and I need the updates for the mandatory optional attribute for schemes if the host model has an active attribute with a value other than the default .true.. Once I've got that update, I can make the optional var changes in prebuild similar to this PR for capgen, which will unblock the transition to contiguous arrays instead of blocked DDTs for the UFS.

gold2718 commented 4 months ago

We need to merge this so that I can bring feature/capgen down to main. That's needed, because ccpp_prebuild uses the capgen metadata parser and I need the updates for the mandatory optional attribute for schemes if the host model has an active attribute with a value other than the default .true.. Once I've got that update, I can make the optional var changes in prebuild similar to this PR for capgen, which will unblock the transition to contiguous arrays instead of blocked DDTs for the UFS.

Is there time to submit at least comments from partial reviews in progress?

climbfuji commented 4 months ago

We need to merge this so that I can bring feature/capgen down to main. That's needed, because ccpp_prebuild uses the capgen metadata parser and I need the updates for the mandatory optional attribute for schemes if the host model has an active attribute with a value other than the default .true.. Once I've got that update, I can make the optional var changes in prebuild similar to this PR for capgen, which will unblock the transition to contiguous arrays instead of blocked DDTs for the UFS.

Is there time to submit at least comments from partial reviews in progress?

Oh yes, for sure. Just wanted everyone to know why we need to get his in soon. Thanks for reviewing.

climbfuji commented 4 months ago

I also created https://github.com/NCAR/ccpp-framework/issues/545 so that I can go ahead and merge this.