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

Replace pika::merge with std::merge #977

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Replace pika::merge with std::merge #977

merged 1 commit into from
Sep 13, 2023

Conversation

aurianer
Copy link
Collaborator

@aurianer aurianer commented Sep 8, 2023

No description provided.

@aurianer aurianer self-assigned this Sep 8, 2023
@aurianer aurianer force-pushed the benchmark_std_merge branch 4 times, most recently from a1fd4e3 to 997ec6f Compare September 8, 2023 13:19
@msimberg
Copy link
Collaborator

msimberg commented Sep 8, 2023

Could you update the title please?

@aurianer aurianer changed the title Benchmark pika::merge vs std::merge Replace pika::merge with std::merge Sep 8, 2023
@aurianer
Copy link
Collaborator Author

aurianer commented Sep 12, 2023

So I ran the problematic miniapp with the specific configuration (see the comment of the gist for the exact options) and I got 2 segmentation faults on 86 iterations, each of 20 runs for the std::merge version and I got 1 segmentation fault in 72 iterations of 20 runs for the pika::merge version.
https://gist.github.com/aurianer/e73bdbdf644f10fbbd7e242e5b545702
https://gist.github.com/aurianer/0f8f7e3b9280b45c4ff363a649226793
So there is a problem but it's unlikely to be triggered by this PR

@aurianer
Copy link
Collaborator Author

cscs-ci run

@aurianer aurianer marked this pull request as ready for review September 12, 2023 16:12
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2023

Codecov Report

Merging #977 (e499a8f) into master (a9b539d) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #977      +/-   ##
==========================================
- Coverage   93.97%   93.94%   -0.04%     
==========================================
  Files         143      143              
  Lines        8682     8684       +2     
  Branches     1113     1113              
==========================================
- Hits         8159     8158       -1     
- Misses        336      338       +2     
- Partials      187      188       +1     
Files Changed Coverage Δ
...af/eigensolver/tridiag_solver/index_manipulation.h 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@rasolca rasolca merged commit d89739a into master Sep 13, 2023
3 checks passed
@rasolca rasolca deleted the benchmark_std_merge branch September 13, 2023 12:09
github-actions bot pushed a commit that referenced this pull request Sep 13, 2023
@msimberg msimberg added Type:Bug Something isn't working TODO:Task Category:CI not planned Feature currently outside of the roadmap that might be considered in the future Priority:Low Type:Cleanup labels Sep 27, 2023
@msimberg msimberg removed Type:Bug Something isn't working TODO:Task Category:CI not planned Feature currently outside of the roadmap that might be considered in the future labels Sep 27, 2023
@rasolca rasolca linked an issue Oct 6, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Replace use of pika::merge with std::merge
4 participants