Move the IMS snow process from the preplandobs job into the landanl job #2232

jiaruidong2017 · 2024-01-17T16:58:17Z

Description

This PR is to re-organize the snow DA related jobs by moving the IMS snow process from the preplandobs job into the landanl job, that way the preprocessed file specific to each member only lives in $DATA and is not retained (only the input file is retained in $COM_OBS). This PR contributes to the test case for adding JEDI-based snow DA in the global-workflow CI test (PR #2199).

Change characteristics

Is this a breaking change (a change in existing functionality)? NO
Does this change require a documentation update? NO

How has this been tested?

Checklist

Any dependent changes have been merged and published
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 generate no new warnings
New and existing tests pass with my changes
I have made corresponding changes to the documentation if necessary

…for PR NOAA-EMC#2199

aerorahul · 2024-01-17T18:38:19Z

ush/python/pygfs/task/land_analysis.py

@@ -45,7 +45,7 @@ def __init__(self, config):
                'npz': self.config.LEVS - 1,
                'LAND_WINDOW_BEGIN': _window_begin,
                'LAND_WINDOW_LENGTH': f"PT{self.config['assim_freq']}H",
-                'OPREFIX': f"{self.runtime_config.RUN}.t{self.runtime_config.cyc:02d}z.",
+                'OPREFIX': f"{self.runtime_config.CDUMP}.t{self.runtime_config.cyc:02d}z.",


This should be RUN and not CDUMP.

To use CDUMP here is for either gdas or enkfgdas. However, RUN is the only for gdas. We will use the same land_analysis.py code for the later hybrid runs.

Removed the changes.

aerorahul · 2024-01-17T18:39:10Z

ush/python/pygfs/task/land_analysis.py

@@ -187,6 +187,7 @@ def prepare_IMS(self) -> None:
        os.symlink(exe_src, exe_dest)

        # execute CALCFIMSEXE to calculate IMS snowdepth
+        os.chdir(localconf.DATA)


Shouldn't you already be in DATA?

This is for the enkfgdas run, and the DATA are different for the different ensemble members.

We will have RUN=enkfgdas for the ensemble component.

@aerorahul Your this comment is for the above conversation, correct? If RUN=enkfgdas is available, I can change CDUMP back to RUN.

Removed os.chdir(localconf.DATA) for now.

aerorahul · 2024-01-17T18:43:14Z

ush/python/pygfs/task/land_analysis.py

+    def get_obs_dict(self) -> None:
+        """Compile a dictionary of observation files to copy
+
+        This method uses the OBS_LIST configuration variable to generate a dictionary
+        from a list of YAML files that specify what observation files are to be
+        copied to the run directory from the observation input directory
+
+        Parameters
+        ----------
+
+        Returns
+        ----------
+        obs_dict: Dict
+            a dictionary containing the list of observation files to copy for FileHandler
+        """
+        logger.debug(f"OBS_LIST: {self.task_config['OBS_LIST']}")
+        obs_list_config = parse_j2yaml(self.task_config["OBS_LIST"], self.task_config)
+        logger.debug(f"obs_list_config: {obs_list_config}")
+        # get observers from master dictionary
+        observers = obs_list_config['observers']
+        copylist = []
+        for ob in observers:
+            obfile = ob['obs space']['obsdatain']['engine']['obsfile']
+            basename = os.path.basename(obfile)
+            if 'ims_snow' not in obfile:
+                copylist.append([os.path.join(self.task_config['COM_OBS'], basename), obfile])
+        obs_dict = {
+            'mkdir': [os.path.join(self.runtime_config['DATA'], 'obs')],
+            'copy': copylist
+        }
+        return obs_dict


This needs to be configured so that it conforms with the generic method in class Analysis.

@CoryMartin-NOAA suggested to create this new get_obs_dict() function in the class LandAnalysis. The only difference for this new get_obs_dict() compared with the generic method in class Analysis is to skip putting the IMS observations into the copylist.

@aerorahul can you clarify what you mean by your comment here?

aerorahul · 2024-01-17T21:57:29Z

ush/python/pygfs/task/land_analysis.py

+        logger.info("Generating IMS snow OBS")
+        if f"{ self.runtime_config.cyc }" == '18':
+            self.prepare_IMS()


This is preparing the observation. IIRC, this was part of the preplandobs job

@aerorahul From our email conversations on Nov 30, 2023 as below, we have agreement to bring this IMS snow processing into landanl job.

Jiarui/Mike, Rahul and I just had a quick chat, and he is fine with combining the obs preprocessing with the OI job, especially since we will have 1 high res OI job and a set of meta tasks for multiple ensemble OI jobs. Jiarui, please proceed and let me know if you have any questions or how I can help. -Cory

yes. That is true and is fine.
However, this is not complete and I don't think is necessary for the snowDA test to work.
Besides, if this is going to be added here, then the counterpart in exglobal_prep_land_obs.py should be removed.

Moving of prepare_IMS from the prep job to the inline analysis job should be its own issue and PR after the test is added.

The IMS process in the scripts/exglobal_prep_land_obs.py was removed.

The support to move IMS processing from the prep job to the landanl job was already merged into the GDASApp develop. If you don't want to include this change in this PR, we need to revert the changes made in the GDASApp. Please let me know what you think. Thanks.

WalterKolczynski-NOAA · 2024-01-18T06:27:56Z

@jiaruidong2017 Would you please update the title and description to reflect the work being done without users needing to jump to other PRs or issues

jiaruidong2017 · 2024-01-18T13:43:48Z

@jiaruidong2017 Would you please update the title and description to reflect the work being done without users needing to jump to other PRs or issues

Updated. Thanks.

aerorahul · 2024-01-18T14:24:37Z

ush/python/pygfs/task/land_analysis.py

@@ -244,7 +245,12 @@ def initialize(self) -> None:
            Instance of the LandAnalysis object
        """

-        super().initialize()


Undoing this, eliminates the hierarchical class and works against what the design here is supposed to accomplish.

Yes, I understand. However, if super().initialize() is left here, we need to add a special logic for the IMS observation type in the class Analysis. Do you want me to do this way or do you have any other suggestions? Thanks.

super.initialize() is generic and agnostic to land (or any other component).
It should be the first thing to do followed by anything that the land-specific implementation needs.

I made small changes in class Analysis to skip putting ims_snow files into the copylist. This approach can solve the our issues, and keep to use super().initialize(). What do you think about this way?

This PR is to address the recent filename changes of the orography data from `$(CASE)_oro_data.nc` to `$(CASE).mx$(OCNRES)_oro_data.nc` in the global-workflow. This PR is a prerequisite to support NOAA-EMC/global-workflow#2232.

CoryMartin-NOAA · 2024-01-25T20:00:33Z

This PR is going to be closed. We will keep a separate preplandobs job but will write out to $DATA. $RUNDIRS/gdaslandanl will only have one ob file for IMS, but $RUNDIRS/enkfgdaslananl will have memNNN directories analogous to the ensemble fcst job which has a root $DATA and then subdirectories.

Pick the relevant changes for the deterministic from PR NOAA-EMC#2033 …

8603933

…for PR NOAA-EMC#2199

aerorahul requested changes Jan 17, 2024

View reviewed changes

aerorahul reviewed Jan 17, 2024

View reviewed changes

Merge branch 'develop' into feature/snow_test

3a36651

Merge branch 'develop' into feature/snow_test

b6a650d

jiaruidong2017 changed the title ~~Pick the relevant changes for the gdas deterministic from PR #2033 for PR #2199~~ Move the IMS snow process from the preplandobs job into the landanl job Jan 18, 2024

aerorahul reviewed Jan 18, 2024

View reviewed changes

Make changes to include 'OCNRES' for orog data.

6846662

jiaruidong2017 requested a review from aerorahul January 19, 2024 14:33

jiaruidong2017 mentioned this pull request Jan 19, 2024

Make changes to include 'OCNRES' in the orography filenames. NOAA-EMC/GDASApp#864

Merged

CoryMartin-NOAA mentioned this pull request Jan 19, 2024

Add a yaml for snow DA testing. #2199

Merged

7 tasks

CoryMartin-NOAA closed this Jan 25, 2024

jiaruidong2017 deleted the feature/snow_test branch February 29, 2024 00:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move the IMS snow process from the preplandobs job into the landanl job #2232

Move the IMS snow process from the preplandobs job into the landanl job #2232

jiaruidong2017 commented Jan 17, 2024 •

edited

Loading

aerorahul Jan 17, 2024

jiaruidong2017 Jan 17, 2024

jiaruidong2017 Jan 19, 2024

aerorahul Jan 17, 2024

jiaruidong2017 Jan 17, 2024

aerorahul Jan 17, 2024

jiaruidong2017 Jan 17, 2024

jiaruidong2017 Jan 19, 2024

aerorahul Jan 17, 2024

jiaruidong2017 Jan 17, 2024

CoryMartin-NOAA Jan 17, 2024

aerorahul Jan 17, 2024

jiaruidong2017 Jan 17, 2024

aerorahul Jan 18, 2024

jiaruidong2017 Jan 19, 2024

WalterKolczynski-NOAA commented Jan 18, 2024

jiaruidong2017 commented Jan 18, 2024

aerorahul Jan 18, 2024

jiaruidong2017 Jan 18, 2024

aerorahul Jan 18, 2024

jiaruidong2017 Jan 19, 2024

CoryMartin-NOAA commented Jan 25, 2024

Move the IMS snow process from the preplandobs job into the landanl job #2232

Move the IMS snow process from the preplandobs job into the landanl job #2232

Conversation

jiaruidong2017 commented Jan 17, 2024 • edited Loading

Description

Change characteristics

How has this been tested?

Checklist

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

WalterKolczynski-NOAA commented Jan 18, 2024

jiaruidong2017 commented Jan 18, 2024

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

CoryMartin-NOAA commented Jan 25, 2024

jiaruidong2017 commented Jan 17, 2024 •

edited

Loading