-
Notifications
You must be signed in to change notification settings - Fork 204
Enable EnKF-only for atmosphere #4345
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
base: develop
Are you sure you want to change the base?
Conversation
… hard-coded (need to revisit)
…del (global_control.nml.IN)
…e temporary file
…e temporary file
TravisElless-NOAA
left a comment
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this clear out the prep jobs you just added/declared previously (line 389)?
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.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
| {% 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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
|
@aerorahul and @TravisElless-NOAA Thanks for your suggestion. In this PR, we would like to set up enkf-only for both GSI and JEDI so that we can run a comparison of both. I used Additionally, I have a another question. The |
@DavidHuber-NOAA I did not see obvious error in the |
|
@bhuang95 by default, shfmt -w dev/job_cards/rocoto/prep.shor if you would like to write a new file so you can do your own comparison, you can use the shfmt dev/job_cards/rocoto/prep.sh -f dev/job_cards/rocoto/prep.sh.shfmt |
|
@bhuang95 can you start by resolving all the conflicts and then we can go from there on scoping out what is needed for this? |
Thanks very much, @CoryMartin-NOAA. I will fix the conflicts including the jcb repo. |
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-gdas#219 Resolve - NOAA-EMC/global-workflow#4339 --------- Co-authored-by: Cory Martin <[email protected]>
@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 <[email protected]>
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 <[email protected]>
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)NOAA-EMC/GDASApp#2010:
NOAA-EMC/jcb-gdas#219:
observations/atmosphere-lgetkfdirectory (not used due to changes in jcb below)NOAA-EMC/jcb#32:
obs distributionandobs localizationsblocks to obs configuration in jcb-gdas/ observations/atmosphere/ .obs distributionandobs localizationsare applied for all assimilated obs. However, ifoverride_obs_distribution_localizationsis defined andoverride: truein GDASApp/parm/atm/atm_ens_obs_dist_localizations.yaml.j2, these predefined two blocks of matching observations will be overridden. This adds the flexibility to apply different localization methods/lengths for different observations.Resolves:
Dependencies:
-NOAA-EMC/GDASApp#2010
-NOAA-EMC/jcb-gdas#219
-NOAA-EMC/jcb#32
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