-
Notifications
You must be signed in to change notification settings - Fork 13
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
amendment to amendent of Proposal amendment #190
amendment to amendent of Proposal amendment #190
Conversation
* Redefine task "incompletion" to avoid problem edge cases. * Permit pre-conditions in completion expressions. * Remove redundent safety check that was only required for the previous proposal
1fc7465
to
86389bd
Compare
Similarly, a task's outputs are "incomplete" or "not complete" if the completion | ||
condition is not satisfied. | ||
|
||
3.5 A task is "finished" if it has final task status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Finished" already has a well established meaning (succeeded or failed) so I don't think we should use that. I suggest we stick with "final status" (as you've done in 3.6 below) or "final task status".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, succeeded and failed are the two primary final statuses, and "finished" would be a nice term for a task that is not going to change automatically anymore, whether its outputs are complete or not.
But I have to agree, unfortunately, that the old :finished
pseudo output is a spanner in those works.
So ok, I guess we have to stick with just "final status" for that.
N.B. giving a short name for "finished with complete outputs" is not strictly | ||
necessary, but it is convenient. I propose "done": a that finishes with completed | ||
outputs has done its job and can be forgotten by the scheduler. "Not done" is the | ||
logical opposite of that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we are distinguishing between a task and it's outputs I don't don't think we need a new name.
A tasks output's are either "complete" or "incomplete" (I think we're all happy with that).
A task can be:
- not final
- final status with complete outputs
- I think it's fine to refer to this as a "complete" task (we're talking about a task here, not it's outputs)
- final status with incomplete outputs
- I think it's fine to refer to this as an "incomplete" task
- As long as we define an "incomplete" task as "a task that has reached final status with incomplete outputs" I don't see any cause for confusion
- (I think it would too easy to confuse "not done" with "not run" or "not finished")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[oops, previous agreement retracted - I read it late last night whilst semi-conscious].
A tasks output's are either "complete" or "incomplete" (I think we're all happy with that).
Correct.
I'm fine with this:
- a task is complete if it has final status and complete outputs
- (i.e., what I meant by "done" in an attempt to avoid this inexplicable controversy!)
So long as it goes with this:
- a task is incomplete if it is not complete
Then, we can simply say tasks are retained in the pool IF they attain a final status without completing.
Two questions:
- what is wrong with that?
- are you really disagreeing with the idea that incomplete should mean the same thing as "not complete"?!
Obviously an incomplete task that has final status IS "incomplete", but by the definition of "complete", so is a task that is not complete and does not have final status.
We can't just define "incomplete" independently of "complete" - we might as well say False != not True
!
It's not as if it's difficult to avoid this problem, there are several ways to do it without inconsistent use of these terms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused. A task can be:
- not final
- final status with complete outputs
- final status with incomplete outputs
So we're trying to agree terminology for those 3 task states.
Agreed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, a task can be:
- not final and incomplete outputs
- not final and complete outputs
- final and incomplete outputs
- final and complete outputs
Your proposed definition of a "complete task" is:
- final status and complete outputs
- (so far so good)
Therefore an "incomplete task" (i.e., "not complete") is:
- NOT (final and complete outputs), i.e., not final OR incomplete outputs
- (this includes waiting tasks, and active tasks that have not completed their outputs - and it is perfectly sensible to call those "incomplete")
But you evidently want to define (your words) an "incomplete task" as final status AND incomplete outputs. That contradicts the basic logic of completeness. Final and incomplete outputs is just a sub-category of "incomplete tasks".
It's completely (no pun intended 😁) unnecessary to do that. We just need to say:
- a complete task is final and has complete outputs
- otherwise the task is incomplete
- task pool logic is:
- remove tasks that are complete
- retain tasks that are final and incomplete
So maybe now you can see why I went through various other self-consistent options too. The concept of an "incomplete task" is not quite so clean when the definition of "complete task" includes final status as well as outputs.
Still it is one of the self-consistent options.
Other self-consistent options are:
- say only outputs can be complete or not, and tasks can be final or not
- (then we might want to say a "final task with complete outputs" is "done" or similar, for convenience)
- tie "in/complete task" entirely to "in/complete outputs" (i.e., no final status)
- (it works and is self-consistent, but is kinda redundant, and it is probably more confusing to have "complete tasks" that don't have final status)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently define an "incomplete" task as a task that finishes without completing all required outputs.
https://cylc.github.io/cylc-doc/stable/html/glossary.html#term-incomplete-task
We now need to tweak this to say final status rather than finishes but fundamentally I'm proposing we keep the existing definition which is widely used in the documentation. I don't see any logical inconsistently - a task can can only be complete or incomplete once it has reached a final status.
Note that "final with incomplete outputs" is an important state that we need an easy way to reference. We need to flag tasks in this state in the GUI and add an event for this state (cylc/cylc-flow#4957).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK ... here's my response.
Using "complete" and "incomplete" in this way IS inconsistent with the normal meaning of those terms, despite both your claims to the contrary. They are not instantaneous binaries like "succeeded" and "failed". Just Google the meaning of incomplete" - anything that becomes complete is logically incomplete until that point. It doesn't suddenly become complete or incomplete in an instant.
@dpmatthews you said (on Element) It's no more inconsistent than it used to be! ... which is clearly an admission that it is inconsistent!
Further, it's completely unnecessary to do this. We could easily (as per my unassailable logic above) just say that tasks achieve a final status with complete or incomplete outputs, and act accordingly.
IMO this is likely to confuse users, because they will naturally assume that they know what complete and incomplete mean, especially when we also link the concept to completion of outputs.
How much of a problem that is, is at this point a matter of opinion, but I'm not ecstatic about voluntarily choosing inconsistent terminology when we really don't have to do it.
The ONLY point in favour of this is evidently our own recent precedent: we already use "incomplete task" in this way. But:
- adding the definition of "complete task" in opposition to this makes the inconsistency more glaring
- and do you think many users will be put out by fixing this now? I'm pretty sure no one at NIWA is invested in the concept yet - we've been holding off pushing this stuff out till 8.3.0.
BUT if you guys really ain't gonna back down on this, I can live with it so long as we explicitly document that "complete task" and "incomplete task" have this weird meaning - whereby a task is not incomplete prior to achieving completion. Which does frankly warrant another use of the 🤯 emoji).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dpmatthews you said (on Element) It's no more inconsistent than it used to be! ... which is clearly an admission that it is inconsistent!
Dave is pointing out that the "inconsistency" you perceive was not introduced by my amendment, but comes from the original proposal.
Further, it's completely unnecessary to do this. We could easily (as per my unassailable logic above) just say that tasks achieve a final status with complete or incomplete outputs, and act accordingly.
Ok, it's going to be easier to bypass this argument than resolve it. I had already adopted your wording on #191 which you may have missed. Please see if this works for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dave is pointing out that the "inconsistency" you perceive was not introduced by my amendment, but comes from the original proposal.
Yes, I acknowledged that (and thus take some responsibility for the situation myself). But the amendment did make the inconsistency more obvious, and we can and should still fix things we got wrong in the past.
Ok, it's going to be easier to bypass this argument than resolve it
It's amusing that you both keep disagreeing with me on this without ever actually countering my argument, other than to say you don't see any inconsistency - which I don't believe, since incomplete != not complete
is transparently inconsistent!
But ok, it's a free(ish) world ...
I had already adopted your wording on #191 which you may have missed. Please see if this works for you.
... I'll take a look - I had missed it sorry, in a bit of a rush after returning from Easter break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would prefer not to introduce "done" and "not done" as using facing concepts.
Note also done !== not done
😁
Hopefully #191 will suffice?
That was just my attempt to introduce self-consistent short terms for when a task can be removed from the pool or not, by divorcing the terminology from the concept of output completion.
Is that a typo? I certainly agree that
Hopefully! ... |
Yep, all good, phew! |
My small tweak of #189 - terminology only, but it's critical terminology for how we document this functionality.
My (only!) problem with #189 is that it changes the meaning of terms in such a way that
incomplete task
no longer means the same thing asa task that is not complete
. Given the universally agreed meaning of those very commonly used words, we can't do that.We have to (or at least it makes undeniable sense to) describe outputs as "complete" or "incomplete", but we do not have to call tasks "complete" or "incomplete". The only essential concepts are:
So, my proposed amendment to the proposed amendment to the amended proposal is essentially this:
As non-essential bonus I propose we shorten (when appropriate) "finished with complete outputs" to "done" (the task has done its job in the workflow and the scheduler can forget it)
This branch is a clone of 189, with another commit (mine) on top. So the full diff includes all the changes of @oliver-sanders plus mine. Look at the final commit alone to see my on-top changes.