cam6_3_012: Add Horizontal Momentum Tendency Budget Variables#232
cam6_3_012: Add Horizontal Momentum Tendency Budget Variables#232cacraigucar merged 24 commits intoESCOMP:cam_developmentfrom
Conversation
…[UV]TEND_* variables can be output via fincl? or by setting history_budget = .true. No existing variables have been changed.
cacraigucar
left a comment
cacraigucar
left a comment
There was a problem hiding this comment.
Once you make the requested changes and push them back to your fork, please ask for a rereview from me (hit the two circular arrows beside my name in the reviewer section of the PR). At that point, I will ask for review from the other CAM SE's.
cacraigucar
changed the title
src/physics/cam/check_energy.F90
Outdated
| call addfld('DTCORE', (/ 'lev' /), 'A', 'K/s' , 'T tendency due to dynamical core') | ||
| call addfld('UTEND_CORE', (/ 'lev' /), 'A', 'm/s2' , 'U tendency due to dynamical core') | ||
| call addfld('VTEND_CORE', (/ 'lev' /), 'A', 'm/s2' , 'V tendency due to dynamical core') | ||
| call register_vector_field('UTEND_CORE','VTEND_CORE') |
There was a problem hiding this comment.
Inside the check_energy module seems like an odd place for these addfld calls for dycore tendencies diagnostics. Should those be with the other added addfld calls in cam_diagnostics or in phys_init (in the some module source file as the corresponding outfld calls)?
There was a problem hiding this comment.
I agree, and would probably vote to move them to cam_diagnostics, mostly because I think physpkg has enough going on, and I don't want to start adding extra addfld calls to it if we can avoid it.
However, would this move include DTCORE, which I suspect the added tendencies were modeled after? Does anyone know if there is a reason for DTCORE to be located in check_energy?
There was a problem hiding this comment.
Our usual standard is that the addfld call should be in the same module as the corresponding outfld call. Otherwise, it risks being removed in a cleanup pass.
There was a problem hiding this comment.
@gold2718 that's a good point! However, in that case I might vote to move most of the addfld and outfld calls to their respective physics parameterization interfaces. For example, I would move UTEND_RAYLEIGH inside rayleigh_friction_tend, instead of having it exist in tphysac.
Obviously some of the quantities, like UTEND_TOT, may not have a clear location, in which case I am happy to put them in either cam_diagnostics or somewhere in physpkg (whatever we decide).
There was a problem hiding this comment.
@siouxyalie I think having the tendency named after the process instead of the particular parameterization makes complete sense, at least for this particular diagnostic.
However, you can still go one-level down, at least for many of these processes, as subroutines like convect_shallow_tend and vertical_diffusion_tend are designed to manage multiple different parameterizations (e.g. between CAM4 and CAM5 physics) in the same location, so the diagnostic can still be independent of the exact parameterization chosen. I agree that the same should be done for the temperature and humidity tendency diagnostics as well, although we don't have to do that for this PR unless you are really feeling up to it (as we can always just make an issue instead and save it for later).
For ?TEND_MACROP, the argument could be made that labeling CLUBB just "MACROP" is not really accurate, as CLUBB also includes shallow convection and vertical diffusion. So if someone was comparing CAM4 to CAM6 and wasn't knowledgeable it would look as if CAM6 had no shallow convection but a huge cloud macrophysics tendency, when in reality this isn't actually the case.
So, I might argue that ?TEND_MACROP should only represent the tendencies from macrop_driver_tend, and just use TEND_CLUBB for CLUBB (as that would remove the redundancy). I also believe you could pass in the macmic sub-stepping info into macrop_driver_tend similar to what is done for clubb_tend_cam and calculate the (properly scaled) tendencies there, again like what is done for ?TEND_CLUBB. However, I don't know this section of the code that well, so it is entirely possible that I am missing something (maybe the other SEs can chime in if they think that is wise or not)?
Finally, if I am the only one pushing for this larger change and the other SEs don't think it is necessary then I am happy to concede, especially if it holds up this PR for too long.
There was a problem hiding this comment.
I believe very strongly that the budget terms should all be consistently written out in the tphysac/tphysbc routines.
One constraint of these budget terms is that adding up all the individual budget terms should yield the total budget term. After much discussion between @siouxyalie and I, we determined that this is up to the user to verify - setting up a test was becoming very cumbersome. He also thought it was standard practice for users to do this verification.
With that in mind, if a new parameterization is added and the individual budget terms for that parameterization are not output the total budget will not match. At that point unless the outputting of the budget terms are in a single routine, the user will have a more difficult task of trying to identify the parameterization which is missing the term. Now it is a simple matter of scanning through the calls and seeing which is missing the outfld calls. Also, as people love to "find examples", this approach will prompt them to create budget terms for the new parameterization.
With that in mind, if the outfld calls are in tphysac/tphysbc, @gold2718 where would you like the addfld calls to reside? The outfld calls I see at this level have their addfld calls in check_energy (as @siouxyalie) pointed out.
There was a problem hiding this comment.
As I said above, if the outfld calls are to stay in physpkg.F90 (a temporary solution as it will obviously not translate to the new infrastructure), then the addfld calls should also be there. Is there a problem with that?
There was a problem hiding this comment.
No problem other than it shows my indecisiveness of which rule we were going to break - the addfld being in the same module or keeping the interfaces in physpkg "clean". After thinking about it more, I agree with your confirmation of the proper place for the addfld calls. My alternate approach doesn't look so good in the light of Monday morning.
@siouxyalie - this means that you will pull all of your addfld calls for your new outfld calls out of the places where you put them and put them all in the phys_init routine inside physpkg.F90. I would suggest you make a section at the very end of the routine with them. You will also need to add at the top of phys_init a "use cam_history, only: addfld".
There was a problem hiding this comment.
Thanks for this guidance. On it.
gold2718
left a comment
gold2718
left a comment
There was a problem hiding this comment.
Not a complete review but a couple of items that need to be addressed.
src/physics/cam/check_energy.F90
Outdated
| call addfld('DTCORE', (/ 'lev' /), 'A', 'K/s' , 'T tendency due to dynamical core') | ||
| call addfld('UTEND_CORE', (/ 'lev' /), 'A', 'm/s2' , 'U tendency due to dynamical core') | ||
| call addfld('VTEND_CORE', (/ 'lev' /), 'A', 'm/s2' , 'V tendency due to dynamical core') | ||
| call register_vector_field('UTEND_CORE','VTEND_CORE') |
There was a problem hiding this comment.
As I said above, if the outfld calls are to stay in physpkg.F90 (a temporary solution as it will obviously not translate to the new infrastructure), then the addfld calls should also be there. Is there a problem with that?
…UCORE, and DVCORE.
…iagnostics into phys_init, as their outfld calls are in tphysac, tphysbc
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Mostly looks good to me, but I do have some (hopefully minor) requests.
parameterization modifies only one wind component, the tendency is still output.
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Looks great now, thanks! Just a couple last comments which appear to have not been updated.
nusbaume
left a comment
nusbaume
left a comment
There was a problem hiding this comment.
Thanks, everything looks good to me now!
cacraigucar
changed the title
gold2718
left a comment
gold2718
left a comment
There was a problem hiding this comment.
Looks good now, thanks!
cacraigucar
changed the title
Refactor QBO namelist settings; fix -chem none issue; remove reprosum settings from FSD test
cacraigucar
changed the title
CAM's bld/configure routine needs to be updated for MPAS to allow it to build under CAM or CESM checkouts. The namelist defaults also need to be updated to get a MPAS CESM regression test to pass. Fixes ESCOMP#337
…l_noresm3_beta01 Summary: Control all output from CAM-Oslo Contributors: Johannes Fjeldså Reviewers: Steve Goldhaber Purpose of changes: Contain output from CAM-Oslo that is always outputted to be namelist options. Changes in CAM will allow for changes in OsloAero to take place. Github PR URL: NorESMhub#232 Changes made to build system: 'None' Changes made to the namelist: build-namelist, namelist_definitions and namelist_defaults_cam has six new namelist flags. Changes to the defaults for the boundary datasets: 'None' Substantial timing or memory changes: 'None' Introduce six new history flags to contain output that is always outputted from OsloAero. Similar work as done for noresm2.3 (see NorESMhub#198 (comment)). In addition these flags will govern summation fields aggregated by OsloAero as opposed to performing this aggregation in diagnostics or post processing. Should remain a draft PR until NorESMhub/OSLO_AERO#60 is merged. fixes: NorESMhub#195 Testing: aux_cam_noresm Expected fails due to: NorESMhub#180 NorESMhub/NorESM#695
5 participants
Mods allow outputing UTEND_TOT, UTEND_CORE, UTEND_PHYSTOT, UTEND_MACROP, UTEND_VDIFF, UTEND_DCONV, UTEND_SHCONV, UTEND_GWDTOT, UTEND_NDG, UTEND_RAYLEIGH, UTEND_QBORLX, UTEND_LUNART, and UTEND_IONDRG. Same for V as well.
Features:
Some of the tendency variables above may be zero, depending on the CAM configuration. However, setting history_budget = .true. will output all of them so that the sum of all the UTEND_* variables (except for UTEND_PHYSTOT and UTEND_TOT) is equal UTEND_TOT regardless of configuration. That said, I have only tested this in SCAM and with in nudged runs with the FV dycore.
As mentioned, by default, these variables will not be output. However, with history_budget = .true., a closed U and V tendency budget will be output in addition to the existing, untouched T and Q budgets.
Caveats:
Some of these tendencies can already be output (e.g. UTGW_TOTAL = UTEND_GWDTOT, UTEND_CLUBB = UTEND_MACROP for default CAM6, ZMMTU = UTEND_DCONV). I have not altered existing variables, but rather added variables with a consistent naming convention. Some of the added variables will be redundant.
Some parameterizations currently do not influence horizontal momentum, but may in the future (e.g. microphysics, dry adjustment). Additional variables would need to added should such development ever occur.
Important: time integrating UTEND_TOT will not give the instantaneous U. However, time integrating UTEND_TOT will give instantaneous UAP (U after physics). This is because U is output in the middle of physics (after deep convection but before radiation). UAP is essentially U at the end of the time step loop (at least for the FV core...).
Default averaging flag for UAP, VAP (U,V after physics) is "A". I think it makes more sense for these variables to be output instantaneously. This way, time integrating the time averaged UTEND_TOT will give UAP. However, it looks like the the averaging flag set by avgflat_pertape overrides the default set by addfld, making the potential change from "A" to "I" in UAP, VAP addfld calls irrelevant. So, solely setting history_budget = .true. will output a closed budget, but time integrating this budget will not give U,V or UAP,VAP. This is potentially very confusing. I did not want to alter any existing code, nor did I know how to somehow make it so UAP, VAP are only ever output instantaneously, so I'll leave it up to the real developers to decide.
resolves #226