-
Notifications
You must be signed in to change notification settings - Fork 175
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 JGDAS_ENKF_SFC and JGDAS_ENKF_UPDATE jobs with COMIN/COMOUT instead of COM #3170
base: develop
Are you sure you want to change the base?
Update JGDAS_ENKF_SFC and JGDAS_ENKF_UPDATE jobs with COMIN/COMOUT instead of COM #3170
Conversation
@mingshichen-noaa FYI, please ignore CI test messages in your PR. We are using your PR to test a new CI setup on WCOSS2. We will rerun the CI tests for this PR once reviews have completed. Thanks! |
CI Tests set up to run in /lfs/h2/emc/ptmp/emc.global/PR/PR_3170/RUNTESTS on WCOSS |
@KateFriedman-NOAA |
@mingshichen-noaa CI tests that you were running under your account? Why did you stop them? Was there an error? |
|
@KateFriedman-NOAA |
@KateFriedman-NOAA |
@mingshichen-noaa Glad to hear your tests on Hercules succeeded. Please leave this PR open until it's merged or the code managers decide to close. Thanks! |
@WalterKolczynski-NOAA |
scripts/exgdas_enkf_update.sh
Outdated
@@ -412,7 +412,7 @@ $APRUN_ENKF ${DATA}/$(basename $ENKFEXEC) 1>stdout 2>stderr | |||
export err=$?; err_chk | |||
|
|||
# Cat runtime output files. | |||
cat stdout stderr > "${COM_ATMOS_ANALYSIS_STAT}/${ENKFSTAT}" | |||
cat stdout stderr > "${COMIN_ATMOS_ANALYSIS_STAT}/${ENKFSTAT}" |
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 an output. It's possible COM_ATMOS_ANALYSIS is both an IN and an OUT for this job. Please evaluate what the script is doing carefully to determine what are inputs and what are outputs.
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 second Walter's comment. It appears that COM_ATMOS_ANALYSIS
is both an IN and an OUT for this job. Please examine and if you agree, please define the variable twice, once as an IN and once as an OUT. Then update the script to use the appropriate IN or OUT version of the variable. Thanks!
jobs/JGDAS_ENKF_UPDATE
Outdated
@@ -43,7 +43,7 @@ status=$? | |||
# Send Alerts | |||
############################################## | |||
if [ ${SENDDBN} = YES ] ; then | |||
"${DBNROOT}/bin/dbn_alert" "MODEL" "ENKF1_MSC_enkfstat" "${job}" "${COM_ATMOS_ANALYSIS_STAT}/${APREFIX}enkfstat" | |||
"${DBNROOT}/bin/dbn_alert" "MODEL" "ENKF1_MSC_enkfstat" "${job}" "${COMIN_ATMOS_ANALYSIS_STAT}/${APREFIX}enkfstat" |
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 notification should be about an output too.
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 agree with Walter's comment. This should be a COMOUT
variable here since it's reporting the existence of an output file via dbn_alert
.
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.
@WalterKolczynski-NOAA @KateFriedman-NOAA
Yes, I agree with your comments. I am working on the modifications.
@WalterKolczynski-NOAA @KateFriedman-NOAA |
Description
NCO has requested that each COM variable specify whether it is an input or an output. This completes that process for the global JGDAS_ENKF_SFC and JGDAS_ENKF_UPDATE jobs.
Refs #2451
Type of change
Change characteristics
How has this been tested?
Checklist