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

#2195: termination: make actions run consistently in a runnable #2196

Merged
merged 4 commits into from
Oct 4, 2023
Merged

#2195: termination: make actions run consistently in a runnable #2196

merged 4 commits into from
Oct 4, 2023

Conversation

lifflander
Copy link
Collaborator

Fixes #2195

@lifflander lifflander linked an issue Sep 28, 2023 that may be closed by this pull request
Fix addAction semantics #2195
Closed
@github-actions
Copy link

github-actions bot commented Sep 28, 2023 •
edited
Loading

Pipelines results

PR tests (clang-9, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-12, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (gcc-11, ubuntu, mpich, trace runtime, coverage)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, gcc-9, ubuntu, mpich)

Build for 4345027 (2023-10-03 14:40:25 UTC)

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]"
          detected during:
            instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>]" 
/vt/src/vt/objgroup/proxy/proxy_objgroup.impl.h(154): here
            instantiation of "vt::objgroup::proxy::Proxy<ObjT>::PendingSendType vt::objgroup::proxy::Proxy<ObjT>::reduce<f,Op,Target,Args...>(Target, Args &&...) const [with ObjT=vt::vrt::collection::lb::GreedyLB, f=&vt::vrt::collection::lb::GreedyLB::collectHandler, Op=vt::collective::PlusOp, Target=vt::objgroup::proxy::ProxyElm<vt::vrt::collection::lb::GreedyLB>, Args=<vt::vrt::collection::lb::GreedyPayload>]" 
/vt/src/vt/vrt/collection/balance/greedylb/greedylb.cc(222): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&MyObj::handler, Target=vt::objgroup::proxy::ProxyElm<MyObj>]" 
/vt/examples/callback/callback.cc(147): here

/vt/src/vt/pipe/pipe_manager.impl.h(133): warning: missing return statement at end of non-void function "vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]"
          detected during instantiation of "auto vt::pipe::PipeManager::makeSend<f,Target>(Target) [with f=&colHan, Target=vt::vrt::collection::VrtElmProxy<MyCol, vt::Index1D>]" 
/vt/examples/callback/callback.cc(153%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

@lifflander
Copy link
Collaborator Author

A test still needs to be written for this.

Matthew-Whitlock
Matthew-Whitlock reviewed Oct 3, 2023
View reviewed changes
src/vt/termination/term_action.cc
}

void TermAction::triggerAllEpochActions(EpochType const& epoch) {
void TermAction::produceOn(EpochType epoch) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't used anywhere

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is called from the function in term_action.impl.h which can't call produce directly due to a header dependency.

src/vt/termination/term_action.cc
} else {
return triggerAllEpochActions(epoch);
auto encapsulated_epoch = getCurrentEpoch();
theTerm()->produce(encapsulated_epoch);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a dependency between epoch and encapsulated_epoch? The runtime could detect circular dependencies that way and I think(?) it lowers the number of termination detection messages.

If we changed the order of handling an epoch finishing so that when epoch x finishes we inform dependent epochs only after enqueuing x's actions, we could also remove the explicit produce/consumes and just use epoch dependencies and runnables.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've created an issue to address this. Right now, this isn't quite possible due to how addDepenency is implemented. #2197

@lifflander lifflander mentioned this pull request Oct 4, 2023
Open
@lifflander lifflander merged commit 2ff5cc2 into develop Oct 4, 2023
25 checks passed
@Matthew-Whitlock Matthew-Whitlock mentioned this pull request Oct 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Matthew-Whitlock Matthew-Whitlock Matthew-Whitlock approved these changes

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

Successfully merging this pull request may close these issues.

Fix addAction semantics
2 participants
@lifflander @Matthew-Whitlock