Skip to content

Conversation

@teojgo
Copy link
Contributor

@teojgo teojgo commented Mar 16, 2021

Fixes #1726

@vkarak
Copy link
Contributor

vkarak commented Mar 16, 2021

@jenkins-cscs retry daint

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Tutorials fail for the PGI compiler, so we would need to adapt the tests for Daint and perhaps the text...

@teojgo
Copy link
Contributor Author

teojgo commented Mar 17, 2021

Tutorials fail for the PGI compiler, so we would need to adapt the tests for Daint and perhaps the text...

Hmm. I have not considered the pgi compiler since in the basic tutorial it uses only the gnu/clang.

@teojgo
Copy link
Contributor Author

teojgo commented Mar 18, 2021

Tutorials fail for the PGI compiler, so we would need to adapt the tests for Daint and perhaps the text...

Maybe we should move the mention of the hook in the threaded test so that we can set the flags based on the environment

@pep8speaks
Copy link

pep8speaks commented Mar 30, 2021

Hello @teojgo, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2021-04-06 09:29:04 UTC

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Perhaps we also need to add some text later on when we introduce the hooks:

Original text:

and we set them in the setflags() pipeline hook. Let’s explain what is this all about. 

Proposed text:

and we set them in the setflags() pipeline hook. We have first seen the pipeline hooks in the multithreaded "Hello, World!" example, but let’s explain now what is this all about. 

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

lgtm

@codecov-io
Copy link

Codecov Report

Merging #1870 (d371ecf) into master (2704799) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1870   +/-   ##
=======================================
  Coverage   87.90%   87.90%           
=======================================
  Files          49       49           
  Lines        8452     8452           
=======================================
  Hits         7430     7430           
  Misses       1022     1022           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2704799...d371ecf. Read the comment docs.

@vkarak
Copy link
Contributor

vkarak commented Apr 2, 2021

@jenkins-cscs retry daint

@vkarak vkarak changed the title [doc] Add '-pthread' to the multithreaded tutorial test [doc] Add -pthread to the multithreaded tutorial test Apr 2, 2021
@vkarak vkarak merged commit 725c78f into reframe-hpc:master Apr 6, 2021
@teojgo teojgo deleted the doc/pthread_tutorial_option branch May 21, 2021 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReFrame tutorial: missing -pthread in A Multithreaded “Hello, World!”

4 participants