Enable EnKF-only for atmosphere #4345
Conversation
… hard-coded (need to revisit)
…del (global_control.nml.IN)
…e temporary file
…e temporary file
TravisElless-NOAA
left a comment
TravisElless-NOAA
left a comment
There was a problem hiding this comment.
Thanks for picking up the ball on this @bhuang95. As I'm starting to relearn things, I have a few questions along with some other details I noticed initially.
| # If DOENKFONLY_ATM="YES", skip PROCESS_TROPCY | ||
| if [[ "${DOENKFONLY_ATM:-NO}" == "YES" ]] ; then | ||
| export PROCESS_TROPCY="NO" | ||
| fi |
There was a problem hiding this comment.
Is this block actually needed? I haven't run with this enkf setting yet, but in default testing PROCESS_TROPCY=NO gets set 3 lines earlier. Does this default change for enkf only?
There was a problem hiding this comment.
@TravisElless-NOAA I remember I added this block because PROCESS_TROPCY was set to "YES" somewhere and it failed the task when DOENKFONLY_ATM = "YES". Looks like this block is not needed any more here. I will remove this block and test it in the updated PR.
| 'DOLETKF_OCN', 'IAUFHRS_ENKF', 'NET', 'NMEM_ENS_GFS', 'DO_GSISOILDA', 'DO_LAND_IAU', | ||
| 'DOENKFONLY_ATM'] |
There was a problem hiding this comment.
Cleans up the format of this block better
| 'DOLETKF_OCN', 'IAUFHRS_ENKF', 'NET', 'NMEM_ENS_GFS', 'DO_GSISOILDA', 'DO_LAND_IAU', | |
| 'DOENKFONLY_ATM'] | |
| 'DOLETKF_OCN', 'IAUFHRS_ENKF', 'NET', 'NMEM_ENS_GFS', | |
| 'DO_GSISOILDA', 'DO_LAND_IAU', 'DOENKFONLY_ATM'] |
There was a problem hiding this comment.
@TravisElless-NOAA Will modify as suggested in the updated PR. Thanks.
|
|
||
| # Reset tasks to run enkf-only for atm if do_enkfonly_atm=true | ||
| if options['do_enkfonly_atm']: | ||
| task_names[run] = [] |
There was a problem hiding this comment.
Does this clear out the prep jobs you just added/declared previously (line 389)?
There was a problem hiding this comment.
@TravisElless-NOAA Yes. Line 385-390, and Line 429- basically reset task_names[run]. It was meant to only include necessary tasks for enkf-only run and to avoid excessive addition of "if" statements before these lines.
Looking at @aerorahul 's comments suggesting using DO_GSIATMVAR, DO_GSIATMENS, DO_JEDIATMVAR, DO_JEDIATMENS to set up enkf-only run for GSI and JEDI. this may not be preferred and I may need to add if statements instead.
| dep_dict = {'type': 'metatask', 'name': 'gdas_fcst', 'offset': f"-{timedelta_to_HMS(self._base['interval_gdas'])}"} | ||
| deps.append(rocoto.add_dependency(dep_dict)) |
There was a problem hiding this comment.
Did this need to be in your enkfonly if block? Currently this new dependency is being added to the entire suite of workflow options, where it wasn't before. So asking if this was intended or not.
There was a problem hiding this comment.
@TravisElless-NOAA Line 143-144 is not necessary to be include in the enkfonly if block. Because the enkfgdas_epmn in the enkfonly if block is dependent on the completion of enkfgdas_fcst on Line 143-144. But we can add them there too.
parm/archive/enkf_grp.yaml.j2
Outdated
| {% if not DOENKFONLY_ATM %} | ||
| {% for iaufhr in IAUFHRS %} | ||
| {% set iaufhr = iaufhr %} | ||
| - "{{ COMIN_ATMOS_ANALYSIS_MEM | relpath(ROTDIR) }}/{{ head }}recentered_increment.atm.i{{ '%03d' % iaufhr }}.nc" | ||
| {% endfor %} # iaufhr in IAUFHRS | ||
| {% endfor %} # iaufhr in IAUFHRS | ||
| {% endif %} |
There was a problem hiding this comment.
This section will need to be changed to match the JEDI version in lines 27-32. If running GSI version of ENKFONLY will want to archive increment files for that run as well.
There was a problem hiding this comment.
@TravisElless-NOAA I see. I will add one block for DOENKFONLY_ATM=YES to match Line 27-29 and add the increment files for GSI enkf-only run as well. Thanks.
|
@bhuang95
With
|
@CoryMartin-NOAA I fixed most of shell script failure given what was suggested. I am not sure why the shellcheck still failed and I don't see any obvious issues now (some warning triggered this?). Is there a log file for this shellcheck failure? |
@WalterKolczynski-NOAA The |
|
the |
|
@CoryMartin-NOAA Finally fixed the shellcheck failure and passed all checks. Learnt a lot how this process works, thank you all! |
This PR works along with the following three dependent PRs to enable the EnKF-only configuration for the atmosphere within the global workflow (see detailed description NOAA-EMC/global-workflow#4345) Dependencies: -NOAA-EMC/global-workflow#4345 -NOAA-EMC/GDASApp#2010 -NOAA-EMC/jcb#32 Resolve - NOAA-EMC/global-workflow#4339 --------- Co-authored-by: Cory Martin <cory.r.martin@noaa.gov>
This PR works along with the following three dependent PRs to enable the EnKF-only configuration for the atmosphere within the global workflow (see detailed description NOAA-EMC/global-workflow#4345) Dependencies: -NOAA-EMC/global-workflow#4345 -#2010 -NOAA-EMC/jcb#32 Resolve - NOAA-EMC/global-workflow#4339 --------- Co-authored-by: Cory Martin <cory.r.martin@noaa.gov>
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| if [[ ! -d "${COMOUT_ATMOS_ANALYSIS_PREV}" ]]; then mkdir -p "${COMOUT_ATMOS_ANALYSIS_PREV}"; fi | ||
| if [[ ! -d "${OUTPUT}" ]]; then mkdir -p "${OUTPUT}"; fi |
There was a problem hiding this comment.
You can safely reduce this to just
mkdir -p "${COMOUT_ATMOS_ANALYSIS_PREV}"
mkdir -p "${OUTPUT}"mkdir -p does not raise an error or touch directories if they already exist. If you prefer to keep this in if-blocks, we prefer them in multi-line blocks.
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
|
|
||
| if [[ ${DO_JEDIATMENS} == "YES" ]]; then | ||
| # JEDI run | ||
| cd "${DATA}" || exit |
There was a problem hiding this comment.
| cd "${DATA}" || exit | |
| cd "${DATA}" || exit 1 |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| if [[ ${DO_JEDIATMENS} == "YES" ]]; then | ||
| # JEDI run | ||
| cd "${DATA}" || exit | ||
| if [[ ! -d "testrun/varbc" ]]; then mkdir -p "testrun/varbc"; fi |
There was a problem hiding this comment.
| if [[ ! -d "testrun/varbc" ]]; then mkdir -p "testrun/varbc"; fi | |
| mkdir -p "testrun/varbc" |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| if [[ ! -d "testrun/varbc" ]]; then mkdir -p "testrun/varbc"; fi | ||
| /bin/ln -sf "${ABIAS_SAT}" "./satbias_in" | ||
| /bin/ln -sf "${ABIASPC_SAT}" "./satbias_pc" | ||
| grep -i "NaN" satbias_in && echo "Stop. There are NaN in ${ABIAS}." && exit 1 |
There was a problem hiding this comment.
Prefer this as an if-block and err_exit or err_chk should be called:
| grep -i "NaN" satbias_in && echo "Stop. There are NaN in ${ABIAS}." && exit 1 | |
| if ! grep -i "NaN" satbias_in > /dev/null 2>&1 ; then | |
| export err=1 | |
| err_chk "FATAL ERROR There are NaN in ${ABIAS}." | |
| fi |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
|
|
||
| export err=$? | ||
| if [[ ${err} -ne 0 ]]; then | ||
| err_exit "satbias2iodas.x failed for ${instrument}, ABORT!" |
There was a problem hiding this comment.
Fatal errors should be labeled:
| err_exit "satbias2iodas.x failed for ${instrument}, ABORT!" | |
| err_exit "FATAL ERROR satbias2iodas.x failed for ${instrument}, ABORT!" |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| cd ./testrun/varbc/ | ||
| grep "${instrument}" ../../satbias_in | awk '{print $2" "$3" "$4}' > \ | ||
| "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".tlapse.txt | ||
| /bin/cp -p satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias.nc |
There was a problem hiding this comment.
We try to use cpreq unless the file is optional. If it is optional, then a warning should be printed if it is missing
| /bin/cp -p satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias.nc | |
| cpreq -p satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias.nc |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| grep "${instrument}" ../../satbias_in | awk '{print $2" "$3" "$4}' > \ | ||
| "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".tlapse.txt | ||
| /bin/cp -p satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias.nc | ||
| /bin/mv satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias_cov.nc |
There was a problem hiding this comment.
It's probably fine to use /bin/mv on any POSIX system, but in general, we should allow the PATH variable to determine the command to use.
| /bin/mv satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias_cov.nc | |
| mv satbias_"${instrument}".nc4 "${OUTPUT}"/gdas.t"${gcyc}"z."${instrument}".satbias_cov.nc |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| # GSI run | ||
| for file in ${BIASFILES}; do | ||
| cpreq "${BIASDIR}/${file}" "${COMOUT_ATMOS_ANALYSIS_PREV}/enkf${file}.txt" | ||
| err=$? |
There was a problem hiding this comment.
err needs to be exported for err_exit to capture it properly
| err=$? | |
| export err=$? |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| cpreq "${BIASDIR}/${file}" "${COMOUT_ATMOS_ANALYSIS_PREV}/enkf${file}.txt" | ||
| err=$? | ||
| if [[ ${err} -ne 0 ]]; then | ||
| err_exit "Error copying operational anl bias correction files" |
There was a problem hiding this comment.
| err_exit "Error copying operational anl bias correction files" | |
| err_exit "FATAL ERROR copying operational anl bias correction files" |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| tar -cvf "${ABIAS_AIR_JEDI_TAR}" "./enkf${GDUMP}.t${gcyc}z.abias_air.txt" | ||
| rm -rf "./enkf${GDUMP}.t${gcyc}z.abias_air.txt" | ||
| tar -cvf "${ABIAS_SAT_JEDI_TAR}" ./*.nc ./*.txt | ||
| err=$? |
There was a problem hiding this comment.
| err=$? | |
| export err=$? |
dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS
Outdated
| fi | ||
|
|
||
| if [[ ${err} -ne 0 ]]; then | ||
| err_exit "Error executing ${EXSCRIPT}" |
There was a problem hiding this comment.
| err_exit "Error executing ${EXSCRIPT}" | |
| err_exit "FATAL ERROR executing ${EXSCRIPT}" |
| if [[ "${DOENKFONLY_ATM:-NO}" == "YES" ]] ; then | ||
| export RECENTER_ENKF="NO" # Turn off recentering ensemble analysis | ||
| export DO_VERFOZN="NO" # Ozone data assimilation monitoring | ||
| export DO_VERFRAD="NO" # Radiance data assimilation monitoring | ||
| export DO_VMINMON="NO" # GSI minimization monitoring | ||
| export DO_METP="NO" # Run METPLUS jobs - set METPLUS settings in config.metp | ||
| export DO_FIT2OBS="NO" # Run fit to observations package | ||
| export DO_TRACKER="NO" # Hurricane track verification | ||
| export DO_GENESIS="NO" # Cyclone genesis verification | ||
| fi |
There was a problem hiding this comment.
@aerorahul I wonder if this would be better to have in dev/workflow/applications/gfs_*.py. What do you think?
This PR works along with the following three dependent PRs to enable the EnKF-only configuration for the atmosphere within the global workflow (see detailed description NOAA-EMC/global-workflow#4345) Dependencies: -NOAA-EMC/global-workflow#4345 -NOAA-EMC/jcb-gdas#219 -NOAA-EMC/jcb#32 Resolve - NOAA-EMC/global-workflow#4339 --------- Co-authored-by: Bo Huang <Bo.Huang@gaea65.ncrc.gov>
This PR works along with the following three dependent PRs to enable the EnKF-only configuration for the atmosphere within the global workflow (see detailed description NOAA-EMC/global-workflow#4345) Dependencies: -NOAA-EMC/global-workflow#4345 -NOAA-EMC/jcb-gdas#219 -NOAA-EMC/jcb#32 Resolve - NOAA-EMC/global-workflow#4339 --------- Co-authored-by: Bo Huang <Bo.Huang@gaea65.ncrc.gov>
|
This PR requires the changes in the following PRs The @bhuang95 : Do you intended to add an EnKF-only atmosphere g-w CI case at some point? Failure to do so opens us to the possibility of the EnKF-only capability being unknowingly broken by future g-w PRs. Addition of an EnKF-only atmosphere g-w CI case could be the topic of a future g-w issue and PR. |
|
@bhuang95 , whether before or after the above PRs are merged into their respective |
|
@bhuang95 : GDASApp PR #2051 has been merged into GDASApp Please update the I can test once |
|
@RussTreadon-NOAA I need to work on some other tasks today. I will update my g-w bhuang95:feature/enkf_only_dev (including updated sorc/gdas.cd) and address @DavidHuber-NOAA's suggested changes tomorrow morning. Quick question to your CI test question for this PR: yes, we need to do that probably in the future PR as @aerorahul suggested before. |
|
Thank you @bhuang95 for the update. I'll resume work on this PR after you address remaining issues. It should be fairly easy to create a new CI EnKF-only atmosphere CI case from existing cases. |
|
@DavidHuber-NOAA Thanks for your suggested changes. I have addressed most of them in this updated PR. @RussTreadon-NOAA A quick sanity check on this branch would be with no further changes it would not affect the existing CI tests because of by default DOENKFONLY_ATM: NO. Please let me know how your test goes. @aerorahul Here are some thoughts for your previous high-level suggestions.
In the current implementation, my goal was to minimize changes to the workflow so I chose to define this high-level My impression: it would require more extensive changes if we use Thank you all for reviewing this PR. |
6 participants
Description
This PR, along with its three dependent PRs listed below, enables running enkf only for atmosphere in the global-workflow with necessary tasks. It will help test JEDI EnKF capabilities in the global workflow and compare with GSI in support of hybrid 4DEnVar appplication for the atmosphere.
This was further modified based on the original branch from Travis J Elless at NOAA EMC.
Major change summary from three PRs:
This PR:
DOENKFONLY_ATM(false by default) in the dev/parm/config/gfs/config.base.j2 to activate the GSI and JEDI EnKF-only run.fetchatmanlbiasandprepatmanlbiasto fetch operational analysis bias correction files at the previous cycle and process them for JEDI and GSI EnKF use (dev/jobs/JGLOBAL_ATM_PREP_ANL_BIAS)Resolves:
Dependencies:
Acceptance Criteria
DOENKFONLY_ATM="NO"by default, it should not affect existing applications in the current global workflow. In other words, it should pass all relevant tests following the general guidelines of building the global workflow.DOENKFONLY_ATM="YES"in the config.base during the workflow setup, it should generate related tasks for GSI and JEDI EnKF-only run with additional variable changes in config.base, respectively, and pass all these tasks through rocoto jobs.Type of change
Change characteristics
How has this been tested?
-Built and cycled successfully on Ursa as follows.
/scratch4/BMC/gsienkf/Bo.Huang/expCodes/Workflow/Data/IC/TravisIC-2022010312/Checklist