Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suicide trigger prevents later cycles starting #6602

Open
wants to merge 1 commit into
base: 8.4.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes.d/6602.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug where suicide triggers could prevent initial cycle point tasks spawning.
7 changes: 4 additions & 3 deletions cylc/flow/taskdef.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,10 @@ def generate_graph_parents(
) -> Dict['SequenceBase', List[TaskTuple]]:
"""Determine concrete graph parents of task tdef at point.

Infer parents be reversing upstream triggers that lead to point/task.
Infer parents by reversing upstream triggers that lead to point/task.
"""
graph_parents: Dict['SequenceBase', List[TaskTuple]] = {}

for seq, triggers in tdef.graph_parents.items():
if not seq.is_valid(point):
# Don't infer parents if the trigger belongs to a sequence that
Expand Down Expand Up @@ -339,8 +340,8 @@ def has_only_abs_triggers(self, point):
if (
trig.offset_is_absolute or
trig.offset_is_from_icp or
# Don't count self-suicide as a normal trigger.
dep.suicide and trig.task_name == self.name
# Don't count suicide as a normal trigger:
dep.suicide
Copy link
Member

@oliver-sanders oliver-sanders Feb 11, 2025

Choose a reason for hiding this comment

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

This undoes a change @hjoliver made in #4970

5e7ffad#diff-b46dc58446bab522ef576c6d2e32a4276f57d60ff62e930e6f47762d47bf9507R296

I think this change makes sense, but will need to be reviewed in the context of whatever #4970 was trying to resolve.

Copy link
Member

Choose a reason for hiding this comment

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

Good spotting ...

Copy link
Member

@oliver-sanders oliver-sanders Feb 26, 2025

Choose a reason for hiding this comment

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

(leaving you two to resolve the differences between these changes)

Copy link
Member Author

Choose a reason for hiding this comment

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

@hjoliver what do you think we should do about this?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I got distracted on the way to looking at that...

):
has_abs = True
else:
Expand Down
44 changes: 44 additions & 0 deletions tests/integration/test_taskdef.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.


async def test_almost_self_suicide(flow, scheduler, start):
"""Suicide triggers should not count as upstream tasks when looking
to spawn parentless tasks.

https://github.com/cylc/cylc-flow/issues/6594

For the example under test, pre-requisites for ``!a`` should not be
considered the same as pre-requisites for ``a``. If the are then then
is parentless return false for all cases of ``a`` not in the inital cycle
and subsequent cycles never run.
"""
wid = flow({
'scheduler': {'cycle point format': '%Y'},
'scheduling': {
'initial cycle point': 1990,
'final cycle point': 1992,
'graph': {
'R1': 'install_cold',
'P1Y': 'install_cold[^] => a? => b?\nb:fail? => !a?'
}
}
})
schd = scheduler(wid)
async with start(schd):
tasks = [str(t) for t in schd.pool.get_tasks()]
for task in ['1990/a:waiting', '1991/a:waiting', '1992/a:waiting']:
assert task in tasks
Loading