-
Notifications
You must be signed in to change notification settings - Fork 179
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
Modify workflow to access files from obs component #1202
Modify workflow to access files from obs component #1202
Conversation
Modify config and archive list scripts to access file from obs component. GitHub: 1198
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.
shellcheck found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
jobs/rocoto/prep.sh
Outdated
fi | ||
|
||
# exception handling to ensure no dead link | ||
[[ $(find ${COMOBS} -xtype l | wc -l) -ge 1 ]] && exit 9 |
Check notice
Code scanning / shellcheck
Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).
|
||
# exception handling to ensure no dead link | ||
[[ $(find ${COMOBS} -xtype l | wc -l) -ge 1 ]] && exit 9 | ||
[[ $(find ${gCOMOBS} -xtype l | wc -l) -ge 1 ]] && exit 9 |
Check notice
Code scanning / shellcheck
Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).
|
||
# exception handling to ensure no dead link | ||
[[ $(find ${COMOBS} -xtype l | wc -l) -ge 1 ]] && exit 9 | ||
[[ $(find ${gCOMOBS} -xtype l | wc -l) -ge 1 ]] && exit 9 |
Check notice
Code scanning / shellcheck
Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).
…ile linking from GDA location. Using COMIN_OBS for file reference in obs component. GitHub: 1198
jobs/JGLOBAL_ATMOS_ANALYSIS
Outdated
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | ||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | ||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | ||
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/${COMPONENT}" |
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.
Same as above
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.
Same as above
fi | ||
|
||
# exception handling to ensure no dead link | ||
[[ $(find ${COMIN_OBS} -xtype l | wc -l) -ge 1 ]] && exit 9 |
Check notice
Code scanning / shellcheck
Consider invoking this command separately to avoid masking its return value (or use '|| true' to ignore).
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.
For file specific changes, please see the inline comments.
A number of files have changed their permissions from 100755 -> 100644
.
Please revert those permission changes.
if [[ "$CDATE" -ge "2019042300" ]]; then | ||
export AHIBF="$DMPDIR/${CDUMP}x.${PDY}/${cyc}/atmos/${CDUMP}.t${cyc}z.ahicsr.tm00.bufr_d" | ||
if [[ "${CDATE}" -ge "2019042300" ]]; then | ||
export AHIBF="${COMIN_OBS}/${CDUMP}.t${cyc}z.ahicsr.tm00.bufr_d" |
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 changes are consistent w/ the PR, but I am not sure this is entirely correct.
This is going to be true, if script that gets the obs copies them into the COMIN_OBS
. But if it doesn't, we have to go to the DMPDIR
.
May be @RussTreadon-NOAA might know the details of these conditionals
scripts/exglobal_atmos_sfcanl.sh
Outdated
export FNACNA=${FNACNA:-${COMIN}/${OPREFIX}seaice.5min.blend.grb} | ||
export FNSNOA=${FNSNOA:-${COMIN}/${OPREFIX}snogrb_t${JCAP_CASE}.${LONB_CASE}.${LATB_CASE}} | ||
[[ ! -f ${FNSNOA} ]] && export FNSNOA="${COMIN_OBS}/${OPREFIX}snogrb_t1534.3072.1536" | ||
FNSNOG=${FNSNOG:-${COMIN_GES_OBS}/${GPREFIX}snogrb_t${JCAP_CASE}.${LONB_CASE}.${LATB_CASE}} |
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.
Shouldn't this be looking in COMIN_GES
since we are retaining this file in the atmos
component directory?
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 looks like the logic is to access these files in COMIN. Maybe there is no need to config the COMIN_GES.
echo "${obs_dirname}${head}nsstbufr " >>gfsa.txt | ||
echo "${obs_dirname}${head}prepbufr " >>gfsa.txt | ||
echo "${obs_dirname}${head}prepbufr.acft_profiles " >>gfsa.txt |
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.
fix indentation.
echo "${obs_dirname}${head}nsstbufr " >>gdas_restarta.txt | ||
echo "${obs_dirname}${head}prepbufr " >>gdas_restarta.txt | ||
echo "${obs_dirname}${head}prepbufr.acft_profiles " >>gdas_restarta.txt |
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.
fix indentation.
…in COM assignment as requested
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.
a couple of if-blocks and an indentation fix.
we can worry about the leftover $COMPONENT
in the files in a later PR.
Please fix the if-blocks at the very least and we can merge this PR.
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.
There are still permissions issues with execute being removed from several scripts.
Please provide more detail. Which one? |
755 permission restored to 9 scripts. |
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 branch at its current state fails in the gdasanal
and gdaseobs
jobs.
This branch needs to merge develop
into it.
|
@aerorahul Conflicting resolved. |
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.
Several of the scripts have had their permissions changed again. Please correct.
755 permission updated. |
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.
Seems okay now.
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.
Some suggestions as they won't break anything (ROTDIR_DUMP=NO
does not work), but other than those changes look ok.
I have not tested and assume that @lgannoaa tested these changes for a few cycles for both GDAS and GFS.
export COMIN_OBS=${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs} | ||
export COMIN_GES_OBS=${COMIN_GES_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs} |
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.
Is there a reason this needs to have a variable? This is a configuration file and nothing is set before calling config.base
export COMIN_OBS=${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs} | |
export COMIN_GES_OBS=${COMIN_GES_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs} | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" | |
export COMIN_GES_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@@ -38,8 +38,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
else | |||
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMOUT_ENS="${ROTDIR}/enkfgdas.${PDY}/${cyc}" | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@@ -35,8 +35,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${GDUMP}.${gPDY}/${gcyc}/atmos} | |||
else | |||
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
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.
ROTDIR_DUMP=NO
does not work, so this change is untested.
@@ -42,8 +42,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_OBS=${COMIN_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${RUN}.${PDY}/${cyc}/atmos} | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${GDUMP}.${gPDY}/${gcyc}/atmos} | |||
else | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@@ -36,8 +36,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_OBS=${COMIN_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${RUN}.${PDY}/${cyc}/${COMPONENT}} | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${GDUMP}.${gPDY}/${gcyc}/${COMPONENT}} | |||
else | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/${COMPONENT}" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/${COMPONENT}" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
jobs/JGLOBAL_ATMOS_ANALYSIS
Outdated
@@ -38,8 +38,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${GDUMP}.${gPDY}/${gcyc}/atmos} | |||
else | |||
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@@ -38,8 +38,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-${ROTDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos} | |||
else | |||
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_OBS="${DMPDIR}/${CDATE}/${CDUMP}" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDATE}/${GDUMP}" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@@ -42,8 +42,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_OBS=${COMIN_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${RUN}.${PDY}/${cyc}/atmos} | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${GDUMP}.${gPDY}/${gcyc}/atmos} | |||
else | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@@ -36,8 +36,8 @@ if [ ${RUN_ENVIR} = "nco" -o ${ROTDIR_DUMP:-NO} = "YES" ]; then | |||
export COMIN_GES_OBS=${COMIN_GES_OBS:-$(compath.py ${envir}/obsproc/${obsproc_ver})/${GDUMP}.${gPDY}/${gcyc}/atmos} | |||
else | |||
export COMOUT="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_OBS="${DMPDIR}/${CDUMP}.${PDY}/${cyc}/atmos" | |||
export COMIN_GES_OBS="${DMPDIR}/${GDUMP}.${gPDY}/${gcyc}/atmos" | |||
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" |
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.
export COMIN_OBS="${COMIN_OBS:-${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs}" | |
export COMIN_OBS="${ROTDIR}/${CDUMP}.${PDY}/${cyc}/obs" |
@lgannoaa I noticed some files changed permissions again. You might want to look at what is making this change. |
@RussTreadon-NOAA |
Has a cycled test been run from the feature branch for sufficient duration to confirm that the proposed changes do not alter cycled results from a control run using develop? If so, approval from @WalterKolczynski-NOAA and @aerorahul is sufficient. |
The permission change is to fix the config file permission issue from the develop branch. There issues are not introduced from this PR. |
I discovered the Global Workflow develop #f98433f (Jan 23, 2023) has an internal silent failure "unary operator expected" in gdassfcanl and gfssfcanl. File exglobal_atmos_sfcanl.sh contain incorrect application logic in FNSNOG assignment. @RussTreadon-NOAA |
When the conflicts were resolved in this PR, the permission on some of the |
Description
Observations in the current cycled system are linked/copied or created into the atmos/ subdirectory under ROTDIR
There are three files linked from emc dump to atmos subdirectory:
syndata.tcvitals.tm00
snogrb_t1534.3072.1536
seaice.5min.blend.grb
The nsstbufr, prepbufr will be created in obs subdirectory.
There should be only prep jobs need to access GDA location to link files to the obs component. All other jobs should be using obsproc files under the obs component.
Modify the following scripts to use COMIN_OBS in access files under obs component.
JGDAS_ATMOS_ANALYSIS_DIAG
JGDAS_ATMOS_CHGRES_FORENKF
JGDAS_ENKF_DIAG
JGDAS_ENKF_ECEN
JGDAS_ENKF_SELECT_OBS
JGDAS_ENKF_SFC
JGLOBAL_ATMOS_ANALYSIS
JGLOBAL_ATMOS_ANALYSIS_CALC
JGLOBAL_ATMOS_SFCANL
config.anal
config.base.emc.dyn
exglobal_atmos_sfcanl.sh
Fixes #1198
How Has This Been Tested?
Cycled tests on WCOSS
HOMEgfs:/lfs/h2/emc/global/noscrub/lin.gan/git/PR/issue-1184
EXPDIR:/lfs/h2/emc/global/noscrub/lin.gan/expdir/obsproc_da
KEEPDATA=YES
The following log file show a cycle with MAKE_NSSTBUFR="NO"
/lfs/h2/emc/ptmp/lin.gan/com/obsproc_da/gfs/v16.3/logs/2022062006
The following log file show a cycle with MAKE_NSSTBUFR="YES"
/lfs/h2/emc/ptmp/lin.gan/com/obsproc_da/gfs/v16.3/logs/2022062012
You may execute the following CMD to exam the log:
(In gdas/gfs prep step, the linking of the obsproc files is in the correct target /lfs/h2/emc/ptmp/lin.gan/com/obsproc_da/gfs/v16.3/gfs.20220620/12/obs/)
grep "ln " prep |grep -v "20220620/12/obs/"
(The bufr files located in new directory have been used in gdas prep, gfs prep, gdas anal, gfs anal, gdassfcanl, gfssfcanl, eobs, )
grep "ln " * |grep "20220620/12/obs/"
(No reference to emc dump archive other then the prep step. I.e. all other step used files from the new location under obs component)
grep "ln " * |grep bufr|grep "emc.global/dump"|grep -v "prep.log"|wc -l
(Exam the creation of the nsstbufr file when MAKE_NSSTBUFR="YES")
grep nsst *prep.log
Forecast-only tested on Hera
Checklist