Compile yaml integration and parse correctly #168

singhd789 · 2024-08-23T21:34:07Z

Describe your changes

Integrate the am5 model and compile yamls with the model yaml created for fre pp.
reworking parsing scripts in fremake to get correct information from a combined yaml

Issue ticket number and link (if applicable)

Associated with issue #141

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

… pp)

- this is for later organization and parsing when yamls are combined

- another `-e` option was added to represent the experiment name

- combine model, compile, and platform yaml - parse combined yaml

- added `-e` experiment and `--execute` options - redefine variables with parsed information - createCheckout: fix checkout script running if `--execute` used and checkour script already created

…-integration

- this is done in the separate tool `combine-yamls`

- certain variables were overwritten when bind mode is implemented - these variables are saved in `BACKUP` variables

…-integration

- accidentally removed

singhd789 · 2024-08-30T14:18:50Z

TO-DO:

create pytests: Add tests for fre make tools #142
update am5 yamls: Update/Clean AM5 yamls for fre make #152
create schema for combined yaml: Update schema.json file for validation for combined-[experiment].yaml #167

- combine-yamls here if tool wasn't used

- reflects changes in `fre yamltools combine-yamls` - references defined `init_pp_yamls` class in `combine-yamls` to make combining a default behavior (on top of tool still being available if user wants to combine separately) - update schema.json for pp combined yaml

- new test for validation of combiend yamls - new fail test for if compile yaml path not correct - new fail test for if a value in the compile.yaml is not of right data type

ceblanton · 2024-09-20T14:23:35Z

fre/make/createCheckout.py

+    # If fre yammltools combine-yamls tools was used, the combined yaml should exist
+    if Path(combined_path).exists():
+        full_combined = combined_path
+        print("\nNOTE: Yamls previously merged.")


Can the combined yaml be overwritten each time, maybe with a message?

It seems like fre pp configure-yaml -y am5.yaml should configure the pp workflow based on the am5.yaml but it won't if the combined- file is there.

Yeah, I started to look into making it rewrite as a default, but if we wanted to keep the functionality for a user to use the tool by itself first (fre yamltools combine-yaml), the combined yaml will already exist. Maybe we keep the creating/rewriting of the yaml as a default behavior (in compilation and pp), then have the actual tool itself just be a check of some sorts. Like if someone just wanted to see what the combined yaml would output (without doing any compilation or post-processing).

Thanks, that sounds good. Another option would be to compare the combined- yaml to the one that would be re-created and give a message whether it has changed or is up-to-date.

Oooo yeah, we could do that as well! Maybe that could be a small change in another PR. I'm pretty comfortable making it rewrite as a default behavior in this one, then making a small edit to compare yamls in another

- there was a false success with the previous schema

- update paths in test script based on reorganizations - update test_combine_yamls for validation test, wrong compile file test, wrong data type (invalid yaml) test

fre/make/createCompile.py

thomas-robinson

We need a README.

Address the comments and we should be good.

fre/make/createDocker.py

fre/make/createCompile.py

fre/make/createDocker.py

fre/make/createMakefile.py

thomas-robinson · 2024-09-24T17:22:29Z

fre/make/gfdlfremake/yamlfre.py

+    def __init__(self,compileinfo):
+        """
+        Brief: Read get the compile yaml and fill in the missing pieces
+        Param:
+            - self the compile Yaml object
+            - compileinfo dictionary with compile information from the combined yaml
+        """
+        # compile information from the combined yaml
+        self.yaml = compileinfo
+
+        ## Check the yaml for required things
+        ## Check for required experiment name
+        try:
+            self.yaml["experiment"]
+        except:
+            print("You must set an experiment name to compile \n")
+            raise
+        ## Check for optional libraries and packages for linking in container
+        try:
+            self.yaml["container_addlibs"]
+        except:
+            self.yaml["container_addlibs"]=""
+        ## Check for optional libraries and packages for linking on bare-metal system
+        try:
+            self.yaml["baremetal_linkerflags"]
+        except:
+            self.yaml["baremetal_linkerflags"]=""
+        ## Check for required src
+        try:
+            self.yaml["src"]
+        except:
+            print("You must set a src to specify the sources in modelRoot/"+self.yaml["experiment"]+"\n")
+            raise
+        ## Loop through the src array
+        for c in self.yaml['src']:
+        ## Check for required componenet name
+            try:
+                c['component']
+            except:
+                print("You must set the 'componet' name for each src component")
+                raise
+            ## Check for required repo url
+            try:
+                c['repo']
+            except:
+                print("'repo' is missing from the component "+c['component']+" in "+self.yaml["experiment"]+"\n")
+                raise
+            # Check for optional branch. Otherwise set it to blank
+            try:
+                c['branch']
+            except:
+                c['branch']=""
+            # Check for optional cppdefs. Otherwise set it to blank
+            try:
+                c['cppdefs']
+            except:
+                c['cppdefs']=""
+            # Check for optional doF90Cpp. Otherwise set it to False
+            try:
+                c['doF90Cpp']
+            except:
+                c['doF90Cpp']=False
+            # Check for optional additional instructions. Otherwise set it to blank
+            try:
+                c['additionalInstructions']
+            except:
+                c['additionalInstructions']=""
+            # Check for optional paths. Otherwise set it to blank
+            try:
+                c['paths']
+            except:
+                c['paths']=[c['component']]
+            # Check for optional requires. Otherwise set it to blank
+            try:
+                c['requires']
+            except:
+                c['requires']=[]
+            # Check for optional overrides. Otherwise set it to blank
+            try:
+                c['makeOverrides']
+            except:
+                c['makeOverrides']=""
+            # Check for optional flags. Otherwise set it to blank.
+            try:
+                c["otherFlags"]
+            except:
+                c["otherFlags"]=""


The sad situation of someone else having to clean up an inexperienced python coder's mess :-(

fre/make/runFremake.py

fre/yamltools/combine_yamls.py

thomas-robinson · 2024-09-24T17:34:40Z

fre/yamltools/combine_yamls.py


-    with open(combined,'w',encoding='UTF-8') as f:
-        yaml.safe_dump(full_yaml,f,sort_keys=False)
+    if use == "compile":


im not convinced this is the best variable name here. maybe we can bring it up at a fre meeting.

fre/yamltools/freyamltools.py

singhd789 · 2024-09-25T16:10:56Z

fremake README going to be pushed here: issue #187

ceblanton · 2024-09-25T18:55:37Z

All the tests pass, except one with a known cause, and a clear issue to track (#189).

FAILED fre/tests/test_fre_catalog_cli.py::test_cli_fre_catalog_builder - assert False

where False = all([True, False])

ilaflott · 2024-09-27T17:41:36Z

it's weird that this PR messes with fre/tests/test_fre_cmor_cli.py, is there any aggressive test-file clean up happening in this PR? This test passes in main

singhd789 · 2024-09-27T17:47:48Z

it's weird that this PR messes with fre/tests/test_fre_cmor_cli.py, is there any aggressive test-file clean up happening in this PR? This test passes in main

Not pertaining to test_fre_cmor_cli.py, I don't think. I'm about to check if I have to update my branch from main at all.

…-integration

Otherwise, the cmor test will fail when run afterwards

ilaflott · 2024-09-30T13:55:27Z

looks good, merging!

singhd789 added 12 commits August 14, 2024 00:11

#141 Add fremake model yaml info to standard model yaml created (for…

eec3239

… pp)

#141 Put compile info under compile section

b7df975

- this is for later organization and parsing when yamls are combined

#141 Redefine fre_properties using reusable yaml variables

c4248e3

#141 Change -e option to --execute

0350674

- another `-e` option was added to represent the experiment name

#141 Fix combining and parsing yamls

1b445e9

- combine model, compile, and platform yaml - parse combined yaml

#141 Rework code that saves parsed yaml info

eb67d23

- added `-e` experiment and `--execute` options - redefine variables with parsed information - createCheckout: fix checkout script running if `--execute` used and checkour script already created

Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 141.am5-yaml…

43de62d

…-integration

#141 Remove combining yamls functionality from yamlfre.py

32ad88b

- this is done in the separate tool `combine-yamls`

#141 Update parsing information from combined yaml

a4c8df1

#141 Update runscript for container

86e5b72

- certain variables were overwritten when bind mode is implemented - these variables are saved in `BACKUP` variables

#141 Update platforms yaml path

975094b

#141 Add test yaml files

b612f80

singhd789 requested a review from thomas-robinson August 27, 2024 13:45

singhd789 added 2 commits August 27, 2024 13:39

#141 Fix up pylint messages

03db8ba

Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 141.am5-yaml…

31bc40c

…-integration

singhd789 force-pushed the 141.am5-yaml-integration branch from 9d4bb23 to 31bc40c Compare August 27, 2024 17:47

singhd789 added 3 commits August 30, 2024 09:59

#141 Add freVars object

c9cf806

- accidentally removed

#141 Use freVars object to parse and fill in missing info

96bfcf3

#141 Update README

cf8c89c

singhd789 marked this pull request as ready for review August 30, 2024 14:17

singhd789 marked this pull request as draft August 30, 2024 14:37

singhd789 added 8 commits September 4, 2024 09:56

#141 Update schema for combined yaml

36860a9

#141 Reference class or use of combine-yamls tool

309b59f

- combine-yamls here if tool wasn't used

#141 Add validation of yaml

be9e821

#141 Update example yamls

999d70d

#141 Add yaml validation

f553b20

#141 Update messages and where validation happens

d36233f

#141 Fix where validation happens

ac18852

singhd789 added 3 commits September 13, 2024 09:54

#141 Fix paths and add test cases

9976464

- new test for validation of combiend yamls - new fail test for if compile yaml path not correct - new fail test for if a value in the compile.yaml is not of right data type

#141 Fix error checking

579f34e

#141 Fix location

dd6cff8

rem1776 mentioned this pull request Sep 16, 2024

Add null model yaml example #185

Merged

7 tasks

ceblanton reviewed Sep 20, 2024

View reviewed changes

singhd789 added 3 commits September 23, 2024 10:33

#141 Combine yamls/rewrite combined yaml as default behavior

3d330f0

#141 Update schema for validation (compilation yamls)

7c79554

- there was a false success with the previous schema

#141 Reorganize test yamls and update tests

f333294

- update paths in test script based on reorganizations - update test_combine_yamls for validation test, wrong compile file test, wrong data type (invalid yaml) test

thomas-robinson reviewed Sep 24, 2024

View reviewed changes

fre/make/createCompile.py Outdated Show resolved Hide resolved

thomas-robinson reviewed Sep 24, 2024

View reviewed changes

fre/make/createCompile.py Outdated Show resolved Hide resolved

thomas-robinson requested changes Sep 24, 2024

View reviewed changes

singhd789 added 2 commits September 25, 2024 11:34

#141 Wrap code in function and add error check

77e123c

#141 Set acceptable values for click option

5736653

thomas-robinson approved these changes Sep 25, 2024

View reviewed changes

singhd789 mentioned this pull request Sep 25, 2024

Yaml parsing errors with esm4.yaml #173

Closed

singhd789 and others added 3 commits September 27, 2024 00:58

Add pathlib import

fd2d8fc

Merge remote-tracking branch 'origin/main' into 141.am5-yaml-integration

1b809ac

Merge remote-tracking branch 'origin/main' into 141.am5-yaml-integration

e1424e0

singhd789 and others added 3 commits September 27, 2024 13:53

Merge branch 'main' of github.com:NOAA-GFDL/fre-cli into 141.am5-yaml…

e2cfbea

…-integration

#141 Update yaml test to chdir back to original pytest directory.

f48c7ab

Otherwise, the cmor test will fail when run afterwards

#141 Add one more chdir back to original working directory

d27a4bc

ilaflott merged commit a283e9a into main Sep 30, 2024
2 checks passed

ilaflott deleted the 141.am5-yaml-integration branch September 30, 2024 13:56

This was linked to issues Oct 25, 2024

Fremake yaml integration #141

Closed

Yaml parsing errors with esm4.yaml #173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compile yaml integration and parse correctly #168

Compile yaml integration and parse correctly #168

singhd789 commented Aug 23, 2024 •

edited

Loading

singhd789 commented Aug 30, 2024

ceblanton Sep 20, 2024

singhd789 Sep 20, 2024 •

edited

Loading

ceblanton Sep 20, 2024

singhd789 Sep 20, 2024

thomas-robinson left a comment

thomas-robinson Sep 24, 2024

thomas-robinson Sep 24, 2024

singhd789 commented Sep 25, 2024

ceblanton commented Sep 25, 2024

ilaflott commented Sep 27, 2024

singhd789 commented Sep 27, 2024

ilaflott commented Sep 30, 2024

Compile yaml integration and parse correctly #168

Compile yaml integration and parse correctly #168

Conversation

singhd789 commented Aug 23, 2024 • edited Loading

Describe your changes

Issue ticket number and link (if applicable)

Checklist before requesting a review

singhd789 commented Aug 30, 2024

ceblanton Sep 20, 2024

Choose a reason for hiding this comment

singhd789 Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

ceblanton Sep 20, 2024

Choose a reason for hiding this comment

singhd789 Sep 20, 2024

Choose a reason for hiding this comment

thomas-robinson left a comment

Choose a reason for hiding this comment

thomas-robinson Sep 24, 2024

Choose a reason for hiding this comment

thomas-robinson Sep 24, 2024

Choose a reason for hiding this comment

singhd789 commented Sep 25, 2024

ceblanton commented Sep 25, 2024

ilaflott commented Sep 27, 2024

singhd789 commented Sep 27, 2024

ilaflott commented Sep 30, 2024

singhd789 commented Aug 23, 2024 •

edited

Loading

singhd789 Sep 20, 2024 •

edited

Loading