Add CAM diagnostic schemes #105
Conversation
|
Partial approval: I implemented two ZM diagnostic fields using the diagnostic template and was able to compile the new code. Unable to run as ZM is not running in CAM-SIMA yet. Will review the Fortran code mods after @nusbaume completes his initial review |
cam_diagnostics/cam_diagnostics.F90
Outdated
| character(len=65) :: const_std_names(4) = & | ||
| (/'water_vapor_mixing_ratio_wrt_moist_air_and_condensed_water ', & | ||
| 'cloud_liquid_water_mixing_ratio_wrt_moist_air_and_condensed_water', & | ||
| 'rain_mixing_ratio_wrt_moist_air_and_condensed_water ', & | ||
| 'cloud_ice_mixing_ratio_wrt_moist_air_and_condensed_water '/) | ||
|
|
||
| character(len=6) :: const_diag_names(4) = (/'Q ', & | ||
| 'CLDLIQ', & | ||
| 'CLDICE', & | ||
| 'RAINQM'/) |
There was a problem hiding this comment.
Shouldn't the contents of these two variables be dictated by whatever is populating the constituent object and loop over what is in that object?
There was a problem hiding this comment.
we're looping over the constituents object, yes. But this is a mapping of the standard name to the diagnostic name (which is not contained within the object). Not all of these will necessarily be "used". Does that make any sense?
There was a problem hiding this comment.
This still feels "fragile" to me, as the diagnostic name index may not match the constituent index. Right now in ZM, I am assuming that the tendency array maps one-to-one to the constituent array. I would naively think that the diagnostic array would also use the same indexing. Let me know if we should have a discussion on this.
There was a problem hiding this comment.
Hi @cacraigucar I'm not quite following. Is the problem that someone could modify one of the lists (const_std_names or const_diag_names) and then they'd be off? To clarify, when I'm looping over the constituents, I don't expect them to be in this exact order. This is just a mapping between standard name and diagnostic name (which is not contained in the properties object). Let me know if I'm still missing something!
There was a problem hiding this comment.
Hi @peverwhee, @cacraigucar. If I understand this fragility correctly it might be possible that user error could cause the lists to become out-of-sync. I wonder of ways to mitigate this, e.g., checking the lists are of same length at initialization (to prevent the obvious typo of adding one but not the other), but all bets are off if an entry is added to the wrong place in one of the lists.
My immediate thought was to assign these into a Nx2 array instead of two N-sized arrays but a quick Google didn't reveal if Fortran had syntax like this, which would prevent typing errors as much as possible:
const_std_and_diag_names(4,2) = (/ /'water_vapor_mixing_ratio..', 'Q'/, ...
/'cloud_liquid_water...', 'CLDLIQ'/, ...
)
There was a problem hiding this comment.
When I do this, I always use a parameter for each dimension size.
The only example of this I found in CAM code is from the very ancient (well before my time) src/physics/camrt/radae.F90
Newer code, such as src/dynamics/se/dycore/fvm_consistent_se_cslam.F90 does have a parameter for the changeable dimension which I think addresses your main safety concern.
There was a problem hiding this comment.
Thanks! I like this in fvm_consistent_se_cslam.F90, which I guess is what you're referring to: it's as close to safe and 'automatic' as we can get before the clean-up issue #137 is addressed:
integer, parameter :: num_area=5
integer, dimension(num_area*4), parameter :: idy_shift_tmp = (/-1, 0, 0, 0,-1,& !iside=1
-1,-1, 0, 1, 1,& !iside=2
1, 0, 0, 0, 1,& !iside=3
1, 1, 0,-1,-1/) !iside=4
integer, dimension(num_area,4), parameter :: idy_shift = RESHAPE(idy_shift_tmp,(/num_area,4/))There was a problem hiding this comment.
While this conversation has been good, my concern was not actually about there being two arrays, but rather about the necessity of having them at all. I had a google chat with Courtney which resulted in #137 and will eventually eliminate the need for these arrays.
There was a problem hiding this comment.
Thanks @cacraigucar, my apologies for misunderstanding the original issue. Until #137 becomes a more permanent fix the current code looks good!
There was a problem hiding this comment.
No apology needed. Nobody understood what my concern was until I met with Courtney and explained it in person!
cam_diagnostics/cam_diagnostics.F90
Outdated
| character(len=65) :: const_std_names(4) = & | ||
| (/'water_vapor_mixing_ratio_wrt_moist_air_and_condensed_water ', & | ||
| 'cloud_liquid_water_mixing_ratio_wrt_moist_air_and_condensed_water', & | ||
| 'rain_mixing_ratio_wrt_moist_air_and_condensed_water ', & | ||
| 'cloud_ice_mixing_ratio_wrt_moist_air_and_condensed_water '/) | ||
|
|
||
| character(len=6) :: const_diag_names(4) = (/'Q ', & | ||
| 'CLDLIQ', & | ||
| 'CLDICE', & | ||
| 'RAINQM'/) |
There was a problem hiding this comment.
This still feels "fragile" to me, as the diagnostic name index may not match the constituent index. Right now in ZM, I am assuming that the tendency array maps one-to-one to the constituent array. I would naively think that the diagnostic array would also use the same indexing. Let me know if we should have a discussion on this.
Description
Introduces the following schemes:
Updates the existing SDFs to include cam_state_ and cam_tend_diagnostic schemes (tendencies at the end of physics_after_coupler and state at the end of physics_before_coupler to match CAM simple physics physpkg.F90)
Testing