Don't recompute runahead unnecessarily in main loop - #7435
Conversation
| # the parentless task "a" should be spawned out to the runahead limit | ||
| assert schd.pool.get_task_ids() == {'2/a', '3/a'} | ||
| # the parentless task "a" should be spawned out to the next cycle | ||
| assert schd.pool.get_task_ids() == {'2/a'} |
There was a problem hiding this comment.
Small change in behaviour - but not one that will affect real running of workflows. After the cylc set operation, tasks are not spawned out to runahead limit until the main loop gets round to it.
This test goes on to check that we continue to spawn out to the runeahead limit below.
| <span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">-</span><span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">̿○</span><span style="color:#000000;background:#e5e5e5"> 2 </span> | ||
| <span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">-</span><span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">̿○</span><span style="color:#000000;background:#e5e5e5"> A </span> | ||
| <span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">̿○</span><span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">a</span><span style="color:#000000;background:#e5e5e5"> </span> | ||
| <span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">-</span><span style="color:#000000;background:#e5e5e5"> </span><span style="color:#000000;background:#e5e5e5">○</span><span style="color:#000000;background:#e5e5e5"> 2 </span> |
There was a problem hiding this comment.
Minor difference in these TUI tests:
-̿ ○
+○The waiting task is no longer queued immediately after the cylc set operation, but will be queued in the next main loop iteration.
This does not matter for the purposes of the two tests:
/tests/integration/tui/test_app.py::test_auto_expansion/tests/integration/tui/test_mutations.py::test_set_mutation
f3f1fef to
e7c6b37
Compare
| '5/foo', | ||
| '6/foo', | ||
| '7/foo', | ||
| } |
There was a problem hiding this comment.
Another minor change in behaviour - the task pool immediately after removing the other tasks does not spawn out to the runahead limit. Again this is OK because we test below that the workflow continues out to the runahead limit naturally anyway.
| # BACK COMPAT: spawn the task pool out to RH limit on startup. | ||
| # Not strictly necessary, it will spawn ahead per main loop iteration, | ||
| # but useful back-compat for tests that expect this prior to | ||
| # https://github.com/cylc/cylc-flow/pull/7237 | ||
| # FROM: 8.6.x | ||
| # TO: 8.7.0 | ||
| # REMOVE AT: 8.7.0 | ||
| self.pool.compute_runahead() | ||
| for _ in range(10): | ||
| # (Arbitrary limit to avoid infinite loop if something goes wrong) | ||
| if not self.pool.release_runahead_tasks(): | ||
| for itask in self.pool.get_tasks(): | ||
| self.pool.queue_if_ready(itask) | ||
| break |
There was a problem hiding this comment.
Extracted the shim out into the scheduler and deliberately not in its own method, to stem any temptation to call it anywhere else in future on 8.6.x
e7c6b37 to
ace9057
Compare
Co-authored-by: Hilary Oliver <hilary.j.oliver@gmail.com>
ace9057 to
8fc9f80
Compare
hjoliver
left a comment
hjoliver
left a comment
There was a problem hiding this comment.
Good. #7400 minus shim removal and test adaptation to that.
IF we were removing the shim on 8.6.x (as I was trying to do) this needs to be done together with that to avoid having to pointlessly adapt the tests twice, but fine to delay the shim removal in current circumstances.
| or self.pool.tasks_removed | ||
| ) | ||
|
|
||
| if has_updated: |
There was a problem hiding this comment.
Do those emoji suggest you don't like the pool.tasks_removed bit? (It's there because removing a task can change the runahead base point) or the has_updated bit? (which isn't this PR).
3 participants
Partially supersedes #7400 - contains minimal set of changes that I'm happy with
Check List
CONTRIBUTING.mdand added my name as a Code Contributor.?.?.xbranch.