-
Notifications
You must be signed in to change notification settings - Fork 9
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
#2171: Implement addUserDefinedData function to PhaseManager #2199
Conversation
Pipelines resultsPR tests (gcc-12, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (gcc-9, ubuntu, mpich, zoltan, json schema test) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (clang-9, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (gcc-10, ubuntu, openmpi, no LB) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (clang-11, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (clang-13, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (clang-12, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (gcc-8, ubuntu, mpich, address sanitizer) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (clang-14, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (clang-10, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (intel icpc, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich) Build for 242e4ed (2023-11-06 23:16:51 UTC)
|
ce3e9d0
to
d54ee9b
Compare
b5be7fd
to
cb055a5
Compare
cb055a5
to
8dc9607
Compare
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 good to me
e15f5de
to
4ac5ada
Compare
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.
Please see concern below.
src/vt/phase/phase_manager.h
Outdated
auto j = std::make_shared<nlohmann::json>(); | ||
j->emplace(key, value); | ||
|
||
theNodeLBData()->getLBData()->user_per_phase_json_[phase] = j; |
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.
Does this approach limit the user-defined data to a single key-value pair? I think we should allow as much as the application wants to provide.
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.
Updated
f28d4c1
to
242e4ed
Compare
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 good
Fixes #2171