Fail "a => b & !b" in validation.

[hjoliver]

This is a small change with no associated Issue. Follow up #4334

Requirements 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.py and conda-environment.yml. (None)

• Appropriate tests are included (unit and/or functional).

• Appropriate change log entry included.

• No documentation update required.

[kinow]

Looks good to me!

Tested and confirmed this is now invalid and raises an error:

a & b => c
a => !c
b => !c

@hjoliver the example below is not invalid, because c is executed only if a and b succeed. But is not when only one of a or b succeeds, right?

a & b => c
a | b => !c
b | a => !c

Also, this runs OK, with no errors:

a & b => c
!a => c

While this one raises a GraphParseError:

a & b => c
a:fail => c

Isn't !a the same as a:fail?

My Cylc-Fu is a bit rusty, sorry!

[hjoliver]

Good testing @kinow 👍

Isn't !a the same as a:fail?

No, a:fail is when the task job fails (or is killed) while executing.

!a is a suicide trigger. a => !b means "if a succeeds, remove the task proxy b from the scheduler task pool". You can be forgiven for not remembering that as spawn-on-demand largely consigned suicide triggers to history. This all just came up again in the context of backward compatibility, to support Cylc 7 graphs full of the bastards.

This should fail, but the parser doesn't recognize a | b and b | a as the same expression.

a & b => c
a | b => !c
b | a => c

It might be easy to fix that for simple expressions, but in general equivalence of complex logical expressions is ah, beyond the scope of this PR!

(Your version of this above has a|b => !c and b|a => !c which is redundant but OK).

[hjoliver]

Also, this runs OK, with no errors:

and

While this one raises a GraphParseError:

Did you get those two the wrong way around?

The first one should fail, because a suicide ! doesn't make sense on the LHS of a trigger.

The second one should validate and run, but shutdown with c still waiting on a:failed.

[hjoliver]

It might be easy to fix that for simple expressions, but in general equivalence of complex logical expressions is ah, beyond the scope of this PR!

Probably not worth the effort given the limited-to-no use for suicide triggers in future.

[kinow]

Did you get those two the wrong way around?

Yes! Sorry 🤣 I used your unit test for the examples. And I got confused when trying to say whether the test passed (i.e. an expected exception was raised) or not.

PR looks good! And I'll be one of the first to review our new documentation & tutorial to brush up on Cylc graph syntax

Thanks @hjoliver !

[kinow]

And the coverage report is wrong. I debugged @hjoliver and it is testing the lines of code added in this PR (which codecov added a comment above saying it is not 😠 ).