From 90d8c74813df5a6e6c9389c3d5bf539381fc61ee Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Mon, 9 Mar 2020 15:01:22 +0100 Subject: [PATCH 1/3] Impose units in performance reference tuple * Fix checks that don't contain units in performance reference tuples. * Add unit test. * Documentation fixes. --- cscs-checks/compile/haswell_fma_check.py | 1 - cscs-checks/libraries/hpx/hpx_hello_world.py | 1 - cscs-checks/mch/automatic_arrays_acc.py | 12 +++--- cscs-checks/mch/collectives_halo.py | 6 +-- cscs-checks/mch/cuda_stress_test.py | 6 +-- cscs-checks/mch/fieldextra_check.py | 2 +- cscs-checks/mch/g2g_meteoswiss_check.py | 2 +- cscs-checks/mch/gpu_direct_acc.py | 1 - cscs-checks/prgenv/helloworld.py | 1 + .../gperftools_mpi_omp.py | 2 +- docs/running.rst | 2 +- docs/tutorial.rst | 10 ++++- reframe/core/pipeline.py | 20 +++++----- unittests/test_pipeline.py | 38 +++++++++++-------- 14 files changed, 60 insertions(+), 44 deletions(-) diff --git a/cscs-checks/compile/haswell_fma_check.py b/cscs-checks/compile/haswell_fma_check.py index 4ac601794b..3c4c680c8b 100644 --- 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 index 420028bafc..1c21304374 100644 --- 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 index 52b8a51cf3..3d3cf787e3 100644 --- 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, None)}, + 'dom:gpu': {'time': (5.7E-05, None, 0.15, None)}, + 'kesch:cn': {'time': (2.9E-04, None, 0.15, None)}, }, '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, None)}, + 'dom:gpu': {'time': (7.5e-05, None, 0.15, None)}, + 'kesch:cn': {'time': (1.4E-04, None, 0.15, None)}, } } diff --git a/cscs-checks/mch/collectives_halo.py b/cscs-checks/mch/collectives_halo.py index 166142baae..42608a6a87 100644 --- 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, None) }, 'daint': { - 'elapsed_time': (ref, None, 0.15) + 'elapsed_time': (ref, None, 0.15, None) }, 'dom': { - 'elapsed_time': (ref, None, 0.15) + 'elapsed_time': (ref, None, 0.15, None) }, } diff --git a/cscs-checks/mch/cuda_stress_test.py b/cscs-checks/mch/cuda_stress_test.py index 463574fcaa..773864dacb 100644 --- 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, None) }, 'dom:gpu': { - 'time': (1.39758, None, 0.05) + 'time': (1.39758, None, 0.05, None) }, 'kesch:cn': { - 'time': (2.25, None, 0.05) + 'time': (2.25, None, 0.05, None) } } self.tags = {'production', 'mch', 'craype'} diff --git a/cscs-checks/mch/fieldextra_check.py b/cscs-checks/mch/fieldextra_check.py index b0dece55d3..3934d0b41e 100644 --- a/cscs-checks/mch/fieldextra_check.py +++ b/cscs-checks/mch/fieldextra_check.py @@ -84,6 +84,6 @@ def __init__(self): } self.reference = { 'kesch': { - 'perf': (420., None, 0.10) + 'perf': (420., None, 0.10, None) } } diff --git a/cscs-checks/mch/g2g_meteoswiss_check.py b/cscs-checks/mch/g2g_meteoswiss_check.py index 616240e606..e1afe474a5 100644 --- a/cscs-checks/mch/g2g_meteoswiss_check.py +++ b/cscs-checks/mch/g2g_meteoswiss_check.py @@ -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, None)} } self.variables = {'G2G': str(g2g)} diff --git a/cscs-checks/mch/gpu_direct_acc.py b/cscs-checks/mch/gpu_direct_acc.py index 27623a42c6..02f3c7ca4a 100644 --- 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 index 64fc66739c..5286610cc2 100644 --- 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 index ab3bd0ab75..cef0a66ade 100644 --- 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 index 4ba8e2d452..e4b7d26812 100644 --- 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 `__, 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 index 19bc4c7005..4bdac4c749 100644 --- 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 available then :class:`None` has to be used for the measurement unit. 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,13 @@ 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. :class:`None` must be + used if no unit is available. + + .. versionchanged:: 3.0 + + Combining It All Together ------------------------- diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 5cc2727d40..acc9008f77 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -467,9 +467,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 @@ -487,7 +487,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` @@ -1293,11 +1299,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 index 141bb0adc6..b7cbfc218b 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -751,11 +751,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), } } @@ -858,12 +858,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), } } @@ -879,11 +887,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() @@ -901,9 +909,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), } } @@ -915,11 +923,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() From 658863dd4a8729b650030efd900e81f89b0e7a3c Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 12 Mar 2020 12:41:35 +0100 Subject: [PATCH 2/3] Address PR comments --- cscs-checks/mch/automatic_arrays_acc.py | 12 ++++++------ cscs-checks/mch/collectives_halo.py | 6 +++--- cscs-checks/mch/cuda_stress_test.py | 6 +++--- cscs-checks/mch/fieldextra_check.py | 2 +- cscs-checks/mch/g2g_meteoswiss_check.py | 4 ++-- docs/tutorial.rst | 6 ++---- 6 files changed, 17 insertions(+), 19 deletions(-) diff --git a/cscs-checks/mch/automatic_arrays_acc.py b/cscs-checks/mch/automatic_arrays_acc.py index 3d3cf787e3..0c48c0f2e4 100644 --- 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, None)}, - 'dom:gpu': {'time': (5.7E-05, None, 0.15, None)}, - 'kesch:cn': {'time': (2.9E-04, None, 0.15, None)}, + '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, None)}, - 'dom:gpu': {'time': (7.5e-05, None, 0.15, None)}, - 'kesch:cn': {'time': (1.4E-04, None, 0.15, None)}, + '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 index 42608a6a87..f4e803cec3 100644 --- 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, None) + 'elapsed_time': (ref, None, 0.15, 's') }, 'daint': { - 'elapsed_time': (ref, None, 0.15, None) + 'elapsed_time': (ref, None, 0.15, 's') }, 'dom': { - 'elapsed_time': (ref, None, 0.15, None) + '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 index 773864dacb..f2e28e2960 100644 --- 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, None) + 'time': (1.41184, None, 0.05, 's') }, 'dom:gpu': { - 'time': (1.39758, None, 0.05, None) + 'time': (1.39758, None, 0.05, 's') }, 'kesch:cn': { - 'time': (2.25, None, 0.05, None) + '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 index 3934d0b41e..1384270378 100644 --- a/cscs-checks/mch/fieldextra_check.py +++ b/cscs-checks/mch/fieldextra_check.py @@ -84,6 +84,6 @@ def __init__(self): } self.reference = { 'kesch': { - 'perf': (420., None, 0.10, None) + 'perf': (420., None, 0.10, 's') } } diff --git a/cscs-checks/mch/g2g_meteoswiss_check.py b/cscs-checks/mch/g2g_meteoswiss_check.py index e1afe474a5..8ac939d393 100644 --- 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, None)} + 'kesch:cn': {'time': (3.461, None, 0.2, 's')} } self.variables = {'G2G': str(g2g)} diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 4bdac4c749..b81079bbc2 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -675,7 +675,7 @@ The first occurrence will be used as the reference value of the ``perf`` perform 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 four-tuple comprising the reference value, the lower and upper thresholds and the measurement unit. -If no unit is available then :class:`None` has to be used for 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. @@ -694,9 +694,7 @@ 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. :class:`None` must be - used if no unit is available. - + Reference tuples now require the measurement unit. .. versionchanged:: 3.0 From 2009e26e648fc81cd4ed17a2875d6ba9f5bdbc02 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 12 Mar 2020 13:22:44 +0100 Subject: [PATCH 3/3] Address PR comments (version 2) --- cscs-checks/mch/fieldextra_check.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cscs-checks/mch/fieldextra_check.py b/cscs-checks/mch/fieldextra_check.py index 1384270378..9e401f97ea 100644 --- 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\S+)', - 'fieldextra.diagnostic', 'perf', float) + 'time': sn.extractsingle(r'WALL CLOCK\s*SPEEDUP\D*(?P