Feature/gfs config dirpath fixedfiles #1474

HenryRWinterbottom · 2023-04-17T17:31:31Z

Description

This PR addresses issue #1473 .

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

The directory tree and expected fixed files are defined/copied.

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my own code
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 generate no new warnings
New and existing tests pass with my changes
Any dependent changes have been merged and published

It can be difficult to debug MPMD jobs because their logs are all written concurrently to a single file. While the use of tags to designate which task via the preamble and PS4 can help identify which line is from which task, it is still difficult to follow a single task through the log, particularly for larger MPMD jobs with dozens of tasks. Individual stdout files are now created by using the `srun` `--output` option. These files are written to the working directory (in `$DATA`). Fixes: #1468

…files

aerorahul

there is way too much in this PR that
a. we did not discuss in the plan
b. irrelevant to the topic
c. changes that were made that are counter to the ones in the previous PR.

aerorahul · 2023-04-17T17:42:11Z

ush/python/pygfs/ufswm/gfs.py

+
+        # HRW: `atmos_run` will need to be defined dynamically once
+        # additional GFS applications are added.
+        config_attr_dict = {'aero_run': 'DO_AERO',


I think we prefer do_aero etc. The less renaming we do, the better.

That comes from this in the configuration/environment: DO_AERO False.

aerorahul · 2023-04-17T17:43:04Z

ush/python/pygfs/ufswm/gfs.py

+                            'ice_cpl': 'cplice',
+                            'ice_res': 'ICERES',
+                            'ice_run': 'DO_ICE',
+                            'ocn_cpl': 'cpl',


if chm_cpl is cplchm for e.g., then why is ocn_cpl just cpl?

This comes from DO_COUPLED False or cpl False. Which do you want to use?

aerorahul · 2023-04-17T17:46:27Z

ush/python/pygfs/ufswm/gfs.py

+        self.ufs_config.FIX_orog = os.path.join(self._config.FIXgfs, 'orog')
+        self.ufs_config.FIX_ugwd = os.path.join(self._config.FIXgfs, 'ugwd')
+
+        self.ufs_config.HOMEgfs = self._config.HOMEgfs


Lets think about this:
If we use gfs in the name of an object that is "ufs", would that cause issues?
Does ufs_config need to know where HOMEgfs is? It needs FIX and parm, but they could be independent of "HOMEgfs"

Maybe not in the UFS class but we could add it in the GFS class.

aerorahul · 2023-04-17T17:57:42Z

parm/ufs/fix/gfs/atmos.fixed_files.yaml

-copy:
-  # Atmosphere mosaic file linked as the grid_spec file (atm only)
-  - [$(FIX_orog)/$(atm_res)/$(atm_res)_mosaic.nc, $(DATA)/INPUT/grid_spec.nc]
+dirtree:


We don't need this dirtree section.

Does the FileHandler know how to build the directory tree prior to copying the respective files?

No. it does not, but that is not the point of these fix files.
They are containing information about what files are needed, where is the source and what is the destination. And what is the action (copy).
Anything else is not within scope of this file.

I am really confused about this. How does the directory tree get built such that the files can be copied? Last time we discussed this piece of the workflow this was needed. Why is it no longer needed?

aerorahul · 2023-04-17T17:57:59Z

parm/ufs/fix/gfs/atmos.fixed_files.yaml

-  - [$(FIX_orog)/$(atm_res)/$(atm_res)_grid.tile4.nc, $(DATA)/INPUT/]
-  - [$(FIX_orog)/$(atm_res)/$(atm_res)_grid.tile5.nc, $(DATA)/INPUT/]
-  - [$(FIX_orog)/$(atm_res)/$(atm_res)_grid.tile6.nc, $(DATA)/INPUT/]
+fixedfiles:


Keep these as they were before.

aerorahul · 2023-04-17T17:59:42Z

ush/python/pygfs/ufswm/gfs.py

+    Parameters
+    ----------
+
+    config: AttrDict
+
+        A Python dictionary containing the run-time configuration
+        attributes.
+


There are no parameters to a class. They are to the constructor __init__.

Is this a reference to the documentation?

aerorahul · 2023-04-17T18:04:31Z

ush/python/pygfs/ufswm/gfs.py

+        fixedfile_list = [item for item in self.ufs_config.fixedfile_list if (
+            "atmos" in item.lower() or "land" in item.lower())]


This is where a configuration yaml is necessary to elevate this from code to yaml

Agreed. I tried to keep this simple for the time-being until we are at that point.

aerorahul · 2023-04-17T18:09:37Z

ush/python/pygfs/ufswm/gfs.py

+        self.config_fcstapp()
+
+    @logit(logger)
+    def build_dirtree(self: UFS) -> None:


Hold this for another PR.

aerorahul · 2023-04-17T18:15:15Z

ush/python/pygfs/ufswm/ufs.py


        with open(output_file, 'w') as fho:
            fho.write(file_out)
+
+    @staticmethod
+    def __get_fixedfile_list(fixedfile_path) -> List:


I don't know why we care about getting a list of all files in a directory.

I thought this was what we discussed. In that path are only fixed-file YAMLs. They are relevant depending on the application of the GFS.

HenryRWinterbottom · 2023-04-17T18:27:45Z

there is way too much in this PR that a. we did not discuss in the plan b. irrelevant to the topic c. changes that were made that are counter to the ones in the previous PR.

I am not sure how I am supposed to test the the pieces that were put in place without the remainder of the other bits. My understanding was the following:

PR GFS v16 workflow development #1 -- Add YAML files.
PR GFSv15.2.6 production changes and bug fix #2 -- Add infrastructure to build directory tree; this includes linking static files.

HenryRWinterbottom and others added 4 commits April 17, 2023 11:20

Bringing branch UTD with WIP.

2435f39

Temporary for rebase.

cc22279

Updated fixed files.

fd2bc1f

Merge branch 'dev/fcstsprint' into feature/gfs_config_dirpath_fixedfiles

640d6a9

HenryRWinterbottom requested review from aerorahul and WalterKolczynski-NOAA April 17, 2023 17:33

WalterKolczynski-NOAA and others added 2 commits April 17, 2023 13:37

Merge branch 'NOAA-EMC:develop' into feature/gfs_config_dirpath_fixed…

40c6980

…files

aerorahul requested changes Apr 17, 2023

View reviewed changes

HenryRWinterbottom requested a review from aerorahul April 17, 2023 18:24

Bringing branch UTD prior to rebase.

15ebec8

HenryRWinterbottom closed this Apr 24, 2023

HenryRWinterbottom deleted the feature/gfs_config_dirpath_fixedfiles branch April 26, 2023 17:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/gfs config dirpath fixedfiles #1474

Feature/gfs config dirpath fixedfiles #1474

HenryRWinterbottom commented Apr 17, 2023

aerorahul left a comment

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

aerorahul Apr 17, 2023

aerorahul Apr 17, 2023

HenryRWinterbottom Apr 17, 2023

HenryRWinterbottom commented Apr 17, 2023

		fixedfile_list = [item for item in self.ufs_config.fixedfile_list if (
		"atmos" in item.lower() or "land" in item.lower())]

Feature/gfs config dirpath fixedfiles #1474

Feature/gfs config dirpath fixedfiles #1474

Conversation

HenryRWinterbottom commented Apr 17, 2023

aerorahul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HenryRWinterbottom commented Apr 17, 2023