parentless sequential clock trigger spawns #5732

dwsutherland · 2023-09-15T01:29:45Z

superseded by #5738

Spawning parentless clock triggered tasks out to the run ahead limit is unnecessary, and creates a lot of UI clutter.

Naturally clock trigger tasks are sequential in time, so it makes sense to only spawn the next parentless clock-trigger task when the current clock trigger is satisfied.

This PR attempts this.. In the simplest case;

[scheduling]
    initial cycle point = 20230915T0140Z
    [[xtriggers]]
        clock_1 = wall_clock()
    [[graph]]
        PT1M = """
@clock_1 => a
a => b
"""
[runtime]
    [[root]]
        script = sleep 5
    [[a,b]]

cutting down this;

to this:

(would only show one cycle, but obviously the trigger is satisfied)

Real workflows often have a much greater chain than a => b (and possibly much greater run-ahead limits), so you can imagine the clutter. (such as those at NIWA)

One thing to note; if your task has multiple xtriggers including a clock trigger, i.e.;

    [[xtriggers]]
        clock_1 = wall_clock()
        up_1 = workflow_state(workflow=clockspawn, task=b, point=%(point)s, offset=-P1Y):PT10S
    [[graph]]
        PT1M = """
@clock_1 => a
a => b
"""
        +PT1M/PT1M = """
@up_1 => b
"""

Then the task will still be clock blocked from spawning out to the RH (will instead spawn sequentially on trigger satisfaction, as described).
And if for some fringe reason a user wants the non-clock xtrigger to be checked/run out into the future/RH-limit and associated task clock bound to when it actually runs, then they can simply split them up:

    [[xtriggers]]
        clock_1 = wall_clock()
        up_1 = workflow_state(workflow=clockspawn, task=b, point=%(point)s, offset=-P1Y):PT10S
    [[graph]]
        PT1M = """
@clock_1 => a
a => b
"""
        +PT1M/PT1M = """
@up_1 => up_xtrig
up_xtrig => a
"""

(or w/e they desire.. alternatively turning their bespoke xtrigger into a task if that works)

In this way, I believe the behavior introduced here should be the default/natural restriction..

Check List

I have read CONTRIBUTING.md and 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 (and conda-environment.yml if present).
Appears to be covered by existing Tests.
CHANGES.md entry included if this is a change that can affect users
Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.

dwsutherland · 2023-09-15T04:49:23Z

Actually, I might broaden this to optionally specify other xtriggers as sequential-spawn, e.g:

    [[xtriggers]]
        up_a = workflow_state(workflow=aflow, task=a, point=%(point)s):PT10S
        [[[settings]]]
            sequential-spawn = up_a
    [[graph]]
        PT1M = """
@up_1 => b
"""

Because, for some workflows, I wouldn't want workflow_state xtriggers to flood the UI (not to mention the process pool)..

Even though you could add a clock-trigger to make them sequential, this would clutter the workflow definition and be too narrow (i.e. you may want them to run sequential but not bound by real time)..

Wall clock xtriggers should still be sequential.

hjoliver

Had a good play around with this; it appears to do what it says on the tin 👍

Couple of comments...

hjoliver · 2023-09-18T00:23:54Z

cylc/flow/xtrigger_mgr.py

@@ -211,6 +211,11 @@ def __init__(
        # Signatures of active functions (waiting on callback).
        self.active: list = []

+        # Gather named wall-clocks closet to current time.


Suggested change

# Gather named wall-clocks closet to current time.

# Gather named wall-clock triggers closest to current time.

hjoliver · 2023-09-18T00:24:41Z

cylc/flow/xtrigger_mgr.py

@@ -211,6 +211,11 @@ def __init__(
        # Signatures of active functions (waiting on callback).
        self.active: list = []

+        # Gather named wall-clocks closet to current time.
+        # (no need to check or spawn future clocks of tasks respectively).


Can you rephrase this comment? It doesn't seem to make much sense as worded!)

hjoliver · 2023-09-18T04:49:41Z

cylc/flow/scheduler.py

            if housekeep_xtriggers:
+                # Spawn parentless clock-triggered
+                self.pool.spawn_parentless_clock_triggered()
                # (Could do this periodically?)
                self.xtrigger_mgr.housekeep(self.pool.get_tasks())


Is this the right place to handle the spawning? The reason for the if block here is to "delete satisfied xtriggers no longer needed by any tasks" (from the xtrigger_mgr.housekeep() docstring).

I noticed that housekeep_xtriggers is satisfied if xtriggers are, so just piggy backed off that.. But could just use that spawns list.

hjoliver · 2023-09-18T04:54:17Z

cylc/flow/xtrigger_mgr.py

+        # Gather named wall-clocks closet to current time.
+        # (no need to check or spawn future clocks of tasks respectively).
+        self.wall_clock_labels: set = set()
+        self.wall_clock_spawns: list = []


Could this variable be named better? like clock_spawn_next or something?

Yes, will change.. maybe to broaden sequential_spawn_next

hjoliver · 2023-09-18T05:01:28Z

My test case:

[scheduling]
    initial cycle point = now
    [[xtriggers]]
        c2 = "wall_clock(offset=PT1M)"
        x1 = "xrandom(percent=50, _=%(point)s)"
    [[graph]]
        PT1M = """
            @wall_clock => foo1
            @c2 => foo2
            @x1 => bar
            qux
        """
[runtime]
    [[foo1, foo2, bar, qux]]
        script = sleep 5

One weird thing noticed. Spawned tasks waiting on clock triggers should be visible as waiting in n=0. When I run this, foo1 disappears from Tui after a few cycles, except when it is actually running. But foo2 is always visible as a waiting task. Both are present in cylc dump -t, however, so presumably that implies a bug in Tui, not the datastore?

hjoliver · 2023-09-18T05:09:31Z

Tested with a parent in one cycle too, all good. As above, with this additional bit of graph:

       R1/+PT2M/PT0M = "qux => foo2"

hjoliver · 2023-09-18T05:13:07Z

[x] Appears to be covered by existing Tests.

Coverage passed, but we can't be verifying the new spawning behavior as you haven't modified any of the test files.

dwsutherland · 2023-09-18T07:20:49Z

When I run this, foo1 disappears from Tui after a few cycles, except when it is actually running.

Yes, I found the same.. Will have a look (maybe after next revision)

Coverage passed, but we can't be verifying the new spawning behavior as you haven't modified any of the test files.

I guess what I mean is; the current tests appear to cover 100% of this patch.. But, yes, will need to add specific tests (especially with expanded to optionally other xtriggers)

dwsutherland · 2023-09-20T04:49:32Z

superseded by #5738

parentless sequential clock trigger spawns

3b4f1e3

dwsutherland added the sod-follow-up label Sep 15, 2023

dwsutherland added this to the cylc-8.3.0 milestone Sep 15, 2023

dwsutherland self-assigned this Sep 15, 2023

dwsutherland added the efficiency For notable efficiency improvements label Sep 15, 2023

dwsutherland requested a review from hjoliver September 15, 2023 02:05

dwsutherland mentioned this pull request Sep 15, 2023

Get rid of the hidden pool for partially satisfied tasks #5638

Closed

dwsutherland added small and removed small labels Sep 15, 2023

hjoliver reviewed Sep 18, 2023

View reviewed changes

dwsutherland mentioned this pull request Sep 20, 2023

parentless sequential xtrigger spawning #5738

Merged

7 tasks

dwsutherland closed this Sep 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parentless sequential clock trigger spawns #5732

parentless sequential clock trigger spawns #5732

dwsutherland commented Sep 15, 2023 •

edited

Loading

dwsutherland commented Sep 15, 2023

hjoliver left a comment

hjoliver Sep 18, 2023

hjoliver Sep 18, 2023

hjoliver Sep 18, 2023

dwsutherland Sep 18, 2023

hjoliver Sep 18, 2023

dwsutherland Sep 18, 2023

hjoliver commented Sep 18, 2023

hjoliver commented Sep 18, 2023

hjoliver commented Sep 18, 2023 •

edited

Loading

dwsutherland commented Sep 18, 2023 •

edited

Loading

dwsutherland commented Sep 20, 2023

	# Gather named wall-clocks closet to current time.
	# Gather named wall-clock triggers closest to current time.

parentless sequential clock trigger spawns #5732

parentless sequential clock trigger spawns #5732

Conversation

dwsutherland commented Sep 15, 2023 • edited Loading

dwsutherland commented Sep 15, 2023

hjoliver left a comment

Choose a reason for hiding this comment

hjoliver Sep 18, 2023

Choose a reason for hiding this comment

hjoliver Sep 18, 2023

Choose a reason for hiding this comment

hjoliver Sep 18, 2023

Choose a reason for hiding this comment

dwsutherland Sep 18, 2023

Choose a reason for hiding this comment

hjoliver Sep 18, 2023

Choose a reason for hiding this comment

dwsutherland Sep 18, 2023

Choose a reason for hiding this comment

hjoliver commented Sep 18, 2023

hjoliver commented Sep 18, 2023

hjoliver commented Sep 18, 2023 • edited Loading

dwsutherland commented Sep 18, 2023 • edited Loading

dwsutherland commented Sep 20, 2023

dwsutherland commented Sep 15, 2023 •

edited

Loading

hjoliver commented Sep 18, 2023 •

edited

Loading

dwsutherland commented Sep 18, 2023 •

edited

Loading