MetOffice / MetOffice/dagrunner
Order of predecessors to a node can matter
- Dominant language
- Python
- Stars
- 5
- Forks
- 6
- PR merge metrics
- No merged PRs in 30d
Description
When a node has multiple predecessors the order in which they are defined and then passed to create the networkx DiGraph matters. However, it's not clear to me that it should matter for a directed acyclic graph. As far as I can tell, for a collection of predecessors for a node in a DAG has no notion of ordering of those predecessors (however, instead, they could be labelled).
For example, in the [demo notebook](https://github.com/MetOffice/dagrunner/blob/main/docs/demo.ipynb) in cell 3 if you swap the order:
```python
# node2 -> node5
node5 = Node(step="step5", leadtime=leadtime)
EDGES.append((node2, node5))
# node4 -> node5
node4 = Node(step="step4", leadtime=leadtime)
EDGES.append((node4, node5))
```
for
```python
# node4 -> node5
node4 = Node(step="step4", leadtime=leadtime)
EDGES.append((node4, node5))
# node2 -> node5
node5 = Node(step="step5", leadtime=leadtime)
EDGES.append((node2, node5))
```
Then the result for node5 (see cell 8) is `3_4_1_2_5` not `1_2_3_4_5`.
This is an issue that I believe is inherited from paraflow, in a sense. I came across this in the context of paraflow while experimenting with filtering the DiGraph object (for hindcasting) and recreating a new DiGraph object messed up the order. For the IMPROVER suite the `${INPUT}` argument matters.
I think this is something to consider in the design. I understand that in the case of the demo notebook if the `concatenate_id` function were defined differently this problem could be removed. However it still could lead to some confusion. And, in general, we cannot rely on the ordering of the predecessors to a node, especially if we generate and manipulate the DiGraph object itself.
@cpelley (and others): I'd be interested to hear your thoughts on this.
---
As an aside, I'm investigating the use of DAGRunner for a verification workflow at the Bureau of Meteorology. The use of DAGRunner would be advantageous as it allows for multiple schedulers rather than just Cylc as for paraflow/
Contributor guide
Research direction
Start with docs/demo.ipynb, especially cell 3 where the predecessor edges are defined and cell 8 where node5's result is shown. Reproduce the changed output after swapping the edge order, then inspect how the DiGraph is filtered and recreated. The issue needs a design decision and acceptance criteria for predecessor ordering before implementation can begin.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100