-
Notifications
You must be signed in to change notification settings - Fork 40
Add task list based profiling #1337
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
base: develop
Are you sure you want to change the base?
Conversation
…ly chosen chunk size, and to be sparse aware
Co-authored-by: Luke Roberts <[email protected]>
Yurlungur
left a comment
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.
Looks like some of the new tests you added are failing. Otherwise LGTM.
| PARTHENON_REQUIRE(dict_.count(label) > 0, "Asking for non-existent timing region."); | ||
| return dict_[label]; |
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.
Why not just do this?
| PARTHENON_REQUIRE(dict_.count(label) > 0, "Asking for non-existent timing region."); | |
| return dict_[label]; | |
| return dict_.at(label); |
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 like having the descriptive error message and I think this isn't performance critical.
PR Summary
This PR adds the capability to time groupings of tasks in a task list for profiling. For instance, it was used to create the the by multigrid level timing plot shown below. For an example of using the task list timing, see the unit test.
Additionally, this PR starts on adding some unit tests for the tasking infrastructure itself.
mg_timing.pdf
PR Checklist