Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#269: added printing and calculating statistics from sub-phases #276

Closed
wants to merge 1 commit into from
Closed

#269: added printing and calculating statistics from sub-phases #276

wants to merge 1 commit into from

Conversation

marcinwrobel1986
Copy link
Contributor

  • added get_load from sub-phases in Rank
  • added compute_subphases to work models
  • added printing statistics from sub-phases in algorithms and main LBAF routine
  • added get_subphases_time to object
  • added printing statistics from sub-phases when populating from log
  • fixed returned type for is_sentinel in Rank

closes #269

@marcinwrobel1986 marcinwrobel1986 added the enhancement New feature or request label Aug 5, 2022
@marcinwrobel1986 marcinwrobel1986 requested a review from ppebay August 5, 2022 16:17
@marcinwrobel1986 marcinwrobel1986 self-assigned this Aug 5, 2022
@marcinwrobel1986
Copy link
Contributor Author

@ppebay Philippe please take a look if what I have done make sense. Best would be to try with nolb-8color-16nodes-data dataset. Thanks!

@ppebay ppebay marked this pull request as ready for review August 17, 2022 17:11
@PhilMiller PhilMiller self-requested a review August 17, 2022 17:14
@PhilMiller
Copy link
Member

It looks like there was a small mis-communication of what we were looking for. We primarily want the statistics computed and reported for each subphase, not sums over all of them.

Once we have that, then we can start doing comparisons between what an aggregate of the subphases shows and what's seen for the entire phase.

@ppebay
Copy link
Contributor

ppebay commented Oct 11, 2022

@marcinwrobel1986 are we ready to review this PR?

@marcinwrobel1986
Copy link
Contributor Author

We will be tomorrow.

@cz4rs cz4rs self-requested a review November 9, 2022 18:33
@marcinwrobel1986 marcinwrobel1986 changed the title WIP #269: added printing and calculating statistics from sub-phases #269: added printing and calculating statistics from sub-phases Nov 9, 2022
@marcinwrobel1986 marcinwrobel1986 marked this pull request as draft November 9, 2022 18:34
#269: - added get_load from sub-phases in Rank
70cd6fc 
- added compute_subphases to work models
- added printing statistics from sub-phases in algorithms and main LBAF routine
- added get_subphases_time to object
- added printing statistics from sub-phases when populating from log
- fixed returned type for is_sentinel in Rank
@marcinwrobel1986 marcinwrobel1986 force-pushed the 269-report-subphase-sensitive-imbalance-alongside-general-imbalance branch from feede4d to 70cd6fc Compare November 10, 2022 17:45
@ppebay
Copy link
Contributor

ppebay commented Nov 18, 2022

@marcinwrobel1986 is this PR ready for review?

@marcinwrobel1986
Copy link
Contributor Author

@marcinwrobel1986 is this PR ready for review?

@ppebay this is calculating the load from subphases, but all the subphases. This part is finished and ready for review and to be merged. In current situation it would be a good idea to merge that and create another issue, with proper description that Phil created, where one would implement statistics computed and reported for each subphase. Below is Phil's comment:

It looks like there was a small mis-communication of what we were looking for. We primarily want the statistics computed and reported for each subphase, not sums over all of them. Once we have that, then we can start doing comparisons between what an aggregate of the subphases shows and what's seen for the entire phase.

@PhilMiller
Copy link
Member

Computing a summed load across all of the subphases is not especially useful functionality. The point is to do the subphase-sensitive calculations that would let us apply reasoning as described in this report: https://github.com/DARMA-tasking/vector-balancing-report

@ppebay
Copy link
Contributor

ppebay commented Nov 18, 2022

@PhilMiller I agree.

Should we nevertheless merge this current PR -- or discard it altogether, as the summation across all subphases is not useful?

@PhilMiller
Copy link
Member

Discard it, or fix it to do the useful things.

@ppebay
Copy link
Contributor

ppebay commented Jan 9, 2023

Discarding this PR that is not doing what is needed

@ppebay ppebay closed this Jan 9, 2023
@ppebay ppebay deleted the 269-report-subphase-sensitive-imbalance-alongside-general-imbalance branch January 9, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppebay ppebay Awaiting requested review from ppebay

@PhilMiller PhilMiller Awaiting requested review from PhilMiller

@cz4rs cz4rs Awaiting requested review from cz4rs

Assignees

@PhilMiller PhilMiller

@marcinwrobel1986 marcinwrobel1986

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Report subphase-sensitive imbalance alongside general imbalance
3 participants
@marcinwrobel1986 @PhilMiller @ppebay