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.

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

723 Add debugging epoch labels #724

Merged
merged 15 commits into from
Mar 9, 2020
Merged

723 Add debugging epoch labels #724

merged 15 commits into from
Mar 9, 2020

Conversation

lifflander
Copy link
Collaborator

@lifflander lifflander commented Mar 6, 2020

Fixes #723

Add a new optional parameter when creating a new epoch to specify a label that is dumped in debugging output (DOT files). In order to make this change correctly, a breaking change is made on non-default overloads of TerminationDetector::makeEpochCollective and TerminationDetector::makeEpochRooted due to char* to bool overload problems.

Thus, a tag class is used to specify/capture if the successor epoch should be the current epoch (or an manual override by the user) or whether a DS epoch should be used. This has the added benefit of making the interface more explicit and less error prone.

outfile-global.pdf

This has potential for beta.6, depending on feedback.

@codecov
Copy link

codecov bot commented Mar 6, 2020

Codecov Report

Merging #724 into develop will increase coverage by 0.01%.
The diff coverage is 94.11%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #724      +/-   ##
===========================================
+ Coverage    79.06%   79.08%   +0.01%     
===========================================
  Files          333      334       +1     
  Lines        10162    10174      +12     
===========================================
+ Hits          8035     8046      +11     
- Misses        2127     2128       +1
Impacted Files Coverage Δ
src/vt/termination/termination.h 100% <ø> (ø) ⬆️
src/vt/messaging/collection_chain_set.h 100% <100%> (ø) ⬆️
.../termination/test_termination_action_common.impl.h 92.53% <100%> (ø) ⬆️
tests/unit/termination/test_term_dep_send_chain.cc 100% <100%> (ø) ⬆️
src/vt/termination/epoch_tags.h 100% <100%> (ø)
...mination/test_termination_action_nested_collect.cc 100% <100%> (ø) ⬆️
src/vt/messaging/dependent_send_chain.h 100% <100%> (ø) ⬆️
...rmination/test_termination_action_nested_rooted.cc 100% <100%> (ø) ⬆️
tests/unit/termination/test_term_cleanup.cc 97.18% <100%> (+0.08%) ⬆️
src/vt/vrt/collection/manager.impl.h 81.89% <50%> (-0.08%) ⬇️
... and 1 more

@lifflander lifflander force-pushed the 723-epoch-labels branch 2 times, most recently from 141ad1e to 1f3d8bb Compare March 6, 2020 07:03

SuccessorEpochCapture::SuccessorEpochCapture()
: epoch_(
(theMsg()->getEpoch() != no_epoch and
Copy link
Contributor

@pnstickne pnstickne Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider TU-local/file method accepting theMsg()->getEpoch() -> maybe_epoch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this means.

Add a new optional parameter when creating a new epoch to specify a
label that is dumped in debugging output (DOT files). In order to make
this change correctly, a breaking change is made on non-default
overloads of TermminationDetector::makeEpochCollective and
TermminationDetector::makeEpochRooted due to char* to bool overload
problems.

Thus, a tag class is used to specify if the epoch should use the
current epoch as the succesor or whether a DS epoch should be
used. This has the added benefit of making the interface more explicit
and less error prone.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add labels to epochs for debugging (e.g., output to DOT file)
3 participants