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

add example workflows #5773

Merged
merged 2 commits into from
Jan 29, 2024
Merged

add example workflows #5773

merged 2 commits into from
Jan 29, 2024

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Oct 17, 2023

This is essentially a cut-down version of what we had at Cylc 7 - https://github.com/cylc/cylc-flow/tree/7.8.x/etc/examples

These examples will get auto-documented in the user-guide in cylc-doc.

One obvious omission, sub-workflows, @hjoliver, got a good flow.cylc to contribute?

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.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@oliver-sanders oliver-sanders added the doc Documentation label Oct 17, 2023
@oliver-sanders oliver-sanders added this to the cylc-8.3.0 milestone Oct 17, 2023
@oliver-sanders oliver-sanders self-assigned this Oct 17, 2023
oliver-sanders added a commit to oliver-sanders/cylc-doc that referenced this pull request Oct 17, 2023
* Document the major design patters for implementing Cylc workflows.
* Along with cylc/cylc-flow#5773, this closes cylc#627
* Add some functional examples for users to play with.
* The aim is to help assist workflow writers by providing them with
  minimal templates to flush out.
Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Nice, a few minor changes suggested. Lemme think about a sub-workflow example.

cylc/flow/etc/examples/converging-workflow/flow.cylc Outdated Show resolved Hide resolved
cylc/flow/etc/examples/converging-workflow/flow.cylc Outdated Show resolved Hide resolved
cylc/flow/etc/examples/datetime-cycling/flow.cylc Outdated Show resolved Hide resolved
cylc/flow/etc/examples/datetime-cycling/flow.cylc Outdated Show resolved Hide resolved
cylc/flow/etc/examples/datetime-cycling/flow.cylc Outdated Show resolved Hide resolved
cylc/flow/etc/examples/event-driven-cycling/README.rst Outdated Show resolved Hide resolved
cylc/flow/etc/examples/event-driven-cycling/README.rst Outdated Show resolved Hide resolved
cylc/flow/etc/examples/hello-world/index.rst Outdated Show resolved Hide resolved
cylc/flow/etc/examples/inter-workflow-triggers/index.rst Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member

This took me longer than it should have, but here's a minimal subworkflow example, with documentation sufficient to understand exactly how it works:

https://github.com/hjoliver/cylc-subwf-example-min

Even though it is (necessarily) more complicated than the other examples, I think it probably should be added. Use of subworkflows does come up occasionally. I'll format for this PR if you agree.

@oliver-sanders
Copy link
Member Author

Aah, that sub-workflow example relies on no-detach mode which has caveats and conflicts with auto-restart functionality.

I thought we had an example kicking around that used detaching with workflow_state polling on selected end tasks?

Might leave this out for the mo as it'll take a while to write up, we should document it though.

@oliver-sanders
Copy link
Member Author

Unlikely to get the time to flush these out further right now so suggest going forward with this.

@oliver-sanders oliver-sanders force-pushed the examples branch 2 times, most recently from 6dc9443 to 8e4c41a Compare November 23, 2023 16:33
@oliver-sanders oliver-sanders marked this pull request as ready for review November 23, 2023 16:55
@hjoliver
Copy link
Member

Aah, that sub-workflow example relies on no-detach mode which has caveats and conflicts with auto-restart functionality.

Yes, because that's meant to be a minimal example of sub-workflows, to illustrate the concept without the additional complexities of handling detaching sub-workflows. It does however say this:

Detaching sub-workflows, on the other hand, need a wrapper to poll for completion and kill them in response to a launcher task kill.

I thought we had an example kicking around that used detaching with workflow_state polling on selected end tasks?

Yes, I have a more complex example that can do it either way, but as-is probably too much for this purpose:

https://github.com/hjoliver/cylc-subwf-example

IMO no-detach is preferable for a minimal example that shows exactly what a sub-workflow is.

But I don't mind if we do that as a follow-up.

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Dec 13, 2023

Ok, best to come back to the sub-workflow example another day. FYI, my reasons for preferring the detaching solution:

  • The scheduler's exit code is a crude way of determining success e.g. the scheduler will exit with code 0 if the workflow is requested to shut down, e.g. cylc stop '*'.
  • Polling for the success of a specific task/output is a more general / robust solution, e.g. what if there are multiple tasks in the sub-workflow you want to trigger off of.
  • The approach is not compatible with Cylc's auto restart functionality.

#5875

@oliver-sanders oliver-sanders requested review from MetRonnie and hjoliver and removed request for hjoliver December 20, 2023 14:34
cylc/flow/etc/examples/datetime-cycling/flow.cylc Outdated Show resolved Hide resolved
cylc/flow/etc/examples/converging-workflow/index.rst Outdated Show resolved Hide resolved
cylc/flow/etc/examples/event-driven-cycling/README.rst Outdated Show resolved Hide resolved
cylc/flow/etc/examples/event-driven-cycling/bin/trigger Outdated Show resolved Hide resolved
cylc/flow/etc/examples/event-driven-cycling/bin/trigger Outdated Show resolved Hide resolved
cylc/flow/etc/examples/hello-world/index.rst Outdated Show resolved Hide resolved
cylc/flow/etc/examples/datetime-cycling/index.rst Outdated Show resolved Hide resolved
@hjoliver
Copy link
Member

hjoliver commented Jan 15, 2024

[subworkflow example]

FYI, my reasons for preferring the detaching solution:

Non-detaching provides a simpler first example, to help users understand the workflow-level concepts. Auto-migration and early subworkflow exit really doesn't matter for that use case.

For the detaching case, we'll probably have to provide the polling wrapper. Most users would struggle to understand how to do that.

Polling for the success of a specific task/output is a more general / robust solution, e.g. what if there are multiple tasks in the sub-workflow you want to trigger off of.

FYI, my detaching examples don't require you to specify a final task: if the task pool table is empty when the scheduler shuts down, the workflow is complete. (Triggering off mid-flow tasks is another matter, but again not really appropriate for a minimal example).

[UPDATE]: I guess the intention here is a template that users can adapt for use, not just a minimal example, so on that basis I'll do a follow-up PR with a minimal detaching example, polling wrapper included.

@oliver-sanders oliver-sanders mentioned this pull request Jan 22, 2024
12 tasks
* Partially addresses cylc/cylc-doc#627
* Add some examples of Cylc workflow implementation patterns.

---

Co-authored-by: Hilary James Oliver <[email protected]>
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Thanks. Don't know if it needs a second look by Hilary before merging

@oliver-sanders
Copy link
Member Author

This is the diff since Hillary's last review: 457ace5

Should be uncontroversial.

@oliver-sanders oliver-sanders merged commit 08d9c53 into cylc:master Jan 29, 2024
31 of 33 checks passed
@oliver-sanders oliver-sanders deleted the examples branch January 29, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants