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.

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

Fix builds with newest stdexec #1105

Merged
merged 12 commits into from
May 6, 2024

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented Apr 24, 2024

Opts in to the newer stdexec sender/receiver concepts (with using sender_concept = stdexec::sender_t etc.). Also fixes a number of compilation failures resulting from stricter checks etc. in stdexec.

@msimberg msimberg self-assigned this Apr 24, 2024
@msimberg msimberg force-pushed the stdexec-sender-concept-fixes branch 3 times, most recently from 2d67b72 to 1d4f37d Compare April 29, 2024 09:40
@msimberg msimberg marked this pull request as ready for review April 29, 2024 09:54
@msimberg msimberg added this pull request to the merge queue Apr 29, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 29, 2024
@msimberg msimberg force-pushed the stdexec-sender-concept-fixes branch from 1d4f37d to 378cb43 Compare April 29, 2024 14:44
@pika-bot
Copy link
Collaborator

Performance test report

pika Performance

Comparison

BENCHMARKRESULT
Task Overhead - Create Thread Hierarchical - Latch--

Info

PropertyBeforeAfter
pika Commit0abc084a7f7494
pika Datetime2024-02-19T15:15:15+00:002024-04-29T14:45:13+00:00
Envfile
Hostnamenid00025nid00020
Datetime2024-02-19T16:26:16.072067+01:002024-04-29T16:52:07.222181+02:00
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Clusternamedaintdaint

Explanation of Symbols

SymbolMEANING
=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 msimberg added this to the 0.25.0 milestone Apr 29, 2024
Copy link
Contributor

@aurianer aurianer left a comment

Choose a reason for hiding this comment

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

Looks nice thanks!

@msimberg msimberg force-pushed the stdexec-sender-concept-fixes branch 2 times, most recently from 6accc07 to 7c18530 Compare May 6, 2024 07:51
@pika-bot
Copy link
Collaborator

pika-bot commented May 6, 2024

Performance test report

pika Performance

Comparison

BENCHMARKRESULT
Task Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
pika Datetime2024-02-19T15:15:15+00:002024-05-06T07:51:55+00:00
pika Commit0abc084649ddd8
Envfile
Datetime2024-02-19T16:26:16.072067+01:002024-05-06T09:59:39.751493+02:00
Clusternamedaintdaint
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Hostnamenid00025nid00025

Explanation of Symbols

SymbolMEANING
=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 added 11 commits May 6, 2024 11:14
…d_pool_scheduler

The bulk callable expects an extra argument that was not being sent from the predecessor then sender
since the then callable returns void. This adds a return to the then callable. Newer versions of
stdexec diagnose this issue without connecting a receiver.
Actually return a sender from callable, and send error to let_error.
MPI, precompiled headers, and stdexec seem to conflict, leading to duplicate mangled symbols:

/usr/local/include/stdexec/__detail/__env.hpp:310:9: error: definition with same mangled name '_ZNK7stdexec5__env9get_env_tclINS_7__sexprIXtlNS_12_GLOBAL__N_1UlvE7_EEENS4_6__anonEEEEEDTcl10tag_invokeclL_ZNS_9__declvalIS1_EEOT_vEEcl9__declvalIRKS9_EEEESC
_' as another definition
        operator()(const _EnvProvider& __env_provider) const noexcept
        ^
/usr/local/include/stdexec/__detail/__env.hpp:310:9: note: previous definition is here

Since precompiled headers don't change functionality, this disables them, rather than disabling MPI
or stdexec in CI.
@msimberg msimberg force-pushed the stdexec-sender-concept-fixes branch from 7c18530 to 014f808 Compare May 6, 2024 09:14
@msimberg msimberg enabled auto-merge May 6, 2024 09:14
@msimberg msimberg added this pull request to the merge queue May 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2024
@msimberg msimberg added this pull request to the merge queue May 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 6, 2024
…nv specialization

nvc++ is not able to compile the function using constexpr. Since get_env isn't typically required at
constexpr-time, simply remove the constexpr.
@msimberg msimberg force-pushed the stdexec-sender-concept-fixes branch from c27b4f6 to a9dcd11 Compare May 6, 2024 15:00
@msimberg msimberg enabled auto-merge May 6, 2024 15:01
@pika-bot
Copy link
Collaborator

pika-bot commented May 6, 2024

Performance test report

pika Performance

Comparison

BENCHMARKRESULT
Task Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
pika Datetime2024-02-19T15:15:15+00:002024-05-06T15:00:25+00:00
pika Commit0abc084d6a7b85
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Envfile
Datetime2024-02-19T16:26:16.072067+01:002024-05-06T17:09:23.459316+02:00
Clusternamedaintdaint
Hostnamenid00025nid00025

Explanation of Symbols

SymbolMEANING
=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 msimberg added this pull request to the merge queue May 6, 2024
Merged via the queue into pika-org:main with commit dcdcfd7 May 6, 2024
36 of 38 checks passed
@msimberg msimberg deleted the stdexec-sender-concept-fixes branch May 6, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants