[develop] Add jinja templating to all config file formats. #701

danielabdi-noaa · 2023-03-28T03:02:10Z

DESCRIPTION OF CHANGES:

Mainly addresses issue #700

Add jinja templating for a "block of lines" for all config file formats. This is different from what extend_yaml does, namely, fill template values of dictionary entries. Given a second file containing the context (dictionary) for rendering the jinja template, you can now do this:

{% if DO_REAL_TIME %}
WALL_LIMIT_ANAL: 16:00:00
WALL_LIMIT_FCST: 24:00:00
{% else %}
WALL_LIMIT_ANAL: ""
WALL_LIMIT_FCST: ""
{% endif %}

instead of

WALL_LIMIT_ANAL: '{% if DO_REAL_TIME %}"16:00:00"{% else %}""{% endif %}'
WALL_LIMIT_FCST: '{% if DO_REAL_TIME %}"16:00:00"{% else %}""{% endif %}'

If these variables are defined only for realtime runs, the first option without the {% else %} will not define them, while the second option still has to define the variables to null string.

I think this feature is required for the upcoming rrfs workflow with yaml template files.

Other changes include:

Parse xml attributes as dictionary entries
Parse repeated entries into a list
Bug fix with matching single/double quotes

With this change you should be able to parse a rendered xml file (not the one in parm) and convert it to any of the five formats.

./config_utils.py -c $EXPTDIR/FV3LAM_wflow.xml -o yaml

If the context for rendering is available in file context.yaml, we can render and convert the templated file instead like this

./config_utils.py -c $PARMdir/FV3LAM_wflow.xml -x $PWD/context.yaml -o yaml

Edit: Work done after this PR's approval

Added F90 namelist as one of the supported config formats. You should be able to convert to/from nml files now including workflow xml files.
Made shell/ini formats work with deeply nested dictionaries. This means workflow files in these formats should work now, but the format does not look pretty, so this is just for completeness i.e. enable all six formats for workflow configuration.
Some bug fixes to make sure that a yaml/json workflow file work as intended. Tested with:

custom_GFDLgrid
community_ensemble_2mems

The test is conducted by converting the generated FV3LAM_wflow.xml file say to yaml using this tool, and then back to xml.
The resulting xml file replaces the one in EXPTDIR and the test re-run again. The above tests succeeded using this verification method.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

TESTS CONDUCTED:

DEPENDENCIES:

None

DOCUMENTATION:

ISSUE:

Jinja templating for config file formats #700

CHECKLIST

My code follows the style guidelines in the Contributor's Guide
I have performed a self-review of my own code using the Code Reviewer's Guide
I have commented my code, particularly in hard-to-understand areas
My changes need updates to the documentation. I have made corresponding changes to the documentation
My changes do not require updates to the documentation (explain).
My changes generate no new warnings
New and existing tests pass with my changes
Any dependent changes have been merged and published

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

CONTRIBUTORS (optional):

panll · 2023-04-04T05:38:03Z

I tested on Hera. It has following running error messages:
./config_utils.py -c ../parm/FV3LAM_wflow.xml -o yaml
Traceback (most recent call last):
File "/scratch2/BMC/fv3lam/lpan/test/review/04032023/ufs-srweather-app/ush/./config_utils.py", line 14, in
cfg_main()
File "/scratch2/BMC/fv3lam/lpan/test/review/04032023/ufs-srweather-app/ush/python_utils/config_parser.py", line 765, in cfg_main
cfg = load_config_file(args.cfg, 2, args.context)
File "/scratch2/BMC/fv3lam/lpan/test/review/04032023/ufs-srweather-app/ush/python_utils/config_parser.py", line 703, in load_config_file
return load_xml_config(config_file_or_string, return_string)
File "/scratch2/BMC/fv3lam/lpan/test/review/04032023/ufs-srweather-app/ush/python_utils/config_parser.py", line 527, in load_xml_config
tree = ET.parse(config_file_or_string)
File "/scratch1/NCEPDEV/nems/role.epic/miniconda3/4.12.0/envs/regional_workflow/lib/python3.9/xml/etree/ElementTree.py", line 1229, in parse
tree.parse(source, parser)
File "/scratch1/NCEPDEV/nems/role.epic/miniconda3/4.12.0/envs/regional_workflow/lib/python3.9/xml/etree/ElementTree.py", line 580, in parse
self._root = parser._parse_whole(source)
xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 1, column 0

danielabdi-noaa · 2023-04-04T10:27:52Z

@panll You can parse the xml workflow file under parm/ directory only iff you provide a context file with dictionary to render the jinja templates. Without it, it will consider the xml file invalid.

./config_utils.py -c $PARMdir/FV3LAM_wflow.xml -x $PWD/context.yaml -o yaml

If you try an xml file in an EXPTDIR instead, it should work with the commands you used.

./config_utils.py -c $EXPTDIR/FV3LAM_wflow.xml -o yaml

You may encounter issue #691 with an old experiment directory, but it should work with experiment generated from latest develop or this PR. For example this is the output with the community test case:

$ ./config_utils.py -c ../../expt_dirs/test_community/FV3LAM_wflow.xml -o yaml
cycledef:
- attrib:
    group: at_start
  valuea: 201906151800 201906151800 24:00:00
- attrib:
    group: forecast
  valuea: 201906151800 201906151800 24:00:00
log:
  cyclestr: /scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/test_community/log/FV3LAM_wflow.log
task:
- attrib:
    name: make_grid
    cycledefs: at_start
    maxtries: '2'
  account: zrtrr
  native: --export=NONE
  nodes: 1:ppn=24
  nodesize: 40
  partition: hera
  queue: batch
  walltime: 00:20:00
  command: /scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush/load_modules_run_task.sh
    "make_grid" "/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/jobs/JREGIONAL_MAKE_GRID"
  join:
    cyclestr: /scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/test_community/log/make_grid_@Y@m@[email protected]
  jobname: make_grid
  envar:
  - name: GLOBAL_VAR_DEFNS_FP
    value: /scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/test_community/var_defns.sh
  - name: USHdir
    value: /scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush
  - name: PDY
    value:
      cyclestr: '@Y@m@d'
  - name: cyc
    value:
      cyclestr: '@H'
  - name: subcyc
    value:
      cyclestr: '@M'
  - name: LOGDIR
    value:
      cyclestr: /scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/test_community/log
  - name: nprocs
    value: 24
....

christinaholtNOAA · 2023-04-05T16:42:26Z

It's important to note that the YAML created from this tool will not be sufficient to create an XML given the tool we recently have.

danielabdi-noaa · 2023-04-05T17:07:23Z

@christinaholtNOAA It can generate the workflow xml file from a rendered yaml file, because we can convert from yaml to xml too. However, it would need a jinja-templated equivalent of the original FV3LAM_wflow.xml file, which will be FV3LAM_wflow.yaml, to generate workflow for any test case.

$ ./config_utils.py -c ../../expt_dirs/custom_GFDLgrid/FV3LAM_wflow.xml -o yaml > wflow.yaml

$ ./config_utils.py -c ./wflow.yaml -o xml

<?xml version="1.0" ?>
<root>
  <cycledef group="at_start">201907010000 201907010000 24:00:00</cycledef>
  <cycledef group="forecast">201907010000 201907010000 24:00:00</cycledef>
  <log>
    <cyclestr>/scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/custom_GFDLgrid/log/FV3LAM_wflow.log</cyclestr>
  </log>
  <task name="make_grid" cycledefs="at_start" maxtries="2">
    <account>zrtrr</account>
    <native>--export=NONE</native>
    <nodes>1:ppn=24</nodes>
    <nodesize>40</nodesize>
    <partition>hera</partition>
    <queue>batch</queue>
    <walltime>00:20:00</walltime>
    <command>/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush/load_modules_run_task.sh "make_grid" "/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/jobs/JREGIONAL_MAKE_GRID"</command>
    <join>
      <cyclestr>/scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/custom_GFDLgrid/log/make_grid_@Y@m@[email protected]</cyclestr>
    </join>
    <jobname>make_grid</jobname>
    <envar>
      <name>GLOBAL_VAR_DEFNS_FP</name>
      <value>/scratch2/BMC/gsd-hpcs/Daniel.Abdi/expt_dirs/custom_GFDLgrid/var_defns.sh</value>
    </envar>
    <envar>
      <name>USHdir</name>
      <value>/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/ush</value>
    </envar>
....

danielabdi-noaa · 2023-04-06T01:19:35Z

@panll If you can re-test and approve this PR, assuming there are no issues, it would be great since I need this for my next PR.

MichaelLueken · 2023-04-07T19:47:29Z

@danielabdi-noaa @christinaholtNOAA While working through issues in PR #707, it has become clear that using config_utils.py to validate a config.yaml file no longer works. Using:

./config_utils.py -c ./config.community.yaml -v ./config_defaults.yaml

leads to the following:

INVALID ENTRY: metatask_run_ensemble={'task_run_fcst_mem#mem#': {'walltime': '02:00:00'}}
FAILURE

Is this something that could be addressed in this PR, or should a separate issue be created for this problem? Thanks!

dshawul · 2023-04-07T20:41:18Z

@MichaelLueken I think ignoring validation of the new rocoto: section should make it work. I will modify it that way

MichaelLueken · 2023-04-07T20:47:34Z

@danielabdi-noaa I should also note that this behavior was noted on Orion. Testing this on Gaea, this wasn't an issue. However, your strategy sounds like it will work for any machine. Thanks!

danielabdi-noaa · 2023-04-07T22:34:44Z

@MichaelLueken Since hard-coding rocoto: section will make the tool less generic, I have made it so that we can pass the section name (in this case rocoto) that we want to exclude from testing.

$ ./config_utils.py -c ./config.community.yaml -v ./config_defaults.yaml -k "(?\!rocoto\b)"
SUCCESS

christinaholtNOAA

My apologies that I did not fully review this PR on my first pass.

This addition is, IMO, not a desired feature. This will absolutely convolute how we do configuration in the App. We should really have one way to do a task, and this one circumvents the path forward that I had been trying to apply for a fairly clean way to create a workflow without prolific numbers of control switches.

I'd propose that we do not go this route at all.

panll · 2023-04-11T06:32:00Z

It works now, thanks! @danielabdi-noaa @MichaelLueken

christinaholtNOAA · 2023-04-14T17:40:33Z

@MichaelLueken I'd suggest a "Do Not Merge" flag until we have a full discussion around the Google Doc that @danielabdi-noaa introduced in the meeting this morning.

MichaelLueken · 2023-04-14T18:03:46Z

Per the discussion during today's SRW CM meeting, I have added the DO_NOT_MERGE label to this PR.

…nity#718) Added the capability for the SRW App to use GDAS nemsio data type as initial conditions. --------- Co-authored-by: Edward Snyder <[email protected]>

MichaelLueken · 2023-05-17T19:56:06Z

@danielabdi-noaa The reviewers voted to close this PR until demonstrably necessary to ensure that the workflow works correctly.

Thank you very much for discussing the issues that you see with the current SRW workflow. It is clear that a lot more work is required to get the existing workflow into a spot where we will all be happy.

danielabdi-noaa linked an issue Mar 28, 2023 that may be closed by this pull request

Jinja templating for config file formats #700

Open

danielabdi-noaa marked this pull request as ready for review March 28, 2023 14:34

danielabdi-noaa force-pushed the feature/config_jinja branch 2 times, most recently from ff841b8 to e1d3c73 Compare March 29, 2023 15:31

danielabdi-noaa added 2 commits April 1, 2023 01:35

Add jinja templating to all config file formats.

005dc18

Use valuea less frequently.

987fce1

danielabdi-noaa force-pushed the feature/config_jinja branch from e1d3c73 to c21ae6b Compare April 1, 2023 16:36

Modification to make rendering be called from setup.

6208b5e

danielabdi-noaa force-pushed the feature/config_jinja branch from c21ae6b to 6208b5e Compare April 1, 2023 18:14

Fix loading config from string.

1741f6b

MichaelLueken added the run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests label Apr 4, 2023

Fix config file validation with rocoto section.

9e1c5e7

MichaelLueken mentioned this pull request Apr 10, 2023

[develop] cicd functional testing script #707

Merged

30 tasks

This was referenced Apr 10, 2023

[develop] Integrate jinja-enabled config files and add rrfs workflow danielabdi-noaa/ufs-srweather-app#2

Closed

[develop] Integrate jinja-templated workflow and add some rrfs workflow files. #721

Closed

christinaholtNOAA reviewed Apr 10, 2023

View reviewed changes

panll approved these changes Apr 11, 2023

View reviewed changes

MichaelLueken added the DO_NOT_MERGE Ensure that a PR isn't merged label Apr 14, 2023

danielabdi-noaa and others added 9 commits April 15, 2023 18:19

Use root of xml if provided.

1ffc402

Treat integers that start with 0 as strings.

9fbad58

Merge branch 'develop' into feature/config_jinja

23c4826

[develop] Add GDAS nemsio data type for initial conditions (ufs-commu…

ae0d994

…nity#718) Added the capability for the SRW App to use GDAS nemsio data type as initial conditions. --------- Co-authored-by: Edward Snyder <[email protected]>

Make sh/ini formats work for nested dictionaries, hence workflow.

fd7f747

Add fortran namelist as one of the config formats.

3ab646b

Merge branch 'develop' into feature/config_jinja

6d47fb8

Bug fix ENSMEM_INDX as int.

4de88fb

Merge branch 'develop' into feature/config_jinja

43f10e8

MichaelLueken closed this May 17, 2023

MichaelLueken mentioned this pull request Jul 14, 2023

[develop] Add verification of snowfall accumulation #853

Merged

19 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[develop] Add jinja templating to all config file formats. #701

[develop] Add jinja templating to all config file formats. #701

danielabdi-noaa commented Mar 28, 2023 •

edited

Loading

panll commented Apr 4, 2023

danielabdi-noaa commented Apr 4, 2023

christinaholtNOAA commented Apr 5, 2023

danielabdi-noaa commented Apr 5, 2023 •

edited

Loading

danielabdi-noaa commented Apr 6, 2023

MichaelLueken commented Apr 7, 2023

dshawul commented Apr 7, 2023

MichaelLueken commented Apr 7, 2023

danielabdi-noaa commented Apr 7, 2023 •

edited

Loading

christinaholtNOAA left a comment

panll commented Apr 11, 2023

christinaholtNOAA commented Apr 14, 2023

MichaelLueken commented Apr 14, 2023

MichaelLueken commented May 17, 2023

[develop] Add jinja templating to all config file formats. #701

[develop] Add jinja templating to all config file formats. #701

Conversation

danielabdi-noaa commented Mar 28, 2023 • edited Loading

DESCRIPTION OF CHANGES:

Type of change

TESTS CONDUCTED:

DEPENDENCIES:

DOCUMENTATION:

ISSUE:

CHECKLIST

LABELS (optional):

CONTRIBUTORS (optional):

panll commented Apr 4, 2023

danielabdi-noaa commented Apr 4, 2023

christinaholtNOAA commented Apr 5, 2023

danielabdi-noaa commented Apr 5, 2023 • edited Loading

danielabdi-noaa commented Apr 6, 2023

MichaelLueken commented Apr 7, 2023

dshawul commented Apr 7, 2023

MichaelLueken commented Apr 7, 2023

danielabdi-noaa commented Apr 7, 2023 • edited Loading

christinaholtNOAA left a comment

Choose a reason for hiding this comment

panll commented Apr 11, 2023

christinaholtNOAA commented Apr 14, 2023

MichaelLueken commented Apr 14, 2023

MichaelLueken commented May 17, 2023

danielabdi-noaa commented Mar 28, 2023 •

edited

Loading

danielabdi-noaa commented Apr 5, 2023 •

edited

Loading

danielabdi-noaa commented Apr 7, 2023 •

edited

Loading