Skip to content

Conversation

@teojgo
Copy link
Contributor

@teojgo teojgo commented Jul 19, 2018

  • Adapt to new regression test syntax.

  • Make a change in coloredSphere.py to load different python modules
    depending on the ParaView version.

Closes #381.

* Adapt to new regression test syntax.

* Make a change in `coloredSphere.py` to load different python modules
  depending on the ParaView version.
@teojgo teojgo added this to the Summer sprint milestone Jul 19, 2018
@teojgo teojgo self-assigned this Jul 19, 2018
@teojgo teojgo requested a review from vkarak July 19, 2018 08:55
@teojgo teojgo changed the title Adapt check for PE 18.07 [bugfix] Adapt check for PE 18.07 Jul 19, 2018
@vkarak vkarak changed the title [bugfix] Adapt check for PE 18.07 [test] Adapt Paraview check for PE 18.07 Jul 19, 2018
@teojgo
Copy link
Contributor Author

teojgo commented Jul 19, 2018

@jenkins-cscs retry all

self.executable_opts = ['coloredSphere.py']

# Needed in order to swap from the default version of gcc
self.pre_run = ['module swap gcc/6.2.0 gcc/7.1.0']
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not too portable. I suggest specifying gcc/7.1.0 in self.modules. Then ReFrame will generate the necessary commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifying into the modules generated the following:

module load daint-gpu
module unload PrgEnv-cray
module load PrgEnv-gnu
module unload PrgEnv-cray
module unload gcc 
module load gcc/7.1.0
module unload PrgEnv-cray
module unload gcc 
module load ParaView

which unloads again gcc before loading Paraview.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that sth is wrong with the module command generation. This looks like related to #59. Can you add a note before pre_run referring to this issue?

@codecov-io
Copy link

Codecov Report

Merging #401 into master will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #401      +/-   ##
==========================================
+ Coverage   91.08%   91.15%   +0.06%     
==========================================
  Files          68       68              
  Lines        8247     8300      +53     
==========================================
+ Hits         7512     7566      +54     
+ Misses        735      734       -1
Impacted Files Coverage Δ
unittests/test_pipeline.py 97.35% <0%> (+0.12%) ⬆️
reframe/core/pipeline.py 95.44% <0%> (+0.73%) ⬆️

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 d91ab74...a1e13fa. Read the comment docs.

# NOTE: This is needed in order to swap from the default
# version of gcc until issue #59 is fixed. Then it should
# be moved to the `self.modules` definition.
self.pre_run = ['module swap gcc/6.2.0 gcc/7.1.0']
Copy link
Contributor

Choose a reason for hiding this comment

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

After merging with master, this workaround should not be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I had it ready and tested with #405

@vkarak
Copy link
Contributor

vkarak commented Jul 25, 2018

@jenkins-cscs retry daint

@vkarak vkarak merged commit 3960193 into master Jul 25, 2018
@vkarak vkarak deleted the regression_test/paraview_pe18.07 branch July 25, 2018 15:17
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.

4 participants