Split get_background into 3d and 4d tasks #268

tariqhamzeyssai · 2023-10-23T15:34:19Z

Description

UPDATED Nov. 16
This PR splits get_background task into get_background3d and get_background4d. It is tested on the hofx suite only.

The important ability on display here is a way for suite questions to dictate which tasks are called in that suite's flow.cylc script. In lieu of a "map between suites, tasks and questions", which is a work in progress, we can achieve this dependency with minimal code changes and a bit of config gaming.

window_type is pulled from task_questions and placed into suite_questions instead.
{{window_type}} is a placeholder in the flow.cylc template where GetBackground is called.
In tasks/task_questions.yaml, where tasks are listed underneath a question, GetBackground is replaced with the placeholder version GetBackground{{window_type}}. This is needed for swell create to grab task questions and populate config parameters. Note the imperfection, not just the ugliness, with this approach. The experiment.yaml file may end up with unneeded parameters, like background_frequency for a 3d run.
Similarly, GetBackground3d and GetBackground4d are added to these task lists, where appropriate. This is needed for swell task GetBackground3d ... and the like.

While in concept we are abstracting window_type away from the config (get_background tasks no longer grab this element from config dictionary), it still appears in experiment.yaml since it is a question that has an answer.

Ultimately a better mapping scheme for suites, tasks, and questions is a necessary precursor for this PR, which attempts to fit a square peg into a round hole.

Dependencies

NA

Impact

NA

danholdaway · 2023-10-27T17:17:47Z

src/swell/tasks/get_background.py

@@ -27,6 +28,56 @@

 class GetBackground(taskBase):

+    def _fetch_all(self, background_prep, r2d2_dict, model_component):


Perhaps it's not necessary to create a function for holding this? Since it's only used in one place?

danholdaway · 2023-10-27T17:26:32Z

src/swell/tasks/prep_background.py

@@ -0,0 +1,140 @@
+#!/usr/bin/env python


I'm not sure this is quite the right design. We ultimately need to map between suites, tasks and questions. If you ask window_type as a suite question then you can set your (e.g.) hofx suite to use the task GetBackground3d if the answer is 3D, otherwise use GetBackground4d. Then you can determine which questions to ask based on what things GetBackground3d is getting out the config. In this case you would never have PrepBackground in the suite so the code wont know to look here for what is coming out of the config. Further, even if it did it would see, for example, background_frequency which is something only needed by GetBackground4d. So I think there really needs to be files like:

tasks/get_background3d.py
tasks/get_background4d.py
utilities/get_background.py

You can put your class in utilities if you like. But everything you access from the config needs to be in the codes in tasks. It doesn't matter that there might be duplicate things being taken from the config in each. The issue is that you otherwise break the connection between suites, tasks and task_questions.

Here is the code that builds a list of tasks from the suite file (flow.cylc)

swell/src/swell/deployment/prep_config_base.py

Line 251 in 60d99fd

def open_flow(self):

Once it has a list of tasks it greps all the task code in the tasks directory to see which things in the config will be requests. From there it can ask all those questions and create experiement.yaml

…ckground, formally split get_background into 3d and 4d tasks

tariqhamzeyssai · 2023-11-16T16:42:55Z

This PR has been reworked. See updated description.

danholdaway

I think this looks great. Thanks for working on it. We can just update the remaining suites if need be and clean up comments. Once we get back on track with swell CI we can get it merged.

danholdaway · 2023-12-11T15:16:50Z

src/swell/suites/hofx/suite_questions.yaml

@@ -1,3 +1,10 @@
+window_type:
+  ask_question: True
+  default_value: 3d


Is it possible to have a defer_to_model here?

I think not, but I'm trying to remember why.

danholdaway · 2023-12-11T15:21:17Z

src/swell/tasks/task_questions.yaml

@@ -5,7 +5,9 @@ analysis_forecast_window_offset:
  - all
  prompt: What is the duration from the middle of the window when forecasts start?
  tasks:
-  - GetBackground
+  - GetBackground{{window_type}}


I don't think we have a mechanism to resolve Jinja2 templates in the task questions. We just need to list each 3D, 4D, etc tasks as you've done below.

Alluded to this issue in the description. When swell create is called, flow.cylc doesn't yet know what window_type we're dealing with; it's still in template form. We need a way to match that template to task_questions. This is the simplest, though imperfect, way to avoid the hassle of regex or other pattern matching codes.

danholdaway · 2023-12-11T15:22:00Z

src/swell/tasks/task_questions.yaml

-  depends:
-    key: window_type
-    value: 4D
+#  depends:


This can indeed be removed. The question will only be asked if the 4D task is found in the suite after it has been resolved.

first commit, prep_background.py

c8d9c31

tariqhamzeyssai requested a review from danholdaway October 23, 2023 15:34

danholdaway suggested changes Oct 27, 2023

View reviewed changes

Tariq Hamzey added 4 commits November 14, 2023 13:06

Back to basics, make window_type a suite question, get rid of prep_ba…

aa3ebde

…ckground, formally split get_background into 3d and 4d tasks

task lists for templated task names

93efcb5

remove GetBackground3d from background_frequency task list

f2d5399

remove comments

de540a7

tariqhamzeyssai changed the title ~~Split 3D and 4D background preparation~~ Split get_background into 3d and 4d tasks Nov 16, 2023

Merge branch 'develop' into feature/split_get_background

9f4fcc5

danholdaway suggested changes Dec 11, 2023

View reviewed changes

danholdaway requested a review from jardizzo December 11, 2023 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split get_background into 3d and 4d tasks #268

Split get_background into 3d and 4d tasks #268

tariqhamzeyssai commented Oct 23, 2023 •

edited

Loading

danholdaway Oct 27, 2023

danholdaway Oct 27, 2023

danholdaway Oct 27, 2023

danholdaway Oct 27, 2023

tariqhamzeyssai commented Nov 16, 2023

danholdaway left a comment

danholdaway Dec 11, 2023

tariqhamzeyssai Dec 12, 2023

danholdaway Dec 11, 2023

tariqhamzeyssai Dec 12, 2023 •

edited

Loading

danholdaway Dec 11, 2023

		@@ -27,6 +28,56 @@

		class GetBackground(taskBase):

		def _fetch_all(self, background_prep, r2d2_dict, model_component):

Split get_background into 3d and 4d tasks #268

Are you sure you want to change the base?

Split get_background into 3d and 4d tasks #268

Conversation

tariqhamzeyssai commented Oct 23, 2023 • edited Loading

Description

Dependencies

Impact

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

tariqhamzeyssai commented Nov 16, 2023

danholdaway 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

tariqhamzeyssai Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tariqhamzeyssai commented Oct 23, 2023 •

edited

Loading

tariqhamzeyssai Dec 12, 2023 •

edited

Loading