Skip to content

Conversation

@hurricane642
Copy link
Contributor

This is a continuation of #2084, devoted exclusively to the Amber test. A new file nve.py has been added, introducing the main class for the Amber test, as well as the CSCS tests inherited from it.

@jenkins-cscs
Copy link
Collaborator

Can I test this patch?

@teojgo
Copy link
Contributor

teojgo commented Sep 2, 2021

Ok to test

@pep8speaks
Copy link

pep8speaks commented Sep 2, 2021

Hello @hurricane642, 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-10-05 21:45:31 UTC

@victorusu
Copy link
Contributor

@jenkins-cscs retry all

@vkarak vkarak added this to the ReFrame sprint 21.09.1 milestone Sep 14, 2021
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.

I also believe that the CSCS version of the test can be simplified significantly. I will work a bit on that.

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.

I have a proposal for both the library file and the test. I will push tomorrow or do a PR to your branch.

@hurricane642
Copy link
Contributor Author

I have a proposal for both the library file and the test. I will push tomorrow or do a PR to your branch.

Ok, cool, I'll wait :))

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2021

Codecov Report

Merging #2172 (8cda9fe) into master (c610815) will not change coverage.
The diff coverage is n/a.

❗ Current head 8cda9fe differs from pull request most recent head 0e27602. Consider uploading reports for the commit 0e27602 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2172   +/-   ##
=======================================
  Coverage   85.54%   85.54%           
=======================================
  Files          55       55           
  Lines        9746     9746           
=======================================
  Hits         8337     8337           
  Misses       1409     1409           

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 43042a7...0e27602. Read the comment docs.

@vkarak
Copy link
Contributor

vkarak commented Sep 15, 2021

@hurricane642 I've force pushed to your branch, since I made some substantial changes especially in the CSCS version of the test. I think that both the library and the CSCS version of the test -which is a bit weird to say the least- are much better now. @victorusu @jjotero comments?

@vkarak
Copy link
Contributor

vkarak commented Sep 15, 2021

ok to test

@vkarak
Copy link
Contributor

vkarak commented Sep 15, 2021

The simplest tests I could write and pass were the following:

@rfm.simple_test
class my_amber_check(Amber_NVE):
    modules = ['Amber']
    valid_prog_environs = ['builtin']

    @run_after('init')
    def scope_systems(self):
        if self.platform == 'gpu':
            self.valid_systems = ['daint:gpu']
        else:
            self.valid_systems = ['daint:mc']
            self.num_tasks = 2

or these two:

@rfm.simple_test
class my_amber_check(Amber_NVE):
    modules = ['Amber']
    valid_systems = ['daint:mc']
    valid_prog_environs = ['builtin']
    num_tasks = 2
    platform = parameter(['cpu'])


@rfm.simple_test
class my_amber_check(Amber_NVE):
    modules = ['Amber']
    valid_systems = ['daint:gpu']
    valid_prog_environs = ['builtin']
    platform = parameter(['gpu'])

Actually, the Cellulose_production_NVE was the only one not to pass, because it hit the default time limit.

I also noticed that num_tasks must be >=2 for the cpu version of the test, which I think it should be reflected in the library.

@vkarak
Copy link
Contributor

vkarak commented Oct 4, 2021

Actually, the simplest functional test is this!

import reframe as rfm
from hpctestlib.apps.amber.nve import amber_nve_check


@rfm.simple_test
class my_amber_check(amber_nve_check):
    valid_prog_environs = ['*']
    valid_systems = ['*']

Which you can scope and run from the command line:

./bin/reframe <options> --system=dom:gpu -n '.*cuda$' -p builtin -S modules=Amber -S num_tasks=1 -r

or

./bin/reframe <options> --system=dom:mc -n '.*mpi$' -p builtin -S modules=Amber -S num_tasks=2 -r

@vkarak vkarak changed the title [test] Add generic Amber check for the cscs-supported apps into hpctestlib/apps [test] Add new Amber NVE library test and refactor the CSCS Amber check Oct 4, 2021
@vkarak vkarak closed this Oct 5, 2021
@vkarak vkarak reopened this Oct 5, 2021
@vkarak
Copy link
Contributor

vkarak commented Oct 6, 2021

@jenkins-cscs retry daint

@vkarak vkarak merged commit 3abb6db into reframe-hpc:master Oct 6, 2021
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.

9 participants