Updates for ZM round 4 #345
Conversation
There was a problem hiding this comment.
Thanks @cacraigucar! I had a couple questions on the physics_types naming and another regarding a subroutine call change, hopefully easy to address.
| <dimensions>horizontal_dimension</dimensions> | ||
| <ic_file_input_names>tpert pbuf_tpert</ic_file_input_names> | ||
| </variable> | ||
| <variable local_name="ql" |
There was a problem hiding this comment.
Could I ask why the local name in SIMA is set to ql instead of icwmrdp as in the snapshot? I was not totally sure that icwmrdp corresponds exactly to ql since there is a icwmrsh which is the shallow convection equivalent, but I couldn't find a clear definition of the ql in the current CAM's state to be sure.
There was a problem hiding this comment.
The simple answer is that that is what the initial developer (whoever that was) used it in their code. Inside zm_convr_intr.F90 which resides in ESCOMP/CAM, there is a pbuf call call pbuf_get_field(pbuf, icwmrdp_idx, ql ). That links the two variable names together.
| access="protected" > | ||
| <initial_value>1</initial_value> | ||
| </variable> | ||
| <variable local_name="dpdry" |
There was a problem hiding this comment.
Same question about the choice of the local name dpdry here (which usually implies contiguous, non-gathered dry delta-p layer thickness?) It appears the CCPP standard names spreadsheet calls this fake_dpdry. Since this is convective gathered columns maybe the naming could be clear that this is related to convection.
There was a problem hiding this comment.
Again, this name is from the original developer and comes from ESCOMP/CAM zm_conv_intr.F90. The dpdry name is used both as a value which has meaning and a fake_dpdry. The time that dpdry has a value, it gets it from: dpdry(i,:) = state%pdeldry(ideep(i),:)/100._r8
There was a problem hiding this comment.
Thanks @cacraigucar. I don't think a change would need to be made in ESCOMP/CAM nor in the scheme to rename these in SIMA registry.
The "link" of these variables to CAM snapshots is through <ic_file_input_names>. The link to the CCPP schemes themselves is through the standard name.
I think the local_name here is only used for when these variables are USEd through the physics_types object.
To confirm this, I renamed ql and dpdry in src/data/registry.xml to icwmrdp and dpdry_ideep and built CAM-SIMA with ZM, and was able to get the same results as before the renaming - so I believe changing the local_name here won't affect anything else.
I am concerned that names that are too specific (in the case of ql/icwmrdp here, it is specific to deep convection: _due_to_deep_convection; for dpdry, it is for gathered columns: _for_deep_convection_for_gathered_convective_columns) are being given generic names (ql and dpdry) that otherwise have meanings outside of deep convection. The naming here (in the registry) applies across CAM-SIMA and not just within the ZM scheme, so I'd like to advocate for not using generic names but populating them with scheme-specific data.
Sorry for insisting on this, I'm happy to discuss further!
There was a problem hiding this comment.
I lean towards @jimmielin 's approach here. These names are only used in the generated code in SIMA and the caps, so an update to atmospheric_physics wouldn't be necessary. And if there's a way to make them a little clearer so someone down the line doesn't think "hey, I need something called dpdry, i'll just use this standard name here in the registry", I think it'd be worth it.
There was a problem hiding this comment.
I don't believe we should change either of the names that @jimmielin has concerns about since changes would need to be made is ESCOMP/CAM and ESCOMP/atmospheric_physics, though I am open to being overruled.
| <dimensions>horizontal_dimension</dimensions> | ||
| <ic_file_input_names>tpert pbuf_tpert</ic_file_input_names> | ||
| </variable> | ||
| <variable local_name="ql" |
There was a problem hiding this comment.
The simple answer is that that is what the initial developer (whoever that was) used it in their code. Inside zm_convr_intr.F90 which resides in ESCOMP/CAM, there is a pbuf call call pbuf_get_field(pbuf, icwmrdp_idx, ql ). That links the two variable names together.
| access="protected" > | ||
| <initial_value>1</initial_value> | ||
| </variable> | ||
| <variable local_name="dpdry" |
There was a problem hiding this comment.
Again, this name is from the original developer and comes from ESCOMP/CAM zm_conv_intr.F90. The dpdry name is used both as a value which has meaning and a fake_dpdry. The time that dpdry has a value, it gets it from: dpdry(i,:) = state%pdeldry(ideep(i),:)/100._r8
Tag name (required for release branches):
Originator(s): cacraig, peverley, nusbaume
Description (include the issue title, and the keyword ['closes', 'fixes', 'resolves'] followed by the issue number):
Describe any changes made to build system: N/A
Describe any changes made to the namelist: N/A
List any changes to the defaults for the input datasets (e.g. boundary datasets): N/A
List all files eliminated and why: N/A
List all files added and what they do: N/A
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>)M cime_config/atm_in_paramgen.py
M src/data/physconst.meta
M src/data/ref_pres.meta
M src/data/registry.xml
M src/physics/ncar_ccpp
M src/physics/utils/phys_comp.F90
A src/physics/utils/ppgrid.F90
M src/utils/time_manager.F90
M test/unit/sample_files/write_init_files/physics_inputs_4D.F90
M test/unit/sample_files/write_init_files/physics_inputs_bvd.F90
M test/unit/sample_files/write_init_files/physics_inputs_cnst.F90
M test/unit/sample_files/write_init_files/physics_inputs_ddt.F90
M test/unit/sample_files/write_init_files/physics_inputs_ddt2.F90
M test/unit/sample_files/write_init_files/physics_inputs_ddt_array.F90
M test/unit/sample_files/write_init_files/physics_inputs_host_var.F90
M test/unit/sample_files/write_init_files/physics_inputs_mf.F90
M test/unit/sample_files/write_init_files/physics_inputs_no_horiz.F90
M test/unit/sample_files/write_init_files/physics_inputs_protect.F90
M test/unit/sample_files/write_init_files/physics_inputs_scalar.F90
M test/unit/sample_files/write_init_files/physics_inputs_simple.F90
If there are new failures (compared to the
test/existing-test-failures.txtfile),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:
derecho/gnu/aux_sima:
If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced:
CAM-SIMA date used for the baseline comparison tests if different than latest: