Do not create a combined yaml file anymore #348

singhd789 · 2025-02-10T21:19:59Z

Describe your changes

instead of copying and combining files, read yaml info as strings
append strings to combine yaml information across yamls
load combined yaml info string
split up compile and pp classes to make script simpler and easier to follow

Issue ticket number and link (if applicable)

Checklist before requesting a review

I ran my code
I tried to make my code readable
I tried to comment my code
I wrote a new test, if applicable
I wrote new instructions/documentation, if applicable
I ran pytest and inspected it's output
I ran pylint and attempted to implement some of it's feedback

- instead read yaml info as strings - append strings to combine yaml information across yamls - load combined yaml info string - split up compile and pp classes to make script simpler and easier to follow

- `output` option would dump final, cleaned yaml info to a file

…-comb-file

- Multiple scripts rely/use this tool - PR would be too big if I tried to fix each one with this reworked tool

- this file wouldn't exist anymore

- outputs a combined yaml file for other tools to use/reference

- other tools also need to be changed with these PR changes so this older script is kept to not break multiple tools

ilaflott · 2025-02-19T18:14:22Z

I'm working on a different part that requires yaml combining, and incidentally i've tested part of this, so i'll comment on that here now. Overall impression is this is solid work-in-progress!

The following call works great:

> fre yamltools combine-yamls -y fre/yamltools/tests/AM5_example/am5.yaml -e c96L65_am5f7b12r1_amip -p ncrc5.intel -t prod-openmp --use pp --output FOO.yaml
Combining yaml files into one dictionary: 
   model yaml: fre/yamltools/tests/AM5_example/am5.yaml
   experiment yaml: pp.c96_amip.yaml
   experiment yaml: pp-TEST.c96_amip.yaml
   analysis yaml: clouds.yaml
   analysis yaml: land.yaml
> cat FOO.yaml
name: c96L65_am5f7b12r1_amip
platform: ncrc5.intel
target: prod-openmp
build:
  compileYaml: compile_yamls/compile.yaml
  platformYaml: compile_yamls/platforms.yaml
directories:
  history_dir: /archive/$USER/am5/am5f7b12r1/c96L65_am5f7b12r1_amip/ncrc5.intel-prod-openmp/history
  pp_dir: /archive/$USER/am5/am5f7b12r1/c96L65_am5f7b12r1_amip/ncrc5.intel-prod-openmp/pp
  analysis_dir: /nbhome/$USER/am5/am5f7b12r1/c96L65_am5f7b12r1_amip
  ptmp_dir: /ptmp/$USER
  fre_analysis_home: /home/fms/local/opt/fre-analysis/test
  pp_grid_spec: /archive/oar.gfdl.am5/model_gen5/inputs/c96_grid/c96_OM4_025_grid_No_mg_drag_v20160808.tar
postprocess:
  settings:
    history_segment: P1Y
    site: ppan
    pp_start: '1980'
    pp_stop: '2020'
    pp_chunk_a: P1Y
    pp_components: atmos atmos_scalar
  switches:
    do_statics: false
    do_timeavgs: true
    clean_work: true
    do_refinediag: false
    do_atmos_plevel_masking: true
    do_preanalysis: false
    do_analysis: true
  components:
  - type: atmos_cmip-TEST
    sources: atmos_month_cmip atmos_8xdaily_cmip atmos_daily_cmip
    sourceGrid: cubedsphere
    xyInterp: 180,360
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos-TEST
    sources: atmos_month
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos_cmip
    sources: atmos_month_cmip atmos_8xdaily_cmip atmos_daily_cmip
    sourceGrid: cubedsphere
    xyInterp: 180,360
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos
    sources: atmos_month
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos_level_cmip
    sources: atmos_level_cmip
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos_level
    sources: atmos_month
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos_month_aer
    sources: atmos_month_aer
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order1
    inputRealm: atmos
  - type: atmos_diurnal
    sources: atmos_diurnal
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order2
    inputRealm: atmos
  - type: atmos_scalar
    sources: atmos_scalar
  - type: aerosol_cmip
    xyInterp: 180,288
    sources: aerosol_month_cmip
    sourceGrid: cubedsphere
    interpMethod: conserve_order1
    inputRealm: atmos
  - type: land
    sources: land_month
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order1
    inputRealm: land
  - type: land_cmip
    sources: land_month_cmip
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order1
    inputRealm: land
  - type: tracer_level
    sources: atmos_tracer
    sourceGrid: cubedsphere
    xyInterp: 180,288
    interpMethod: conserve_order1
    inputRealm: atmos
analysis:
  land-test:
    required:
      data_frequency: mon-test
      date_range:
      - '1980'
      - '2020'
    workflow:
      components:
      - land-test
      cumulative: false
      product: ts-test
      script_frequency: R1
      switch: true
  clouds-test:
    required:
      data_frequency: mon
      date_range:
      - '1980'
      - '2020'
    workflow:
      components:
      - atmos-test
      cumulative: false
      product: t-test
      script_frequency: P1Y
      switch: true

switching the flag from pp to compile generates an error however, that I'd wager @singhd789 is well-aware of, but i'll put my call+output here to help w/ debugging efforts:

> fre yamltools combine-yamls -y fre/yamltools/tests/AM5_example/am5.yaml -e c96L65_am5f7b12r1_amip -p ncrc5.intel -t prod-openmp --use compile --output BAR.yaml
Traceback (most recent call last):
  File "/home/Ian.Laflotte/conda/envs/fre_cli/bin/fre", line 33, in <module>
    sys.exit(load_entry_point('fre-cli', 'console_scripts', 'fre')())
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1161, in __call__
    return self.main(*args, **kwargs)
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1082, in main
    rv = self.invoke(ctx)
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1443, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 788, in invoke
    return __callback(*args, **kwargs)
  File "/home/Ian.Laflotte/Working/fre-cli/fre/yamltools/freyamltools.py", line 43, in combine_yamls
    combine_yamls_script.consolidate_yamls(yamlfile,experiment,platform,target,use,output)
  File "/home/Ian.Laflotte/Working/fre-cli/fre/yamltools/combine_yamls_script.py", line 102, in consolidate_yamls
    combined = cip.InitCompileYaml(yamlfile, platform, target, join_constructor)
AttributeError: module 'fre.yamltools.compile_info_parser' has no attribute 'InitCompileYaml'

singhd789 · 2025-02-19T18:23:45Z

@ilaflott I think that compile issue was actually just fixed with one of the last pushes (forgot to change the class name previously)

Also realized I forgot to update/create tests for the updated version of the tool though

ilaflott · 2025-02-19T18:49:59Z

You were right- i needed a pull. Different error-

> fre yamltools combine-yamls -y fre/yamltools/tests/AM5_example/am5.yaml -e c96L65_am5f7b12r1_amip -p ncrc5.intel -t prod-openmp --use compile --output BAR.yaml
Combining yaml files into one dictionary: 
Traceback (most recent call last):
  File "/home/Ian.Laflotte/conda/envs/fre_cli/bin/fre", line 33, in <module>
    sys.exit(load_entry_point('fre-cli', 'console_scripts', 'fre')())
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1161, in __call__
    return self.main(*args, **kwargs)
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1082, in main
    rv = self.invoke(ctx)
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 1443, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/Ian.Laflotte/conda/envs/fre_cli/lib/python3.9/site-packages/click/core.py", line 788, in invoke
    return __callback(*args, **kwargs)
  File "/home/Ian.Laflotte/Working/fre-cli/fre/yamltools/freyamltools.py", line 43, in combine_yamls
    combine_yamls_script.consolidate_yamls(yamlfile,experiment,platform,target,use,output)
  File "/home/Ian.Laflotte/Working/fre-cli/fre/yamltools/combine_yamls_script.py", line 109, in consolidate_yamls
    get_combined_compileyaml(combined,experiment,output)
TypeError: get_combined_compileyaml() takes 1 positional argument but 3 were given

singhd789 · 2025-02-19T19:36:35Z

@ilaflott Alright, pull one more time ....

A test is still failing - not sure why yet (printing the dictionaries that are being compared, they both look the same)

ilaflott · 2025-02-24T20:51:09Z

So i did come up against the dictionary-comparison thing the other day, and I figured the lack-of-equality was because the order of the elements isn't equivalent. I.e., the dictionaries are automatically "ordered", see stack overflow

singhd789 · 2025-02-24T21:14:34Z

So i did come up against the dictionary-comparison thing the other day, and I figured the lack-of-equality was because the order of the elements isn't equivalent. I.e., the dictionaries are automatically "ordered", see stack overflow

@ilaflott Huh interesting. I had that though initially too but then the test passed locally, which led me to believe the order might not matter (I also read that it shouldn't somewhere)

EDIT: got it to find the error - not sure what I was doing before 😅

singhd789 · 2025-02-24T21:42:25Z

Found the last test was combining pp2 (defined in test) twice - not sure why (when I list the experiment yamls that it should be combining, all 3 are only listed once)

UPDATE: dumb typo made by moi

ilaflott

looks good- merging for my branch's sake...

Dana Singh added 16 commits February 10, 2025 16:10

#346 Do not make the combined yaml file anymore

daeb5bd

- instead read yaml info as strings - append strings to combine yaml information across yamls - load combined yaml info string - split up compile and pp classes to make script simpler and easier to follow

#346 Add in cleaning the yaml info

9d005b6

#346 Fix functions for merging yaml info for post-processing

5f59da1

#346 Add in an output option

213dde2

- `output` option would dump final, cleaned yaml info to a file

#346 Update combine_yamls script

c13cf6e

#346 Rename combine_compile.py and combine_pp.py

2ed5072

#346 Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 346.no…

3078409

…-comb-file

#346 Keep old combine-yamls tool

e425faa

- Multiple scripts rely/use this tool - PR would be too big if I tried to fix each one with this reworked tool

#346 Move print statements, return yaml dictionary instead

12519a6

#346 Use re-worked combine-yamls tool in configure_script_yaml.py

703310c

#346 Remove test for combined yaml

df7686f

- this file wouldn't exist anymore

#346 combined file would not exist

c28814a

#346 Provide output option for pp tools

6ffb6fd

- outputs a combined yaml file for other tools to use/reference

#346 Add back test to check for existence of combined yaml

1e5aafa

#346 Update scripts

1c6a681

#346 Note original combine_yamls.py kept

ce7c8f1

- other tools also need to be changed with these PR changes so this older script is kept to not break multiple tools

singhd789 marked this pull request as ready for review February 19, 2025 18:06

ilaflott self-requested a review February 19, 2025 18:10

singhd789 marked this pull request as draft February 19, 2025 18:24

ilaflott mentioned this pull request Feb 19, 2025

320.cmor yaml parsing #363

Closed

8 tasks

Dana Singh added 2 commits February 19, 2025 14:16

#346 Add test file for combine_yamls_script.py

8639fa1

#346 Fix output option for combining compile yamls

c44e93c

Dana Singh added 2 commits February 19, 2025 15:04

#346 Fix tests for combined compile operations

0a126ad

#346 Update comment

9e73466

singhd789 marked this pull request as ready for review February 19, 2025 21:54

#346 Try to get rid of some nested if statements

fbea263

Dana Singh added 2 commits February 20, 2025 15:24

#346 Fix error messages

55766cb

#346 Try to get rid of oh so many nested if statements

66e8931

Dana Singh added 2 commits February 24, 2025 16:53

#346 Add sort_keys = False

ab83cb5

#346 Remove repetitive addition to a list

4f391e3

ilaflott approved these changes Feb 25, 2025

View reviewed changes

ilaflott merged commit a16e093 into main Feb 25, 2025
2 checks passed

ilaflott deleted the 346.no-comb-file branch February 25, 2025 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not create a combined yaml file anymore #348

Do not create a combined yaml file anymore #348

singhd789 commented Feb 10, 2025 •

edited

Loading

ilaflott commented Feb 19, 2025

singhd789 commented Feb 19, 2025 •

edited

Loading

ilaflott commented Feb 19, 2025

singhd789 commented Feb 19, 2025 •

edited

Loading

ilaflott commented Feb 24, 2025 •

edited

Loading

singhd789 commented Feb 24, 2025 •

edited

Loading

singhd789 commented Feb 24, 2025 •

edited

Loading

ilaflott left a comment

Do not create a combined yaml file anymore #348

Do not create a combined yaml file anymore #348

Conversation

singhd789 commented Feb 10, 2025 • edited Loading

Describe your changes

Issue ticket number and link (if applicable)

Checklist before requesting a review

ilaflott commented Feb 19, 2025

singhd789 commented Feb 19, 2025 • edited Loading

ilaflott commented Feb 19, 2025

singhd789 commented Feb 19, 2025 • edited Loading

ilaflott commented Feb 24, 2025 • edited Loading

singhd789 commented Feb 24, 2025 • edited Loading

singhd789 commented Feb 24, 2025 • edited Loading

ilaflott left a comment

Choose a reason for hiding this comment

singhd789 commented Feb 10, 2025 •

edited

Loading

singhd789 commented Feb 19, 2025 •

edited

Loading

singhd789 commented Feb 19, 2025 •

edited

Loading

ilaflott commented Feb 24, 2025 •

edited

Loading

singhd789 commented Feb 24, 2025 •

edited

Loading

singhd789 commented Feb 24, 2025 •

edited

Loading