Constituents #206
Constituents #206
Conversation
Moved setting of physics suite name to phys_readnl Change scripts and tests to python3 Update gen_registry and test files for metadata change from ddt_type to type Collect scheme names with call to ccpp_physics_suite_schemes Update CCPP framework external Make modifications needed for doctests to be properly caught by Github Actions. Add 'fail-fast' option to Actions workflow to prevent test skipping. Improve formatting of CAM config option description strings Fix documentation of physics-suites configuration option Fix name of Held-Suarez SDF name First pass of getting constituent info from physics suite Working on runtime constituents from physics Add missing DDT metadata Add advected property to registry, use for qv, remove other water tracers Progress on adding constituents, need to rethink how to check constituent status or value Changed is_read_from_file to subroutine Add missing metadata (spmd_utils) Mark host variables as initialized Use num_advected instead of pcnst Implemeneted const wet/dry and min val interfaces
Move composition-dependent variables to registry
…d additions to enable null dycore to run
nusbaume
added
enhancement
There was a problem hiding this comment.
Thanks for bringing this in! I do have a fair amount of change requests, but hopefully most of them are pretty small. I also held back on reviewing the SE dycore changes and the cam_constituents additions until we get them working and/or get additional feedback from Steve. Of course if you have any questions or concerns with my requests please let me know!
src/data/air_composition.F90
Outdated
| @@ -31,9 +32,7 @@ module air_composition | |||
| ! composition of air | |||
| ! | |||
| integer, parameter :: num_names_max = 30 | |||
There was a problem hiding this comment.
Is there a way that you don't need num_names_max and instead allocate const_is_water_species to num_advected? If not, then a check should be made somewhere that num_advected <= num_names_max
There was a problem hiding this comment.
const_is_water_species allocated to num_advected
There was a problem hiding this comment.
I believe you can now remove num_names_max
| call model_grid_init() | ||
|
|
||
| ! Initialize constituent data | ||
| call cam_ccpp_initialize_constituents(columns_on_task, pver, errflg, errmsg) |
There was a problem hiding this comment.
I've gone back and forth on this one, but thought I'd put it out there for discussion. I am uncomfortable about requiring host models to add a call for a subroutine which does not exist, but is auto-generated. Is there a way that this call could be hidden away in auto-generated CCPP code?
Up until this PR, we had the 5 phases as the only special code needed to interface with the CCPP. With this PR, we are adding "register", "initialize" and "number" for constituents. Each have their own API and there is no code to look at until the CCPP is run. Obviously this will need to be extensively documented, but I am wondering if the calls be hidden away somehow?
This code to interface with the CCPP is not adhering to the "just add the 5 phase calls and supply metadata for your scheme and it should work".
There was a problem hiding this comment.
Would making another routine (cam_initialize_constituents?) at the end of this module to "hide" this away be sufficient?
There was a problem hiding this comment.
I've gone back and forth on this one, but thought I'd put it out there for discussion. I am uncomfortable about requiring host models to add a call for a subroutine which does not exist, but is auto-generated. Is there a way that this call could be hidden away in auto-generated CCPP code?
This has been in the system for years. It was discussed and approved way back when the NRL folks met with the CCPP Framework committee and pointed out the lack of support for communicating with the dycore. It is not a new interface. It is not one of the 'required 5' phase calls (so it is technically optional) but models need it to function.
There was a problem hiding this comment.
@gold2718 - I don't remember this, but that doesn't mean anything. So you are saying the Dom and Grant approved this, correct, that there are three new optional interfaces to the CCPP?
Thinking about this further, I'd like to propose another possible solution. That would be instead to add an optional "6th" phase that would be analogous to our "register" phase in CAM. I do remember early on you and I discussed this phase and you said we'd implement that phase if needed. @peverwhee, @gold2718 and @nusbaume, would this be a way to hide these three object interfaces?
I'm concerned that if we end up adding additional CCPP-blessed objects in the future that the number of interfaces will proliferate. To me that makes the threshold of host models adopting CCPP higher (with every additional interface that needs to be potentially added).
There was a problem hiding this comment.
Thinking about this further, I'd like to propose another possible solution. That would be instead to add an optional "6th" phase that would be analogous to our "register" phase in CAM.
I think the problem with this approach to initializing advected consitituents is that any interface that hides what is going on has to be compatible with how all the models (e.g., CAM, UFS, NEPTUNE) bring up their dycore structures. Lumping everything together might look 'simpler' from a 'number of CCPP interfaces' perspective but it will lead to more complex implementation in the host models (including CAMDEN) which will then have to parse out the data returned by the new pre-packaged interface and add the data to their internal structures. We can consider this if you get buy-in from NOAA and NRL (more NRL since the UFS still has their advected constituents listed in a separate text file that has to magically work with the CCPP fields at runtime).
There was a problem hiding this comment.
opened #215 for continued discussion
There was a problem hiding this comment.
Intentionally not resolving this, so the discussion is readily viewable when #215 is worked upon.
… dict instead of wiki
There was a problem hiding this comment.
Given that some python code was modified, I figured I would review those changes so that @cacraigucar doesn't have to.
|
Ran the following tests today and they all work as expected.
These tests are in addition to the previously reported tests: pear_run ( My original pear : Adds a constituent (and probably shouldn't per Jesse's suggestion since it does not have pear initialized in the init phase so that there are valid values for the dycore) |
Overview
Incorporate constituents into CAMDEN.
The constituent ddt code is auto-generated by the framework. In the CAMDEN infrastructure code, the cam_constituents module keeps track of the constituent properties and indices for use by the host.
Key Changes
Externals Update: new framework tag (on my fork; will open PR to NCAR/ccpp-framework/feature/capgen after this PR)
cam_constituents.F90: constituents module; initialize constituent properties & provide get methods
cam_comp.F90: add register constituents
air_composition.F90: Move composition-dependent variables to the registry to avoid circular dependency; enable constituents
inputnames_to_stdnames.py: script to convert input names in snapshot file to appropriate standard names [will need to be used until we have a better system for input names in the framework (needs design)
write_init_files.py: Updates to handle constituents differently than other required variables
closes #197
closes #207
closes #211
addresses #168