MetOffice / dagrunner

⛔[EXPERIMENTAL] Directed acyclic graph (DAG) runner and tools
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

Overriding parameters with settings #53

Closed cpelley closed 2 weeks ago

cpelley commented 3 weeks ago

Overriding Node parameters with node attributes (settings). Exposed this requirement as part of my tackling https://github.com/MetOffice/epp_workflows/pull/66 Technically, this is non-blocking as IMPROVER or EPP should be responsible for generating the networkx graph. Dagrunner ExecuteGraph simply provides the means to pass it nodes and edges to do this for you.

Additional minor changes:

Why do we want this?

More easily explained with an example (illustrative). precipitation accumulation is defined with a period in its diagnostic name. This is a function of the leadtime (x, y here). We don't want to unnecessarily be defining our node identity with a period in this diagnostic name or through adding a period parameter, since the period is only characteristic of the output filename, not its identity (the leadtime is enough to characterise its identity and we don't want to be deducing the period for a node everywhere we want to connect to it). Instead, what we want is to override the diagnostic property using a node attribute (setting) for the node where it matters (saving). That is, when saving to disk, as the filepath will be derived using this period in the name.

diagnostic_name = "precipitation_accumulation"
...
for leadtime in [x, y]:
    NODES.append(
        [
            Node(model="gl", diagnostic="precipitation_accumulation", step=..., leadtime=leadtime),
            Node(model="gl", diagnostic="precipitation_accumulation", step="save", leadtime=leadtime),
        ]
    )

    period = PERIOD_LOOKUP[leadtime/HOUR]
SETTINGS[Node(model="gl", diagnostic="precipitation_accumulation", step="save", leadtime=leadtime)] = {
        "call": {...}, "diagnostic": f"{diagnostic_name}{period}"
    }
...

Issues

SamGriffithsMO commented 2 weeks ago

I thought overriding was something we were actively trying to remove when going to dagrunner, won't this increase the scope to reintroduce mass overriding again?

cpelley commented 2 weeks ago

I thought overriding was something we were actively trying to remove when going to dagrunner, won't this increase the scope to reintroduce mass overriding again?

The PR title could be somewhat misleading. This is to do with simultaneously supporting node parameters for its identity while retaining support for a node attribute that doesn't impact this identity (the node parameter remains the same for node identity, not overridden). The overriding behaviour that I have mention previously (EG) concerned more how settings are defined with no way to understand what they are, besides debugging the graph due to how they are a merger at many places.