2027: Create Runnable inside Objgroup's invoke #2039

JacobDomagala · 2022-12-07T22:01:10Z

Fixes #2027

src/vt/runnable/make_runnable.h

…ypes

github-actions · 2023-02-02T19:45:13Z

Pipelines results

PR tests (gcc-12, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.0, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
FAILED: tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o 
/usr/bin/ccache /nvcc_wrapper/build/nvcc_wrapper -DJSON_USE_IMPLICIT_CONVERSIONS=1 -DVT_NO_COLOR_ENABLED -I/vt/tests/unit -I/vt/lib/CLI -I/vt/lib/json/include -I/vt/lib/brotli/c/include -Irelease -I/vt/src -isystem /vt/tests/extern/googletest/googletest/include -isystem /vt/tests/extern/googletest/googletest -isystem /vt/lib/fmt/include -isystem /vt/lib/EngFormat-Cpp/include -isystem /build/checkpoint/install/include -isystem /build/detector/install/include -Wno-deprecated-gpu-targets -O3 -DNDEBUG -fdiagnostics-color=always -Wall -pedantic -Wshadow -Wno-unknown-pragmas -Wsign-compare -ftemplate-backtrace-limit=100 -Werror -fPIC -std=c++1z -MD -MT tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o -MF tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o.d -o tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o -c tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
/build/checkpoint/install/include/checkpoint/container/tuple_serialize.h(86): error%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (clang-9, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 703a972 (2023-02-08 18:01:15 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-12, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-11, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (nvidia cuda 11.2, ubuntu, mpich)

Build for ( UTC)

nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
FAILED: tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o 
/usr/bin/ccache /nvcc_wrapper/build/nvcc_wrapper -DJSON_USE_IMPLICIT_CONVERSIONS=1 -DVT_NO_COLOR_ENABLED -I/vt/tests/unit -I/vt/lib/CLI -I/vt/lib/json/include -I/vt/lib/brotli/c/include -Irelease -I/vt/src -isystem /vt/tests/extern/googletest/googletest/include -isystem /vt/tests/extern/googletest/googletest -isystem /vt/lib/fmt/include -isystem /vt/lib/EngFormat-Cpp/include -isystem /build/checkpoint/install/include -isystem /build/detector/install/include -Wno-deprecated-gpu-targets -O3 -DNDEBUG -fdiagnostics-color=always -Wall -pedantic -Wshadow -Wno-unknown-pragmas -Wsign-compare -ftemplate-backtrace-limit=100 -Werror -fPIC -std=c++1z -MD -MT tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o -MF tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o.d -o tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx.o -c tests/CMakeFiles/epoch_nompi.dir/Unity/unity_0_cxx.cxx
nvcc_wrapper does not accept standard flags -std=c++1z since partial standard flags and standards after C++17 are not supported. nvcc_wrapper will use -std=c++14 instead. It is undefined behavior to use this flag. This should only be occurring during CMake configuration.
/build/checkpoint/install/include/checkpoint/container/tuple_serialize.h(86): error: namespace "std" has no member "launder"

/build/checkpoint/install/include/checkpoint/container/tuple_serialize.h(86): error: type name is not allowed

/vt/src/vt/runnable/runnable.h(237): warning: constexpr if statements are a C++17 feature

/vt/src/vt/runnable/runnable.h(237): error: namespace "std" has no member "is_void_v"

/vt/src/vt/runnable/runnable.h(237): error: namespace "std" has no member "invoke_result_t"

/vt/src/vt/runnable/runnable.h(237): error: type name is not allowed-=-%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (intel icpc, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.
intel-cc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '-diag-disable=10441' to disable this message.-%0D%0A%0D%0A%0D%0A ==> And there is more. Read log. <==

Build log

PR tests (clang-13, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-10, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

PR tests (clang-14, ubuntu, mpich)

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

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

Build for 8820ec9 (2023-02-08 19:56:12 UTC)

Compilation - successful

Testing - passed

Build log

…roup::invoke

codecov · 2023-02-03T10:26:26Z

Codecov Report

Merging #2039 (703a972) into develop (cf4c06a) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 703a972 differs from pull request most recent head 8820ec9. Consider uploading reports for the commit 8820ec9 to get more accurate results

@@             Coverage Diff             @@
##           develop    #2039      +/-   ##
===========================================
+ Coverage    84.83%   84.87%   +0.03%     
===========================================
  Files          720      720              
  Lines        25632    25639       +7     
===========================================
+ Hits         21746    21760      +14     
+ Misses        3886     3879       -7

Impacted Files	Coverage Δ
src/vt/objgroup/manager.h	`100.00% <ø> (ø)`
src/vt/objgroup/proxy/proxy_objgroup_elm.h	`100.00% <ø> (ø)`
src/vt/runnable/runnable.cc	`87.69% <ø> (-0.88%)`	⬇️
src/vt/vrt/collection/invoke/invokable.impl.h	`100.00% <ø> (ø)`
src/vt/vrt/collection/manager.h	`100.00% <ø> (ø)`
examples/collection/jacobi1d_vt.cc	`90.08% <100.00%> (ø)`
examples/collection/jacobi2d_vt.cc	`94.89% <100.00%> (ø)`
src/vt/objgroup/manager.impl.h	`97.89% <100.00%> (+0.02%)`	⬆️
src/vt/objgroup/proxy/proxy_objgroup_elm.impl.h	`100.00% <100.00%> (ø)`
src/vt/runnable/make_runnable.h	`100.00% <100.00%> (ø)`
... and 6 more

src/vt/objgroup/manager.impl.h

PhilMiller · 2023-02-07T23:15:27Z

src/vt/objgroup/manager.impl.h

+            runnable::invoke<Type, f>(get(proxy), std::forward<Args>(args)...);
+          result = std::move(ret);
+        }
+      });


Does this all do the correct thing if f returns an l-value reference to a moveable but non-copyable type? Without looking at FunctionWrapper, if Ret preserves the reference, then Ret result just won't compile, I think.

My concern is that we may get back a reference to something that needs to be left alone, and then move from it.

More deeply, is there a reason that runLambda doesn't itself pass through the return value from the thing it runs, such that we could do

return runnable::makeRunnableVoid(...).runLambda([] { return runnable::invoke<Type, f>(get(proxy), forward(args)); });

If it needs to manage making finish calls on the various context bits, those could be move to a scope guard structure that will call them in their destructors, rather than in straight-line code, so that we can return without declaring temporaries that would need to be conscientious about reference types.

Good point. Now that runLambda does it immediately we can push the return value through

src/vt/runnable/make_runnable.h

src/vt/objgroup/manager.h

src/vt/vrt/collection/manager.h

JacobDomagala self-assigned this Dec 7, 2022

JacobDomagala linked an issue Dec 7, 2022 that may be closed by this pull request

Objgroup invoke does not create a runnable #2027

Closed

JacobDomagala commented Dec 7, 2022

View reviewed changes

src/vt/runnable/make_runnable.h Outdated Show resolved Hide resolved

JacobDomagala requested review from lifflander and PhilMiller December 7, 2022 22:04

JacobDomagala force-pushed the 2027-objgroup-invoke-does-not-create-a-runnable branch 2 times, most recently from be99d06 to 37b4f63 Compare December 13, 2022 18:20

nmm0 mentioned this pull request Dec 13, 2022

Meeting Agenda [do not close] #925

Open

JacobDomagala force-pushed the 2027-objgroup-invoke-does-not-create-a-runnable branch from 37b4f63 to 3bab5fd Compare January 9, 2023 22:25

JacobDomagala added 3 commits February 2, 2023 20:23

#2027: Tests: Add test to reproduce Non-copyable invoke call

137536f

#2027: Objgroup: Add separate invoke functions for different return t…

48c96ce

…ypes

#2027: Objgroup: Use withObjGroup for runnables in ObjGroup

2fa7392

JacobDomagala force-pushed the 2027-objgroup-invoke-does-not-create-a-runnable branch from 3bab5fd to 2fa7392 Compare February 2, 2023 19:23

JacobDomagala force-pushed the 2027-objgroup-invoke-does-not-create-a-runnable branch from 3ac6c5d to d903adc Compare February 3, 2023 09:10

JacobDomagala marked this pull request as ready for review February 3, 2023 10:04

#2027: ObjGroup: Use if constexpr to avoid multiple functions in ObjG…

caa92ea

…roup::invoke

JacobDomagala force-pushed the 2027-objgroup-invoke-does-not-create-a-runnable branch from d903adc to caa92ea Compare February 3, 2023 10:04

JacobDomagala requested review from cz4rs, nlslatt, nmm0, stmcgovern and thearusable February 3, 2023 12:22

lifflander approved these changes Feb 6, 2023

View reviewed changes

PhilMiller reviewed Feb 7, 2023

View reviewed changes

src/vt/objgroup/manager.impl.h Outdated Show resolved Hide resolved

PhilMiller reviewed Feb 7, 2023

View reviewed changes

src/vt/objgroup/manager.impl.h Outdated Show resolved Hide resolved

#2027: Objgroup: Inline checks for invoke's return type

6043acd

PhilMiller reviewed Feb 7, 2023

View reviewed changes

#2027: invoke: refactor greatly for C++17

703a972

PhilMiller reviewed Feb 8, 2023

View reviewed changes

src/vt/runnable/make_runnable.h Outdated Show resolved Hide resolved

PhilMiller reviewed Feb 8, 2023

View reviewed changes

src/vt/objgroup/manager.h Show resolved Hide resolved

PhilMiller reviewed Feb 8, 2023

View reviewed changes

src/vt/vrt/collection/manager.h Show resolved Hide resolved

#2027: runnable: clever workaround to avoid branching on return type

8820ec9

PhilMiller approved these changes Feb 8, 2023

View reviewed changes

lifflander merged commit 5444a54 into develop Feb 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2027: Create Runnable inside Objgroup's invoke #2039

2027: Create Runnable inside Objgroup's invoke #2039

JacobDomagala commented Dec 7, 2022

github-actions bot commented Feb 2, 2023 •

edited

Loading

codecov bot commented Feb 3, 2023 •

edited

Loading

PhilMiller Feb 7, 2023

PhilMiller Feb 7, 2023

lifflander Feb 7, 2023

2027: Create Runnable inside Objgroup's invoke #2039

2027: Create Runnable inside Objgroup's invoke #2039

Conversation

JacobDomagala commented Dec 7, 2022

github-actions bot commented Feb 2, 2023 • edited Loading

Pipelines results

codecov bot commented Feb 3, 2023 • edited Loading

Codecov Report

PhilMiller Feb 7, 2023

Choose a reason for hiding this comment

PhilMiller Feb 7, 2023

Choose a reason for hiding this comment

lifflander Feb 7, 2023

Choose a reason for hiding this comment

github-actions bot commented Feb 2, 2023 •

edited

Loading

codecov bot commented Feb 3, 2023 •

edited

Loading