Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[Breaking Change] Tasking rewrite #987
[Breaking Change] Tasking rewrite #987
Changes from 27 commits
bee5684
e881ad9
90f3e59
92564e1
6fde57d
2320c0e
95818ba
d602a35
10a67f1
23803d0
a4db040
8b7d42a
52f0d5a
e6eb2e3
ce7a6bb
1ddc2e0
0bd54cf
6082812
07ae71a
c1dbcb3
fbbe02a
d39a31a
b074ee6
829e047
fc16f0f
b400c11
9957538
6a33dd6
bf290fc
6029f7d
ae047de
cf59020
dc16a32
18628be
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can a TL be cyclic and ever run to completion? I thought being cyclic implied that it would never complete.
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.
Maybe I'm not using quite the right words. The graph can have cycles, but at least one node in the cycle acts as a conditional, either continuing the cycle or exiting it depending on some condition. Is there a better way to describe 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.
I think it may be that we just have different mental models for the graph. In my mind, the graph is just defined by task dependencies and its structure is static. Each node in the graph is then associated with a status and I look for tasks that are available to do and try to switch their status. In this model, iterative task lists are just groupings of tasks in the graph where a task can return iterate and that sets the status of all tasks in the iterative list to incomplete.
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.
Is there ever a use case for
global_sync
? It seems like there will always be some other MPI communication (like a reduction) associated with any point in the task list that requires all ranks to be at the same point.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 think so. The important distinction of
global_sync
is that it does a reduction on the statuses across all MPI ranks to determine if a task is complete or not. Technically this may not be required if all MPI ranks are guaranteed to return the same status, as would be the case if they all checked some condition on some globally reduced quantity. Given that, we can probably avoid usingglobal_sync
in some places where one might naively think that a reduction on statuses is required. But I can imagine use cases where different ranks evaluate different things and the global reduction of statuses would be required.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.
My thought is that if we want to sync all ranks, there is always going to be some information that gets communicated between them (more than just that they are done). Maybe I am not being imaginative enough though.
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.
let me try to make something up quickly. Say we're pushing particles around that can move across many cells in a given time step. One algorithm you could code up would have every rank push all the particles that live on it's blocks until the end of the time step or until it leaves the rank's domain (at which point it sends them to the right neighbor). You could write this as an iterative thing with the completion check being a check of whether all particles on the rank have reached the end of the time step. Then the completion check is totally local to the rank, but you need to continue iterating globally until everybody agrees things are done.
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.
Sure, but that could also just be written as a reduction to get the total number of active particles in the simulation.