Bug in suicide trigger handling

[hjoliver]

See #4324 (comment)

Consider this Cylc 7 graph:

[scheduling]
  [[graph]]
      R1 = """
a1 => b1
a1:fail | b1 => c => d1
a1:fail & c => !a1 & !b1 & !d1
a2 => b2
a2:fail | b2 => c => d2
a2:fail & c => !a2 & !b2 & !d2
"""
[runtime]
  [[root]]
     script = sleep 5
  [[a1, b1, d1]]
  [[a2, b2, d2]]
  [[c]]
  [[a2]]
     script = false

(Example first noted here: cylc/cylc-admin#129 (comment))

With a2 failing, sometimes d1 runs, sometimes it doesn't. In the latter case we get spawn-time warnings of invalid cycle point (out of sequence bounds) for d1 (and sometimes d2).

[hjoliver]

Phew, easy fix. The problem is the graph says both c => d1 and c => !d1. So this simple graph exhibits the same behaviour:

R1 = """
   c => d1
   c => !d1
"""

The result depends on which trigger the graph parser gets to first, because trigger can be normal or suicide, not both. Arguably the graph is invalid, but the full workflow above is trying to work around a graph parsing deficiency (see
cylc/cylc-admin#129 (comment)) so there might not be an alternative until we have dynamic subgraphs (Cylc 9). Ensuring that suicide triggers take precedence should fix the problem, if the parser encounters both for the same dependency.

[hjoliver]

Arguably the graph is invalid,

God damn it. It is invalid. And in fact the error results from a mistranslation (by me) of the original example, and is not necessary for the aforementioned workaround.

I'll just make it fail validation instead.

(a => b and a => !b looks like how suicide triggers actually work in Cylc 8 - they spawn the target task if it hasn't been spawned already, in case it has multiple suicide triggers ... but not surprisingly trying to support this in the syntax results in nasty problems ... and it isn't necessary anyway).