Merge pull request #1223 from teojgo/enhancement/impose_units

Vasileios Karakasis · web-flow · commit 978a67099464 · 2020-03-12T23:16:48.000+01:00
[feat] Impose units in performance reference tuple
diff --git a/cscs-checks/compile/haswell_fma_check.py b/cscs-checks/compile/haswell_fma_check.py
@@ -55,4 +55,3 @@ def setflags(self):
             if self.current_environ.name == 'PrgEnv-cray':
                 self.build_system.cflags = ['-Ofast', '-S']
                 self.build_system.cxxflags = ['-Ofast', '-S']
-
diff --git a/cscs-checks/libraries/hpx/hpx_hello_world.py b/cscs-checks/libraries/hpx/hpx_hello_world.py
@@ -62,4 +62,3 @@ def set_sanity(self):
         self.sanity_patterns = sn.all(sn.chain([assert_num_tasks],
                                                assert_threads,
                                                assert_localities))
-
diff --git a/cscs-checks/mch/automatic_arrays_acc.py b/cscs-checks/mch/automatic_arrays_acc.py
@@ -41,14 +41,14 @@ def __init__(self):
         }
         self.arrays_reference = {
             'PrgEnv-cray': {
-                'daint:gpu': {'time': (5.7E-05, None, 0.15)},
-                'dom:gpu': {'time': (5.7E-05, None, 0.15)},
-                'kesch:cn': {'time': (2.9E-04, None, 0.15)},
+                'daint:gpu': {'time': (5.7E-05, None, 0.15, 's')},
+                'dom:gpu': {'time': (5.7E-05, None, 0.15, 's')},
+                'kesch:cn': {'time': (2.9E-04, None, 0.15, 's')},
             },
             'PrgEnv-pgi': {
-                'daint:gpu': {'time': (7.5E-05, None, 0.15)},
-                'dom:gpu': {'time': (7.5e-05, None, 0.15)},
-                'kesch:cn': {'time': (1.4E-04, None, 0.15)},
+                'daint:gpu': {'time': (7.5E-05, None, 0.15, 's')},
+                'dom:gpu': {'time': (7.5e-05, None, 0.15, 's')},
+                'kesch:cn': {'time': (1.4E-04, None, 0.15, 's')},
             }
         }
 
diff --git a/cscs-checks/mch/collectives_halo.py b/cscs-checks/mch/collectives_halo.py
@@ -97,13 +97,13 @@ def __init__(self, variant, bench_reference):
 
         self.reference = {
             'kesch:cn': {
-                'elapsed_time': (ref, None, 0.15)
+                'elapsed_time': (ref, None, 0.15, 's')
             },
             'daint': {
-                'elapsed_time': (ref, None, 0.15)
+                'elapsed_time': (ref, None, 0.15, 's')
             },
             'dom': {
-                'elapsed_time': (ref, None, 0.15)
+                'elapsed_time': (ref, None, 0.15, 's')
             },
         }
 
diff --git a/cscs-checks/mch/cuda_stress_test.py b/cscs-checks/mch/cuda_stress_test.py
@@ -36,13 +36,13 @@ def __init__(self):
         }
         self.reference = {
             'daint:gpu': {
-                'time': (1.41184, None, 0.05)
+                'time': (1.41184, None, 0.05, 's')
             },
             'dom:gpu': {
-                'time': (1.39758, None, 0.05)
+                'time': (1.39758, None, 0.05, 's')
             },
             'kesch:cn': {
-                'time': (2.25, None, 0.05)
+                'time': (2.25, None, 0.05, 's')
             }
         }
         self.tags = {'production', 'mch', 'craype'}
diff --git a/cscs-checks/mch/fieldextra_check.py b/cscs-checks/mch/fieldextra_check.py
@@ -79,11 +79,11 @@ def __init__(self):
             r'%INFO fieldextra: Program successfully completed', self.stdout
         )
         self.perf_patterns = {
-            'perf': sn.extractsingle(r'WALL CLOCK\s*SPEEDUP\D*(?P<perf>\S+)',
-                                     'fieldextra.diagnostic', 'perf', float)
+            'time': sn.extractsingle(r'WALL CLOCK\s*SPEEDUP\D*(?P<time>\S+)',
+                                     'fieldextra.diagnostic', 'time', float)
         }
         self.reference = {
             'kesch': {
-                'perf': (420., None, 0.10)
+                'time': (420., None, 0.10, 's')
             }
         }
diff --git a/cscs-checks/mch/g2g_meteoswiss_check.py b/cscs-checks/mch/g2g_meteoswiss_check.py
@@ -29,7 +29,7 @@ def __init__(self, g2g):
                                          '-DCMAKE_BUILD_TYPE=Release',
                                          '-DENABLE_MPI_TIMER=ON']
         self.build_system.max_concurrency = 1
-        self.maintainers = ['TM', 'JG']
+        self.maintainers = ['AJ', 'LM']
         self.tags = {'production', 'mch'}
         self.num_tasks = 2
         self.num_gpus_per_node  = 2
@@ -46,6 +46,6 @@ def __init__(self, g2g):
                                      self.stdout, 'time', float)
         }
         self.reference = {
-            'kesch:cn': {'time': (3.461, None, 0.2)}
+            'kesch:cn': {'time': (3.461, None, 0.2, 's')}
         }
         self.variables = {'G2G': str(g2g)}
diff --git a/cscs-checks/mch/gpu_direct_acc.py b/cscs-checks/mch/gpu_direct_acc.py
@@ -72,4 +72,3 @@ def setflags(self):
                 self.build_system.fflags += ['-ta=tesla:cc35']
             elif self.current_system.name in ['arolla', 'tsa']:
                 self.build_system.fflags += ['-ta=tesla:cc70']
-
diff --git a/cscs-checks/prgenv/helloworld.py b/cscs-checks/prgenv/helloworld.py
@@ -145,6 +145,7 @@ def __init__(self, lang, linkage):
             self.valid_prog_environs += ['PrgEnv-pgi-nompi',
                                          'PrgEnv-gnu-nompi']
 
+
 @rfm.required_version('>=2.14')
 @rfm.parameterized_test(*([lang, linkage]
                           for lang in ['cpp', 'c', 'f90']
diff --git a/cscs-checks/tools/profiling_and_debugging/gperftools_mpi_omp.py b/cscs-checks/tools/profiling_and_debugging/gperftools_mpi_omp.py
@@ -86,7 +86,7 @@ def __init__(self, lang):
             # check pdf report:
             sn.assert_found('PDF document', self.rpt_file_doc),
         ])
-        self.perf_patterns = { 'jacobi_elapsed%': self.report_flat_pctg, }
+        self.perf_patterns = {'jacobi_elapsed%': self.report_flat_pctg, }
         self.maintainers = ['JG']
         self.tags = {'performance-tools'}
 
diff --git a/docs/running.rst b/docs/running.rst
@@ -998,7 +998,7 @@ The attributes of this handler are the following:
   - ``check_perf_ref``: The reference performance value of a certain performance variable.
   - ``check_perf_value``: The performance value obtained by this test for a certain performance variable.
   - ``check_perf_var``: The name of the `performance variable <tutorial.html#writing-a-performance-test>`__, whose value is logged.
-  - ``check_perf_unit``: The unit of measurement for the measured performance variable, if specified in the corresponding tuple of the :attr:`reframe.core.pipeline.RegressionTest.reference` attribute.
+  - ``check_perf_unit``: The unit of measurement for the measured performance variable specified in the corresponding tuple of the :attr:`reframe.core.pipeline.RegressionTest.reference` attribute.
 
 .. note::
    .. versionchanged:: 2.20
diff --git a/docs/tutorial.rst b/docs/tutorial.rst
@@ -674,7 +674,8 @@ The ``perf`` subkey will then be searched in the following scopes in this order:
 The first occurrence will be used as the reference value of the ``perf`` performance variable.
 In our example, the ``perf`` key will be resolved in the ``daint:gpu`` scope giving us the reference value.
 
-Reference values in ReFrame are specified as a three-tuple or four-tuple comprising the reference value, the lower and upper thresholds and, optionally, the measurement unit.
+Reference values in ReFrame are specified as a four-tuple comprising the reference value, the lower and upper thresholds and the measurement unit.
+If no unit is relevant, then you have to insert :class:`None` explicitly.
 Thresholds are specified as decimal fractions of the reference value. For nonnegative reference values, the lower threshold must lie in the [-1,0], whereas the upper threshold may be any positive real number or zero.
 In our example, the reference value for this test on ``daint:gpu`` is 50 Gflop/s ±10%. Setting a threshold value to :class:`None` disables the threshold.
 If you specify a measurement unit as well, you will be able to log it the performance logs of the test; this is handy when you are inspecting or plotting the performance values.
@@ -692,6 +693,11 @@ This is useful when using ReFrame for benchmarking purposes and you would like t
 
    .. versionadded:: 2.19
 
+.. note::
+   Reference tuples now require the measurement unit.
+   .. versionchanged:: 3.0
+
+
 Combining It All Together
 -------------------------
 
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
@@ -479,9 +479,9 @@ class RegressionTest(metaclass=RegressionTestMeta):
     #: The reference values are specified as a scoped dictionary keyed on the
     #: performance variables defined in :attr:`perf_patterns` and scoped under
     #: the system/partition combinations.
-    #: The reference itself is a three- or four-tuple that contains the
-    #: reference value, the lower and upper thresholds and, optionally, the
-    #: measurement unit.
+    #: The reference itself is a four-tuple that contains the reference value,
+    #: the lower and upper thresholds and the measurement unit.
+    #:
     #: An example follows:
     #:
     #: .. code:: python
@@ -499,7 +499,13 @@ class RegressionTest(metaclass=RegressionTestMeta):
     #:
     #: :type: A scoped dictionary with system names as scopes or :class:`None`
     #: :default: ``{}``
-    reference = fields.ScopedDictField('reference', typ.Tuple[object])
+    #:
+    #: .. note::
+    #:     .. versionchanged:: 3.0
+    #:        The measurement unit is required. The user should explicitly
+    #:        specify `None` if no unit is available.
+    reference = fields.ScopedDictField(
+        'reference', typ.Tuple[object, object, object, object])
     # FIXME: There is not way currently to express tuples of `float`s or
     # `None`s, so we just use the very generic `object`
 
@@ -1307,11 +1313,7 @@ def check_performance(self):
                 keyparts = key.split(self.reference.scope_separator)
                 system = keyparts[0]
                 varname = keyparts[-1]
-                try:
-                    unit = ref[3]
-                except IndexError:
-                    unit = None
-
+                unit = ref[3]
                 variables.add((varname, unit))
                 if system == '*':
                     has_default = True
diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py
@@ -795,11 +795,11 @@ class MyTest(rfm.RegressionTest):
         self.test.setup(self.partition, self.prgenv)
         self.test.reference = {
             'testsys': {
-                'value1': (1.4, -0.1, 0.1),
-                'value2': (1.7, -0.1, 0.1),
+                'value1': (1.4, -0.1, 0.1, None),
+                'value2': (1.7, -0.1, 0.1, None),
             },
             'testsys:gpu': {
-                'value3': (3.1, -0.1, 0.1),
+                'value3': (3.1, -0.1, 0.1, None),
             }
         }
 
@@ -902,12 +902,20 @@ def test_performance_failure(self):
         with pytest.raises(PerformanceError):
             self.test.check_performance()
 
+    def test_performance_no_units(self):
+        with pytest.raises(TypeError):
+            self.test.reference = {
+                'testsys': {
+                    'value1': (1.4, -0.1, 0.1),
+                }
+            }
+
     def test_unknown_tag(self):
         self.test.reference = {
             'testsys': {
-                'value1': (1.4, -0.1, 0.1),
-                'value2': (1.7, -0.1, 0.1),
-                'foo': (3.1, -0.1, 0.1),
+                'value1': (1.4, -0.1, 0.1, None),
+                'value2': (1.7, -0.1, 0.1, None),
+                'foo': (3.1, -0.1, 0.1, None),
             }
         }
 
@@ -923,11 +931,11 @@ def test_unknown_system(self):
                                       performance3=3.3)
         self.test.reference = {
             'testsys:login': {
-                'value1': (1.4, -0.1, 0.1),
-                'value3': (3.1, -0.1, 0.1),
+                'value1': (1.4, -0.1, 0.1, None),
+                'value3': (3.1, -0.1, 0.1, None),
             },
             'testsys:login2': {
-                'value2': (1.7, -0.1, 0.1)
+                'value2': (1.7, -0.1, 0.1, None)
             }
         }
         self.test.check_performance()
@@ -945,9 +953,9 @@ def test_default_reference(self):
                                       performance3=3.3)
         self.test.reference = {
             '*': {
-                'value1': (1.4, -0.1, 0.1),
-                'value2': (1.7, -0.1, 0.1),
-                'value3': (3.1, -0.1, 0.1),
+                'value1': (1.4, -0.1, 0.1, None),
+                'value2': (1.7, -0.1, 0.1, None),
+                'value3': (3.1, -0.1, 0.1, None),
             }
         }
 
@@ -959,11 +967,11 @@ def test_tag_resolution(self):
                                       performance3=3.3)
         self.test.reference = {
             'testsys': {
-                'value1': (1.4, -0.1, 0.1),
-                'value2': (1.7, -0.1, 0.1),
+                'value1': (1.4, -0.1, 0.1, None),
+                'value2': (1.7, -0.1, 0.1, None),
             },
             '*': {
-                'value3': (3.1, -0.1, 0.1),
+                'value3': (3.1, -0.1, 0.1, None),
             }
         }
         self.test.check_performance()

Original file line number	Diff line number	Diff line change
`@@ -36,13 +36,13 @@ def __init__(self):`
`36`	`36`	`}`
`37`	`37`	`self.reference = {`
`38`	`38`	`'daint:gpu': {`
`39`		`- 'time': (1.41184, None, 0.05)`
	`39`	`+ 'time': (1.41184, None, 0.05, 's')`
`40`	`40`	`},`
`41`	`41`	`'dom:gpu': {`
`42`		`- 'time': (1.39758, None, 0.05)`
	`42`	`+ 'time': (1.39758, None, 0.05, 's')`
`43`	`43`	`},`
`44`	`44`	`'kesch:cn': {`
`45`		`- 'time': (2.25, None, 0.05)`
	`45`	`+ 'time': (2.25, None, 0.05, 's')`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`	`self.tags = {'production', 'mch', 'craype'}`
Original file line number	Diff line number	Diff line change
`@@ -79,11 +79,11 @@ def __init__(self):`
`79`	`79`	`r'%INFO fieldextra: Program successfully completed', self.stdout`
`80`	`80`	`)`
`81`	`81`	`self.perf_patterns = {`
`82`		`- 'perf': sn.extractsingle(r'WALL CLOCK\sSPEEDUP\D(?P<perf>\S+)',`
`83`		`- 'fieldextra.diagnostic', 'perf', float)`
	`82`	`+ 'time': sn.extractsingle(r'WALL CLOCK\sSPEEDUP\D(?P<time>\S+)',`
	`83`	`+ 'fieldextra.diagnostic', 'time', float)`
`84`	`84`	`}`
`85`	`85`	`self.reference = {`
`86`	`86`	`'kesch': {`
`87`		`- 'perf': (420., None, 0.10)`
	`87`	`+ 'time': (420., None, 0.10, 's')`
`88`	`88`	`}`
`89`	`89`	`}`