Fix regional dependencies for sub-TaskLists and make solvers work for abitrary partitioning
#1132
Conversation
TaskLists
| @@ -177,7 +177,7 @@ CalcIndices(const NeighborBlock &nb, MeshBlock *pmb, | |||
| (neighbor_bounds[dir].e - neighbor_bounds[dir].s + 1); | |||
| s[dir] += nb.origin_loc.l(dir) % 2 == 1 ? extra_zones - interior_offset : 0; | |||
| e[dir] -= nb.origin_loc.l(dir) % 2 == 0 ? extra_zones - interior_offset : 0; | |||
| if (ir_type == IndexRangeType::InteriorSend) { | |||
| if (ir_type == IndexRangeType::InteriorSend && !prores) { | |||
There was a problem hiding this comment.
Changes to CalcIndices are required since we are actually passing the correct neighbor information in GetInterior*.
| @@ -59,6 +59,7 @@ struct BndInfo { | |||
| bool allocated = true; | |||
| bool buf_allocated = true; | |||
| int alloc_status; | |||
| bool same_to_same = false; | |||
There was a problem hiding this comment.
This flag denotes that this buffer represents a communication from this block to itself and that the buffer does not need to be filled (a message just needs to be sent).
| int other = (nb.gid == pmb->gid && (btype == BoundaryType::gmg_restrict_recv || | ||
| btype == BoundaryType::gmg_restrict_send)) | ||
| ? 1 | ||
| : 0; | ||
| return {sender_id, receiver_id, pcv->label(), location_idx, other}; |
There was a problem hiding this comment.
Add more information to the communication key so that we can represent same-to-same restriction and same-to-same prolongation as different channels.
| : Task(std::forward<TID>(dep), label, 0, func, limits) {} | ||
| template <typename TID> | ||
| Task(TID &&dep, const std::string &label, int verbose_level, | ||
| const std::function<TaskStatus()> &func, std::pair<int, int> limits = {1, 1}) | ||
| : label_(label), verbose_level_(verbose_level), f(func), exec_limits(limits) { |
There was a problem hiding this comment.
Add verbosity flag to tasks, if verbose_level_ > 0 the task will print its name out to stdout before starting the task. Useful for debugging.
There was a problem hiding this comment.
This is a useful feature. I wonder if the verbosity should be blanket applied to all tasks based off a verbose flag set in pin, rather than enrolled via each AddTask?
EDIT: this proposal could actually be handled via the downstream app
There was a problem hiding this comment.
I agree that would be nice, but I think that it requires a bit of threading to make it work (since the tasking stuff doesn't directly access pin currently). I would leave this for a future PR.
In practice, I just manually comment out the verbose_level_ > 0 test and recompile if I want to see everything...
| return AddTaskImpl(tq, dep, std::forward<Arg1>(arg1), 0, | ||
| std::forward<Args>(args)...); | ||
| } else if constexpr (is_tuple_t<Arg1>::value) { | ||
| return AddTaskImpl(tq, dep, std::get<0>(arg1), std::get<1>(arg1), std::get<2>(arg1), | ||
| std::forward<Args>(args)...); |
There was a problem hiding this comment.
Allow passing a tuple containing the name of the task, the verbosity level of the task, and the task function itself. Allows easy print out of the order in which tasks are run.
| std::vector<TaskList *> GetAllTaskLists() { | ||
| std::vector<TaskList *> list; | ||
| GetAllTaskListsInternal(list); | ||
| return list; | ||
| } |
There was a problem hiding this comment.
Necessary for linking regional tasks in iterative task lists.
TaskListsTaskLists and make solvers work for abitrary partitioning
| : Task(std::forward<TID>(dep), label, 0, func, limits) {} | ||
| template <typename TID> | ||
| Task(TID &&dep, const std::string &label, int verbose_level, | ||
| const std::function<TaskStatus()> &func, std::pair<int, int> limits = {1, 1}) | ||
| : label_(label), verbose_level_(verbose_level), f(func), exec_limits(limits) { |
There was a problem hiding this comment.
This is a useful feature. I wonder if the verbosity should be blanket applied to all tasks based off a verbose flag set in pin, rather than enrolled via each AddTask?
EDIT: this proposal could actually be handled via the downstream app
PR Summary
Currently, sublists of
TaskLists do not have their regional dependencies set correctly. This results in the bug described in #1125. This PR changesTaskRegion::BuildGraph()to correctly register regional dependencies among allTaskLists and closes #1125.Additionally, it takes a second step to make the multigrid solver work when
parthenon/mesh/pack_size != -1by including "self-communication" between coarse blocks on a finer two-level composite grid and fine leaf blocks on the next coarser two-level composite grid. Although no data is communicated in these communication channels, this introduces a dependency (outside of the tasking framework) that ensures operations on these blocks that are part of multiple gmg levels occur in the correct order.Getting the multigrid stuff to work was a painful debugging process. As a result, I introduced some capability for more customized naming of tasks and the ability to write task labels to screen as the tasks are called. This turned out to be fairly useful for debugging what was going on.
PR Checklist