data store: add absolute graph edges
Closes #5845
Include absolute graph edges into the data store.
At present, absolute graph parents apply only when the parent is in the same cycle as the child.
E.G. Taking this example:
R1 = build
P1 = build[^] => run
The build => run graph edge would only exist in the R1 cycle (the one in which the build task was scheduled to run) rather than all subsequent cycles to which it applied.
The cause of the bug was a bit of code that was purposefully filtering out absolute graph edges which point to tasks in cycles other than the one that the child task is in. The code contained this comment which suggests doing this resolved some other issue:
# If 'foo[^] => bar' only spawn off of '^'.
Taking this code out fixes #5845, but I'm worried that this code served a purpose and that by deleting it I'm solving one problem by introducing another.
The comment first appeared here:
https://github.com/cylc/cylc-flow/commit/416240f63873b120a26447558767241df6f0b6e5#diff-e67a65e5cc6c5da815c97f19bf1afc269d56a7f032351aa72364a414ae726048R84
I can't track down why it was added by scanning through #3811. Long shot, @dwsutherland you don't have any memory of this (3 years ago!)?
Check List
- [ ] I have read
CONTRIBUTING.mdand added my name as a Code Contributor. - [ ] Contains logically grouped changes (else tidy your branch by rebase).
- [ ] Does not contain off-topic changes (use other PRs for other changes).
- [ ] Applied any dependency changes to both
setup.cfg(andconda-environment.ymlif present). - [ ] Tests are included (or explain why tests are not needed).
- [ ]
CHANGES.mdentry included if this is a change that can affect users - [ ] Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
- [ ] If this is a bug fix, PR should be raised against the relevant
?.?.xbranch.
I might have shifted some code about also.. I guess this is separate to the issue of spawning off ^ ? (where you'd spawn out to the RH limit if this dependency was in more than just R1)
Actually yeah, so I've only included it in the same cycle (like you said)... it makes sense WRT limiting the edges to ^ cycle, but it does feel artificial ..
Added a test and changelog, rebased onto 8.3.x.
@wxtim Assigned for review 2, well worth a good play with the graph view (static cylc graph and live in the GUI).
What about this equivalent bit in generate_graph_children()?
https://github.com/cylc/cylc-flow/blob/4d6430a7f0905201c1206f18c2f348d26a82b32e/cylc/flow/taskdef.py#L49-L51
What about this equivalent bit in generate_graph_children()?
(haven't forgotten about this, still catching up with my inbox).
A good point that I either don't have an answer to or simply can't remember.
I don't think I've seen any consequences of this (abs deps seem to work fine, perhaps this is the graph walking algorithm doing its job?).
Opened an issue.