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

Update docs for implicit/naked tasks #207

Merged
merged 6 commits into from
Mar 12, 2021

Conversation

MetRonnie
Copy link
Member

Companion to cylc/cylc-flow#4109

Formerly naked tasks
@MetRonnie MetRonnie added content Addition or modification of documentation small labels Mar 4, 2021
@MetRonnie MetRonnie self-assigned this Mar 4, 2021
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Great, would suggest dropping the old "dummy task" and "naked task" terminology.

Implicit Tasks
--------------

An :term:`implicit task` appears in the workflow graph but has no
explicit runtime configuration section. Such tasks automatically
inherit the default "dummy task" configuration from the root
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
inherit the default "dummy task" configuration from the root
inherit the default configuration from the root

src/glossary.rst Outdated
@@ -412,6 +412,29 @@ Glossary
* :term:`job`
* :term:`qualifier`

implicit task
naked task
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
naked task

Copy link
Member Author

Choose a reason for hiding this comment

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

What about changing the first line to "Implicit tasks (previously known as naked tasks)..."

Copy link
Member

Choose a reason for hiding this comment

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

Could do, however, the terminology before was a mixture of, dummy (incorrect), naked (strange) and naked-dummy (really strange and somewhat suggestive).

@MetRonnie MetRonnie marked this pull request as draft March 5, 2021 12:52
@MetRonnie

This comment has been minimized.

@MetRonnie
Copy link
Member Author

Great, would suggest dropping the old "dummy task" and "naked task" terminology.

The only issue is that, as I understand it, a task can be non-implicit but still be a dummy task (e.g. if it has an empty but explicit section under [runtime]). So I think there still has to be a term for dummy task, but I can't think of a good replacement

@oliver-sanders
Copy link
Member

I think we can probably get away without having a term for that, I'm not sure this is a concept we use often enough is it?

There isn't really such a thing as a dummy task in Cylc. You can have a task which has not been configured to do anything in particular, however, that task will still run. Also all tasks inherit from root and it is a common practice to set [runtime][root]script = rose task-run so just because the [runtime] entry is bare doesn't mean it doesn't do anything, although root (along with other families) are expanded out by cylc config --sparse '[runtime]'.

From memory I think there is one legit use in the suite design guide where we advise inserting "dummy" tasks between large families to reduce the number of edges between them:

# A*B edges
FAMILY_A:succeed-all => FAMILY_B

# A+B edges
FAMILY_A:succeed-all => dummy => FAMILY_B

Perhaps in that section we could call it a "blank task" or something like that, however, since SoD we may be able to just remove this section alltogether.

@MetRonnie
Copy link
Member Author

If you run a workflow in "dummy mode", does that mean it treats every task as having [runtime][X]script = """true"""?

@oliver-sanders
Copy link
Member

oliver-sanders commented Mar 9, 2021

Ach, not quite, dummy mode is a bit icky, it is actually sets script = sleep <time>.

I would expect it to unset everything that you had defined (besides the inherit config), however, it doesn't quite do that meaning that dummy mode isn't quite as dummy as one might think e.g. in "dummy" mode tasks are still submitted to batch systems etc.

Here's the code:

https://github.com/cylc/cylc-flow/blob/07c7f5c9c785f097066ffdf8d4d5630b5914d734/cylc/flow/config.py#L1264-L1325

The closest thing to a "dummy" i.e. a "bare" task (one with no config) occurs when you run the flow in dummy-local mode but even that is a sleeper task which produces every possible [output] before it exits.

https://cylc.github.io/cylc-doc/nightly/html/user-guide/running-suites.html?#simulating-suite-behaviour

I think we were trying to kill dummy mode but there was some use case left over, trying to remember. There's some note of it here - https://github.com/cylc/cylc-admin/blob/765e182557f6a7b702432cdae36b97f0a4c60b79/docs/feb2020-workshop-notes.md#wednesday

(Dummy and dummy-local would probably map better onto sleep and sleep-local).

@MetRonnie MetRonnie marked this pull request as ready for review March 12, 2021 11:28
@MetRonnie MetRonnie requested a review from datamel March 12, 2021 12:23
@MetRonnie MetRonnie removed the small label Mar 12, 2021
Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @MetRonnie

@datamel datamel merged commit 0452fdb into cylc:master Mar 12, 2021
@MetRonnie MetRonnie deleted the allow-implict-tasks branch March 12, 2021 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Addition or modification of documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants