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

1265 lb data replay without collection #1720

Merged
merged 41 commits into from
Nov 9, 2023
Merged

1265 lb data replay without collection #1720

merged 41 commits into from
Nov 9, 2023

Conversation

nlslatt
Copy link
Collaborator

@nlslatt nlslatt commented Mar 25, 2022

This PR allows replaying LB data (workloads) read from json files. It does not correctly handle communications and thus will not work with comm-aware load models or load balancers. To handle communications correctly, a vt refactor where communications are treated analogously to load data by the ProposedReassignment load model and the LBManager would likely be the best approach.

Closes #1265

@nlslatt nlslatt force-pushed the 1265-stats-replay-without-collection branch 5 times, most recently from 855587d to a765426 Compare March 25, 2022 18:53
@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (clang-3.9, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (gcc-5, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (gcc-7, ubuntu, mpich, trace runtime, LB)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@codecov
Copy link

codecov bot commented Mar 25, 2022 •
edited
Loading

Codecov Report

Merging #1720 (fa6c1bc) into develop (7be56f2) will decrease coverage by 1.50%.
Report is 201 commits behind head on develop.
The diff coverage is 94.43%.

❗ Current head fa6c1bc differs from pull request most recent head 794299d. Consider uploading reports for the commit 794299d to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1720      +/-   ##
===========================================
- Coverage    85.48%   83.98%   -1.50%     
===========================================
  Files          722      776      +54     
  Lines        25907    27297    +1390     
===========================================
+ Hits         22146    22925     +779     
- Misses        3761     4372     +611
Files Coverage Δ
src/vt/configs/arguments/app_config.h 100.00% <ø> (ø)
src/vt/configs/arguments/args.cc 95.37% <100.00%> (+0.80%) ⬆️
src/vt/runtime/runtime_banner.cc 64.85% <ø> (+1.95%) ⬆️
src/vt/vrt/collection/balance/baselb/baselb.h 100.00% <ø> (ø)
.../vt/vrt/collection/balance/lb_invoke/lb_manager.cc 76.52% <100.00%> (-4.40%) ⬇️
...c/vt/vrt/collection/balance/lb_invoke/lb_manager.h 100.00% <ø> (ø)
src/vt/vrt/collection/balance/model/raw_data.cc 100.00% <100.00%> (ø)
src/vt/vrt/collection/balance/workload_replay.h 100.00% <100.00%> (ø)
...sts/unit/collection/test_workload_data_migrator.cc 100.00% <100.00%> (ø)
src/vt/vrt/collection/balance/workload_replay.cc 84.21% <84.21%> (ø)

... and 491 files with indirect coverage changes

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (gcc-9, ubuntu, mpich, zoltan)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (clang-5.0, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (gcc-10, ubuntu, openmpi, no LB)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (gcc-6, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (clang-9, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (gcc-8, ubuntu, mpich, address sanitizer)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (nvidia cuda 10.1, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (clang-10, alpine, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@github-actions
Copy link

github-actions bot commented Mar 25, 2022 •
edited
Loading

PR tests (clang-10, ubuntu, mpich)

Build for cd90005

Compilation - successful

Testing - passed

Build log

@nlslatt nlslatt marked this pull request as ready for review March 25, 2022 20:52
@nlslatt
Copy link
Collaborator Author

nlslatt commented Mar 25, 2022

@ppebay I would love to run one of your (load-only) toy problems through this if you can get it into exactly the right json format. Until some changes are made to the json reader, the data effectively needs to be dumped by vt itself (so that the special bits of the object IDs are consistent with vt's usage).

@nlslatt nlslatt marked this pull request as draft March 29, 2022 18:03
@lifflander lifflander mentioned this pull request Mar 29, 2022
Open
nlslatt added 26 commits November 9, 2023 10:46
@nlslatt
#1265: tests: leverage refactor in testing
31cea61
@nlslatt
#1265: replay: clean up code
35f05da
@nlslatt
#1265: tests: add more replay tests
b693bbe
@nlslatt
#1265: replay: update license headers
3f07a72
@nlslatt
#1265: replay: add doxygen
39112db
@nlslatt
#1265: tests: add subphases to workload migrator test
9144b7b
@nlslatt
#1265: replay: allow in-memory testing
34e77ff
@nlslatt
#1265: tests: run replay without verifying
cb7d762
@nlslatt
#1265: replay: clarify usage in tool
3839d30
@nlslatt
#1265: replay: add replay namespace
3ec982e
@nlslatt
#1265: replay: clean up code
a74cfc9
@nlslatt
#1265: replay: update to use lb callbacks
b24acee
@nlslatt
#1265: replay: update to reflect stats renaming
d124d63
@nlslatt
#1265: replay: allow custom stats callback for testing
dafa15d
@nlslatt
#1265: tests: verify statistics under replay
b9c9263
@nlslatt
#1265: replay: fix doxygen
c896627
@nlslatt
#1265: replay: make compatibility updates
3af9b7b
@nlslatt
#1265: tests: make compatibility updates
d1b2194
@nlslatt
#1265: replay: make compatibility updates
3f7c972
@nlslatt
#1265: tests: make compatibility updates
d9e0f14
@nlslatt
#1265: phase manager: use phase passed from lb manager
245e27b
@nlslatt
#1265: lb manager: add accessors needed for statistics computation
ae5f091
@nlslatt
#1265: replay: print phase summary when simulating phase
638bc0b
@nlslatt
#1265: build: make tools build option lowercase
64dd099
@nlslatt
#1265: build: remove testing comment from macro
43b7755
@nlslatt
#1265: replay: remove unnecessary barrier
794299d
@nlslatt nlslatt force-pushed the 1265-stats-replay-without-collection branch from af9ad32 to 794299d Compare November 9, 2023 18:46
@nlslatt nlslatt requested a review from lifflander November 9, 2023 18:46
lifflander
lifflander approved these changes Nov 9, 2023
View reviewed changes
src/vt/vrt/collection/balance/workload_replay.cc
auto json = r.readFile();
auto sd = std::make_shared<LBDataHolder>(*json);

for (auto &phase_data : sd->node_data_) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

A minor improvement would be to conditionalize these loops on if (theConfig()->vt_debug_replay) {

@lifflander lifflander merged commit 65e7ebc into develop Nov 9, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lifflander lifflander lifflander approved these changes

@stmcgovern stmcgovern Awaiting requested review from stmcgovern

@nmm0 nmm0 Awaiting requested review from nmm0

@thearusable thearusable Awaiting requested review from thearusable

@JacobDomagala JacobDomagala Awaiting requested review from JacobDomagala

@ppebay ppebay Awaiting requested review from ppebay

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement general test driver for LB testing
2 participants
@nlslatt @lifflander