276 Callback parameterization #2136

lifflander · 2023-04-18T23:24:14Z

Fixes #276

This replaces the current proxy.reduce syntax with three calls that are symmetrical across object groups and collections.

proxy.allreduce<&MyCol::handler, collective::PlusOp>(value1, value2);
proxy.reduce<&MyCol::handler, collective::PlusOp>(proxy[1], value1, value2);
proxy.reduce<collective::PlusOp>(some_callback, value1, value2);

github-actions · 2023-04-19T00:08:18Z

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for 110388b (2023-04-26 17:07:55 UTC)

FAILED: src/CMakeFiles/vt.dir/Unity/unity_20_cxx.cxx.o 
/usr/bin/ccache /nvcc_wrapper/build/nvcc_wrapper -DJSON_USE_IMPLICIT_CONVERSIONS=1 -DVT_NO_COLOR_ENABLED -I/vt/lib/CLI -I/build/vt/release -I/vt/src -I/vt/lib/json/include -I/vt/lib/brotli/c/include -isystem /vt/lib/fmt/include -isystem /vt/lib/EngFormat-Cpp/include -isystem /build/checkpoint/install/include -Wno-deprecated-gpu-targets -O3 -DNDEBUG -Wall -pedantic -Wshadow -Wno-unknown-pragmas -Wsign-compare -ftemplate-backtrace-limit=100 -Werror -std=c++17 -MD -MT src/CMakeFiles/vt.dir/Unity/unity_20_cxx.cxx.o -MF src/CMakeFiles/vt.dir/Unity/unity_20_cxx.cxx.o.d -o src/CMakeFiles/vt.dir/Unity/unity_20_cxx.cxx.o -c /build/vt/src/CMakeFiles/vt.dir/Unity/unity_20_cxx.cxx
/vt/src/vt/collective/reduce/reduce.h: In member function 'vt::collective::reduce::Reduce::PendingSendType vt::collective::reduce::Reduce::reduce(vt::Node, Params&& ...)':
/vt/src/vt/collective/reduce/reduce.h:186:51: error: 'tuple' was not declared in this scope; did you mean 'std::tuple'?
  186 |     auto msg = vt::makeMessage<MsgT>(std::tuple{std::forward<Params>(params)...});
      |                                                   ^~~~~
      |                                                   std::tuple
/usr/include/c++/9/type_traits:2461:37: note: 'std::tuple' declared here
 2461 |   template<typename... _Elements>
      |                                     ^    
/vt/src/vt/collective/reduce/get_reduce_stamp.h: In static member function 'static auto vt::collective::reduce::GetReduceStamp<enable, Args>::getMsg(Args&& ...)':
/vt/src/vt/collective/reduce/get_reduce_stamp.h:60:32: error: 'tuple' was not declared in this scope; did you mean 'std::tuple'?
   60 |     return vt::makeMessage<MsgT>(std::tuple{std::forward<Args>(args)...});
      |                                ^~~~~
      |                                std::tuple
/usr/include/c++/9/type_traits:2461:37: note: 'std::tuple' declared here
 2461 |   template<typename... _Elements>
      |                                     ^    
/vt/src/vt/collective/reduce/get_reduce_stamp.h: In static member function 'static constexpr auto vt::collective::reduce::GetReduceStamp<typename std::enable_if<is_same_v<typename std::decay<typename std::tuple_element<(sizeof... (Args) - 1), std::tuple<_Tps ...> >::type>::type, vt::util::adt::AlignedCharUnion<vt::util::strong::detail::Strong<int, -1, vt::collective::reduce::detail::tags::TagTag>, vt::collective::reduce::detail::TagPair, vt::util::strong::detail::Strong<long unsigned int, 18446744073709551615, vt::collective::reduce::detail::tags::SeqTag>, vt::util::strong::detail::Strong<long unsigned int, 18446744073709551615, vt::collective::reduce::detail::tags::UserIDTag>, vt::epoch::EpochType> >, void>::type, Args ...>::getMsgHelper(std::tuple<_Args1 ...>, std::index_sequence<Idx ...>)':
/vt/src/vt/collective/reduce/ge%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (clang-9, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, ubuntu, mpich)

Build for 5de43d3 (2023-05-04 00:21:29 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(159): 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

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

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 1012272 (2023-06-13 17:01:22 UTC)

Compilation - successful

Testing - passed

Build log

nlslatt

Here's my feedback so far. I think it's important that others review this as well.

tutorial/tutorial_2b.h

src/vt/collective/reduce/operators/default_op.impl.h

src/vt/utils/fntraits/fntraits.h

src/vt/vrt/collection/balance/greedylb/greedylb.h

tests/unit/collection/test_collection_group.extended.cc

tests/unit/collection/test_collection_group_recreate.cc

tests/unit/pipe/test_callback_send_collection.extended.cc

src/vt/collective/reduce/reduce.h

src/vt/vrt/collection/balance/lb_data_restart_reader.h

PhilMiller · 2023-05-09T19:42:18Z

I haven't reviewed this in full detail, but it generally looks good. My comments above are marked 'Outdated', but they still apply to the code as it appears now.

src/vt/collective/reduce/get_reduce_stamp.h

src/vt/collective/reduce/operators/default_op.impl.h

src/vt/vrt/collection/balance/baselb/baselb.h

tutorial/tutorial_2b.h

PhilMiller · 2023-06-05T20:57:00Z

What does/would it mean for the elements of a collection A to call proxyB.reduce(...) for a different collection B?

lifflander marked this pull request as ready for review April 25, 2023 05:09

nmm0 mentioned this pull request Apr 25, 2023

Meeting Agenda [do not close] #925

Open

lifflander requested review from thearusable, JacobDomagala, cz4rs, PhilMiller, stmcgovern, nmm0 and nlslatt and removed request for thearusable and JacobDomagala April 25, 2023 22:22

lifflander force-pushed the 276-automatically-synthesize-message-types-for-sends-to-arbitrary-handlers-cb branch from 110388b to 066c1b3 Compare May 2, 2023 21:34

nlslatt reviewed May 5, 2023

View reviewed changes

PhilMiller reviewed May 9, 2023

View reviewed changes

src/vt/collective/reduce/reduce.h Outdated Show resolved Hide resolved

PhilMiller reviewed May 9, 2023

View reviewed changes

src/vt/vrt/collection/balance/lb_data_restart_reader.h Outdated Show resolved Hide resolved

lifflander force-pushed the 276-automatically-synthesize-message-types-for-sends-to-arbitrary-handlers-cb branch 2 times, most recently from 81f658a to 3d2346d Compare June 5, 2023 20:19

PhilMiller reviewed Jun 5, 2023

View reviewed changes

src/vt/collective/reduce/get_reduce_stamp.h Show resolved Hide resolved

PhilMiller reviewed Jun 5, 2023

View reviewed changes

src/vt/collective/reduce/operators/default_op.impl.h Outdated Show resolved Hide resolved

PhilMiller reviewed Jun 5, 2023

View reviewed changes

src/vt/vrt/collection/balance/baselb/baselb.h Show resolved Hide resolved

PhilMiller reviewed Jun 5, 2023

View reviewed changes

tutorial/tutorial_2b.h Outdated Show resolved Hide resolved

lifflander added 5 commits June 13, 2023 10:01

#276: pipe: start refactoring for c++17

73605a3

#276: pipe: make callback work with parameters

8455e1e

#276: pipe: finish up parameterization

792ef0f

#276: pipe: remove old traits

0587b24

#276: pipe: improve docs

adce89d

lifflander added 27 commits June 13, 2023 10:01

#276: example: small cleanup

d671855

#276: pipe: make send more explicit

cc20c21

#276: traits: work around difference with clang and gcc

59b93d6

#276: lb: update zoltanlb for new reduce interface

c7da00b

#276: reduce: add missing headers

8a7366c

#276: pipe: add returns to make nvcc pass

baac2fa

#276: reduce: ugly hack to work around nvcc/intel compiler bugs

1aea01b

#276: reduce: reorder arguments for bare reduce to match

e454bc5

#276: reduce: fix code to create a tuple

f7a945d

#276: lb: make reduce vector const&

4b056b0

#276: perf: use new interface for performance test

c376d16

#276: lb: make vector<bool> const&

27ba5fa

#276: tutorial: update and cleanup with new callback/reduce

6ae9090

#276: tests: fix spelling

374ccfc

#276: tutorial: factor out of tutorial number of elements

258220d

#276: reduce: fix typo in comment

4af5e3b

#276: greedylb: remove old message no longer needed

caa45c1

#276: tests: update print to reflect allreduce

cee09f2

#276: tests: remove dead code from test during refactor

60896c3

#276: reduce: simplify stamp and message extraction

32a1bd2

#276: tests: remove duplicated code

600105e

#276: utils: simplify traits when argument is not a pointer

d3fe8b4

#276: lb: remove another dead reduce message

0268648

#276: reduce: fix grammar in comment

063a12c

#276: tutorial: fix missing operator

d18eae3

#276: reduce: require empty data or sentinel for NoneOp

535b12d

#276: tests: remove unused variable

1012272

lifflander force-pushed the 276-automatically-synthesize-message-types-for-sends-to-arbitrary-handlers-cb branch from 5128584 to 1012272 Compare June 13, 2023 17:02

stmcgovern approved these changes Jun 13, 2023

View reviewed changes

lifflander merged commit 0db918f into develop Jun 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

276 Callback parameterization #2136

276 Callback parameterization #2136

lifflander commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 19, 2023 •

edited

Loading

nlslatt left a comment

PhilMiller commented May 9, 2023

PhilMiller commented Jun 5, 2023

276 Callback parameterization #2136

276 Callback parameterization #2136

Conversation

lifflander commented Apr 18, 2023 • edited Loading

github-actions bot commented Apr 19, 2023 • edited Loading

Pipelines results

nlslatt left a comment

Choose a reason for hiding this comment

PhilMiller commented May 9, 2023

PhilMiller commented Jun 5, 2023

lifflander commented Apr 18, 2023 •

edited

Loading

github-actions bot commented Apr 19, 2023 •

edited

Loading