-
Notifications
You must be signed in to change notification settings - Fork 29
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
Adding benchmark-like tests #67
Conversation
MOM6_RESTART_SETTING = 'r' | ||
|
||
# set locations of ICs | ||
BM_IC="/scratch1/NCEPDEV/nems/Bin.Li/S2S/FROM_HPSS/@[CDATE]" |
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.
Generally we don't allow personal directory in the repository code, maybe we can change this later. E.g. get run directory $RUNDIR first , then define BM_IC=$RUNDIR/FROM_HPSS/@[CDATE]
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're working on moving all the ICs to the climpara area and we could point to that in the date suite/benchmark tests (this location). I don't know if you want to add all the ICs to the RT area for the datesuite test area though. For the regression test, I added the benchmark IC files to the regression test baseline area:
https://github.com/ufs-community/ufs-s2s-model/blob/develop/compsets/benchmarkRT_cold.input#L45
BM_IC="@[plat%INPUTS]/BM_IC/@[CDATE]"
Maybe we should add the 2012010100 and 2012010100 IC dates there for what are actually regression tests?
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 prefer all the ICs to be the RT area as we consider the RT could be independent. We do the same for the fix files in weather. It will be easier for code porting and testing as we don't need to have climpara ready to do RT.
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 ICs for 2012010100 and 2012070100 will be in the RT area in the next ufs-s2s-model update.
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's 4TB worth of data, so I don't think you want all of the ICs in the RT area. I would think the 3 tests that are regression tests or at most the 8 ICs currently set up in the datesuite tests? @DeniseWorthen if we point to the RT baseline area for those 8 datesuite tests, are we then loosing the ability to run any date which I think was part of your desire when you originally made these tests?
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.
Yes. The datesuites would still need to point to the original data location. It should work to add a line to the datesuite tests re-setting the BM_IC location w/in each datesuite test (both cold and warm):
# - set date YYYYMMDDHH
SYEAR='2012'
SMONTH='01'
SDAY='01'
SHOUR='00'
CDATE="@[SYEAR]@[SMONTH]@[SDAY]@[SHOUR]"
# set locations of ICs
BM_IC="/scratch1/NCEPDEV/nems/Bin.Li/S2S/FROM_HPSS/@[CDATE]"
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 Bin to also update fv3 to the latest version.
Test cpld_fv3_ccpp_384_mom6_cice_ww3_cold_bmark_rt_repro starting at Sun Apr 12 23:21:19 UTC 2020 (Coupled FV3-CCPP-MOM6-CICE-WW3 system - cold 384 - repro) | ||
Sun Apr 12 23:21:19 UTC 2020 | ||
WORKFLOW REPORT AT Sun Apr 12 23:54:22 2020 (+1586735662) | ||
Tests: 3 failed, 45 passed out of 48 |
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.
3 tests failed?
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.
These 3 failed tests have been rerun until they are successful.
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.
Good catch!
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.
These 3 failed tests have been rerun until they are successful.
So the 3 failed tests have run and all passed?
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.
Yes, they have been run successfully before this pull request.
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. I did not understand the earlier comment. I will complete the review now.
This pull request includes the following updates:
fv3_ccpp_384_mom6_cice_cold_2012010100
fv3_ccpp_384_mom6_cice_cold_2012070100
fv3_ccpp_384_mom6_cice_35d_2012010100
fv3_ccpp_384_mom6_cice_35d_2012070100
By @JessicaMeixner-NOAA and @binli2337