Skip to content

Conversation

@teojgo
Copy link
Contributor

@teojgo teojgo commented Jul 24, 2018

  • Adapt to new regression test syntax.

  • Change the sanity check so that the last residual norm of the KSP
    method is checked for convergence.

  • Adapt the source code to be in sync with the newest version of the
    petsc library.

Closes #380.
Closes #388.

* Adapt to new regression test syntax.

* Change the sanity check so that the last residual norm of the KSP
  method is checked for convergence.

* Adapt the source code to be in sync with the newest version of the
  petsc library.
@teojgo teojgo self-assigned this Jul 24, 2018
@teojgo teojgo requested a review from vkarak July 24, 2018 06:18
@teojgo teojgo added this to the Summer sprint milestone Jul 24, 2018
@teojgo teojgo changed the title [bugfix] Adapt the petsc test for PE 18.07 [test] Adapt the petsc test for PE 18.07 Jul 24, 2018
@teojgo
Copy link
Contributor Author

teojgo commented Jul 24, 2018

This should also be adapted to address issue #388 once #340 is merged

@codecov-io
Copy link

Codecov Report

Merging #404 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #404   +/-   ##
=======================================
  Coverage   91.11%   91.11%           
=======================================
  Files          68       68           
  Lines        8268     8268           
=======================================
  Hits         7533     7533           
  Misses        735      735

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 571a258...9b2c7b9. Read the comment docs.

@vkarak
Copy link
Contributor

vkarak commented Jul 24, 2018

I will merge this after the build systems are merged, so as to adapt it to the syntax as well.

@@ -1,22 +1,13 @@
import filecmp
import os
Copy link
Contributor

Choose a reason for hiding this comment

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

These two imports are not needed anymore.

def __init__(self, variant):
super().__init__()
self.descr = ('Compile/run PETSc 2D Poisson example with cray-petsc '
'(%s linking case)') % variant
Copy link
Contributor

Choose a reason for hiding this comment

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

Better remove the "case" here.

self.sanity_patterns = sanity_filecmp(
'petsc_poisson2d.out', 'petsc_poisson2d.ref')
# Check the final residual norm for convergence
self.sanity_patterns = sn.assert_lt(norms[-1], 1.0e-5)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you don't do anything with the rest of the norms that you extract, you could use sn.extractsingle() and pass item=-1, so to extract the last match. The advantage of this is that it will raise a SanityError instead of IndexError in case of an out-of-bounds error, which means that it will be printed nicely in the failure report. See here for the full spec of the extractsingle() function.

* Also use the build systems instead of the `compile` method.
@vkarak
Copy link
Contributor

vkarak commented Jul 25, 2018

@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

@vkarak vkarak merged commit b5cad8f into master Jul 25, 2018
@vkarak vkarak deleted the bugfix/petsc_adapt_pe18.07 branch July 25, 2018 10:45
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