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

Introduce optional arguments in ccpp schemes and metadata #189

Merged

Conversation

climbfuji
Copy link

@climbfuji climbfuji commented Mar 22, 2024

Description

This PR makes necessary changes for supporting optional arguments in CCPP (see ufs-community/ufs-weather-model#2205 and PRs listed there). This involves adding the optional attribute tot he CCPP metadata for all variables that have matching host model variable with an active attribute other than the default value .true. (i.e. it may not be allocated). In addition, each of these variables need to be declared as optional variable in the Fortran code.

90% of all the changes in this PR were contributed by @dustinswales - kudos to him. The new and clean way to deal with optional arguments also allowed us to discover a few bugs/inconsistencies that we had to fix to get b4b identical results (and successful runs in the first place) with Intel and GNU on Hera.

Two source files need to be compiled with slightly lower optimization (-O1 instead of -O2 with Intel in release mode) to work around what seems to be bugs in the Intel compiler: gcycle.F90 and mynnedmf_wrapper.F90).
This should have negligible effect on the runtime, and it does not affect the results (see below).

This is part of a large set of PRs:

NCAR/ccpp-framework#552
NOAA-EMC/fv3atm#807
https://github.com/ufs-community/ufs-weather-model/pull/2205
#189
NCAR/ccpp-framework#556 (can be scheduled and merged anytime beforehand)
NOAA-GFDL/GFDL_atmos_cubed_sphere#338
NOAA-PSL/stochastic_physics#79

Issue(s) addressed

Working towards NCAR/ccpp-framework#540

Testing

ufs-weather-model full regression testing on Hera (all tests b4b)

@climbfuji climbfuji force-pushed the feature/ccpp_prebuild_opt_args branch from 5bac829 to d1e97a1 Compare April 14, 2024 02:40
climbfuji and others added 24 commits April 13, 2024 20:53
…ariables but internally bend everything back to where it's expected (affects some, but not all variables)
…oud_mp.F90 and physics/Interstitials/UFS_SCM_NEPTUNE/GFS_rrtmgp_cloud_overlap.F90
Add optional attribute to Fortran files.
@@ -68,12 +68,12 @@ subroutine mp_nssl_init(ncol, nlev, errflg, errmsg, threads, restart, &
real(kind_phys), intent(inout) :: qi (:,:) !(1:ncol,1:nlev)
real(kind_phys), intent(inout) :: qs (:,:) !(1:ncol,1:nlev)
real(kind_phys), intent(inout) :: qh (:,:) !(1:ncol,1:nlev) graupel
real(kind_phys), intent(inout) :: ccw(:,:) !(1:ncol,1:nlev)
real(kind_phys), intent(inout), optional :: ccw(:,:) !(1:ncol,1:nlev)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note: ccw should always be active (i.e., not optional)

@@ -320,6 +320,7 @@
type = real
kind = kind_phys
intent = inout
optional = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

@climbfuji @dustinswales The majority of variables that you add optional to, are not optional. For example smois - it is a state soil moisture variable.

@dustinswales
Copy link
Collaborator

@tanyasmirnova @MicroTed
Any host field that was conditionally allocated, for example smois, now has the optional attribute defined in the physics.
This does not mean that these fields should be optional, but rather a relic of whatever logic was controlling these fields on the host side.

Copy link
Collaborator

@AnningCheng-NOAA AnningCheng-NOAA left a comment

Choose a reason for hiding this comment

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

look fine with me.

@MicroTed
Copy link
Collaborator

@dustinswales

@tanyasmirnova @MicroTed Any host field that was conditionally allocated ... This does not mean that these fields should be optional, but rather a relic of whatever logic was controlling these fields on the host side.

I'm not sure I understand why the CCPP interface should care about the host model this way? What if different host models have different logic? I do like that the optional attribute is back after being removed.

@FernandoAndrade-NOAA
Copy link

Testing for #2205 has completed successfully, please continue with merging this PR.

@grantfirl grantfirl merged commit 16a1d88 into ufs-community:ufs/dev May 21, 2024
3 checks passed
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.