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

1933: Add spec file for LB #1982

Merged
merged 10 commits into from
Oct 12, 2022
Merged

1933: Add spec file for LB #1982

merged 10 commits into from
Oct 12, 2022

Conversation

JacobDomagala
Copy link
Contributor

Fixes #1933

@github-actions
Copy link

github-actions bot commented Sep 27, 2022

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-3.9, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-5.0, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (gcc-6, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


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

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (gcc-5, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


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

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-9, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-13, alpine, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (nvidia cuda 10.1, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


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

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-11, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-13, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-10, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (intel icpx, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-14, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (clang-12, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (gcc-11, ubuntu, mpich, json schema test)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


PR tests (intel icpc, ubuntu, mpich)

Build for 9061c71

Compilation - successful

Testing - passed

Build log


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

Build for 9061c71

Compilation - successful

Testing - passed

Build log


@JacobDomagala JacobDomagala force-pushed the 1933-add-spec-file-for-lb branch 5 times, most recently from 566e2e0 to 4aaf7e7 Compare October 4, 2022 15:58
@JacobDomagala JacobDomagala marked this pull request as ready for review October 4, 2022 15:59
@nlslatt
Copy link
Collaborator

nlslatt commented Oct 4, 2022

@JacobDomagala You'll need to re-push now that you've marked this as ready in order for the full PR testing to run.

@JacobDomagala JacobDomagala force-pushed the 1933-add-spec-file-for-lb branch 2 times, most recently from 4326559 to 566e2e0 Compare October 4, 2022 16:14
@codecov
Copy link

codecov bot commented Oct 4, 2022

Codecov Report

Merging #1982 (9061c71) into develop (eb27363) will increase coverage by 0.03%.
The diff coverage is 93.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1982      +/-   ##
===========================================
+ Coverage    84.35%   84.38%   +0.03%     
===========================================
  Files          730      730              
  Lines        25569    25644      +75     
===========================================
+ Hits         21568    21640      +72     
- Misses        4001     4004       +3     
Impacted Files Coverage Δ
src/vt/configs/arguments/app_config.h 100.00% <ø> (ø)
src/vt/trace/trace.cc 49.71% <0.00%> (ø)
src/vt/vrt/collection/balance/node_lb_data.h 100.00% <ø> (ø)
src/vt/utils/file_spec/spec.h 95.83% <83.33%> (ø)
src/vt/utils/file_spec/spec.cc 92.30% <90.00%> (ø)
src/vt/configs/arguments/args.cc 94.54% <100.00%> (+0.05%) ⬆️
src/vt/runtime/runtime.cc 74.88% <100.00%> (+0.05%) ⬆️
src/vt/vrt/collection/balance/node_lb_data.cc 83.33% <100.00%> (+2.03%) ⬆️
tests/unit/collection/test_lb.extended.cc 92.83% <100.00%> (+0.79%) ⬆️
tests/unit/trace/test_trace_spec_reader.cc 100.00% <100.00%> (ø)
... and 5 more

Copy link
Collaborator

@nlslatt nlslatt left a comment

Choose a reason for hiding this comment

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

Looks good to me

src/vt/configs/arguments/args.cc Outdated Show resolved Hide resolved
@JacobDomagala JacobDomagala force-pushed the 1933-add-spec-file-for-lb branch from 6b2a117 to a937c36 Compare October 5, 2022 17:25
Copy link
Collaborator

@nmm0 nmm0 left a comment

Choose a reason for hiding this comment

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

Can you add documentation for the format and its usage (you can just point the reader to the trace spec file since the format is the same)? I think it is also important in the documentation to highlight how this is different from the "LB Specification File" (--vt_lb_file_name)

src/vt/configs/arguments/args.cc Show resolved Hide resolved
@JacobDomagala JacobDomagala force-pushed the 1933-add-spec-file-for-lb branch from 12f2208 to 9061c71 Compare October 10, 2022 20:46
Copy link
Collaborator

@nmm0 nmm0 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement LB specification file for specifying which phases output LB data when enabled
3 participants