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

Replace p2300 fetch_content by a find_package to allow using p2300 spack package #436

Merged
merged 5 commits into from
Sep 20, 2022
Merged

Replace p2300 fetch_content by a find_package to allow using p2300 spack package #436

merged 5 commits into from
Sep 20, 2022

Conversation

aurianer
Copy link
Contributor

@aurianer aurianer commented Sep 6, 2022 •
edited
Loading

No description provided.

@aurianer aurianer added category: CMake M1: P2300 reference implementation Fully move to using the P2300 reference implementation and remove our own implementation. labels Sep 6, 2022
@aurianer aurianer added this to the 0.9.0 milestone Sep 6, 2022
@aurianer aurianer self-assigned this Sep 6, 2022
@aurianer
Copy link
Contributor Author

aurianer commented Sep 6, 2022

bors try

bors bot added a commit that referenced this pull request Sep 6, 2022
@bors
Try #436:
ecc6094
@bors
Copy link
Contributor

bors bot commented Sep 6, 2022

try

Build failed:

msimberg
msimberg requested changes Sep 6, 2022
View reviewed changes
Copy link
Contributor

@msimberg msimberg left a comment

Choose a reason for hiding this comment

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

Very good, but let's skip the stage of using fetchcontent and go directly to only using an external install. The CI environments need to be updated to have it available.

@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from c32e13b to db5f243 Compare September 6, 2022 14:35
@pika-bot
Copy link
Collaborator

pika-bot commented Sep 6, 2022

Performance test report

pika Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
pika Datetime2022-05-17T10:12:00+00:002022-09-06T14:35:52+00:00
pika Commitd45937d305dd20
Hostnamenid01398nid00025
Datetime2022-05-17T12:20:04.953138+02:002022-09-06T16:45:09.578967+02:00
Envfile
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

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch+

Info

PropertyBeforeAfter
pika Datetime2022-06-24T10:19:16+00:002022-09-06T14:35:52+00:00
pika Commit453f6a4305dd20
Hostnamenid00025nid00025
Datetime2022-06-24T12:43:43.615347+02:002022-09-06T16:45:24.250504+02:00
Envfile
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

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
Stream Benchmark - Add(=)-(=)
Stream Benchmark - Scale+++=+++
Stream Benchmark - Triad+++++
Stream Benchmark - Copy(=)(=)(=)

Info

PropertyBeforeAfter
pika Datetime2022-05-17T10:12:00+00:002022-09-06T14:35:52+00:00
pika Commitd45937d305dd20
Hostnamenid01398nid00025
Datetime2022-05-17T12:20:34.006375+02:002022-09-06T16:45:38.812302+02:00
Envfile
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

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
Copy link
Contributor

msimberg commented Sep 7, 2022

Could you update the title to something a bit more descriptive?

@aurianer aurianer changed the title Add find_package in case package available Replace p2300 fetch_content by a find_package to allow using p2300 spack package Sep 8, 2022
@aurianer
Copy link
Contributor Author

aurianer commented Sep 8, 2022

Could you update the title to something a bit more descriptive?

Is it better now?

@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from db5f243 to d9326fe Compare September 8, 2022 11:03
@msimberg msimberg changed the title Replace p2300 fetch_content by a find_package to allow using p2300 spack package Replace p2300 fetch_content by a find_package to allow using p2300 spack package Sep 8, 2022
@msimberg msimberg mentioned this pull request Sep 13, 2022
Merged
@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from d9326fe to 407057a Compare September 13, 2022 12:11
bors bot added a commit that referenced this pull request Sep 13, 2022
@bors @msimberg
Merge #451
7ea4d09 
451: Add `+p2300` variant to Piz Daint Jenkins CI configurations r=msimberg a=msimberg

I've updated the Piz Daint spack commit to include the `p2300` and `whip` packages. This PR doesn't actually attempt to use them, but just adds them to the environment for #436 and #423. Will do ault separately.

`@aurianer` note that I haven't constrained the P2300 commit in any way. If you rebase #436 and it turns out that there are now build problems, it may due to bugs in P2300 that are not there in the pinned commit in `pika_setup_p2300.cmake`. We can either try an older commit or ideally try to fix the problems, if there are any.

Co-authored-by: Mikael Simberg <simberg@cscs.ch>
@msimberg msimberg mentioned this pull request Sep 15, 2022
Merged
@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from 407057a to 53bc821 Compare September 15, 2022 15:32
@aurianer aurianer force-pushed the add_find_package_for_p2300 branch 2 times, most recently from 19df4e9 to c0a67a7 Compare September 16, 2022 08:18
@pika-bot
Copy link
Collaborator

Performance test report

pika Performance

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
For Each(=)(=)(=)

Info

PropertyBeforeAfter
pika Commitd45937d190f189
pika Datetime2022-05-17T10:12:00+00:002022-09-16T08:18:06+00:00
Hostnamenid01398nid00074
Datetime2022-05-17T12:20:04.953138+02:002022-09-16T10:24:46.872417+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
Envfile

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
pika Commit453f6a4190f189
pika Datetime2022-06-24T10:19:16+00:002022-09-16T08:18:06+00:00
Hostnamenid00025nid00074
Datetime2022-06-24T12:43:43.615347+02:002022-09-16T10:25:01.976661+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
Envfile

Comparison

BENCHMARKFORK_JOIN_EXECUTORPARALLEL_EXECUTORSCHEDULER_EXECUTOR
Stream Benchmark - Add(=)(=)--
Stream Benchmark - Scale(=)=+++
Stream Benchmark - Triad+++++
Stream Benchmark - Copy=(=)(=)

Info

PropertyBeforeAfter
pika Commitd45937d190f189
pika Datetime2022-05-17T10:12:00+00:002022-09-16T08:18:06+00:00
Hostnamenid01398nid00074
Datetime2022-05-17T12:20:34.006375+02:002022-09-16T10:25:16.891600+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
Envfile

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 mentioned this pull request Sep 16, 2022
Merged
@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from c0a67a7 to a468d09 Compare September 16, 2022 13:53
@aurianer
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Sep 16, 2022
@bors
Try #436:
5ace08e
@bors
Copy link
Contributor

bors bot commented Sep 16, 2022

try

Build failed:

@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from a468d09 to 91e030c Compare September 19, 2022 19:19
@aurianer
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Sep 19, 2022
@bors
Try #436:
267103e
@bors
Copy link
Contributor

bors bot commented Sep 19, 2022

try

Build failed:

@aurianer
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Sep 19, 2022
@bors
Try #436:
9809fe5
@bors
Copy link
Contributor

bors bot commented Sep 20, 2022

try

Build succeeded:

@msimberg
Copy link
Contributor

Would you mind cleanup up the Tmp! commit (the commit should probably stay, just not the message...)? Feel free to merge after that.

@aurianer aurianer force-pushed the add_find_package_for_p2300 branch from fb473a0 to 7b5685d Compare September 20, 2022 07:00
@aurianer
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Sep 20, 2022

Build succeeded:

@bors bors bot merged commit dfb96df into pika-org:main Sep 20, 2022
@aurianer aurianer deleted the add_find_package_for_p2300 branch October 5, 2022 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msimberg msimberg msimberg approved these changes

@biddisco biddisco Awaiting requested review from biddisco biddisco is a code owner

Assignees

@aurianer aurianer

Labels
category: CMake M1: P2300 reference implementation Fully move to using the P2300 reference implementation and remove our own implementation.
Projects
pika planning board
Archived in project
Milestone
0.9.0
Development

Successfully merging this pull request may close these issues.
3 participants
@aurianer @pika-bot @msimberg