-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update master branch to use master branches of ccpp-physics and ccpp-framework #185
Conversation
…d a namelist that exercises the hurricane version of moninedmf
…ting with Weiguo Wang
…e; add a multi_run configuration file for the using the hurricane PBL namelist for all supported cases
…e initialized to something other than 0 in gfdl_sfc_layer)
… HWRF LSM and surface layer for testing
…-framework (temporary)" This reverts commit de084cb.
…th latest dtc/develop branch; update submodule pointers
Update dtc/develop branch to work with ccpp-physics/framework as of 20200608
…se new phys_tend scheme and MYNN-related changes; add new MYNN-related variables, new tendencies, and new aux[2,3]d variables to GFS_typedefs.[meta,F90]; edit gmtb_scm.F90 to call output_init after physics are initialized and to be able to calculate non-physics tendencies; edit do_time_step to calculate non-physics tendencies; edit gmtb_scm_output to write out new tendencies and aux variables
…_analysis.py to read new tendency variables; add new supported_suites.ini plotting configuration file
…k with the latest CCPP framework; update optional arguments to match FV3
…e the way more than one column is implemented -- use the horizontal_dimension of physics variables rather than having arrays of GFS DDTs with horizontal extent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the cleanup work, glad to see that the new ccpp-framework code is working as expected in SCM as well (when no blocking is used). I left several comments, but happy to approve if you think that it is not worth addressing them now.
@@ -113,6 +116,7 @@ module GFS_typedefs | |||
real(kind=kind_phys) :: dt_phys !< physics time step in seconds | |||
!--- restart information | |||
logical :: restart !< flag whether this is a coldstart (.false.) or a warmstart/restart (.true.) | |||
logical :: cycling !< flag whether this is a coldstart (.false.) or a cycled run (.true.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this will be removed again in fv3atm in the near future, see NOAA-EMC/fv3atm#140.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks. No one would (should) ever change that from the default false value when running the SCM anyway. It was only included to minimize differences between the SCM/FV3 GFS_typedefs.
write(0,*) "Error, the SCM has not implemented time averaging of diagnostics in GFS_typedefs.F90" | ||
stop | ||
end if | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is the time averaging done in SCM? The tendencies are accumulated in ccpp-physics. Are you subtracting the previous accumulated value after each time step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diag_phys_zero is called every time step and right before it is written out, these accumulated change variables are divided by the timestep to get the tendencies.
@@ -3290,6 +3537,15 @@ subroutine control_initialize (Model, nlunit, fn_nml, me, master, & | |||
Model%fhlwr = fhlwr | |||
Model%nsswr = nint(fhswr/Model%dtp) | |||
Model%nslwr = nint(fhlwr/Model%dtp) | |||
if (restart) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried this in SCM? What differences do you see? Would be interesting to check (not for merging this PR, though).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. Restarts are not implemented in the SCM since they are so short to run. This is another GFS_typedefs variable that is only there to minimize differences between FV3/SCM. There should probably be a list of namelist and other vars to not change from their default in the SCM.
@@ -92,66 +90,68 @@ subroutine gmtb_scm_main_sub() | |||
case default | |||
cdata_time_index = 2 | |||
end select | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this cleans up the code quite a bit.
@@ -219,6 +221,31 @@ subroutine output_init(scm_state) | |||
CALL CHECK(NF90_PUT_ATT(NCID=ncid,VARID=dummy_id,NAME="description",& | |||
VALUES="temperature tendency due to microphysics scheme")) | |||
CALL CHECK(NF90_PUT_ATT(NCID=ncid,VARID=dummy_id,NAME="units",VALUES="K s-1")) | |||
CALL CHECK(NF90_DEF_VAR(NCID=ncid,NAME='dT_dt_ogwd',XTYPE=NF90_FLOAT,DIMIDS= (/ hor_dim_id, vert_dim_id, time_id /), & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if we could suppress the output of the 3d tendencies if ldiag3d/qdiag3d are false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about if we just used a netCDF missing value if the logicals are false? Right now, it outputs zeros if they are false, but I suppose that could be misleading. I prefer to keep the variables actually in the output file so that scripts don't need to check for variables existing or not. I guess they would need to check for missing values instead though.
@@ -318,8 +430,23 @@ subroutine output_init(scm_state) | |||
CALL CHECK(NF90_DEF_VAR(NCID=ncid,NAME='init_hour',XTYPE=NF90_FLOAT,VARID=hour_id)) | |||
CALL CHECK(NF90_PUT_ATT(NCID=ncid,VARID=hour_id,NAME="description",VALUES="model initialization hour")) | |||
CALL CHECK(NF90_PUT_ATT(NCID=ncid,VARID=hour_id,NAME="units",VALUES="")) | |||
|
|||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do the same for the 3d tendencies, or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I could have an array of variable names and descriptions and shorten the code with a loop for sure. At some point, I was hoping to reimplement the output for the SCM though, so I'm not sure that it is worth it right now. I'd like to implement something where the users could specify which variables to write out by their CCPP standard names, implement time-averaging (instead of only instantaneous), etc.
do i=1, scm_state%n_cols | ||
dummy_1D(i) = physics%Interstitial(i)%dusfc1(1) | ||
dummy_2D(i,:) = physics%Diag%dt3dt(i,:,1)/scm_state%dt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, this is where the time averaging happens for the 3d tendencies.
end do | ||
CALL CHECK(NF90_INQ_VARID(NCID=ncid,NAME="lw_dn_sfc_clr",VARID=var_id)) | ||
CALL CHECK(NF90_PUT_VAR(NCID=ncid,VARID=var_id,VALUES=dummy_1D,START=(/1,scm_state%itt_out /))) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use the time_avg logical arrays in the same way as it is done in fv3atm - if it is .true. for a specific slice of the array, simply divide by the integration time (same as you do for the 3d tendencies).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll look into that for the output rewrite.
Thanks for the comments and review. I'd like to go ahead and merge as-is and do a rewrite of the output routines later (which I had already planned on doing at some point anyway) taking your suggestions into account then. Mike Iacono is waiting for these changes and I need a good base to start some other development. |
Update master branch to use master branches of ccpp-physics and ccpp-framework
This PR allows the SCM to work with the latest master branches of ccpp-physics and ccpp-framework. It also contains changes to work with HWRF physics, which will presumably be going to the master branches of the CCPP soon too.
Most of the changes are to facilitate new/different variables in the physics, although a few of the commits in July (specifically 75f3475) change the way multiple independent columns could be used. Rather than have multiple instances of GFS DDTs holding the different columns (and potentially allowing different physics suites to be run in each of them simultaneously), the horizontal dimension of the individual variables is used instead. E.g. instead of physics%Statein(i)%tgrs(1,k), you now have physics%Statein%tgrs(i,k). This closes the door on the potential to run multiple physics suites simultaneously, but this wasn't being done in practice due to module-level data anyway.
This PR also marks the ability to use the updated tendencies, although the non-physics tendency is not working due to memory management. This can be enabled in a future PR.
Lastly, the GFS_DCNV_generic_pre routines have been rearranged in many SDFs.