Initial addition for energy fixer for the MPAS dycore#23
Conversation
…or MPAS for adiabatic setup (so it follows the approach for other dycores)
Merge Lauritzen's mpas_fixes_phl branch with Truesdale's maps_fixes branch
…or MPAS for adiabatic setup (so it follows the approach for other dycores)
…en/CAM into mpas_fixes_phl
…90, regression tested for QPC6
…mpas_fixes_phl
…umbers added to similar error messages to aid debug, ChangeLog updates, remove unused variable and code in geopotential.F90, Make MPAS external non-optional again
cam6_3_020: MPAS bug fixes, performance enhancements, allow initial files writes, refactor/cleanup PR mods: . Update CIME external to include NUOPC compliance for MPAS . Update MPAS external: NUOPC compliance, timer output, new sponge layer diffusion . Remove MPAS Streams generation (unused) . dyn_in and dyn_out refactored to point to the same structure. . Fixes to allow NAG compilation . Fix to allow running on 1 pe. . Refactor for weak scaling. . bug fixed in MPAS sub stepping. . fixed a small discrepancy in the diagnosed pressure of one of the simple model cases. . Update default MPASA grids, initial files, and boundary data for newly supported resolutions . New defaults for MPAS dycore timestep. Changed the 3rd-order scaler transport upwind coefficient from 600s to 900s. . Add functionality to output initial condition files. . Add new sponge layer diffusion options for model stability 1) Rayleigh 2) diffusion similar to SE Fixes ESCOMP#385
| compute_energy_diags=& | ||
| (hist_fld_active('SE_dDP').or.hist_fld_active('SE_dDM').or.hist_fld_active('SE_dPD').or.& | ||
| hist_fld_active('KE_dDP').or.hist_fld_active('KE_dDM').or.hist_fld_active('KE_dPD').or.& | ||
| hist_fld_active('WV_dDP').or.hist_fld_active('WV_dDM').or.hist_fld_active('WV_dPD')) |
There was a problem hiding this comment.
I see that this is setting a flag compute_energy_diags to turn on the energy fixer down below, but I'm unfamiliar with the fields listed, SE_dDp, KE_dDP etc. Can you explain, and perhaps in a comment above this statement, on why we turn on the energy fixer with these variables?
If anything, a comment would help other non-scientists understand when we need to turn on the energy fixer.
There was a problem hiding this comment.
They are explained in dyn_comp.F90. Should I repeat description here or we could set the logical in dyn_comp.F90?
There was a problem hiding this comment.
Also: these are energy diagnostics and not the energy fixer. The energy fixer is in physics.
There was a problem hiding this comment.
Also: these are energy diagnostics and not the energy fixer. The energy fixer is in physics.
Ah yes, thanks for the clarifications!
They are explained in dyn_comp.F90. Should I repeat description here or we could set the logical in dyn_comp.F90?
Hmm yes, I see that now. I think it might make most sense to set the logical in dyn_comp.F90. However I'm not sure if dyn_init in dyn_comp.F90 is the best to define these diagnostics fields. @jtruesdal do you know if these diagnostics should be defined in dyn_init? It looks like there's some fields that are defined in stepon.F90, but I'm not familiar with the purpose of that module.
There was a problem hiding this comment.
Most of the history fields specific to a parameterization are defined in the initialization routine. It makes sense to me that would be dyn_init for the dycores. We should probably move the addfld calls taking place in stepon_init to dyn_init.
There was a problem hiding this comment.
@jtruesdal moving the addfld calls into dyn_init sounds good to me, if you think it is good. I'm not familar with the stepon module. Do you know what its purpose is?
@PeterHjortLauritzen after some thought, I think setting compute_energy_diags in dyn_init makes the most sense. As, I'm assuming, that compute_energy_diags wont change throughout a run. i.e. the energy fixer will either be on or off. Is that assumption correct?
Can you also remind me in what cases the energy fixer will be on/off? Or will it always be on?
There was a problem hiding this comment.
Will move to dyn_init
There was a problem hiding this comment.
This is a logical for diagnostics and it will not impact energy fixer in physics.
| real(r8), dimension(qsize,nVertLevels, nCells), intent(in) :: q ! tracer array | ||
| real(r8), dimension(nVertLevels, nCells), intent(in) :: ux ! A-grid zonal velocity component | ||
| real(r8), dimension(nVertLevels, nCells), intent(in) :: uy ! A-grid meridional velocity component | ||
| character*(*), intent(in) :: outfld_name_suffix ! suffix for "outfld" names |
There was a problem hiding this comment.
Other character input arguments in the MPAS dycore are defined as character(len=*), intent(in) ::. Can we update this outfld_name_suffix to do the same for consistency:
character(len=*), intent(in) :: outfld_name_suffix ! suffix for "outfld" namesThere was a problem hiding this comment.
character(len=*) is the common syntax in CESM/CAM as well.
| name_out3 = 'WV_' //trim(outfld_name_suffix) | ||
| name_out4 = 'WL_' //trim(outfld_name_suffix) | ||
| name_out5 = 'WI_' //trim(outfld_name_suffix) | ||
| name_out6 = 'TT_' //trim(outfld_name_suffix) |
There was a problem hiding this comment.
I suggest renaming the name_outx to be more specific. For instance, name_out_se, name_out_ke, etc. if that is appropriate.
This will help the writes on: 872, 873, 874, 888, 902 be clearer to readers and will help ensure that the correct variable is going out to the correct field.
There was a problem hiding this comment.
Sounds good. Since the same code exists in physics we should change there too:
subroutine calc_te_and_aam_budgets in src/physics/cam/check_energy.F90
| name_out6 = 'TT_' //trim(outfld_name_suffix) | ||
|
|
||
| if ( hist_fld_active(name_out1).or.hist_fld_active(name_out2).or.hist_fld_active(name_out3).or.& | ||
| hist_fld_active(name_out4).or.hist_fld_active(name_out5).or.hist_fld_active(name_out6)) then |
There was a problem hiding this comment.
Is this if statement necessary? Is compute_energy_diags preforming the functionality in each of the calls to tot_enegry?
There was a problem hiding this comment.
The if-statements are in there so that we do not compute the vertical integrals if we are not going to output them. The way the subroutine is used in dp_coupling makes the logicals not necessary (as you are pointing out).
There is a similar subroutine in physics (calc_te_and_aam_budgets) and the calls to that subroutine do not have logicals around them in which case we would be computing several vertical integrals even if the energy diagnostics are not active. That is where I got the code from.
I am fine with either approach
There was a problem hiding this comment.
Unless the internal if statement provides some other functionality, I don't think it necessary. I think using compute_energy_diags on the calling code is more explicit to whether the integrals are being computed or not. Rather than having a call to tot_energy which potentially does nothing. That would make the calling code clearer as to when they are being computed.
| ! CAM physics output redistributed to blocks. | ||
| real(r8), allocatable :: t_tend(:,:) | ||
| real(r8), allocatable :: qv_tend(:,:) | ||
| real(r8), allocatable :: q_tend(:,:,:) |
There was a problem hiding this comment.
Now that this variable contains all scalars, and not just qv, would scalars_tend be a better name? MPAS names its scalars array as such.
There was a problem hiding this comment.
The variable only contains thermodynamically active species and not all advected species (in a standard F2000 AMIP setup the thermodynamic active tracers are water vapor and condensates; for WACCM-x it would include components of dry air). I am fine with renaming if you wish.
There was a problem hiding this comment.
I think tracer would be more standard. That name is used throughout CAM including the other dycores (and I see it used a lot in MPAS that way as well). It used to be called the q array in CAM but it is being standardized as tracer. If that's OK with everyone I will rename to tracer_tend
There was a problem hiding this comment.
tracer_tend sounds good to me
There was a problem hiding this comment.
It is only the thermodynamically active species but that would make the name really long: thermodynamically_active_tracer_tend
I am fine with what you choose.
|
@PeterHjortLauritzen did you want me to review other files you changed in |
…ixer does not have liq and ice species. Fix for SPCAM
…into cam_mpas_dev_pel
Fix TSKIN field to correct value
I'm just creating this PR to easily see @PeterHjortLauritzen energy fixer changes in dp_coupling.F90. We can delete it if need be.