-
Notifications
You must be signed in to change notification settings - Fork 79
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
new water and energy budget diagnostics #107
Conversation
6ca9a8b
to
ee63d5c
Compare
Note that this PR removes variables aoflux_grid and gust_fac, these variables are not currently being used but may have been intended as placeholders for future development. |
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'd like to review again once my comments have been addressed. The biggest one is that the med_time_mod.F90 should be from master and not what was in the original diag branch.
cime_config/runseq/runseq_general.py
Outdated
runseq.add_action("MED med_phases_diag_lnd" , run_lnd and diag_mode) | ||
runseq.add_action("MED med_phases_diag_rof" , run_rof and diag_mode) | ||
runseq.add_action("MED med_phases_diag_ocn" , run_ocn and diag_mode) | ||
runseq.add_action("MED med_phases_diag_glc" , run_glc and diag_mode) |
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.
This is really picky - but can yo please keep the indents where they have been placed originally.
@@ -679,7 +679,7 @@ subroutine esmFldsExchange_cesm(gcomp, phase, rc) | |||
call addfld(fldListFr(compice)%flds, 'Faii_'//trim(suffix(n))) | |||
call addfld(fldListTo(compatm)%flds, 'Faxx_'//trim(suffix(n))) | |||
else | |||
! (non aqua-planet) | |||
! CESM (non aqua-planet) |
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.
This is okay - but we don't really need the CESM comment since this code section is already in a CESM specific routine.
@@ -692,7 +692,6 @@ subroutine esmFldsExchange_cesm(gcomp, phase, rc) | |||
mrg_from2=compice, mrg_fld2='Faii_'//trim(suffix(n)), mrg_type2='merge', mrg_fracname2='ifrac', & | |||
mrg_from3=compmed, mrg_fld3='Faox_'//trim(suffix(n)), mrg_type3='merge', mrg_fracname3='ofrac') | |||
|
|||
! (cam, aqua-planet) |
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.
Why did you get rid of this comment?
mediator/med.F90
Outdated
call set_stop_alarm(gcomp, rc) | ||
if (ChkErr(rc,__LINE__,u_FILE_u)) return | ||
|
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.
Why are we calling this new routine? Why is this needed?
mediator/med_fraction_mod.F90
Outdated
type(ESMF_RouteHandle) :: rh_ocn2atm | ||
type(ESMF_RouteHandle) :: rh_lnd2atm | ||
type(ESMF_RouteHandle) :: rh_atm2lnd | ||
integer :: srcTermProcessing_Value = 0 |
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.
Did you bring in my changes from PR #110 into this? These variables are not in my PR branch. So I don't know what you are actually using.
mediator/med_fraction_mod.F90
Outdated
type(ESMF_Field) :: fldsrc | ||
type(ESMF_Field) :: flddst |
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.
The same comment as above applies. These variables are not in my PR branch.
mediator/med_map_mod.F90
Outdated
@@ -105,6 +105,7 @@ subroutine med_map_RouteHandles_init_esmflds(gcomp, llogunit, rc) | |||
use ESMF , only : ESMF_REGRIDMETHOD_NEAREST_STOD | |||
use ESMF , only : ESMF_NORMTYPE_DSTAREA, ESMF_REGRIDMETHOD_PATCH, ESMF_RouteHandlePrint | |||
use NUOPC , only : NUOPC_Write | |||
use shr_sys_mod, only : shr_sys_abort |
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.
Why is this needed? We should be using an ESMF error exit statement.
mediator/med_time_mod.F90
Outdated
optNStep = "nstep" , & | ||
optNSeconds = "nseconds" , & | ||
optNSecond = "nsecond" , & | ||
optNMinutes = "nminutes" , & | ||
optNMinute = "nminute" , & | ||
optNHours = "nhours" , & | ||
optNHour = "nhour" , & | ||
optNDays = "ndays" , & | ||
optNDay = "nday" , & | ||
optNMonths = "nmonths" , & | ||
optNMonth = "nmonth" , & | ||
optNYears = "nyears" , & | ||
optNYear = "nyear" , & | ||
optMonthly = "monthly" , & | ||
optYearly = "yearly" , & | ||
optDate = "date" , & | ||
optIfdays0 = "ifdays0" , & |
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.
Why add these back in. We should be using the code in cmeps master and not this older code. I explicitly took these out between the original diag branch and the current master.
mediator/med_time_mod.F90
Outdated
!=============================================================================== | ||
|
||
subroutine med_time_clockInit(ensemble_driver, esmdriver, logunit, rc) | ||
|
||
! input/output variables | ||
type(ESMF_GridComp) :: ensemble_driver, esmdriver | ||
integer, intent(in) :: logunit | ||
integer, intent(out) :: rc | ||
|
||
! local variables | ||
type(ESMF_Clock) :: clock | ||
type(ESMF_VM) :: vm | ||
type(ESMF_Time) :: StartTime ! Start time | ||
type(ESMF_Time) :: RefTime ! Reference time | ||
type(ESMF_Time) :: CurrTime ! Current time | ||
type(ESMF_Time) :: StopTime ! Stop time | ||
type(ESMF_Time) :: StopTime1 ! Stop time | ||
type(ESMF_Time) :: StopTime2 ! Stop time | ||
type(ESMF_Time) :: Clocktime ! Loop time | ||
type(ESMF_TimeInterval) :: TimeStep ! Clock time-step | ||
type(ESMF_Alarm) :: alarm_stop ! alarm | ||
type(ESMF_Alarm) :: alarm_datestop ! alarm | ||
integer :: ref_ymd ! Reference date (YYYYMMDD) | ||
integer :: ref_tod ! Reference time of day (seconds) | ||
integer :: start_ymd ! Start date (YYYYMMDD) | ||
integer :: start_tod ! Start time of day (seconds) | ||
integer :: curr_ymd ! Current ymd (YYYYMMDD) |
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.
med_time_mod should be from master and not what you have here. This is older code that is no longer needed.
@mvertens I think that I've captured all of your suggested changes. I will restart testing. |
@jedwards4b - thank you. That sounds great.
…On Wed, Oct 7, 2020 at 3:53 PM jedwards4b ***@***.***> wrote:
@mvertens <https://github.com/mvertens> I think that I've captured all of
your suggested changes. I will restart testing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#107 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4XCE56R4I27MGAXOV2OGLSJTPMNANCNFSM4R7D7DKA>
.
--
Mariana Vertenstein
CESM Software Engineering Group Head
National Center for Atmospheric Research
Boulder, Colorado
Office 303-497-1349
Email: [email protected]
|
@@ -83,9 +83,10 @@ def _main_func(): | |||
|
|||
with open(os.path.join(bld_root,'Filepath'), 'w') as out: | |||
cmeps_dir = os.path.join(os.path.dirname(__file__), os.pardir) | |||
# SourceMods dir needs to be first listed | |||
out.write(os.path.join(caseroot, "SourceMods", "src.drv") + "\n") |
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.
This is a bug fix not related to diags - the SourceMods dir needs to be first in Filepath
@@ -165,6 +165,14 @@ | |||
<option name="wallclock"> 00:40:00 </option> | |||
</options> | |||
</test> | |||
<test compset="B1850" grid="f19_g16" name="ERR_Vnuopc_Ld5" testmods="allactive/nuopc_cap_io"> |
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.
Add a B1850 test
@mvertens All tests pass, please review and merge. |
… air-sea flux computation (ESCOMP#107) * Update ufs/ccpp/data/MED_typedefs.meta.
… air-sea flux computation (ESCOMP#107) * Update ufs/ccpp/data/MED_typedefs.meta.
Description of changes
Adds budget diagnostics code to cmeps.
Specific notes
Contributors other than yourself, if any: @mvertens
CMEPS Issues Fixed (include github issue #):
Are changes expected to change answers?
Any User Interface Changes (namelist or namelist defaults changes)?
Testing performed if application target is CESM:(either UFS-S2S or CESM testing is required):
Testing performed if application target is UFS-S2S:
Hashes used for testing: