Replace Analysis class with JEDI class #2789
Conversation
… S2SWA (NOAA-EMC#2757)" This reverts commit fc668aa.
…with app S2SWA (NOAA-EMC#2757)"" This reverts commit a3928d2.
This reverts commit 16eaef3.
There was a problem hiding this comment.
Thanks for doing this @DavidNew-NOAA . Just a few thoughts/comments.
| logger = getLogger(__name__.split('.')[-1]) | ||
|
|
||
|
|
||
| class JEDI(Task): |
There was a problem hiding this comment.
This looks like a class hard-wired for the analysis step, which I think is fine but the name should probably reflect this. JEDIAnalysis? or just keep Analysis?
I think @CoryMartin-NOAA (or maybe that was you @DavidNew-NOAA ?) started a b-matrix base class as well. We probably want to keep these separate.
There was a problem hiding this comment.
The intention is for this class to be used anytime a JEDI application is run in general, not just for running an analysis
| self.link_jedi_exe() | ||
|
|
||
| @logit(logger) | ||
| def execute(self, aprun_cmd: str, jedi_args: Optional[List] = None) -> None: |
There was a problem hiding this comment.
Running only one executable wouldn't work for the "B-matrix" or "prep obs" jobs for example, thus the suggestion above to rename to something descriptive of the analysis job.
| save_as_yaml(self.task_config.jedi_config, self.task_config.jedi_yaml) | ||
| logger.info(f"Wrote YAML to: {self.task_config.jedi_yaml}") | ||
|
|
||
| def link_jedi_exe(self) -> None: |
There was a problem hiding this comment.
we probably should have this in a common utility module
|
|
||
|
|
||
| @logit(logger) | ||
| def find_value_in_nested_dict(nested_dict: Dict, target_key: str) -> Any: |
There was a problem hiding this comment.
same as above, probably belongs to wxflow or some other common utility module
|
I'm closing the pull request until I make some modifications. Rahul and I discussed it and agree, based on previous conversations with Dan and Cory, that the JEDI class should not be inherited from the Task class, but rather any child class of Task (eg AtmAnalysis, AtmEnsAnalysis, etc) should have the JEDI class as a member. |
Description
This PR replaces the PyGFS
Analysisclass with a more generalJEDI class. It is a child class of thewxflowTaskclass, like theAnalysis, but it's intended for any Global Workflow job that runs one or more JEDI applications. At this point, it has only been applied to the atmospheric variational and ensemble analysis jobs, so the Analysis class still remains part of the code base, but can be fully replaced after this PR using the atmospheric analysis jobs as a template for the other analysis jobs (snow, aerosol, etc).In order to make things more generic, the input YAML will now always be named after the JEDI application being run. So, for example, if running
gdas.x, the name of the input YAML isgdas.yaml, rather than, say,gdas.t12z.atmvar.yaml. In the case of the atmospheric variational analysis, thefinalize()method of theAtmAnalysisclass will ensure thatgdas.yamlis saved asgdas.t12z.atmvar.yamlin the COMROT directory anyway, so no need to give it such a descriptive name in the run directory.The
AtmAnalysisandAtmEnsAnalysisclasses, now subclasses ofJEDI, now take care of staging observations and bias corrections in their owninitialize()methods, rather than in theinitialize()method ofJEDI, but theJEDIinitalize()method continues to render the input Jinja2-YAMLS/JCB templates and link the JEDI executable file. TheJEDIclassinitalize()method now also saves the final YAML in the run directory, rather than having theinitalize()method of theAtmAnalysisandAtmEnsAnalysisclasses do that (since all JEDI applications require an input YAML file saved to disk.A generalized
execute()method now exists inJEDIwhich takes the APRUN command as input, and optionally a list of additional arguments that might be passed togdas.x(eg. ['fv3jedi', 'variational']).Type of change
Change characteristics
How has this been tested?
Checklist