cylc set: log warning when no outputs specified and task has no required outputs

[MetRonnie]

R1 = a? => b

$ cylc set workflow//1/a

does nothing and currently produces no warning.

This change makes it log a warning.

https://cylc.discourse.group/t/cylc-set-for-a-task-with-multiple-outputs-doesnt-seem-to-do-anything/1074

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).
• No dependency changes
• Tests are included
• No changelog entry needed as minor
• No docs needed
• If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

[MetRonnie]

I realised there was a slight mistake and the test was not quite what I wanted.

@oliver-sanders Why, for task a in

R1 = """
    a? => b
    a:x
"""

is the auto-generated completion statement (x and succeeded) or failed? This is making x optional even though it is not optional in the graph.

Shouldn't it be x and (succeeded or failed)?

[hjoliver]

Hmm, that does seem wrong.

Logically, it should be (x and succeeded) or (x and failed) i.e. x and (succeeded or failed).

[oliver-sanders]

is the auto-generated completion statement (x and succeeded) or failed? This is making x optional even though it is not optional in the graph.

Shouldn't it be x and (succeeded or failed)?

No, this is correct and as proposed (point 3).

There are four "final" task states:

• succeeded
• failed
• submit-failed
• expired

By default, outputs are associated with the success pathway. The task cannot be expected to complete these outputs in other scenarios as it hasn't necessarily run. Doing it as suggested above would prevent clean graph branching (e.g, in your example, you would need a suicide trigger to remove the task if it failed because it didn't produce the output "x").

Optional outputs as specified in the graph are used to configure the completion expression. The source of truth is in the completion expression itself.

[MetRonnie]

Doing it as suggested above would prevent clean graph branching (e.g, in your example, you would need a suicide trigger to remove the task if it failed because it didn't produce the output "x").

Can't say I understand this. In my example x should be a required output. If it didn't produce the output "x" then that should need an intervention. As succeeded is optional, it doesn't matter what the exit code of the script is, just as long as x is outputted.

[hjoliver]

OK I can see that proposal point 3 does indeed lead to (x and succeeded) or failed:

the expression is automatically generated according to the rules:

i. All required outputs (referenced in the graph) must be satisfied.
ii. Or, if success is optional, then the outputs are complete if it fails.

but I'm no longer entirely convinced that that's correct.

By default, outputs are associated with the success pathway. The task cannot be expected to complete these outputs in other scenarios

You're saying failure is optional, and failure could occur early in execution, before a:x can be generated.

That's true, but I think it's still legit to have a graph that requires the a:x path regardless of a's success or failure - and hence manual intervention if a:x is not generated.

a? => a-success-path
a:fail? => a-failure-path
a:x => a-x-path  # required either way

It's conceivable that a has a known and handle-able failure mode late in execution, and we need a:x to be generated even if it fails.

(Is that what you're saying @MetRonnie ?)

[MetRonnie]

Sorry to stoke an argument 🤭 I suggest we re-visit this after 8.4.0 is released. I will probably just stick completion = x on the task to make the test work

[oliver-sanders]

It's conceivable that a has a known and handle-able failure mode late in execution, and we need a:x to be generated even if it fails.

Yep, and for this niche case, the user can configure a completion expression to achieve the desired behaviour:

completion = x and (succeeded or failed)

But for the more common case where failure means that the task is not able to do what it was supposed to do, the current behavior avoids the need for suicide triggers whilst maintaining consistency with the handling of the other two final outputs, submit-failed and expired (where the output x cannot be produced naturally in any case).

a?
a:x

completion = (x and succeeded) or failed

a:expired?
a:x

completion = (x and succeeded) or expired

a:submit-failed?
a:x

completion = (x and succeeded) or submit_failed

Sorry to stoke an argument 🤭 I suggest we re-visit this after 8.4.0 is released

Yes please. If we do need to revisit this, later, not now!

[hjoliver]

It comes down to what should be the default completion condition vs what should require a user-defined one.

Looking at succeed/fail alone, to my mind we've got it the wrong way around because the current default is not the most literal interpretation of the graph.

Point taken on consistency with submit-failed and expired, but I'm not sure that matters. Succeed and fail are fundamentally different from those because they involve actually executing the job, so it's seems fine to me to treat them slightly differently on that basis.

But OK, on the back-burner for post-8.4.0 revisit!

[MetRonnie]

Btw it seems like instead of logging a warning, we might as well do what skip mode does and just emit the "success pathway" outputs if a user does cylc set task without the --out option and the task has no required outputs.

Arguably an enhancement so should go in 8.5.0 rather than 8.4.1?