Bring in new RRTMPG suite via atmos_phys and mods to get it to work

[peverwhee]

Tag name (required for release branches): TBD
Originator(s): peverwhee

Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):

• initialize needed host model variables
• modify runtime_obj to include name of dycore
• calculate coszrs to be used by radiation (and add namelist variables needed for this)
  • only calculate if we're not using snapshots
• update registry generation code to check initial values vs use statements
• add test

Describe any changes made to build system:

Describe any changes made to the namelist:

List any changes to the defaults for the input datasets (e.g. boundary datasets):

List all files eliminated and why:

List all files added and what they do:

A cime_config/testdefs/testmods_dirs/cam/outfrq_rrtmgp_derecho/shell_commands
A cime_config/testdefs/testmods_dirs/cam/outfrq_rrtmgp_derecho/user_nl_cam
A cime_config/testdefs/testmods_dirs/cam/outfrq_rrtmgp_derecho/user_nl_cpl

• rrtmgp test mods

List all existing files that have been modified, and describe the changes:
(Helpful git command: git diff --name-status development...<your_branch_name>)
M .gitmodules

• update framework tag and atmospheric physics hash

M ccpp_framework
M src/physics/ncar_ccpp

• update submodules

M cime_config/namelist_definition_cam.xml
M src/control/runtime_opts.F90
M src/physics/utils/radiation_namelist.F90
M src/physics/utils/radiation_namelist.meta

• add radiation NL

M cime_config/testdefs/testlist_cam.xml

• add rrtmgp cpu test

M src/control/cam_comp.F90
M src/control/cam_control_mod.F90

• initialize host-model variables needed by rrtmgp

M src/control/runtime_obj.F90
M src/control/runtime_obj.meta
M src/dynamics/mpas/dyn_comp_impl.F90
M src/dynamics/none/dyn_comp.F90
M src/dynamics/se/dyn_comp.F90

• Modify runtime obj to include dycore name; add unset definition & metadata

M src/data/generate_registry_data.py

• check initial value vs use statements

M src/data/registry.xml

• add necessary host model variables

M src/data/registry_v1_0.xsd

• allow capital letters in std name (constituent names)

M src/physics/utils/orbital_data.F90

• conditionally calculate coszrs for radiation

M test/unit/python/sample_files/*

• update tests

If there are new failures (compared to the test/existing-test-failures.txt file),
have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?

derecho/intel/aux_sima:
All tests NLFAIL due to new radiation namelist fields

derecho/gnu/aux_sima:
SMS_Ln2.ne3pg3_ne3pg3_mg37.FPHYStest.derecho_gnu.cam-outfrq_rrtmgp_derecho (Overall: DIFF)

• new test/baseline

All other tests NLFAIL due to new radiation namelist fields

If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced: n/a

CAM-SIMA date used for the baseline comparison tests if different than latest:

[nusbaume]

Looks good, but did have some (hopefully minor) change requests.

[nusbaume]

Not critical for this PR, but should we have a variable labeled ccpp_constant_X if it's not actually provided by the framework itself? If not then maybe it's worth "fixing" this standard name in the future GPU PR?

[peverwhee]

good point - maybe constant_dimension_two ?

[nusbaume]

Sure, that name works for me!

[peverwhee]

implemented in 9797761

[nusbaume]

Nit-pick, but can we make this subroutine a part of the runtime_options DDT, just like get_dycore? I feel like that better matches the OOP design of having both a "getter" and a "setter" within the object itself.

[peverwhee]

I waffled on this!

I had it in there at first, but then I'd have to change the intents in the dynamics init routines to be inout to call the procedures, which meant I'd have to remove protected from cam_runtime_opts which did not seem ideal. But maybe that's fine?

[nusbaume]

That's fair, but I personally think changing the intent to inout and removing the protected attribute would be better than what this is doing now, where we claim the variable should be read-only, but then are actually changing its internal values by calling the external set_cam_dycore subroutine.

So I think I am still fine with implementing this requested change, especially since the actual data inside the DDT can still be "private", and hopefully we can keep this DDT internal to CAM-SIMA itself as much as possible (even with the associated metadata file entry, which we could possibly remove).

[peverwhee]

changed!

[jimmielin]

Thanks @peverwhee! Looks good to me I just had a couple of comments, one for a potential minor comment change and another just for a brief discussion... nothing that holds up merging in the PR.

[jimmielin]

I'm OK with this for now since this is only used in orbital_data, but I have some concerns about this versus just bringing in dycore_is as used in CAM which has 'more functionality'.

Hopefully this property won't be used in physics in the future - if so the 'null' dycore should instead report whatever dycore the snapshot came from (similar to what I made the dycore energy formula code do later in this file).

But maybe whatever physics code that needs to know the dycore name should probably not do it and use something more physical instead. Just wanted to hear your and @nusbaume's thoughts on this!

[peverwhee]

I might be misunderstanding, but having the null dycore report the snapshot dycore wouldn't help my particular issue of not wanting to calculate solar zenith angle if we're reading it from the snapshot.

But maybe there's a way around that that I'm missing.

As for why we did this instead of bringing in dycore_is, I do not recall!

[jimmielin]

Thanks @peverwhee! Yup I understand why it has to report null for orbital_data. I am just slightly worried that once this is available we have to be careful about policing future attempts to use it in the physics code.

[peverwhee]

ah ok. Yes, that is a valid worry.

[nusbaume]

Just wanted to say that while this object and its methods are certainly useful for internal CAM-SIMA functionality, I too would like to see its use avoided in physics schemes. I am actually wondering if we should just remove the metadata entry for the DDT entirely, which would then force us to avoid its use in the physics, as I am not sure it is actually being used now. Thoughts?

[jimmielin]

Sounds good! I agreed not having the ddt in metadata should help with discouraging use of it in physics.

[peverwhee]

removed the metadata entry for the object!

[nusbaume]

Approving this under the assumption that the last two requests I have (excluding the units one which will be fixed in the GPU PR) will be implemented. Thanks again for getting this done @peverwhee!