Add optional attribute to Capgen

[dustinswales]

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:

• Metadata parsing: Check for compatibility between scheme and host metadata. If host has inactive variable, ensure that scheme has variable declared as optional.
• Fortran parsing: Check for consistency between metadata and fortran file. If fortran file does not contain the optional attribute in its declaration, report error.
• For any optional scheme variable, add optional attribute to cap level declarations.
• Declare null pointer at cap level when host does not use optional scheme variable

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]

@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]

@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]

@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]

@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]

Snippets of auto-generated caps:

[climbfuji]

This looks great! Only a few minor comments

[climbfuji]

I have one doctest failure and a suggestion for a fix, other than that I am happy with the changes. (I ran doctests, all tests in test/ and also all tests in test_prebuild.

[climbfuji]

Thanks! doctests are now ok

[peverwhee]

Nice work, @dustinswales !

A couple comments/questions

[peverwhee]

thanks @dustinswales !

[climbfuji]

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

[climbfuji]

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]

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]

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.

[gold2718]

This is not as thorough as usual for me, mainly because I have not found the time to go through the suite_objects logic in detail. However, it seems okay at the level I understand and the generated code is okay (meaning you get what you were looking for) so 👍
I do have a couple of questions and some suggestions if there is time for those.

[gold2718]

It is pretty non standard to have non-optional arguments follow optional arguments in a Fortran subprogram. This is because "there must be no further positional arguments after the first keyword argument."
This 'works' in the CCPP because we generate caps that use keywords for every argument (thanks @davegill), however, I think it is still good practice to write portable code.

[dustinswales]

I agree this is ugly, and will fix, but I believe the standard in this case is referring to the argument list, not the declarations within the procedure.

[gold2718]

Right but for instance, errmsg and errflg are not optional but show up in the argument list after effrg_in, effri_out, and others.

[climbfuji]

This is something we've done consistently - always have errmsg and errflg at the very end. This is even more compelling if we think about the metadata order and the requirement (not sure if it's enforced in capgen, it isn't yet in prebuild) to have the same order as the argument list. Keeping those non-physics and purely framework related variables at the end would be nice.

Since nobody should be calling CCPP-compliant schemes directly - i.e. without ccpp-framework - we can rely on the arguments being listed as keywords?

[climbfuji]

Or, alternatively, move the framework-mandated variables to the very beginning? An awful lot of changes in the current schemes in the physics, but doable.

[gold2718]

Since nobody should be calling CCPP-compliant schemes directly - i.e. without ccpp-framework - we can rely on the arguments being listed as keywords?

At the very least, make this a bold part of the documentation about optional variables. Anyone creating a new scheme can put the variables in any order and anyone is free to use current schemes outside of the CCPP but we should at least document the consequences.

[climbfuji]

I created issue #544 for that.

[gold2718]

Can this be True here given the elif allocatable above? If it can, then what if is also optional?

[dustinswales]

I'm not sure of the purpose of alloval here?
If we are in this block of code, the variable is not an allocatable?

[climbfuji]

I also created #545 so that I can go ahead and merge this.