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 explanatory info to intervention examples #758

Open
wants to merge 1 commit into
base: 8.4.x
Choose a base branch
from

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Aug 15, 2024

The new interventions section is great, but the examples need explaining a bit so that users can understand how they work sufficiently well to extrapolate to similar-but-different use cases.

[This PR was motivated by a user at NIWA who tried to apply the first example (Re-run a Task) to re-run a succeeded task, and wondered why nothing happended downstream of it in that case.]

I've gone for adding a new tab (GUI, Tui, CLI, Explanation) so that the "advanced" explanation doesn't distract from the basic example unless the user wants to see it. Alternatively, it could go in a text box below the example.

If approved in principle I'll add cross-references and add minimal explanations to the other examples as well.

image

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.

@hjoliver hjoliver added this to the 8.4.0 milestone Aug 15, 2024
@hjoliver hjoliver self-assigned this Aug 15, 2024
@hjoliver hjoliver force-pushed the interventions-explanations branch 2 times, most recently from ae73a81 to e5ba03f Compare August 15, 2024 23:00
@hjoliver hjoliver force-pushed the interventions-explanations branch from e5ba03f to 45d1a90 Compare August 16, 2024 00:38
Comment on lines +8 to +9
So Cylc allows you to take manual control of your workflow whilst it's running,
to do things like edit a task's configuration, re-run a section
Copy link
Member

Choose a reason for hiding this comment

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

If we can get rid of "do things like" and make the list happily comprehensive I'd be happier.

Suggested change
So Cylc allows you to take manual control of your workflow whilst it's running,
to do things like edit a task's configuration, re-run a section
Cylc allows you to take manual control of your workflow whilst it's running,
to do things like edit a task's configuration or re-run a section

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to keep this PR strictly to "explanatory info for the examples", rather than a more general revision (note there's a "whilst" in there too 😁 ) ... but I tend to agree with you on that point.

Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

I really like it as a concept, although I'm not completely convinced users will actually read it. I think that you will want @oliver-sanders approval for the idea.

@wxtim wxtim requested a review from oliver-sanders August 16, 2024 09:07
Comment on lines +93 to +101
In this example, a failed task blocked the flow because downstream
tasks depended on its success. Triggering the failed task to run again
(if it succeeds this time) allows the original flow to continue.

The flow will only continue downstream of a triggered task if it has not
already traversed that part of the graph.

So if you trigger a past task that did not block the flow, only the triggered
task itself will run - unless you tell Cylc to start a new flow.
Copy link
Member

Choose a reason for hiding this comment

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

IMO this is too technical, developers will understand it, but users probably won't (flows are an advanced concept). We should be able to explain this more simply without having to mention flows.

E.G:

Suggested change
In this example, a failed task blocked the flow because downstream
tasks depended on its success. Triggering the failed task to run again
(if it succeeds this time) allows the original flow to continue.
The flow will only continue downstream of a triggered task if it has not
already traversed that part of the graph.
So if you trigger a past task that did not block the flow, only the triggered
task itself will run - unless you tell Cylc to start a new flow.
In this example, a task has failed, as a result, some downstream tasks are not running.
Triggering the failed task causes it to run again.
If it succeeds, the downstream tasks will run and the workflow will continue.
Note, any downstream tasks which have already run will not be re-run as a result.

Copy link
Member Author

@hjoliver hjoliver Aug 16, 2024

Choose a reason for hiding this comment

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

I thought you might say that :-)

I don't think a "flow" is very technical. It's an intuitive concept with a very intuitive name. A single logical run of the workflow "flows" through the graph.

Your edit does get pretty close to describing what's going on without using the word "flow" but it stops short of saying how you can rerun "tasks which have already run" if you actually need to do that - which sometimes you might need to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

The aforementioned NIWA user did need to - she was surprised that the flow did not continue after retriggering a succeeded task like it did on retriggering the failed one in the example.

Copy link
Member Author

@hjoliver hjoliver Aug 16, 2024

Choose a reason for hiding this comment

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

Also, it's fine (and good!) to have more detailed "advanced" info available where appropriate so long as it doesn't distract from the core basic info. That's why I tried to hide it in a new tab.

What if we called it "advanced" rather than "explanation"?

Copy link
Member

@oliver-sanders oliver-sanders Aug 16, 2024

Choose a reason for hiding this comment

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

I don't think a "flow" is very technical. It's an intuitive concept with a very intuitive name. A single logical run of the workflow "flows" through the graph.

"Flows" are not part of the minimal terminology / concepts required to use Cylc and are not covered in the tutorial. You only learn about flows when trying to do --flow=new which is a niche requirement, most of our users do not know what "flow" means and there is great confusion amongst those who have tried to use use them (as previously discussed).

But we don't need to talk about flows here, so lets avoid it.

Copy link
Member

@oliver-sanders oliver-sanders Aug 16, 2024

Choose a reason for hiding this comment

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

Suggested change
In this example, a failed task blocked the flow because downstream
tasks depended on its success. Triggering the failed task to run again
(if it succeeds this time) allows the original flow to continue.
The flow will only continue downstream of a triggered task if it has not
already traversed that part of the graph.
So if you trigger a past task that did not block the flow, only the triggered
task itself will run - unless you tell Cylc to start a new flow.
In this example, a task has failed, as a result, some downstream tasks are not running.
Triggering the failed task causes it to run again.
If it succeeds, the downstream tasks will run and the workflow will continue.
Note, any downstream tasks which have already run will not be re-run as a result.
If you want to re-run downstream tasks, see :ref:`interventions.re-run-multiple-tasks`.

You'll need to add a interventions.re-run-multiple-tasks anchor in the next section to use this suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

But you haven't addressed the core point that motivated this PR. I'll repeat it:

If we don't explain why the example works the way it does, users won't be able to apply their knowledge to slightly different use cases. In particular (and as motivated by an actual USER) they need to know (a) why rerunning a failed task typically has different "flow-on" effects than rerunning a succeeded task; and (b) how to make the succeed case flow on if they actually need that.

I don't think intervention documentation is the right place to stick to absolute basics. That's what the tutorial is for.

Copy link
Member

Choose a reason for hiding this comment

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

I think my suggestion explains this more clearly right?

As there is only one flow in this example, we can explain this in the same way we would have at Cylc 7.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Our last two comments passed in the ether - I think you have mostly addressed my concerns now).

Copy link
Member Author

@hjoliver hjoliver Aug 16, 2024

Choose a reason for hiding this comment

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

As there is only one flow in this example, we can explain this in the same way we would have at Cylc 7.

Yes and no - users also need to know why the workflow won't "flow on" after triggering a past task that did not block the flow.

You have sort-of addressed this by referring forward to "re-run multiple tasks", but IMO it would help to have a brief explanation of the sort I suggested in this example too, because it helps put the specific example in a wider context. (After all the section is called "Re-run a Task" (any task!) not just "Re-run a Failed Task).

@oliver-sanders
Copy link
Member

Also, would it not make more sense to include this information in the body text (where it is always visible), rather than putting it into one of the tab groups? We want people to read this!

Comment on lines +90 to +91
.. tab-item:: Explanation
:sync: explanation
Copy link
Member

Choose a reason for hiding this comment

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

IMO this should not be a tab, which is meant to be for your preferred choice of interacting with Cylc. Instead it should be a "spoiler" admonition or ordinary admonition

Copy link
Member Author

Choose a reason for hiding this comment

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

(Yes, see my comment just below!)

Copy link
Member Author

Choose a reason for hiding this comment

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

(It depends on whether we are trying to keep the "advanced" info somewhat hidden, to avoid scaring users with the most basic needs)

Copy link
Member

Choose a reason for hiding this comment

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

Ronnie suggested a "spolier" which hides the content.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep I like that.

@hjoliver
Copy link
Member Author

Also, would it not make more sense to include this information in the body text (where it is always visible), rather than putting it into one of the tab groups? We want people to read this!

Haha, I agree, but I was trying to head off your expected response on "advanced" information as we've just been discussing 😁 but putting it in a place that required some opt-in by users.

@hjoliver
Copy link
Member Author

BTW, "the flow" is a nice intuitive way to describe how a single self-consistent workflow run propagates through the graph.

So e.g., a failed task (when failure is not optional) will typically "block the flow". To my mind that's really not technical and is the clearest way to describe how the system actually works - even if we're not talking about starting new flows in the Cylc 8 sense (although, thankfully, new flows are a perfect fit for the same concept).

@oliver-sanders oliver-sanders removed this from the 8.4.0 milestone Dec 3, 2024
@MetRonnie MetRonnie changed the base branch from master to 8.4.x January 10, 2025 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants