From cd6103f4346dfe779d6d1e02e0dde854c4c8bd57 Mon Sep 17 00:00:00 2001
From: Sergei Kliavinek <klyavinekss@gmail.com>
Date: Wed, 1 Sep 2021 16:52:25 +0300
Subject: [PATCH 01/10] add refactored amber test

---
 cscs-checks/apps/amber/amber_check.py | 180 ++++++++++----------------
 hpctestlib/apps/amber/nve.py          | 145 +++++++++++++++++++++
 2 files changed, 212 insertions(+), 113 deletions(-)
 create mode 100644 hpctestlib/apps/amber/nve.py

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index 4209c33b09..96180125c1 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -5,51 +5,46 @@
 
 import reframe as rfm
 import reframe.utility.sanity as sn
+from hpctestlib.apps.amber.nve import Amber_NVE
 
 
 # FIXME: Use tuples as dictionary keys as soon as
 # https://github.com/eth-cscs/reframe/issues/2022 is in
-daint_gpu_performance = {
-    'Cellulose_production_NVE': (30.0, -0.05, None, 'ns/day'),
-    'FactorIX_production_NVE': (134.0, -0.05, None, 'ns/day'),
-    'JAC_production_NVE': (388.0, -0.05, None, 'ns/day'),
-    'JAC_production_NVE_4fs': (742, -0.05, None, 'ns/day'),
-}
 
 REFERENCE_GPU_PERFORMANCE = {
-    'daint:gpu': daint_gpu_performance,
-    'dom:gpu': daint_gpu_performance
-}
-
-daint_mc_performance_small = {
-    'Cellulose_production_NVE': (8.0, -0.30, None, 'ns/day'),
-    'FactorIX_production_NVE': (34.0, -0.30, None, 'ns/day'),
-    'JAC_production_NVE': (90.0, -0.30, None, 'ns/day'),
-    'JAC_production_NVE_4fs': (150.0, -0.30, None, 'ns/day'),
-}
-
-eiger_mc_performance_small = {
-    'Cellulose_production_NVE': (3.2, -0.30, None, 'ns/day'),
-    'FactorIX_production_NVE': (7.0, -0.30, None, 'ns/day'),
-    'JAC_production_NVE': (30.0, -0.30, None, 'ns/day'),
-    'JAC_production_NVE_4fs': (45.0, -0.30, None, 'ns/day'),
+    'normal':{
+        ('daint:gpu', 'dom:gpu'): {
+            'Cellulose_production_NVE': (30.0, -0.05, None, 'ns/day'),
+            'FactorIX_production_NVE': (134.0, -0.05, None, 'ns/day'),
+            'JAC_production_NVE': (388.0, -0.05, None, 'ns/day'),
+            'JAC_production_NVE_4fs': (742, -0.05, None, 'ns/day'),
+        },
+    },
 }
 
 REFERENCE_CPU_PERFORMANCE_SMALL = {
-    'daint:mc': daint_mc_performance_small,
-    'dom:mc': daint_mc_performance_small,
-    'eiger:mc': eiger_mc_performance_small,
-    'pilatus:mc': eiger_mc_performance_small,
+    ('daint:mc', 'dom:mc'): {
+        'Cellulose_production_NVE': (8.0, -0.30, None, 'ns/day'),
+        'FactorIX_production_NVE': (34.0, -0.30, None, 'ns/day'),
+        'JAC_production_NVE': (90.0, -0.30, None, 'ns/day'),
+        'JAC_production_NVE_4fs': (150.0, -0.30, None, 'ns/day'),
+    },
+    ('eiger:mc', 'pilatus:mc'): {
+        'Cellulose_production_NVE': (3.2, -0.30, None, 'ns/day'),
+        'FactorIX_production_NVE': (7.0, -0.30, None, 'ns/day'),
+        'JAC_production_NVE': (30.0, -0.30, None, 'ns/day'),
+        'JAC_production_NVE_4fs': (45.0, -0.30, None, 'ns/day'),
+    },
 }
 
 REFERENCE_CPU_PERFORMANCE_LARGE = {
-    'daint:mc': {
+    ('daint:mc'): {
         'Cellulose_production_NVE': (10.0, -0.30, None, 'ns/day'),
         'FactorIX_production_NVE': (36.0, -0.30, None, 'ns/day'),
         'JAC_production_NVE': (78.0, -0.30, None, 'ns/day'),
         'JAC_production_NVE_4fs': (135.0, -0.30, None, 'ns/day'),
     },
-    'eiger:mc': {
+    ('eiger:mc'): {
         'Cellulose_production_NVE': (1.3, -0.30, None, 'ns/day'),
         'FactorIX_production_NVE': (3.5, -0.30, None, 'ns/day'),
         'JAC_production_NVE': (17.0, -0.30, None, 'ns/day'),
@@ -57,103 +52,69 @@
     },
 }
 
+REFERENCE_CPU_PERFORMANCE = {
+    'small': REFERENCE_CPU_PERFORMANCE_SMALL,
+    'large': REFERENCE_CPU_PERFORMANCE_LARGE,
+}
+
+REFERENCE_PERFORMANCE = {
+    'gpu': REFERENCE_GPU_PERFORMANCE,
+    'cpu': REFERENCE_CPU_PERFORMANCE,
+}
 
-class AmberBaseCheck(rfm.RunOnlyRegressionTest):
-    valid_prog_environs = ['builtin']
-    strict_check = False
+def inherit_cpu_only(params):
+    return tuple(filter(lambda p: 'cpu' in p[0], params))
+
+
+def inherit_gpu_only(params):
+    return tuple(filter(lambda p: 'gpu' in p[0], params))
+
+
+class AmberCheckCSCS(Amber_NVE):
     modules = ['Amber']
+    valid_prog_environs = ['builtin']
+    strict_check = True
     extra_resources = {
         'switches': {
             'num_switches': 1
         }
     }
     maintainers = ['VH', 'SO']
-    tags = {'scs', 'external-resources'}
-
-    benchmark = parameter([
-        # NVE simulations
-        'Cellulose_production_NVE',
-        'FactorIX_production_NVE',
-        'JAC_production_NVE_4fs',
-        'JAC_production_NVE',
-    ])
-
-    @run_after('init')
-    def download_files(self):
-        self.prerun_cmds = [
-            # cannot use wget because it is not installed on eiger
-            f'curl -LJO https://github.com/victorusu/amber_benchmark_suite'
-            f'/raw/main/amber_16_benchmark_suite/PME/{self.benchmark}.tar.bz2',
-            f'tar xf {self.benchmark}.tar.bz2'
-        ]
-
-    @run_after('init')
-    def set_energy_and_tolerance_reference(self):
-        self.ener_ref = {
-            # every system has a different reference energy and drift
-            'Cellulose_production_NVE': (-443246, 5.0E-05),
-            'FactorIX_production_NVE': (-234188, 1.0E-04),
-            'JAC_production_NVE_4fs': (-44810, 1.0E-03),
-            'JAC_production_NVE': (-58138, 5.0E-04),
-        }
-
-    @run_after('setup')
-    def set_executable_opts(self):
-        self.executable_opts = ['-O',
-                                '-i', self.input_file,
-                                '-o', self.output_file]
-        self.keep_files = [self.output_file]
-
-    @run_after('setup')
-    def set_sanity_patterns(self):
-        energy = sn.extractsingle(r' Etot\s+=\s+(?P<energy>\S+)',
-                                  self.output_file, 'energy', float, item=-2)
-        energy_reference = self.ener_ref[self.benchmark][0]
-        energy_diff = sn.abs(energy - energy_reference)
-        ref_ener_diff = sn.abs(self.ener_ref[self.benchmark][0] *
-                               self.ener_ref[self.benchmark][1])
-        self.sanity_patterns = sn.all([
-            sn.assert_found(r'Final Performance Info:', self.output_file),
-            sn.assert_lt(energy_diff, ref_ener_diff)
-        ])
-
-    @run_after('setup')
-    def set_generic_perf_references(self):
-        self.reference.update({'*': {
-            self.benchmark: (0, None, None, 'ns/day')
-        }})
-
-    @run_after('setup')
-    def set_perf_patterns(self):
-        self.perf_patterns = {
-            self.benchmark: sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
-                                             self.output_file, 'perf',
-                                             float, item=1)
-        }
 
+    @run_before('performance')
+    def set_perf_reference(self):
+        for key, val in REFERENCE_PERFORMANCE[self.platform][self.scale].items():
+            if self.current_partition.fullname in key:
+                self.reference = {'*': val}
+                break
+        else:
+            raise ValueError(
+                f'could not find a reference for the current '
+                f'partition {self.current_partition.fullname!r}'
+            )
 
 @rfm.simple_test
-class AmberGPUCheck(AmberBaseCheck):
+class amber_gpu_check(AmberCheckCSCS):
+    valid_systems = ['daint:gpu', 'dom:gpu']
+    scale = 'normal'
     num_tasks = 1
-    num_tasks_per_node = 1
     num_gpus_per_node = 1
-    valid_systems = ['daint:gpu', 'dom:gpu']
-    executable = 'pmemd.cuda.MPI'
-    input_file = 'mdin.GPU'
-    output_file = 'amber.out'
+    num_tasks_per_node = 1
     descr = f'Amber GPU check'
-    tags = {'maintenance', 'production', 'health'}
-    reference = REFERENCE_GPU_PERFORMANCE
+    tags.update({'maintenance', 'production', 'health'})
+    platform_info = parameter(
+        inherit_params=True,
+        filter_params=inherit_gpu_only)
 
 
 @rfm.simple_test
-class AmberCPUCheck(AmberBaseCheck):
+class amber_cpu_check(AmberCheckCSCS):
+    tags.update({'maintenance', 'production'})
     scale = parameter(['small', 'large'])
     valid_systems = ['daint:mc', 'eiger:mc']
-    executable = 'pmemd.MPI'
-    input_file = 'mdin.CPU'
-    output_file = 'amber.out'
-    tags = {'maintenance', 'production'}
+    platform_info = parameter(
+        inherit_params=True,
+        filter_params=inherit_cpu_only)
 
     @run_after('init')
     def set_description(self):
@@ -169,13 +130,6 @@ def set_hierarchical_prgenvs(self):
         if self.current_system.name in ['eiger', 'pilatus']:
             self.valid_prog_environs = ['cpeIntel']
 
-    @run_after('setup')
-    def set_perf_reference(self):
-        if self.scale == 'small':
-            self.reference = REFERENCE_CPU_PERFORMANCE_SMALL
-        else:
-            self.reference = REFERENCE_CPU_PERFORMANCE_LARGE
-
     @run_after('init')
     def set_num_tasks_cray_xc(self):
         if self.current_system.name in ['daint', 'dom']:
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
new file mode 100644
index 0000000000..c389e21315
--- /dev/null
+++ b/hpctestlib/apps/amber/nve.py
@@ -0,0 +1,145 @@
+# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich)
+# ReFrame Project Developers. See the top-level LICENSE file for details.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
+import reframe as rfm
+import reframe.utility.sanity as sn
+import reframe.utility.typecheck as typ
+
+
+class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
+    '''Base class for the Amber NVE Test.
+
+    Amber is a suite of biomolecular simulation programs. It
+    began in the late 1970's, and is maintained by an active
+    development community (see ambermd.org).
+
+    The presented abstract run-only class checks the amber perfomance.
+    To do this, it is necessary to define in tests the name
+    of the running script (input file), the output file,
+    as well as set the reference values of energy and possible
+    deviations from this value. This data is used to check if
+    the task is being executed correctly, that is, the final energy
+    is correct (approximately the reference). The default assumption
+    is that Amber is already installed on the device under test.
+    '''
+
+    #: Amber output file.
+    #:
+    #: :default: : 'amber.out'
+    output_file = variable(str, value='amber.out')
+
+    #: Amber input file. This file is set by the post-init hook
+    #: :func:`unpack_platform_parameter`.
+    #:
+    #: :default: :class:`required`
+    input_file = variable(str)
+
+    #: Reference value of energy, that is used for the comparison
+    #: with the execution ouput on the sanity step. The absolute
+    #: difference between final energy value and reference value
+    #: should be smaller than energy_tolerance
+    #:
+    #: :type: float
+    #: :default: :class:`required`
+    energy_value = variable(float)
+
+    #: Maximum deviation from the reference value of energy,
+    #: that is acceptable.
+    #:
+    #: :type: float
+    #: :default: :class:`required`
+    energy_tolerance = variable(float)
+
+    #: Parameter pack containing the platform ID, input file and
+    #: executable.
+    platform_info = parameter([
+        ('cpu', 'mdin.CPU', 'pmemd.MPI'),
+        ('gpu', 'mdin.GPU', 'pmemd.cuda.MPI')
+    ])
+
+    #: NVE simulation parameter pack with the benchmark name,
+    #: energy reference and energy tolerance for each case.
+    variant = parameter([
+        ('Cellulose_production_NVE', -443246.0, 5.0E-05),
+        ('FactorIX_production_NVE', -234188.0, 1.0E-04),
+        ('JAC_production_NVE_4fs', -44810.0, 1.0E-03),
+        ('JAC_production_NVE', -58138.0, 5.0E-04)
+    ])
+
+    #: :default: :class:`required`
+    num_tasks_per_node = required
+
+    #: :default: :class:`required`
+    executable = required
+
+    tags = {'sciapp', 'chemistry'}
+
+    @run_after('init')
+    def unpack_platform_parameter(self):
+        '''Set the executable and input file.'''
+
+        self.platform, self.input_file, self.executable = self.platform_info
+
+    @run_after('init')
+    def unpack_variant_parameter(self):
+        '''Set the value of energy and energy tolerance for a
+        specific program.
+        '''
+
+        self.benchmark, self.energy_value, self.energy_tolerance = self.variant
+
+    @run_after('init')
+    def set_keep_files(self):
+        self.keep_files = [self.output_file]
+
+    @performance_function('ns/day', perf_key='nve')
+    def set_perf_patterns(self):
+        return sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
+                                self.output_file, 'perf',
+                                float, item=1)
+
+    @run_before('performance')
+    def set_the_performance_dict(self):
+        self.perf_variables = {self.benchmark:
+                               sn.make_performance_function(
+                                   sn.extractsingle(
+                                       r'ns/day =\s+(?P<perf>\S+)',
+                                       self.output_file, 'perf',
+                                       float, item=1), 'ns/day')}
+
+    @run_before('run')
+    def download_files(self):
+        '''Download program files, which used in test'''
+
+        self.prerun_cmds = [
+            # cannot use wget because it is not installed on eiger
+            f'curl -LJO https://github.com/victorusu/amber_benchmark_suite'
+            f'/raw/main/amber_16_benchmark_suite/PME/{self.benchmark}.tar.bz2',
+            f'tar xf {self.benchmark}.tar.bz2'
+        ]
+
+    @run_before('run')
+    def set_executable_opts(self):
+        '''Set the executable options for the Amber. Determine the
+        using of input and ouput files.
+        '''
+
+        self.executable_opts = ['-O',
+                                '-i', self.input_file,
+                                '-o', self.output_file]
+
+    @sanity_function
+    def assert_energy_readout(self):
+        '''Assert the obtained energy meets the specified tolerances.'''
+
+        energy = sn.extractsingle(r' Etot\s+=\s+(?P<energy>\S+)',
+                                  self.output_file, 'energy', float, item=-2)
+        energy_diff = sn.abs(energy - self.energy_value)
+        ref_ener_diff = sn.abs(self.energy_value *
+                               self.energy_tolerance)
+        return sn.all([
+            sn.assert_found(r'Final Performance Info:', self.output_file),
+            sn.assert_lt(energy_diff, ref_ener_diff)
+        ])

From d55b034d5407697818180937c2ea054548821538 Mon Sep 17 00:00:00 2001
From: Sergei Kliavinek <klyavinekss@gmail.com>
Date: Thu, 2 Sep 2021 13:57:56 +0300
Subject: [PATCH 02/10] delete unneseccary import

---
 cscs-checks/apps/amber/amber_check.py | 1 -
 hpctestlib/apps/amber/nve.py          | 1 -
 2 files changed, 2 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index 96180125c1..2e1a70b950 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -4,7 +4,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 import reframe as rfm
-import reframe.utility.sanity as sn
 from hpctestlib.apps.amber.nve import Amber_NVE
 
 
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index c389e21315..6d884459bc 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -5,7 +5,6 @@
 
 import reframe as rfm
 import reframe.utility.sanity as sn
-import reframe.utility.typecheck as typ
 
 
 class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):

From 1a4364fc141654fe51d0c0e1f15547ed399be530 Mon Sep 17 00:00:00 2001
From: Sergei Kliavinek <klyavinekss@gmail.com>
Date: Thu, 2 Sep 2021 14:11:12 +0300
Subject: [PATCH 03/10] fix pep8 problems with length and blank lines

---
 cscs-checks/apps/amber/amber_check.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index 2e1a70b950..774aefe5e0 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -11,7 +11,7 @@
 # https://github.com/eth-cscs/reframe/issues/2022 is in
 
 REFERENCE_GPU_PERFORMANCE = {
-    'normal':{
+    'normal': {
         ('daint:gpu', 'dom:gpu'): {
             'Cellulose_production_NVE': (30.0, -0.05, None, 'ns/day'),
             'FactorIX_production_NVE': (134.0, -0.05, None, 'ns/day'),
@@ -56,11 +56,12 @@
     'large': REFERENCE_CPU_PERFORMANCE_LARGE,
 }
 
-REFERENCE_PERFORMANCE = {
+REFERENCE_PERF = {
     'gpu': REFERENCE_GPU_PERFORMANCE,
     'cpu': REFERENCE_CPU_PERFORMANCE,
 }
 
+
 def inherit_cpu_only(params):
     return tuple(filter(lambda p: 'cpu' in p[0], params))
 
@@ -82,7 +83,7 @@ class AmberCheckCSCS(Amber_NVE):
 
     @run_before('performance')
     def set_perf_reference(self):
-        for key, val in REFERENCE_PERFORMANCE[self.platform][self.scale].items():
+        for key, val in REFERENCE_PERF[self.platform][self.scale].items():
             if self.current_partition.fullname in key:
                 self.reference = {'*': val}
                 break

From 747b09473b0cd2a16dbe5d103723c20383038bf3 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Tue, 14 Sep 2021 23:41:03 +0200
Subject: [PATCH 04/10] Simplify test

---
 cscs-checks/apps/amber/amber_check.py | 200 +++++++++++---------------
 hpctestlib/apps/amber/nve.py          |  78 +++++-----
 2 files changed, 114 insertions(+), 164 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index 774aefe5e0..ace8fb85fc 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -3,127 +3,69 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
+import contextlib
+
 import reframe as rfm
 from hpctestlib.apps.amber.nve import Amber_NVE
 
 
-# FIXME: Use tuples as dictionary keys as soon as
-# https://github.com/eth-cscs/reframe/issues/2022 is in
-
-REFERENCE_GPU_PERFORMANCE = {
-    'normal': {
-        ('daint:gpu', 'dom:gpu'): {
-            'Cellulose_production_NVE': (30.0, -0.05, None, 'ns/day'),
-            'FactorIX_production_NVE': (134.0, -0.05, None, 'ns/day'),
-            'JAC_production_NVE': (388.0, -0.05, None, 'ns/day'),
-            'JAC_production_NVE_4fs': (742, -0.05, None, 'ns/day'),
-        },
-    },
-}
-
-REFERENCE_CPU_PERFORMANCE_SMALL = {
-    ('daint:mc', 'dom:mc'): {
-        'Cellulose_production_NVE': (8.0, -0.30, None, 'ns/day'),
-        'FactorIX_production_NVE': (34.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE': (90.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE_4fs': (150.0, -0.30, None, 'ns/day'),
-    },
-    ('eiger:mc', 'pilatus:mc'): {
-        'Cellulose_production_NVE': (3.2, -0.30, None, 'ns/day'),
-        'FactorIX_production_NVE': (7.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE': (30.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE_4fs': (45.0, -0.30, None, 'ns/day'),
-    },
-}
-
-REFERENCE_CPU_PERFORMANCE_LARGE = {
-    ('daint:mc'): {
-        'Cellulose_production_NVE': (10.0, -0.30, None, 'ns/day'),
-        'FactorIX_production_NVE': (36.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE': (78.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE_4fs': (135.0, -0.30, None, 'ns/day'),
-    },
-    ('eiger:mc'): {
-        'Cellulose_production_NVE': (1.3, -0.30, None, 'ns/day'),
-        'FactorIX_production_NVE': (3.5, -0.30, None, 'ns/day'),
-        'JAC_production_NVE': (17.0, -0.30, None, 'ns/day'),
-        'JAC_production_NVE_4fs': (30.5, -0.30, None, 'ns/day'),
-    },
-}
-
-REFERENCE_CPU_PERFORMANCE = {
-    'small': REFERENCE_CPU_PERFORMANCE_SMALL,
-    'large': REFERENCE_CPU_PERFORMANCE_LARGE,
-}
-
-REFERENCE_PERF = {
-    'gpu': REFERENCE_GPU_PERFORMANCE,
-    'cpu': REFERENCE_CPU_PERFORMANCE,
-}
-
-
-def inherit_cpu_only(params):
-    return tuple(filter(lambda p: 'cpu' in p[0], params))
-
-
-def inherit_gpu_only(params):
-    return tuple(filter(lambda p: 'gpu' in p[0], params))
-
-
-class AmberCheckCSCS(Amber_NVE):
+@rfm.simple_test
+class amber_check(Amber_NVE):
     modules = ['Amber']
     valid_prog_environs = ['builtin']
-    strict_check = True
     extra_resources = {
         'switches': {
             'num_switches': 1
         }
     }
     maintainers = ['VH', 'SO']
-
-    @run_before('performance')
-    def set_perf_reference(self):
-        for key, val in REFERENCE_PERF[self.platform][self.scale].items():
-            if self.current_partition.fullname in key:
-                self.reference = {'*': val}
-                break
-        else:
-            raise ValueError(
-                f'could not find a reference for the current '
-                f'partition {self.current_partition.fullname!r}'
-            )
-
-@rfm.simple_test
-class amber_gpu_check(AmberCheckCSCS):
-    valid_systems = ['daint:gpu', 'dom:gpu']
-    scale = 'normal'
-    num_tasks = 1
-    num_gpus_per_node = 1
-    num_tasks_per_node = 1
-    descr = f'Amber GPU check'
-    tags.update({'maintenance', 'production', 'health'})
-    platform_info = parameter(
-        inherit_params=True,
-        filter_params=inherit_gpu_only)
-
-
-@rfm.simple_test
-class amber_cpu_check(AmberCheckCSCS):
-    tags.update({'maintenance', 'production'})
+    bench_reference = {
+        'small': {
+            'p100': {
+                'Cellulose_production_NVE': (30.0, -0.05, None, 'ns/day'),
+                'FactorIX_production_NVE': (134.0, -0.05, None, 'ns/day'),
+                'JAC_production_NVE': (388.0, -0.05, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (742, -0.05, None, 'ns/day')
+            },
+            'broadwell': {
+                'Cellulose_production_NVE': (8.0, -0.30, None, 'ns/day'),
+                'FactorIX_production_NVE': (34.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE': (90.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (150.0, -0.30, None, 'ns/day')
+            },
+            'zen2': {
+                'Cellulose_production_NVE': (3.2, -0.30, None, 'ns/day'),
+                'FactorIX_production_NVE': (7.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE': (30.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (45.0, -0.30, None, 'ns/day')
+            }
+        },
+        'large': {
+            'broadwell': {
+                'Cellulose_production_NVE': (10.0, -0.30, None, 'ns/day'),
+                'FactorIX_production_NVE': (36.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE': (78.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (135.0, -0.30, None, 'ns/day')
+            },
+            'zen2': {
+                'Cellulose_production_NVE': (1.3, -0.30, None, 'ns/day'),
+                'FactorIX_production_NVE': (3.5, -0.30, None, 'ns/day'),
+                'JAC_production_NVE': (17.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (30.5, -0.30, None, 'ns/day')
+            }
+        }
+    }
     scale = parameter(['small', 'large'])
-    valid_systems = ['daint:mc', 'eiger:mc']
-    platform_info = parameter(
-        inherit_params=True,
-        filter_params=inherit_cpu_only)
 
     @run_after('init')
-    def set_description(self):
-        self.mydescr = f'Amber parallel {self.scale} CPU check'
-
-    @run_after('init')
-    def set_additional_systems(self):
-        if self.scale == 'small':
-            self.valid_systems += ['dom:mc', 'pilatus:mc']
+    def scope_systems(self):
+        if self.platform == 'gpu':
+            if self.scale == 'small':
+                self.valid_systems = ['daint:gpu', 'dom:gpu']
+        else:
+            self.valid_systems = ['daint:mc', 'eiger:mc']
+            if self.scale == 'small':
+                self.valid_systems += ['dom:mc', 'pilatus:mc']
 
     @run_after('init')
     def set_hierarchical_prgenvs(self):
@@ -131,23 +73,45 @@ def set_hierarchical_prgenvs(self):
             self.valid_prog_environs = ['cpeIntel']
 
     @run_after('init')
-    def set_num_tasks_cray_xc(self):
-        if self.current_system.name in ['daint', 'dom']:
-            self.num_tasks_per_node = 36
+    def set_num_nodes(self):
+        if self.current_system.name in ('daint', 'dom'):
             if self.scale == 'small':
                 self.num_nodes = 6
             else:
                 self.num_nodes = 16
-            self.num_tasks = self.num_nodes * self.num_tasks_per_node
-
-    @run_after('init')
-    def set_num_tasks_cray_shasta(self):
-        if self.current_system.name in ['eiger', 'pilatus']:
-            self.num_tasks_per_node = 128
+        elif self.current_system.name in ('eiger', 'pilatus'):
             if self.scale == 'small':
                 self.num_nodes = 4
             else:
-                # there are too many processors, the large jobs cannot start
-                # need to decrease to just 8 nodes
                 self.num_nodes = 8
-            self.num_tasks = self.num_nodes * self.num_tasks_per_node
+
+    @run_after('init')
+    def set_num_gpus_per_node(self):
+        if self.platform == 'gpu':
+            self.num_gpus_per_node = 1
+
+    @run_after('setup')
+    def skip_if_no_topo(self):
+        proc = self.current_partition.processor
+        pname = self.current_partition.fullname
+        if not proc.info:
+            self.skip(f'no topology information found for partition {pname!r}')
+
+    @run_after('setup')
+    def set_num_tasks(self):
+        proc = self.current_partition.processor
+        pname = self.current_partition.fullname
+        self.num_tasks_per_node = proc.num_cores
+        self.num_tasks = self.num_nodes * self.num_tasks_per_node
+
+    @run_before('performance')
+    def set_perf_reference(self):
+        proc = self.current_partition.processor
+        pname = self.current_partition.fullname
+        if pname in ('daint:gpu', 'dom:gpu'):
+            arch = 'p100'
+        else:
+            arch = proc.arch
+
+        with contextlib.suppress(KeyError):
+            self.reference = self.bench_reference[self.scale][arch][self.benchmark]
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index 6d884459bc..4b74f511de 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -3,6 +3,7 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
+import contextlib
 import reframe as rfm
 import reframe.utility.sanity as sn
 
@@ -53,18 +54,15 @@ class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
 
     #: Parameter pack containing the platform ID, input file and
     #: executable.
-    platform_info = parameter([
-        ('cpu', 'mdin.CPU', 'pmemd.MPI'),
-        ('gpu', 'mdin.GPU', 'pmemd.cuda.MPI')
-    ])
+    platform = parameter(['cpu', 'gpu'])
 
     #: NVE simulation parameter pack with the benchmark name,
     #: energy reference and energy tolerance for each case.
-    variant = parameter([
-        ('Cellulose_production_NVE', -443246.0, 5.0E-05),
-        ('FactorIX_production_NVE', -234188.0, 1.0E-04),
-        ('JAC_production_NVE_4fs', -44810.0, 1.0E-03),
-        ('JAC_production_NVE', -58138.0, 5.0E-04)
+    benchmark = parameter([
+        'Cellulose_production_NVE',
+        'FactorIX_production_NVE',
+        'JAC_production_NVE_4fs',
+        'JAC_production_NVE'
     ])
 
     #: :default: :class:`required`
@@ -76,55 +74,43 @@ class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
     tags = {'sciapp', 'chemistry'}
 
     @run_after('init')
-    def unpack_platform_parameter(self):
-        '''Set the executable and input file.'''
-
-        self.platform, self.input_file, self.executable = self.platform_info
-
-    @run_after('init')
-    def unpack_variant_parameter(self):
-        '''Set the value of energy and energy tolerance for a
-        specific program.
-        '''
-
-        self.benchmark, self.energy_value, self.energy_tolerance = self.variant
+    def prepare_test(self):
+        params = {
+            'cpu': ('mdin.CPU', 'pmemd.MPI'),
+            'gpu': ('mdin.GPU', 'pmemd.cuda.MPI')
+        }
+        with contextlib.suppress(KeyError):
+            self.input_file, self.executable = params[self.platform]
+
+        energies = {
+            'Cellulose_production_NVE': (-443246.0, 5.0E-05),
+            'FactorIX_production_NVE': (-234188.0, 1.0E-04),
+            'JAC_production_NVE_4fs': (-44810.0, 1.0E-03),
+            'JAC_production_NVE': (-58138.0, 5.0E-04)
+        }
+        with contextlib.suppress(KeyError):
+            self.energy_value, self.energy_tolerance = energies[self.benchmark]
 
-    @run_after('init')
-    def set_keep_files(self):
         self.keep_files = [self.output_file]
 
-    @performance_function('ns/day', perf_key='nve')
-    def set_perf_patterns(self):
-        return sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
-                                self.output_file, 'perf',
-                                float, item=1)
-
     @run_before('performance')
-    def set_the_performance_dict(self):
-        self.perf_variables = {self.benchmark:
-                               sn.make_performance_function(
-                                   sn.extractsingle(
-                                       r'ns/day =\s+(?P<perf>\S+)',
-                                       self.output_file, 'perf',
-                                       float, item=1), 'ns/day')}
+    def set_perf_vars(self):
+        self.perf_variables = {
+            self.benchmark: sn.make_performance_function(
+                sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
+                                 self.output_file, 'perf',
+                                 float, item=1), 'ns/day'
+            )
+        }
 
     @run_before('run')
-    def download_files(self):
-        '''Download program files, which used in test'''
-
+    def prepare_run(self):
         self.prerun_cmds = [
             # cannot use wget because it is not installed on eiger
             f'curl -LJO https://github.com/victorusu/amber_benchmark_suite'
             f'/raw/main/amber_16_benchmark_suite/PME/{self.benchmark}.tar.bz2',
             f'tar xf {self.benchmark}.tar.bz2'
         ]
-
-    @run_before('run')
-    def set_executable_opts(self):
-        '''Set the executable options for the Amber. Determine the
-        using of input and ouput files.
-        '''
-
         self.executable_opts = ['-O',
                                 '-i', self.input_file,
                                 '-o', self.output_file]

From 0cb26f1e336661db4ca287a05bdcaa2742c9c28e Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Wed, 15 Sep 2021 00:19:34 +0200
Subject: [PATCH 05/10] Revamp Amber library and CSCS tests

---
 cscs-checks/apps/amber/amber_check.py | 87 +++++++++++++++------------
 hpctestlib/apps/amber/nve.py          | 27 ++-------
 2 files changed, 53 insertions(+), 61 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index ace8fb85fc..fbae1148f8 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -19,20 +19,17 @@ class amber_check(Amber_NVE):
         }
     }
     maintainers = ['VH', 'SO']
-    bench_reference = {
-        'small': {
+    num_nodes = parameter([1, 4, 6, 8, 16])
+    allref = {
+        1: {
             'p100': {
                 'Cellulose_production_NVE': (30.0, -0.05, None, 'ns/day'),
                 'FactorIX_production_NVE': (134.0, -0.05, None, 'ns/day'),
                 'JAC_production_NVE': (388.0, -0.05, None, 'ns/day'),
                 'JAC_production_NVE_4fs': (742, -0.05, None, 'ns/day')
-            },
-            'broadwell': {
-                'Cellulose_production_NVE': (8.0, -0.30, None, 'ns/day'),
-                'FactorIX_production_NVE': (34.0, -0.30, None, 'ns/day'),
-                'JAC_production_NVE': (90.0, -0.30, None, 'ns/day'),
-                'JAC_production_NVE_4fs': (150.0, -0.30, None, 'ns/day')
-            },
+            }
+        },
+        4: {
             'zen2': {
                 'Cellulose_production_NVE': (3.2, -0.30, None, 'ns/day'),
                 'FactorIX_production_NVE': (7.0, -0.30, None, 'ns/day'),
@@ -40,51 +37,53 @@ class amber_check(Amber_NVE):
                 'JAC_production_NVE_4fs': (45.0, -0.30, None, 'ns/day')
             }
         },
-        'large': {
+        6: {
             'broadwell': {
-                'Cellulose_production_NVE': (10.0, -0.30, None, 'ns/day'),
-                'FactorIX_production_NVE': (36.0, -0.30, None, 'ns/day'),
-                'JAC_production_NVE': (78.0, -0.30, None, 'ns/day'),
-                'JAC_production_NVE_4fs': (135.0, -0.30, None, 'ns/day')
-            },
+                'Cellulose_production_NVE': (8.0, -0.30, None, 'ns/day'),
+                'FactorIX_production_NVE': (34.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE': (90.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (150.0, -0.30, None, 'ns/day')
+            }
+        },
+        8: {
             'zen2': {
                 'Cellulose_production_NVE': (1.3, -0.30, None, 'ns/day'),
                 'FactorIX_production_NVE': (3.5, -0.30, None, 'ns/day'),
                 'JAC_production_NVE': (17.0, -0.30, None, 'ns/day'),
                 'JAC_production_NVE_4fs': (30.5, -0.30, None, 'ns/day')
             }
+        },
+        16: {
+            'broadwell': {
+                'Cellulose_production_NVE': (10.0, -0.30, None, 'ns/day'),
+                'FactorIX_production_NVE': (36.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE': (78.0, -0.30, None, 'ns/day'),
+                'JAC_production_NVE_4fs': (135.0, -0.30, None, 'ns/day')
+            }
         }
     }
-    scale = parameter(['small', 'large'])
 
     @run_after('init')
     def scope_systems(self):
-        if self.platform == 'gpu':
-            if self.scale == 'small':
-                self.valid_systems = ['daint:gpu', 'dom:gpu']
-        else:
-            self.valid_systems = ['daint:mc', 'eiger:mc']
-            if self.scale == 'small':
-                self.valid_systems += ['dom:mc', 'pilatus:mc']
+        valid_systems = {
+            'gpu': {1: ['daint:gpu', 'dom:gpu']},
+            'cpu': {
+                4: ['eiger:mc', 'pilatus:mc'],
+                6: ['daint:mc', 'dom:mc'],
+                8: ['pilatus:mc'],
+                16: ['daint:mc']
+            }
+        }
+        try:
+            self.valid_systems = valid_systems[self.platform][self.num_nodes]
+        except KeyError:
+            self.valid_systems = []
 
     @run_after('init')
     def set_hierarchical_prgenvs(self):
         if self.current_system.name in ['eiger', 'pilatus']:
             self.valid_prog_environs = ['cpeIntel']
 
-    @run_after('init')
-    def set_num_nodes(self):
-        if self.current_system.name in ('daint', 'dom'):
-            if self.scale == 'small':
-                self.num_nodes = 6
-            else:
-                self.num_nodes = 16
-        elif self.current_system.name in ('eiger', 'pilatus'):
-            if self.scale == 'small':
-                self.num_nodes = 4
-            else:
-                self.num_nodes = 8
-
     @run_after('init')
     def set_num_gpus_per_node(self):
         if self.platform == 'gpu':
@@ -99,9 +98,13 @@ def skip_if_no_topo(self):
 
     @run_after('setup')
     def set_num_tasks(self):
-        proc = self.current_partition.processor
-        pname = self.current_partition.fullname
-        self.num_tasks_per_node = proc.num_cores
+        if self.platform == 'gpu':
+            self.num_tasks_per_node = 1
+        else:
+            proc = self.current_partition.processor
+            pname = self.current_partition.fullname
+            self.num_tasks_per_node = proc.num_cores
+
         self.num_tasks = self.num_nodes * self.num_tasks_per_node
 
     @run_before('performance')
@@ -114,4 +117,8 @@ def set_perf_reference(self):
             arch = proc.arch
 
         with contextlib.suppress(KeyError):
-            self.reference = self.bench_reference[self.scale][arch][self.benchmark]
+            self.reference = {
+                '*': {
+                    'perf': self.allref[self.num_nodes][arch][self.benchmark]
+                }
+            }
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index 4b74f511de..25131ecdd6 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -64,13 +64,6 @@ class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
         'JAC_production_NVE_4fs',
         'JAC_production_NVE'
     ])
-
-    #: :default: :class:`required`
-    num_tasks_per_node = required
-
-    #: :default: :class:`required`
-    executable = required
-
     tags = {'sciapp', 'chemistry'}
 
     @run_after('init')
@@ -91,20 +84,6 @@ def prepare_test(self):
         with contextlib.suppress(KeyError):
             self.energy_value, self.energy_tolerance = energies[self.benchmark]
 
-        self.keep_files = [self.output_file]
-
-    @run_before('performance')
-    def set_perf_vars(self):
-        self.perf_variables = {
-            self.benchmark: sn.make_performance_function(
-                sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
-                                 self.output_file, 'perf',
-                                 float, item=1), 'ns/day'
-            )
-        }
-
-    @run_before('run')
-    def prepare_run(self):
         self.prerun_cmds = [
             # cannot use wget because it is not installed on eiger
             f'curl -LJO https://github.com/victorusu/amber_benchmark_suite'
@@ -114,6 +93,12 @@ def prepare_run(self):
         self.executable_opts = ['-O',
                                 '-i', self.input_file,
                                 '-o', self.output_file]
+        self.keep_files = [self.output_file]
+
+    @performance_function('ns/day')
+    def perf(self):
+        return sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
+                                self.output_file, 'perf', float, item=1)
 
     @sanity_function
     def assert_energy_readout(self):

From b293d5cf8c5f2494c60c402d97cf8550e187e636 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Wed, 15 Sep 2021 23:57:52 +0200
Subject: [PATCH 06/10] Add CSCS tags

---
 cscs-checks/apps/amber/amber_check.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index fbae1148f8..361611739c 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -18,6 +18,7 @@ class amber_check(Amber_NVE):
             'num_switches': 1
         }
     }
+    tags |= {'maintenance', 'production'}
     maintainers = ['VH', 'SO']
     num_nodes = parameter([1, 4, 6, 8, 16])
     allref = {

From fee80882ac9e32990f484ea808472b5b1b55ebe9 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 1 Oct 2021 23:50:07 +0200
Subject: [PATCH 07/10] Address PR comments

---
 cscs-checks/apps/amber/amber_check.py | 16 ++++++------
 hpctestlib/apps/amber/nve.py          | 35 ++++++++++++++++++---------
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index 361611739c..ce8747654d 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -6,11 +6,11 @@
 import contextlib
 
 import reframe as rfm
-from hpctestlib.apps.amber.nve import Amber_NVE
+from hpctestlib.apps.amber.nve import amber_nve_check
 
 
 @rfm.simple_test
-class amber_check(Amber_NVE):
+class amber_check_cscs(amber_nve_check):
     modules = ['Amber']
     valid_prog_environs = ['builtin']
     extra_resources = {
@@ -67,8 +67,8 @@ class amber_check(Amber_NVE):
     @run_after('init')
     def scope_systems(self):
         valid_systems = {
-            'gpu': {1: ['daint:gpu', 'dom:gpu']},
-            'cpu': {
+            'cuda': {1: ['daint:gpu', 'dom:gpu']},
+            'mpi': {
                 4: ['eiger:mc', 'pilatus:mc'],
                 6: ['daint:mc', 'dom:mc'],
                 8: ['pilatus:mc'],
@@ -76,7 +76,7 @@ def scope_systems(self):
             }
         }
         try:
-            self.valid_systems = valid_systems[self.platform][self.num_nodes]
+            self.valid_systems = valid_systems[self.variant][self.num_nodes]
         except KeyError:
             self.valid_systems = []
 
@@ -87,7 +87,7 @@ def set_hierarchical_prgenvs(self):
 
     @run_after('init')
     def set_num_gpus_per_node(self):
-        if self.platform == 'gpu':
+        if self.variant == 'cuda':
             self.num_gpus_per_node = 1
 
     @run_after('setup')
@@ -99,7 +99,7 @@ def skip_if_no_topo(self):
 
     @run_after('setup')
     def set_num_tasks(self):
-        if self.platform == 'gpu':
+        if self.variant == 'cuda':
             self.num_tasks_per_node = 1
         else:
             proc = self.current_partition.processor
@@ -119,7 +119,7 @@ def set_perf_reference(self):
 
         with contextlib.suppress(KeyError):
             self.reference = {
-                '*': {
+                pname: {
                     'perf': self.allref[self.num_nodes][arch][self.benchmark]
                 }
             }
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index 25131ecdd6..12ef18d514 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -8,7 +8,7 @@
 import reframe.utility.sanity as sn
 
 
-class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
+class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
     '''Base class for the Amber NVE Test.
 
     Amber is a suite of biomolecular simulation programs. It
@@ -54,7 +54,7 @@ class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
 
     #: Parameter pack containing the platform ID, input file and
     #: executable.
-    platform = parameter(['cpu', 'gpu'])
+    variant = parameter(['mpi', 'cuda'])
 
     #: NVE simulation parameter pack with the benchmark name,
     #: energy reference and energy tolerance for each case.
@@ -66,23 +66,36 @@ class Amber_NVE(rfm.RunOnlyRegressionTest, pin_prefix=True):
     ])
     tags = {'sciapp', 'chemistry'}
 
-    @run_after('init')
-    def prepare_test(self):
-        params = {
-            'cpu': ('mdin.CPU', 'pmemd.MPI'),
-            'gpu': ('mdin.GPU', 'pmemd.cuda.MPI')
-        }
-        with contextlib.suppress(KeyError):
-            self.input_file, self.executable = params[self.platform]
+    #: Number of tasks to use
+    num_tasks = required
 
+    @run_after('init')
+    def set_energy_references(self):
         energies = {
             'Cellulose_production_NVE': (-443246.0, 5.0E-05),
             'FactorIX_production_NVE': (-234188.0, 1.0E-04),
             'JAC_production_NVE_4fs': (-44810.0, 1.0E-03),
             'JAC_production_NVE': (-58138.0, 5.0E-04)
         }
-        with contextlib.suppress(KeyError):
+        try:
             self.energy_value, self.energy_tolerance = energies[self.benchmark]
+        except KeyError:
+            raise ValueError(
+                f'no energy reference set for benchmark {self.benchmark!r}'
+            ) from None
+
+    @run_after('init')
+    def prepare_test(self):
+        params = {
+            'mpi':  ('mdin.CPU', 'pmemd.MPI'),
+            'cuda': ('mdin.GPU', 'pmemd.cuda.MPI')
+        }
+        try:
+            self.input_file, self.executable = params[self.variant]
+        except KeyError:
+            raise ValueError(
+                f'test not set up for platform {self.variant!r}'
+            ) from None
 
         self.prerun_cmds = [
             # cannot use wget because it is not installed on eiger

From 5e988f42ed0f272dae0e2f2f887017fde23df07c Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 1 Oct 2021 23:55:43 +0200
Subject: [PATCH 08/10] Fix unused imports

---
 cscs-checks/apps/amber/amber_check.py | 1 -
 hpctestlib/apps/amber/nve.py          | 1 -
 2 files changed, 2 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index ce8747654d..c9af44f6be 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -4,7 +4,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 import contextlib
-
 import reframe as rfm
 from hpctestlib.apps.amber.nve import amber_nve_check
 
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index 12ef18d514..0ce13c4a07 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -3,7 +3,6 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
-import contextlib
 import reframe as rfm
 import reframe.utility.sanity as sn
 

From 0f391023905b46702b729648e740ebd2c445c0d3 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 4 Oct 2021 22:21:51 +0200
Subject: [PATCH 09/10] Fine tune Amber test

---
 cscs-checks/apps/amber/amber_check.py |   2 +-
 hpctestlib/apps/amber/nve.py          | 117 +++++++++++++++-----------
 2 files changed, 70 insertions(+), 49 deletions(-)

diff --git a/cscs-checks/apps/amber/amber_check.py b/cscs-checks/apps/amber/amber_check.py
index c9af44f6be..68e691687c 100644
--- a/cscs-checks/apps/amber/amber_check.py
+++ b/cscs-checks/apps/amber/amber_check.py
@@ -9,7 +9,7 @@
 
 
 @rfm.simple_test
-class amber_check_cscs(amber_nve_check):
+class cscs_amber_check(amber_nve_check):
     modules = ['Amber']
     valid_prog_environs = ['builtin']
     extra_resources = {
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index 0ce13c4a07..52f68a8d00 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -5,6 +5,7 @@
 
 import reframe as rfm
 import reframe.utility.sanity as sn
+import reframe.utility.typecheck as typ
 
 
 class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
@@ -24,67 +25,87 @@ class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
     is that Amber is already installed on the device under test.
     '''
 
-    #: Amber output file.
+    #: The output file to pass to the Amber executable.
     #:
-    #: :default: : 'amber.out'
+    #: :type: `str`
+    #: :default: ``'amber.out'``
     output_file = variable(str, value='amber.out')
 
-    #: Amber input file. This file is set by the post-init hook
-    #: :func:`unpack_platform_parameter`.
+    #: The input file to use.
     #:
-    #: :default: :class:`required`
+    #: The library sets this file to ``mdin.CPU`` or ``mdin.GPU`` depending on
+    #: the test variant.
+    #:
+    #: :default: :obj:`required`
     input_file = variable(str)
 
-    #: Reference value of energy, that is used for the comparison
-    #: with the execution ouput on the sanity step. The absolute
-    #: difference between final energy value and reference value
-    #: should be smaller than energy_tolerance
+    #: The name of the benchmark that this test encodes.
     #:
-    #: :type: float
-    #: :default: :class:`required`
-    energy_value = variable(float)
+    #: :type: `str`
+    #: :required:
+    #: :default: Set by the corresponding value in the :attr:`benchmark_info`
+    #:   parameter pack during initialisation.
+    benchmark = variable(str)
 
-    #: Maximum deviation from the reference value of energy,
-    #: that is acceptable.
+    #: Energy value reference.
     #:
-    #: :type: float
-    #: :default: :class:`required`
-    energy_tolerance = variable(float)
+    #: :type: `float`
+    #: :required:
+    #: :default: Set by the corresponding value in the :attr:`benchmark_info`
+    #:   parameter pack during initialisation.
+    energy_ref = variable(float)
 
-    #: Parameter pack containing the platform ID, input file and
-    #: executable.
-    variant = parameter(['mpi', 'cuda'])
+    #: Energy value tolerance.
+    #:
+    #: :type: `float`
+    #: :required:
+    #: :default: Set by the corresponding value in the :attr:`benchmark_info`
+    #:   parameter pack during initialisation.
+    energy_tol = variable(float)
 
-    #: NVE simulation parameter pack with the benchmark name,
-    #: energy reference and energy tolerance for each case.
-    benchmark = parameter([
-        'Cellulose_production_NVE',
-        'FactorIX_production_NVE',
-        'JAC_production_NVE_4fs',
-        'JAC_production_NVE'
+    #: Parameter pack encoding the benchmark information.
+    #:
+    #: The first element of the tuple refers to the benchmark name,
+    #: the second is the energy reference and the third is the
+    #: tolerance threshold.
+    #:
+    #: :type:`Tuple[str, float, float]`
+    #: :values:
+    #:     .. code-block:: python
+    #:
+    #:        [
+    #:            ('Cellulose_production_NVE', -443246.0, 5.0E-05),
+    #:            ('FactorIX_production_NVE', -234188.0, 1.0E-04),
+    #:            ('JAC_production_NVE_4fs', -44810.0, 1.0E-03),
+    #:            ('JAC_production_NVE', -58138.0, 5.0E-04)
+    #:        ]
+    #:
+    benchmark_info = parameter([
+        ('Cellulose_production_NVE', -443246.0, 5.0E-05),
+        ('FactorIX_production_NVE', -234188.0, 1.0E-04),
+        ('JAC_production_NVE_4fs', -44810.0, 1.0E-03),
+        ('JAC_production_NVE', -58138.0, 5.0E-04)
     ])
+
+    # Parameter encoding the variant of the test.
+    #
+    # :type:`str`
+    # :values: ``['mpi', 'cuda']``
+    variant = parameter(['mpi', 'cuda'])
+
+    # Test tags
+    #
+    # :default: ``{'sciapp', 'chemistry'}``
     tags = {'sciapp', 'chemistry'}
 
-    #: Number of tasks to use
+    #: The :attr:`~reframe.core.pipeline.RegressionTest.num_tasks` is required
     num_tasks = required
 
-    @run_after('init')
-    def set_energy_references(self):
-        energies = {
-            'Cellulose_production_NVE': (-443246.0, 5.0E-05),
-            'FactorIX_production_NVE': (-234188.0, 1.0E-04),
-            'JAC_production_NVE_4fs': (-44810.0, 1.0E-03),
-            'JAC_production_NVE': (-58138.0, 5.0E-04)
-        }
-        try:
-            self.energy_value, self.energy_tolerance = energies[self.benchmark]
-        except KeyError:
-            raise ValueError(
-                f'no energy reference set for benchmark {self.benchmark!r}'
-            ) from None
-
     @run_after('init')
     def prepare_test(self):
+        self.benchmark, self.energy_ref, self.energy_tol = self.benchmark_info
+        self.descr = f'Amber NVE {self.benchmark} benchmark ({self.variant})'
+
         params = {
             'mpi':  ('mdin.CPU', 'pmemd.MPI'),
             'cuda': ('mdin.GPU', 'pmemd.cuda.MPI')
@@ -97,7 +118,6 @@ def prepare_test(self):
             ) from None
 
         self.prerun_cmds = [
-            # cannot use wget because it is not installed on eiger
             f'curl -LJO https://github.com/victorusu/amber_benchmark_suite'
             f'/raw/main/amber_16_benchmark_suite/PME/{self.benchmark}.tar.bz2',
             f'tar xf {self.benchmark}.tar.bz2'
@@ -109,18 +129,19 @@ def prepare_test(self):
 
     @performance_function('ns/day')
     def perf(self):
+        '''The performance of the benchmark expressed in ``ns/day``.'''
         return sn.extractsingle(r'ns/day =\s+(?P<perf>\S+)',
                                 self.output_file, 'perf', float, item=1)
 
     @sanity_function
     def assert_energy_readout(self):
-        '''Assert the obtained energy meets the specified tolerances.'''
+        '''Assert that the obtained energy meets the required tolerance.'''
 
         energy = sn.extractsingle(r' Etot\s+=\s+(?P<energy>\S+)',
                                   self.output_file, 'energy', float, item=-2)
-        energy_diff = sn.abs(energy - self.energy_value)
-        ref_ener_diff = sn.abs(self.energy_value *
-                               self.energy_tolerance)
+        energy_diff = sn.abs(energy - self.energy_ref)
+        ref_ener_diff = sn.abs(self.energy_ref *
+                               self.energy_tol)
         return sn.all([
             sn.assert_found(r'Final Performance Info:', self.output_file),
             sn.assert_lt(energy_diff, ref_ener_diff)

From 30ecdb43a8b67a0bb27c8ad7d2030bceaebf0b67 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 4 Oct 2021 22:48:47 +0200
Subject: [PATCH 10/10] Fix docs + remove unused imports

---
 docs/index.rst               |  1 +
 hpctestlib/apps/amber/nve.py | 69 +++++++++++++++++++-----------------
 2 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/docs/index.rst b/docs/index.rst
index 31137fd2a0..1b1a449de1 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -56,3 +56,4 @@ Publications
    usecases
    migration_2_to_3
    manuals
+   hpctestlib
diff --git a/hpctestlib/apps/amber/nve.py b/hpctestlib/apps/amber/nve.py
index 52f68a8d00..d1e8e7df92 100644
--- a/hpctestlib/apps/amber/nve.py
+++ b/hpctestlib/apps/amber/nve.py
@@ -5,62 +5,63 @@
 
 import reframe as rfm
 import reframe.utility.sanity as sn
-import reframe.utility.typecheck as typ
 
 
 class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
-    '''Base class for the Amber NVE Test.
-
-    Amber is a suite of biomolecular simulation programs. It
-    began in the late 1970's, and is maintained by an active
-    development community (see ambermd.org).
-
-    The presented abstract run-only class checks the amber perfomance.
-    To do this, it is necessary to define in tests the name
-    of the running script (input file), the output file,
-    as well as set the reference values of energy and possible
-    deviations from this value. This data is used to check if
-    the task is being executed correctly, that is, the final energy
-    is correct (approximately the reference). The default assumption
-    is that Amber is already installed on the device under test.
+    '''Amber NVE test.
+
+    `Amber <https://ambermd.org/>`__ is a suite of biomolecular simulation
+    programs. It began in the late 1970's, and is maintained by an active
+    development community.
+
+    This test is parametrized over the benchmark type (see
+    :attr:`benchmark_info`) and the variant of the code (see :attr:`variant`).
+    Each test instance executes the benchmark, validates numerically its output
+    and extracts and reports a performance metric.
+
     '''
 
     #: The output file to pass to the Amber executable.
     #:
-    #: :type: `str`
+    #: :type: :class:`str`
+    #: :required: No
     #: :default: ``'amber.out'``
     output_file = variable(str, value='amber.out')
 
     #: The input file to use.
     #:
-    #: The library sets this file to ``mdin.CPU`` or ``mdin.GPU`` depending on
-    #: the test variant.
+    #: This is set to ``mdin.CPU`` or ``mdin.GPU`` depending on the test
+    #: variant during initialization.
     #:
-    #: :default: :obj:`required`
+    #: :type: :class:`str`
+    #: :required: Yes
     input_file = variable(str)
 
     #: The name of the benchmark that this test encodes.
     #:
-    #: :type: `str`
-    #: :required:
-    #: :default: Set by the corresponding value in the :attr:`benchmark_info`
-    #:   parameter pack during initialisation.
+    #: This is set from the corresponding value in the :attr:`benchmark_info`
+    #: parameter pack during initialization.
+    #:
+    #: :type: :class:`str`
+    #: :required: Yes
     benchmark = variable(str)
 
     #: Energy value reference.
     #:
+    #: This is set from the corresponding value in the :attr:`benchmark_info`
+    #: parameter pack during initialization.
+    #:
     #: :type: `float`
-    #: :required:
-    #: :default: Set by the corresponding value in the :attr:`benchmark_info`
-    #:   parameter pack during initialisation.
+    #: :required: Yes
     energy_ref = variable(float)
 
     #: Energy value tolerance.
     #:
+    #: This is set from the corresponding value in the :attr:`benchmark_info`
+    #: parameter pack during initialization.
+    #:
     #: :type: `float`
-    #: :required:
-    #: :default: Set by the corresponding value in the :attr:`benchmark_info`
-    #:   parameter pack during initialisation.
+    #: :required: Yes
     energy_tol = variable(float)
 
     #: Parameter pack encoding the benchmark information.
@@ -69,7 +70,7 @@ class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
     #: the second is the energy reference and the third is the
     #: tolerance threshold.
     #:
-    #: :type:`Tuple[str, float, float]`
+    #: :type: `Tuple[str, float, float]`
     #: :values:
     #:     .. code-block:: python
     #:
@@ -79,7 +80,6 @@ class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
     #:            ('JAC_production_NVE_4fs', -44810.0, 1.0E-03),
     #:            ('JAC_production_NVE', -58138.0, 5.0E-04)
     #:        ]
-    #:
     benchmark_info = parameter([
         ('Cellulose_production_NVE', -443246.0, 5.0E-05),
         ('FactorIX_production_NVE', -234188.0, 1.0E-04),
@@ -95,10 +95,15 @@ class amber_nve_check(rfm.RunOnlyRegressionTest, pin_prefix=True):
 
     # Test tags
     #
+    # :required: No
     # :default: ``{'sciapp', 'chemistry'}``
     tags = {'sciapp', 'chemistry'}
 
-    #: The :attr:`~reframe.core.pipeline.RegressionTest.num_tasks` is required
+    #: See :attr:`~reframe.core.pipeline.RegressionTest.num_tasks`.
+    #:
+    #: The ``mpi`` variant of the test requires ``num_tasks > 1``.
+    #:
+    #: :required: Yes
     num_tasks = required
 
     @run_after('init')