Handle reference error types in `ensure_started` and `let_error` #338

aurianer · 2022-07-28T16:01:51Z

Fixes partially #284

aurianer · 2022-07-28T16:01:59Z

bors try

bors · 2022-07-28T16:05:29Z

try

Build failed:

ci/circleci: clang_format

pika-bot · 2022-07-28T16:14:38Z

Performance test report

pika Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	+

Info

Property	Before	After
pika Datetime	2021-12-21T15:01:46+00:00	2022-07-28T16:01:53+00:00
pika Commit	01e4980e2184ca113b853ae3a36869f713fcc4b9	`1c972c1`
Envfile
Hostname	nid00932	nid01598
Datetime	2021-12-21T16:09:15.238666+01:00	2022-07-28T18:13:18.991806+02:00
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Clustername	daint	daint

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	+++

Info

Property	Before	After
pika Datetime	2021-12-10T13:50:04+00:00	2022-07-28T16:01:53+00:00
pika Commit	f499a2233385060b8a2612ab88163e62b0881888	`1c972c1`
Envfile
Hostname	nid00243	nid01598
Datetime	2021-12-10T15:19:42.442217+01:00	2022-07-28T18:13:33.570575+02:00
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Clustername	daint	daint

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
Stream Benchmark - Add	(=)	(=)	(=)
Stream Benchmark - Scale	--	-	---
Stream Benchmark - Triad	--	-	(=)
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
pika Datetime	2021-11-12T11:29:27+00:00	2022-07-28T16:01:53+00:00
pika Commit	f64fbd02165a132a6276cedd14c586910abb79e4	`1c972c1`
Envfile
Hostname	nid00007	nid01598
Datetime	2021-11-12T12:57:50.824026+01:00	2022-07-28T18:13:48.287236+02:00
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Clustername	daint	daint

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (>10%)
++/--	Large performance improvement/degradation (>10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

aurianer · 2022-07-29T16:38:00Z

bors try

pika-bot · 2022-07-29T16:45:40Z

Performance test report

pika Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	+

Info

Property	Before	After
pika Commit	01e4980e2184ca113b853ae3a36869f713fcc4b9	`a8e4ed8`
pika Datetime	2021-12-21T15:01:46+00:00	2022-07-29T16:37:44+00:00
Envfile
Clustername	daint	daint
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Hostname	nid00932	nid00005
Datetime	2021-12-21T16:09:15.238666+01:00	2022-07-29T18:44:35.930448+02:00

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	+++

Info

Property	Before	After
pika Commit	f499a2233385060b8a2612ab88163e62b0881888	`a8e4ed8`
pika Datetime	2021-12-10T13:50:04+00:00	2022-07-29T16:37:44+00:00
Envfile
Clustername	daint	daint
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Hostname	nid00243	nid00005
Datetime	2021-12-10T15:19:42.442217+01:00	2022-07-29T18:44:50.504521+02:00

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
Stream Benchmark - Add	(=)	(=)	(=)
Stream Benchmark - Scale	---	--	--
Stream Benchmark - Triad	(=)	(=)	(=)
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
pika Commit	f64fbd02165a132a6276cedd14c586910abb79e4	`a8e4ed8`
pika Datetime	2021-11-12T11:29:27+00:00	2022-07-29T16:37:44+00:00
Envfile
Clustername	daint	daint
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Hostname	nid00007	nid00005
Datetime	2021-11-12T12:57:50.824026+01:00	2022-07-29T18:45:05.164007+02:00

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (>10%)
++/--	Large performance improvement/degradation (>10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

bors · 2022-07-29T16:50:26Z

try

Build failed:

jenkins/cscs/clang-13-debug

msimberg

Looks good, but I still have to have a closer look at let_value/error again.

Could you update the PR title to mention the updated algorithms?

libs/pika/execution/include/pika/execution/algorithms/ensure_started.hpp

pika-bot · 2022-08-03T14:45:32Z

Performance test report

pika Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	+

Info

Property	Before	After
pika Datetime	2021-12-21T15:01:46+00:00	2022-08-03T14:38:33+00:00
pika Commit	01e4980e2184ca113b853ae3a36869f713fcc4b9	`f985242`
Hostname	nid00932	nid00025
Envfile
Datetime	2021-12-21T16:09:15.238666+01:00	2022-08-03T16:44:34.162291+02:00
Clustername	daint	daint
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	+++

Info

Property	Before	After
pika Datetime	2021-12-10T13:50:04+00:00	2022-08-03T14:38:33+00:00
pika Commit	f499a2233385060b8a2612ab88163e62b0881888	`f985242`
Hostname	nid00243	nid00025
Envfile
Datetime	2021-12-10T15:19:42.442217+01:00	2022-08-03T16:44:48.734884+02:00
Clustername	daint	daint
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
Stream Benchmark - Add	-	(=)	--
Stream Benchmark - Scale	-	--	--
Stream Benchmark - Triad	--	+	(=)
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
pika Datetime	2021-11-12T11:29:27+00:00	2022-08-03T14:38:33+00:00
pika Commit	f64fbd02165a132a6276cedd14c586910abb79e4	`f985242`
Hostname	nid00007	nid00025
Envfile
Datetime	2021-11-12T12:57:50.824026+01:00	2022-08-03T16:45:03.546849+02:00
Clustername	daint	daint
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (>10%)
++/--	Large performance improvement/degradation (>10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

aurianer · 2022-08-03T15:13:59Z

bors try

pika-bot · 2022-08-03T15:29:51Z

Performance test report

pika Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	+

Info

Property	Before	After
pika Datetime	2021-12-21T15:01:46+00:00	2022-08-03T15:09:49+00:00
pika Commit	01e4980e2184ca113b853ae3a36869f713fcc4b9	`9ab6177`
Hostname	nid00932	nid00829
Clustername	daint	daint
Envfile
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime	2021-12-21T16:09:15.238666+01:00	2022-08-03T17:28:37.175613+02:00

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	+++

Info

Property	Before	After
pika Datetime	2021-12-10T13:50:04+00:00	2022-08-03T15:09:49+00:00
pika Commit	f499a2233385060b8a2612ab88163e62b0881888	`9ab6177`
Hostname	nid00243	nid00829
Clustername	daint	daint
Envfile
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime	2021-12-10T15:19:42.442217+01:00	2022-08-03T17:28:51.761284+02:00

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
Stream Benchmark - Add	-	(=)	(=)
Stream Benchmark - Scale	--	--	---
Stream Benchmark - Triad	(=)	(=)	-
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
pika Datetime	2021-11-12T11:29:27+00:00	2022-08-03T15:09:49+00:00
pika Commit	f64fbd02165a132a6276cedd14c586910abb79e4	`9ab6177`
Hostname	nid00007	nid00829
Clustername	daint	daint
Envfile
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime	2021-11-12T12:57:50.824026+01:00	2022-08-03T17:29:06.622577+02:00

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (>10%)
++/--	Large performance improvement/degradation (>10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

bors · 2022-08-03T16:13:00Z

try

Build succeeded:

msimberg

Except for the one unnecessary comment this looks good.

libs/pika/execution/tests/unit/algorithm_then.cpp

This test fails with the commit of the P2300 reference implementation used in pika (9159e4925a9895a6959e01e8e0ed22db4199be1a). This same test passes with origin/main i.e. (fb798bb51f66b6df5806a17cc5da1eeef25b25d6).

aurianer · 2022-08-04T10:34:04Z

bors merge

bors · 2022-08-04T10:34:06Z

👎 Rejected by code reviews

pika-bot · 2022-08-04T10:40:43Z

Performance test report

pika Performance

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
For Each	(=)	(=)	+

Info

Property	Before	After
pika Commit	01e4980e2184ca113b853ae3a36869f713fcc4b9	`2498a60`
pika Datetime	2021-12-21T15:01:46+00:00	2022-08-04T10:33:31+00:00
Hostname	nid00932	nid00025
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime	2021-12-21T16:09:15.238666+01:00	2022-08-04T12:39:21.175743+02:00
Clustername	daint	daint
Envfile

Comparison

BENCHMARK	NO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch	+++

Info

Property	Before	After
pika Commit	f499a2233385060b8a2612ab88163e62b0881888	`2498a60`
pika Datetime	2021-12-10T13:50:04+00:00	2022-08-04T10:33:31+00:00
Hostname	nid00243	nid00025
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime	2021-12-10T15:19:42.442217+01:00	2022-08-04T12:39:35.607749+02:00
Clustername	daint	daint
Envfile

Comparison

BENCHMARK	FORK_JOIN_EXECUTOR	PARALLEL_EXECUTOR	SCHEDULER_EXECUTOR
Stream Benchmark - Add	(=)	(=)	--
Stream Benchmark - Scale	-	--	--
Stream Benchmark - Triad	(=)	+	-
Stream Benchmark - Copy	(=)	(=)	(=)

Info

Property	Before	After
pika Commit	f64fbd02165a132a6276cedd14c586910abb79e4	`2498a60`
pika Datetime	2021-11-12T11:29:27+00:00	2022-08-04T10:33:31+00:00
Hostname	nid00007	nid00025
Compiler	/apps/daint/SSL/pika/packages/llvm-11.0.0/bin/clang++ 11.0.0	/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime	2021-11-12T12:57:50.824026+01:00	2022-08-04T12:39:50.340524+02:00
Clustername	daint	daint
Envfile

Explanation of Symbols

Symbol	MEANING
=	No performance change (confidence interval within ±1%)
(=)	Probably no performance change (confidence interval within ±2%)
(+)/(-)	Very small performance improvement/degradation (≤1%)
+/-	Small performance improvement/degradation (>10%)
++/--	Large performance improvement/degradation (>10%)
+++/---	Very large performance improvement/degradation (>10%)
?	Probably no change, but quite large uncertainty (confidence interval with ±5%)
??	Unclear result, very large uncertainty (±10%)
???	Something unexpected…

msimberg · 2022-08-04T10:54:05Z

bors merge

338: Handle reference error types in `ensure_started` and `let_error` r=msimberg a=aurianer Fixes partially #284 Co-authored-by: Auriane R <aurianer@cscs.ch>

bors · 2022-08-04T11:28:35Z

Build failed:

github/macos/debug/tests/unit/container_algorithms

msimberg · 2022-08-04T11:36:46Z

bors merge

bors · 2022-08-04T12:43:52Z

Build succeeded:

aurianer added this to the 0.7.0 milestone Jul 28, 2022

aurianer requested review from biddisco and msimberg as code owners July 28, 2022 16:01

bors bot added a commit that referenced this pull request Jul 28, 2022

Try #338:

2dd96bb

aurianer force-pushed the const_reference_handling branch from 8d087d6 to 005d427 Compare July 29, 2022 16:37

bors bot added a commit that referenced this pull request Jul 29, 2022

Try #338:

82a552a

msimberg requested changes Aug 1, 2022

View reviewed changes

libs/pika/execution/include/pika/execution/algorithms/ensure_started.hpp Show resolved Hide resolved

aurianer modified the milestones: 0.7.0, 0.8.0 Aug 2, 2022

aurianer force-pushed the const_reference_handling branch from 005d427 to 5348649 Compare August 3, 2022 14:38

aurianer force-pushed the const_reference_handling branch from 5348649 to 37d208e Compare August 3, 2022 15:09

bors bot added a commit that referenced this pull request Aug 3, 2022

Try #338:

2e80a31

msimberg requested changes Aug 4, 2022

View reviewed changes

libs/pika/execution/tests/unit/algorithm_then.cpp Outdated Show resolved Hide resolved

msimberg changed the title ~~Const reference handling~~ Handle reference error types in ensure_started and let_error Aug 4, 2022

msimberg assigned aurianer Aug 4, 2022

aurianer added 4 commits August 4, 2022 12:33

Add const reference sender tests in sender adaptors tests

7a56a48

Add test for let_error const reference handling

0127c78

Disable let_error test when using reference implementation

bb35cb9

This test fails with the commit of the P2300 reference implementation used in pika (9159e4925a9895a6959e01e8e0ed22db4199be1a). This same test passes with origin/main i.e. (fb798bb51f66b6df5806a17cc5da1eeef25b25d6).

Change auto with int, fixes the test with ref implementation p2300

790cf6a

aurianer force-pushed the const_reference_handling branch from 37d208e to 790cf6a Compare August 4, 2022 10:33

msimberg approved these changes Aug 4, 2022

View reviewed changes

bors bot added a commit that referenced this pull request Aug 4, 2022

Merge #338

f92849c

338: Handle reference error types in `ensure_started` and `let_error` r=msimberg a=aurianer Fixes partially #284 Co-authored-by: Auriane R <aurianer@cscs.ch>

bors bot merged commit 876eb35 into pika-org:main Aug 4, 2022

msimberg mentioned this pull request Aug 11, 2022

Fix handling of value_types and error_types with references in all sender adaptors #284

Closed

12 tasks

aurianer deleted the const_reference_handling branch April 12, 2023 09:30

Handle reference error types in ensure_started and let_error #338

Handle reference error types in ensure_started and let_error #338

Conversation

aurianer commented Jul 28, 2022

aurianer commented Jul 28, 2022

bors bot commented Jul 28, 2022

try

pika-bot commented Jul 28, 2022

pika Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

aurianer commented Jul 29, 2022

pika-bot commented Jul 29, 2022

pika Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

bors bot commented Jul 29, 2022

try

msimberg left a comment

Choose a reason for hiding this comment

pika-bot commented Aug 3, 2022

pika Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

aurianer commented Aug 3, 2022

pika-bot commented Aug 3, 2022

pika Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

bors bot commented Aug 3, 2022

try

msimberg left a comment

Choose a reason for hiding this comment

aurianer commented Aug 4, 2022

bors bot commented Aug 4, 2022

pika-bot commented Aug 4, 2022

pika Performance

Comparison

Info

Comparison

Info

Comparison

Info

Explanation of Symbols

msimberg commented Aug 4, 2022

bors bot commented Aug 4, 2022

msimberg commented Aug 4, 2022

bors bot commented Aug 4, 2022

Handle reference error types in `ensure_started` and `let_error` #338

Handle reference error types in `ensure_started` and `let_error` #338