cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
335 stars 94 forks source link

config: detect unsatisfiable tasks #4638

Open wxtim opened 2 years ago

wxtim commented 2 years ago

Edit:

Have generalised the issue based on the discussion below.

Cylc 7 used to identify unsatisfiable tasks, this information was provided in get_graph_raw which allowed > graph clients to represent these dependencies in a special manner.

We have lost this feature, we should try to re-implement it and:

  • Put the information back into get_graph_raw.
  • Consider additional outlets for the info e.g. a warning on config load?

Describe the bug

The way Cylc Graph displays dependencies on tasks that don't exist has changed:

Cylc 7 - non-existant task appeared, but grey. Now - non-existant task not plotted at all.

Steps to reproduce the bug

This workflow

[scheduler]
    UTC mode = True
    allow implicit tasks = True
[scheduling]
    initial cycle point = 20000101T00Z
    [[graph]]
        +PT6H/PT6H = """
            forecast[-PT6H] => forecast
        """

when graphed by cylc graph . 2000 20000101T12Z should display the following "ghost" dependency:

image

But doesn't.

Pull requests welcome! This is an Open Source project - please consider contributing a bug fix yourself (please read CONTRIBUTING.md before starting any work though).

Cylc Doc Change Required

May require re-write of exercise in cylc-doc/src/tutorial/scheduling/datetime-cycling

hjoliver commented 2 years ago

Interestingly, if you run that example it just shuts down without doing anything.

Under spawn-on-demand, we start up by auto-spawning any parentless tasks to start the workflow ... here there are no parentless tasks, but the parent is outside the recurrence bounds (but not outside the graph bounds), so nothing happens.

This is not incorrect, but would likely be confusing to non-experts. Extending pre-initial-ignore to individual recurrences would be one solution (I think there's an issue up for that somewhere...).

MetRonnie commented 2 years ago

Interestingly, if you run that example it just shuts down without doing anything.

Under spawn-on-demand, we start up by auto-spawning any parentless tasks to start the workflow ... here there are no parentless tasks, but the parent is outside the recurrence bounds (but not outside the graph bounds), so nothing happens.

This is not incorrect, but would likely be confusing to non-experts. Extending pre-initial-ignore to individual recurrences would be one solution (I think there's an issue up for that somewhere...).

This could prove rather confusing for users.

If the behaviour is to be left as is, could we warn about any instances of this during validation?

hjoliver commented 2 years ago

Extending pre-initial-ignore to individual recurrences would be one solution (I think there's an issue up for that somewhere...).

See #1936

Until then (or if that's complicated by not being able to universally ignore pre-recurrence dependencies) we should be able to get validation to see this.

hjoliver commented 2 years ago

Interestingly, if you run that example it just shuts down without doing anything.

BTW, Cylc 7 would start up and stay up, but stall immediately with unsatisfied prerequisites.

oliver-sanders commented 2 years ago

So this is effectively about detecting unsatisfiable tasks?

If so, and if we can develop a test it would be useful to generate warnings in cylc validate too.

hjoliver commented 2 years ago

So this is effectively about detecting unsatisfiable tasks?

Yes, and they can only exist at the start of a recurrence, offset from the initial cycle point.

oliver-sanders commented 2 years ago

Yes, and they can only exist at the start of a recurrence, offset from the initial cycle point.

I think recurrence exclusions (e.g. PT1H ! T05) could mess with this too.

oliver-sanders commented 2 years ago

Could the cyclic detection algorithm be used to detect this?

oliver-sanders commented 2 years ago

I think we can detect these nodes in get_graph_raw which is already called by the cyclic detection system during validate (usually).

https://github.com/cylc/cylc-flow/compare/master...oliver-sanders:get-graph-raw?expand=1

oliver-sanders commented 2 years ago

@wxtim This is not going to be done for 8.0.0, could you check the scheduling tutorial in the docs to make sure it's still ok.

wxtim commented 2 years ago

@wxtim This is not going to be done for 8.0.0, could you check the scheduling tutorial in the docs to make sure it's still ok.

The chunk of tutorial affected is commented out pending fix.

oliver-sanders commented 2 years ago

Removing the bug label, now the docs have been adapted this is merely a missing feature.

oliver-sanders commented 10 months ago

In the context of https://github.com/cylc/cylc-flow/issues/5945 this may be regarded as a bug.