Address comments

Author: jjotero

docs/configure.rst

@@ -194,7 +194,7 @@ You can view logger's log level as a general cut off.
 For example, if we have set it to ``warning``, no debug or informational messages would ever be printed.
 
 Finally, there is a special set of handlers for handling performance log messages.
-Performance log messages are generated *only* for `performance tests <tutorial_basics.html#writing-a-performance-test>`__, i.e., tests defining the :attr:`perf_variables <reframe.core.pipeline.RegressionTest.perf_variables>` attribute.
+Performance log messages are generated *only* for `performance tests <tutorial_basics.html#writing-a-performance-test>`__, i.e., tests defining the :attr:`~reframe.core.pipeline.RegressionTest.perf_variables` or the :attr:`~reframe.core.pipeline.RegressionTest.perf_patterns` attributes.
 The performance log handlers are stored in the ``handlers_perflog`` property.
 The ``filelog`` handler used in this example will create a file per test and per system/partition combination (``./<system>/<partition>/<testname>.log``) and will append to it the obtained performance data every time a performance test is run.
 Notice how the message to be logged is structured in the ``format`` property, such that it can be easily parsed from post processing tools.

docs/tutorial_advanced.rst

@@ -161,7 +161,7 @@ Let's inspect the build script generated by ReFrame:
 
    trap _onerror ERR
 
-   make -j 1 CPPFLAGS="-DELEM_TYPE=float"
+   make -j 1 CPPFLAGS="-DELEM_TYPE=float" CC=cc CXX=CC
 
 
 The compiler variables (``CC``, ``CXX`` etc.) are set based on the corresponding values specified in the `configuration <config_reference.html#environment-configuration>`__ of the current environment.
@@ -175,7 +175,7 @@ In this case, ``make`` will be invoked as follows:
 
 .. code::
 
-  make -j 1 CPPFLAGS="-DELEM_TYPE=float"
+  make -j 1 CPPFLAGS="-DELEM_TYPE=float" CC=cc CXX=CC
 
 Notice that the ``-j 1`` option is always generated.
 We can increase the build concurrency by setting the :attr:`~reframe.core.buildsystems.Make.max_concurrency` attribute.
@@ -681,8 +681,8 @@ The test will verify that all the nodes print the expected host name:
    :emphasize-lines: 10-
 
 The first thing to notice in this test is that :attr:`~reframe.core.pipeline.RegressionTest.num_tasks` is set to zero as default, which is a requirement for flexible tests.
-However, this value is set to the actual number of tasks during the ``run`` pipeline stage.
-Lastly, the sanity check of this test counts the host names printed and verifies that the total count equals :attr:`~reframe.core.pipeline.RegressionTest.num_tasks`.
+However, with flexible tests, this value is updated right after the job completes to the actual number of tasks that were used.
+Consequenly, this allows the sanity function of the tests to assert that the number host names printed matches :attr:`~reframe.core.pipeline.RegressionTest.num_tasks`.
 
 .. |--flex-alloc-nodes| replace:: :attr:`--flex-alloc-nodes`
 .. _--flex-alloc-nodes: manpage.html#cmdoption-flex-alloc-nodes

docs/tutorial_basics.rst

@@ -664,11 +664,13 @@ For running the benchmark, we need to set the OpenMP number of threads and pin t
 You can set environment variables in a ReFrame test through the :attr:`~reframe.core.pipeline.RegressionTest.variables` dictionary.
 
 What makes a ReFrame test a performance test is the definition of at least one :ref:`performance function<deferrable-performance-functions>`.
-Similarly to a test's :func:`@sanity_function<reframe.core.pipeline.RegressionMixin.sanity_function>`, a performance function is simply a member function decorated with the :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator, which is responsible for extracting a specified performance quantity from a regression test.
-The :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator must be passed the units of the quantity to be extracted, and it also takes the optional argument ``perf_key`` to customize the name of the extracted performance variable.
-If ``perf_key`` is not provided, the performance variable will take the name of the decorated performance function.
+Similarly to a test's :func:`@sanity_function<reframe.core.pipeline.RegressionMixin.sanity_function>`, a performance function is a member function decorated with the :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator, which binds the decorated function to a given unit.
+These functions can be used by the regression test to extract, measure or compute a given quantity of interest; where in this context, the values returned by a performance function are referred to as performance variables.
+Alternatively, performance functions can also be thought as `tools` available to the regression test for extracting performance variables.
+By default, ReFrame will attempt to execute all the available performance functions during the test's ``performance`` stage, producing a single performance variable out of each of the available performance functions.
+These default-generated performance variables are defined in the regression test's attribute :attr:`~reframe.core.pipeline.RegressionTest.perf_variables` during class instantiation, and their default name matches the name of their associated performance function.
+However, one could customize the default-generated performance variable's name by passing the ``perf-key`` argument to the :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator of the associated performance function.
 
-ReFrame identifies all member functions that use the :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator, and will automatically schedule them for execution during the ``performance`` pipeline stage of the test.
 In this example, we extract four performance variables, namely the memory bandwidth values for each of the "Copy", "Scale", "Add" and "Triad" sub-benchmarks of STREAM, where each of the performance functions use the :func:`~reframe.utility.sanity.extractsingle` utility function.
 For each of the sub-benchmarks we extract the "Best Rate MB/s" column of the output (see below) and we convert that to a float.
 
@@ -731,6 +733,45 @@ The :option:`--performance-report` will generate a short report at the end for e
    Log file(s) saved in: '/var/folders/h7/k7cgrdl13r996m4dmsvjq7v80000gp/T/rfm-gczplnic.log'
 
 
+---------------------------------------------------
+Setting explicitly the test's performance variables
+---------------------------------------------------
+
+In the above STREAM example, all four performance functions were almost identical except for a small part of the regex pattern, which led to some code repetition.
+Even though the performance functions were rather simple and the code repetition was not much in that case, this is still not a good practice and it is certainly an approach that would not scale when using more complex performance functions.
+Hence, in this example, we show how to collapse all these four performance functions into a single function and how to reuse this single performance function to create multiple performance variables.
+
+.. code-block:: console
+
+   cat tutorials/basics/stream/stream2.py
+
+.. literalinclude:: ../tutorials/basics/stream/stream2.py
+   :lines: 6-
+   :emphasize-lines: 28-
+
+As shown in the highlighted lines, this example collapses the four performance functions from the previous example into the :func:`extract_bw` function, which is also decorated with the :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator with the units set to ``'MB/s'``.
+However, the :func:`extract_bw` function now takes the optional argument ``kind`` which selects the STREAM benchmark to extract.
+By default, this argument is set to ``'Copy'`` because functions decorated with :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` are only allowed to have ``self`` as a non-default argument.
+Thus, from this performance function definition, ReFrame will default-generate a single performance variable during the test instantiation under the name ``extract_bw``, where this variable will report the performance results from the ``Copy`` benchmark.
+With no further action from our side, ReFrame would just report the performance of the test based on this default-generated performance variable, but that is not what we are after here.
+Therefore, we must modify these default performance variables so that this version of the STREAM test produces the same results as in the previous example.
+As mentioned before, the performance variables (also the default-generated ones) are stored in the :attr:`~reframe.core.pipeline.RegressionTest.perf_variables` dictionary, so all we need to do is to redefine this mapping with our desired performance variables as done in the pre-performance pipeline hook :func:`set_perf_variables`.
+
+.. tip::
+   Performance functions may also be generated inline using the :func:`~reframe.utility.sanity.make_performance_function` utility as shown below.
+
+   .. code-block:: python
+
+      @run_before('performance')
+      def set_perf_vars(self):
+          self.perf_variables = {
+              'Copy': sn.make_performance_function(
+                  sn.extractsingle(r'Copy:\s+(\S+)\s+.*',
+                                   self.stdout, 1, float),
+                  'MB/s'
+               )
+          }
+
 -----------------------
 Adding reference values
 -----------------------
@@ -747,22 +788,23 @@ In the following example, we set the reference values for all the STREAM sub-ben
 
 .. code-block:: console
 
-   cat tutorials/basics/stream/stream2.py
+   cat tutorials/basics/stream/stream3.py
 
 
-.. literalinclude:: ../tutorials/basics/stream/stream2.py
+.. literalinclude:: ../tutorials/basics/stream/stream3.py
    :lines: 6-
    :emphasize-lines: 18-25
 
 
 The performance reference tuple consists of the reference value, the lower and upper thresholds expressed as fractional numbers relative to the reference value, and the unit of measurement.
 If any of the thresholds is not relevant, :class:`None` may be used instead.
+Also, the units in this :attr:`~reframe.core.pipeline.RegressionTest.reference` variable are entirely optional, since they were already provided through the :attr:`@performance_function<reframe.core.pipeline.RegressionMixin.performance_function>` decorator.
 
 If any obtained performance value is beyond its respective thresholds, the test will fail with a summary as shown below:
 
 .. code-block:: console
 
-   ./bin/reframe -c tutorials/basics/stream/stream2.py -r --performance-report
+   ./bin/reframe -c tutorials/basics/stream/stream3.py -r --performance-report
 
 
 .. code-block:: none
@@ -779,30 +821,6 @@ If any obtained performance value is beyond its respective thresholds, the test
      * Rerun with '-n StreamWithRefTest -p gnu --system catalina:default'
      * Reason: performance error: failed to meet reference: Copy=24586.5, expected 55200 (l=52440.0, u=57960.0)
 
-Also, note how the performance syntax for this example is far more compact in comparison to our first iteration of the STREAM test.
-In that first STREAM example, all four performance functions were almost identical, except for a small part of the regex pattern, which led to a lot of code repetition.
-Hence, this example collapses all four performance functions into a single performance function, which now takes an optional argument to select the quantity to extract.
-Then, the performance variables of the test can be defined by setting the respective entries in the :attr:`~reframe.core.pipeline.RegressionTest.perf_variables` dictionary.
-
-.. literalinclude:: ../tutorials/basics/stream/stream2.py
-   :lines: 41-
-
-.. note::
-   Performance functions may also be generated inline using the :func:`~reframe.utility.sanity.make_performance_function` utility as shown below.
-
-   .. code-block:: python
-
-      @run_before('performance')
-      def set_perf_vars(self):
-          self.perf_variables = {
-              'Copy': sn.make_performance_function(
-                  sn.extractsingle(r'Copy:\s+(\S+)\s+.*',
-                                   self.stdout, 1, float),
-                  'MB/s'
-               )
-          }
-
-
 ------------------------------
 Examining the performance logs
 ------------------------------
@@ -1122,9 +1140,9 @@ Let's see and comment the changes:
 
 .. code-block:: console
 
-   cat tutorials/basics/stream/stream3.py
+   cat tutorials/basics/stream/stream4.py
 
-.. literalinclude:: ../tutorials/basics/stream/stream3.py
+.. literalinclude:: ../tutorials/basics/stream/stream4.py
    :lines: 6-
    :emphasize-lines: 8, 27-41, 46-56
 
@@ -1155,18 +1173,18 @@ Let's run our adapted test now:
 
 .. code-block:: console
 
-   ./bin/reframe -c tutorials/basics/stream/stream3.py -r --performance-report
+   ./bin/reframe -c tutorials/basics/stream/stream4.py -r --performance-report
 
 
 .. code-block:: none
 
    [ReFrame Setup]
      version:           3.3-dev0 (rev: cb974c13)
-     command:           './bin/reframe -C tutorials/config/settings.py -c tutorials/basics/stream/stream3.py -r --performance-report'
+     command:           './bin/reframe -C tutorials/config/settings.py -c tutorials/basics/stream/stream4.py -r --performance-report'
      launched by:       user@dom101
      working directory: '/users/user/Devel/reframe'
      settings file:     'tutorials/config/settings.py'
-     check search path: '/users/user/Devel/reframe/tutorials/basics/stream/stream3.py'
+     check search path: '/users/user/Devel/reframe/tutorials/basics/stream/stream4.py'
      stage directory:   '/users/user/Devel/reframe/stage'
      output directory:  '/users/user/Devel/reframe/output'
 

docs/tutorial_tips_tricks.rst

@@ -90,7 +90,7 @@ As suggested by the warning message, passing :option:`-v` will give you the stac
 Debugging deferred expressions
 ==============================
 
-Although deferred expression that are used in sanity and performance functions behave similarly to normal Python expressions, you need to understand their `implicit evaluation rules <deferrable_functions_reference.html#implicit-evaluation-of-sanity-functions>`__.
+Although deferred expressions that are used in sanity and performance functions behave similarly to normal Python expressions, you need to understand their `implicit evaluation rules <deferrable_functions_reference.html#implicit-evaluation-of-sanity-functions>`__.
 One of the rules is that :func:`str` triggers the implicit evaluation, so trying to use the standard :func:`print` function with a deferred expression, you might get unexpected results if that expression is not yet to be evaluated.
 For this reason, ReFrame offers a sanity function counterpart of :func:`print`, which allows you to safely print deferred expressions.
 

tutorials/basics/hellomp/hellomp2.py

@@ -24,6 +24,6 @@ def set_compilation_flags(self):
 
     @sanity_function
     def assert_num_messages(self):
-        num_messages = len(sn.findall(r'\[\s?\d+\] Hello, World\!',
-                                      self.stdout).evaluate())
-        return num_messages == 16
+        num_messages = sn.len(sn.findall(r'\[\s?\d+\] Hello, World\!',
+                                         self.stdout))
+        return sn.assert_eq(num_messages, 16)

tutorials/basics/hellomp/hellomp3.py

@@ -25,6 +25,6 @@ def set_compilation_flags(self):
 
     @sanity_function
     def assert_num_messages(self):
-        num_messages = len(sn.findall(r'\[\s?\d+\] Hello, World\!',
-                                      self.stdout).evaluate())
-        return num_messages == 16
+        num_messages = sn.len(sn.findall(r'\[\s?\d+\] Hello, World\!',
+                                         self.stdout))
+        return sn.assert_eq(num_messages, 16)

tutorials/basics/stream/stream2.py

@@ -20,14 +20,6 @@ class StreamWithRefTest(rfm.RegressionTest):
         'OMP_NUM_THREADS': '4',
         'OMP_PLACES': 'cores'
     }
-    reference = {
-        'catalina': {
-            'Copy':  (25200, -0.05, 0.05, 'MB/s'),
-            'Scale': (16800, -0.05, 0.05, 'MB/s'),
-            'Add':   (18500, -0.05, 0.05, 'MB/s'),
-            'Triad': (18800, -0.05, 0.05, 'MB/s')
-        }
-    }
 
     @run_before('compile')
     def set_compiler_flags(self):
@@ -41,7 +33,8 @@ def validate_solution(self):
     @performance_function('MB/s')
     def extract_bw(self, kind='Copy'):
         '''Generic performance extraction function.'''
-        if kind not in {'Copy', 'Scale', 'Add', 'Triad'}:
+
+        if kind not in ('Copy', 'Scale', 'Add', 'Triad'):
             raise ValueError(f'illegal value in argument kind ({kind!r})')
 
         return sn.extractsingle(rf'{kind}:\s+(\S+)\s+.*',
@@ -50,6 +43,7 @@ def extract_bw(self, kind='Copy'):
     @run_before('performance')
     def set_perf_variables(self):
         '''Build the dictionary with all the performance variables.'''
+
         self.perf_variables = {
             'Copy': self.extract_bw(),
             'Scale': self.extract_bw('Scale'),

tutorials/basics/stream/stream3.py

@@ -8,9 +8,9 @@
 
 
 @rfm.simple_test
-class StreamMultiSysTest(rfm.RegressionTest):
+class StreamWithRefTest(rfm.RegressionTest):
     valid_systems = ['*']
-    valid_prog_environs = ['cray', 'gnu', 'intel', 'pgi']
+    valid_prog_environs = ['gnu']
     prebuild_cmds = [
         'wget http://www.cs.virginia.edu/stream/FTP/Code/stream.c',
     ]
@@ -29,51 +29,29 @@ class StreamMultiSysTest(rfm.RegressionTest):
         }
     }
 
-    # Flags per programming environment
-    flags = variable(dict, value={
-        'cray':  ['-fopenmp', '-O3', '-Wall'],
-        'gnu':   ['-fopenmp', '-O3', '-Wall'],
-        'intel': ['-qopenmp', '-O3', '-Wall'],
-        'pgi':   ['-mp', '-O3']
-    })
-
-    # Number of cores for each system
-    cores = variable(dict, value={
-        'catalina:default': 4,
-        'daint:gpu': 12,
-        'daint:mc': 36,
-        'daint:login': 10
-    })
-
     @run_before('compile')
     def set_compiler_flags(self):
         self.build_system.cppflags = ['-DSTREAM_ARRAY_SIZE=$((1 << 25))']
-        environ = self.current_environ.name
-        self.build_system.cflags = self.flags.get(environ, [])
-
-    @run_before('run')
-    def set_num_threads(self):
-        num_threads = self.cores.get(self.current_partition.fullname, 1)
-        self.num_cpus_per_task = num_threads
-        self.variables = {
-            'OMP_NUM_THREADS': str(num_threads),
-            'OMP_PLACES': 'cores'
-        }
+        self.build_system.cflags = ['-fopenmp', '-O3', '-Wall']
 
     @sanity_function
     def validate_solution(self):
         return sn.assert_found(r'Solution Validates', self.stdout)
 
     @performance_function('MB/s')
     def extract_bw(self, kind='Copy'):
-        if kind not in {'Copy', 'Scale', 'Add', 'Triad'}:
+        '''Generic performance extraction function.'''
+
+        if kind not in ('Copy', 'Scale', 'Add', 'Triad'):
             raise ValueError(f'illegal value in argument kind ({kind!r})')
 
         return sn.extractsingle(rf'{kind}:\s+(\S+)\s+.*',
                                 self.stdout, 1, float)
 
     @run_before('performance')
     def set_perf_variables(self):
+        '''Build the dictionary with all the performance variables.'''
+
         self.perf_variables = {
             'Copy': self.extract_bw(),
             'Scale': self.extract_bw('Scale'),