Skip to content

Add future final-incomplete tasks to n=0 for visibility. - #7248

Draft
hjoliver wants to merge 4 commits into
cylc:masterfrom
hjoliver:future-incomplete-n0
Draft

Add future final-incomplete tasks to n=0 for visibility.#7248
hjoliver wants to merge 4 commits into
cylc:masterfrom
hjoliver:future-incomplete-n0

Conversation

@hjoliver

@hjoliver hjoliver commented Mar 25, 2026

Copy link
Copy Markdown
Member

Close #6383

Add final-incomplete tasks created by cylc setting a future task to the n=0 window, for visibility.

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).
  • Tests are included (or explain why tests are not needed).
  • Changelog entry 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 ?.?.x branch.

@hjoliver hjoliver added this to the 8.7.0 milestone Mar 25, 2026
@hjoliver hjoliver self-assigned this Mar 25, 2026
@hjoliver hjoliver added the could be better Not exactly a bug, but not ideal. label Mar 25, 2026
@hjoliver
hjoliver marked this pull request as ready for review March 25, 2026 20:54
@hjoliver
hjoliver force-pushed the future-incomplete-n0 branch from ee53c54 to 2302d50 Compare March 25, 2026 20:56
@hjoliver hjoliver changed the title Add future incomplete tasks to n=0 for visibility. Add future final-incomplete tasks to n=0 for visibility. Mar 25, 2026
@hjoliver
hjoliver requested a review from oliver-sanders April 9, 2026 03:55

@oliver-sanders oliver-sanders left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and makes sense.

I was playing around with this branch to test out the implications on runahead limit when I discovered that the following example behaves differently on this branch?!

flow.cylc

[scheduler]
    [[events]]
        startup handlers = start; echo

[scheduling]
    cycling mode = integer
    initial cycle point = 1
    [[graph]]
        P1 = a

[runtime]
    [[a]]

bin/start

#!/usr/bin/env bash

cylc set $CYLC_WORKFLOW_ID//10/a --out=failed

sleep 3

cylc set $CYLC_WORKFLOW_ID//1/a
sleep 1
cylc set $CYLC_WORKFLOW_ID//2/a
sleep 1
cylc set $CYLC_WORKFLOW_ID//3/a
sleep 1
cylc set $CYLC_WORKFLOW_ID//4/a
sleep 1
cylc set $CYLC_WORKFLOW_ID//5/a
sleep 1
cylc set $CYLC_WORKFLOW_ID//6/a

Here, run1 is 8.6.x, run2 is this branch:

   - tmp.hBEPWuun2h/run1 - paused 1■
      - ̿○ 7
           ̿○ a
      - ̿○ 8
           ̿○ a
      - ̿○ 9
           ̿○ a
      - ⊗ 10
           ⊗ a
      - ̿○ 11
           ̿○ a
      - ̀○ 12
           ̀○ a
   - tmp.hBEPWuun2h/run2 - paused 1■
      - ̿○ 7
           ̿○ a
      - ̿○ 8
           ̿○ a
      - ̿○ 9
           ̿○ a
      - ⊗ 10
           ⊗ a

And:

$ sqlite3 ~/cylc-run/tmp.hBEPWuun2h/run1/log/db 'SELECT * FROM task_pool'
7|a|[1]|waiting|0
8|a|[1]|waiting|0
9|a|[1]|waiting|0
10|a|[1]|failed|0
11|a|[1]|waiting|0
12|a|[1]|waiting|0
$ sqlite3 ~/cylc-run/tmp.hBEPWuun2h/run2/log/db 'SELECT * FROM task_pool'
10|a|[1]|failed|0
7|a|[1]|waiting|0
8|a|[1]|waiting|0
9|a|[1]|waiting|0

Take out the sleeps and they behave the same?!

Might be an unrelated issue exposed by this change? Or just an unexpected interaction with parentless spawning?

@hjoliver

Copy link
Copy Markdown
Member Author

Well, good spotting - I'll take a look...

@dwsutherland dwsutherland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I'm glad this change is on the pool end 👍 (and not the store)

(pending Oliver's is addressed)

@hjoliver

hjoliver commented Apr 24, 2026

Copy link
Copy Markdown
Member Author

Or just an unexpected interaction with parentless spawning?

Note to self, when I look into this ASAP - parentless spawning is [was] fundamentally broken for non-trivial mixed parented/parentless tasks, fixed by #7237 [now merged]

@oliver-sanders
oliver-sanders marked this pull request as draft May 26, 2026 10:01
@hjoliver

Copy link
Copy Markdown
Member Author

[Coming back to this after #7237 -> #7400 ]

@hjoliver
hjoliver force-pushed the future-incomplete-n0 branch from 2302d50 to d8ffb7f Compare July 29, 2026 04:04
@hjoliver

Copy link
Copy Markdown
Member Author

@oliver-sanders - the last commit fixes your test case. It did reveal a problem for parentless tasks: adding a future incomplete task to the pool skips over the runahead release point, which is when the next parentless instance is spawned.

This revealed a hole in the test suite, which is good: we don't test that the next parentless instances of future final incomplete tasks (made via cylc set) are spawned. On current master, restoring a future final incomplete task from the DB does do that, because it still goes through runahead release despite being final-status - but that's technically unnecessary and even somewhat surprising, so we could easily have broken it with future changes.

I'll leave this as a DRAFT until #7374 is merged, then I'll add a new test here to cover this (otherwise the new test will need to be rejigged after 7374).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

could be better Not exactly a bug, but not ideal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handling future final-incomplete tasks

3 participants