Add capability to output task graph

[lroberts36]

PR Summary

This PR adds the functionality to output the graph of a TaskList, TaskRegion, or TaskCollection in the Graphviz format. I do not explicitly add the connectivity between TaskRegions, so they show up as disconnected graphs. It is pretty trivial to pull out the graph because of #917. Initially, I used the nameof library for writing out function names, but I am not actually convinced this is necessary. I think we get everything we want with

#define TF(...) std::string(#__VA_ARGS__), __VA_ARGS__

Let me know if you have thoughts on this. After reading the internet for a while, I am pretty sure that there is no way to get the names of tasks within task lists themselves since they are effectively function pointers and no name information is available (hence the need to allow passing a string naming the task).

Results look like (from the sparse advection test without refinement and a single block):

PR Checklist

• Code passes cpplint
• New features are documented.
• Adds a test for any bugs fixed. Adds tests for new features.
• Code is formatted
• Changes are summarized in CHANGELOG.md
• Change is breaking (API, behavior, ...)
  • Change is additionally added to CHANGELOG.md in the breaking section
  • PR is marked as breaking
  • Short summary API changes at the top of the PR (plus optionally with an automated update/fix script)
• CI has been triggered on Darwin for performance regression tests.
• Docs build
• (@lanl.gov employees) Update copyright on changed files

[pdmullen]

This is really cool and a powerful debugging tool. My approval comes from testing this utility in various downstream applications. All looks good to me. It even appears to be working with the iterative tasking infrastructure. And it has already uncovered various issues that needed addressing! 😬

My only comment is a non-blocker but for a future PR: It would be cool to thread these graphs through outputs so that they could be dumped to file, e.g., at the beginning of a simulation. In the meantime, however, as a debugging tool, calling std::cout << my_task_collection should be sufficient.

[Yurlungur]

LGTM. Looks like now the user has to call WriteTaskGraph by hand? That's probably for the best.

If I interpret this correctly, the task graph appending is optional--and this doesn't break downstream?

Also could we support

PARTHENON_ADDTASK(tasklist, dependency, _VA_ARGS_)

as sugar for the call around TF?

[lroberts36]

LGTM. Looks like now the user has to call WriteTaskGraph by hand? That's probably for the best.

Yeah, currently the user has to do something like std::cout << tc; to write the graph to screen. Patrick suggested having an option in the driver that would write the task graph to file on the first step.

If I interpret this correctly, the task graph appending is optional--and this doesn't break downstream?

Yes, this should have no impact on downstream codes. Although there is a label attached to all tasks now, it has no impact unless you call the output stream operator on a TaskCollection, TaskRegion, or TaskList. If you do call the output stream operator before completely building the task list, it is possible that it could break things since the output operator calls the graph completion task (which is required so that all the edges are correctly set before printing).

The vector of tasks that gets appended to for output of the graph is a temporary and has no impact on the internal task lists of Task*. Aside from the issue of calling the output operator to early and finalizing the graph to early, you should be able to write the task graph to file and then execute the task graph with no side effects.

Also could we support

PARTHENON_ADDTASK(tasklist, dependency, _VA_ARGS_)

as sugar for the call around TF?

I don't quite see what this would buy us?

[Yurlungur]

Yeah, currently the user has to do something like std::cout << tc; to write the graph to screen. Patrick suggested having an option in the driver that would write the task graph to file on the first step.

👍 would be kind of nice to have the driver do it automatically.

Yes, this should have no impact on downstream codes. Although there is a label attached to all tasks now, it has no impact unless you call the output stream operator on a TaskCollection, TaskRegion, or TaskList. If you do call the output stream operator before completely building the task list, it is possible that it could break things since the output operator calls the graph completion task (which is required so that all the edges are correctly set before printing).

Not super important... but can we add guard rails for that?

The vector of tasks that gets appended to for output of the graph is a temporary and has no impact on the internal task lists of Task*. Aside from the issue of calling the output operator to early and finalizing the graph to early, you should be able to write the task graph to file and then execute the task graph with no side effects.

👍

I don't quite see what this would buy us?

I just find TF a little ugly and I like that macro better. But it's a taste thing. No need to add it.

[lroberts36]

Yeah, currently the user has to do something like std::cout << tc; to write the graph to screen. Patrick suggested having an option in the driver that would write the task graph to file on the first step.

👍 would be kind of nice to have the driver do it automatically.

Yeah, agreed, but let's leave for a future PR.

Yes, this should have no impact on downstream codes. Although there is a label attached to all tasks now, it has no impact unless you call the output stream operator on a TaskCollection, TaskRegion, or TaskList. If you do call the output stream operator before completely building the task list, it is possible that it could break things since the output operator calls the graph completion task (which is required so that all the edges are correctly set before printing).

Not super important... but can we add guard rails for that?

I just included a check in AddTask that the task region containing it hasn't had BuildGraph called.

I just find TF a little ugly and I like that macro better. But it's a taste thing. No need to add it.

Josh and I thought this sort of pattern was a quick to implement, minimally invasive way to update old task list building code, but I agree that it can be a little obscure (which partially inspired the task function acronym choice...).

[Yurlungur]

Yeah, agreed, but let's leave for a future PR.

👍

I just included a check in AddTask that the task region containing it hasn't had BuildGraph called.

👍

Josh and I thought this sort of pattern was a quick to implement, minimally invasive way to update old task list building code, but I agree that it can be a little obscure (which partially inspired the task function acronym choice...).

Yeah. That's fine. Like I said it's a taste thing.