[feat] New performance syntax

[jjotero]

This PR introduces a new syntax to capture and report performance data from a reframe check. In order to facilitate writing generic and composable tests, the reference variable is now entirely optional and each performance variable is extracted with a member function decorated with the performance_function decorator:

import reframe as rfm
import reframe.utility.sanity as sn
import re

@rfm.simple_test
class Base(rfm.RunOnlyRegressionTest):
    valid_systems = ['dom:login']
    valid_prog_environs = ['PrgEnv-gnu']
    executable = 'echo 40 bananas'
    v = variable(str)

    @sanity_function
    def assert_bananas(self): 
        return sn.assert_found(r'bananas', self.stdout)
         
    @performance_function('bananas')
    def perf0(self):
        return sn.extractsingle(r'(\d+)\s*bananas', self.stdout, 1, int)

    @performance_function('bananas', perf_key='b-1')
    def perf1(self, blah=222):
        f = open(self.stdout.evaluate(), 'r')
        stdout = ''.join(f.readlines())
        f.close()
        return int(re.match(r'(\d+)\s*bananas', stdout)[1])

Reframe will pickup these functions marked as @performance_functions and run them all by default:

==============================================================================
PERFORMANCE REPORT
------------------------------------------------------------------------------
Base
- dom:login
   - PrgEnv-gnu
      * num_tasks: 1
      * perf0: 40 bananas
      * b-1: 40 bananas
------------------------------------------------------------------------------

Note that the units are attached to the decorated function, and the name of the resulting performance variable can also be customised with the perf_key decorator argument. If any decorated performance function has more than one argument without a default value (the self placeholder), an error is raised:

def foo(x, y=10):
    # cool

def bar(x, y):
   # not cool

So now, what if someone derives a test and wants to change the performance variables from the base test? We have the perf_variables attribute for that. During class instantiation, ReFrame will initialise perf_variables with all the functions decorated with the performance_function decorator. However, if one wants to modify this mapping, one can do so in a similarly to when dealing with the old perf_patterns as:

@rfm.simple_test
class Derived(Base):
    @run_before('performance')
    def my_report(self):
        l = lambda self: sn.extractsingle(r'(\d+)\s*bananas', self.stdout, 1, int)
        self.perf_variables = {
                'a': self.perf0(),
                'b': sn.make_performance_function(l, 'more_bananas', self),
                'c': sn.make_performance_function(
                    sn.extractsingle(r'(\d+)\s*bananas', self.stdout, 1, int),
                    'other_bananas'
                )
        }

With this syntax, the functions that go into the perf_variables dictionary are guaranteed to be suitable performance functions, otherwise an error is raised. Note the addition of the sn.make_performance_function method, which allows to add more performance functions inline (it works with both a generic callable and a deferred expression). The performance report for this Derived test looks as follows:

==============================================================================
PERFORMANCE REPORT
------------------------------------------------------------------------------
Base
- dom:login
   - PrgEnv-gnu
      * num_tasks: 1
      * a: 40 bananas
      * b: 40 more_bananas
      * c: 40 other_bananas
------------------------------------------------------------------------------

Closes #2064
Closes #2113

[pep8speaks]

Hello @jjotero, Thank you for updating!

• In the file reframe/core/parameters.py:

Line 56:17: E123 closing bracket does not match indentation of opening bracket's line

Do see the ReFrame Coding Style Guide

Comment last updated at 2021-08-23 11:48:31 UTC

[codecov-commenter]

Codecov Report

Merging #2083 (06813e9) into master (7364e80) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2083      +/-   ##
==========================================
+ Coverage   86.00%   86.15%   +0.15%     
==========================================
  Files          53       53              
  Lines        9368     9465      +97     
==========================================
+ Hits         8057     8155      +98     
+ Misses       1311     1310       -1     

Impacted Files	Coverage Δ
reframe/core/deferrable.py	98.21% <100.00%> (+0.16%)	⬆️
reframe/core/hooks.py	89.77% <100.00%> (-0.45%)	⬇️
reframe/core/meta.py	99.11% <100.00%> (+0.19%)	⬆️
reframe/core/namespaces.py	96.72% <100.00%> (+1.63%)	⬆️
reframe/core/parameters.py	100.00% <100.00%> (ø)
reframe/core/pipeline.py	92.24% <100.00%> (+0.34%)	⬆️
reframe/frontend/statistics.py	95.95% <100.00%> (ø)
reframe/utility/__init__.py	92.87% <100.00%> (+0.05%)	⬆️
reframe/utility/sanity.py	97.81% <100.00%> (+0.02%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7364e80...06813e9. Read the comment docs.

[victorusu]

@jjotero, I really loved this PR! This syntax is very clear, extensible, and very powerful.
But I have a question why is the make_performance_function a regression test method? Would it make sense to have it as an independent module that can be used by regression classes? I think that the module approach is more "modular".
And last but not least, I loved, really loved the bananas examples! I do not know if the b-1 example was intentional, but I missed a b-2!

[jjotero]

@victorusu I was not aware of that show! 😂
self.make_performance_function is implemented as a wrapper of the @performance_function decorator, which is not available from the instance. All the other directives are dropped from the instance, but I thought this one made sense just to allow the inlining shown in the examples above. Where else are you thinking this make_performance_function method would be used if not in a regression test instance?

[jjotero]

I have introduced the variable perf_variables of type typ.Dict[str, _DeferredPerformanceExpression]. A _DeferredPerformanceExpression is derived from the _DeferredExpression class with the addition that it has a member unit holding the performance units of each performance variable.

As a result of this internal reorg, I have moved the make_performance_function utility into the pipeline. This function is basically an external convert-constructor for the _DeferredPerformanceExpression class that can take either a callable with its arguments and the units, or just a deferred expression and the units. This could be integrated somehow into the typed descriptor, and it probably makes sense to think of this also in relation to #563 (which also needs some type of conversion function).

[jjotero]

I guess that someone might want to enforce some function to take 2 or more positional arguments without a default value. We don't have such thing in the code right now, but since this is in the utilities, I thought I'd make sense to make it generic.

[jjotero]

I've added the unit tests for all this new syntax. Also, I've moved the make_performance_function method from the pipeline into the utility/sanity module, since this method can be used with a free function. Now, since this make_performance_function returns a _DeferredPerformanceExpression, I think we should be consistent on how we name this function and all the other deferrable and so on. I'll try to think of a better name for this.

[vkarak]

I have the feeling that here we can throw a TypeError and the error would still be traced back to the right user line. Similarly for anything in the performance_function decorator.

[jjotero]

Changed to TypeError, but now I'm confused 😅. I thought the plan was to raise ReframeSyntaxError for everything that we impose and control.

[jjotero]

I also added a few other checks in here to ensure that the inheritance of performance functions adheres to python's MRO.

[vkarak]

Changed to TypeError, but now I'm confused 😅. I thought the plan was to raise ReframeSyntaxError for everything that we impose and control.

If a programming error can be traced back to the user frame that triggered it, then ReFrame can avoid a stack trace automatically. In this case, it can be traced because it's a direct call chain from the decorator call. The problem has been with the variables and parameters, where we do several checks at a later point. In these cases, the user line information is lost and we can't trace back the error to the user code; the stack trace has no user frame. So to avoid printing a stack trace for such user errors, we invented the ReframeSyntaxError. That's the rule of thumb, I hope it makes sense...

[jjotero]

Ah! Got it! Yeah, that makes total sense. Should we move this explanation somewhere in the docs? I think it would be useful.

[vkarak]

I don't know where would be the right place to be honest, because that's mostly a developer's documentation, isn't it?

[vkarak]

Changed to TypeError, but now I'm confused 😅. I thought the plan was to raise ReframeSyntaxError for everything that we impose and control.

If a programming error can be traced back to the user frame that triggered it, then ReFrame can avoid a stack trace automatically. In this case, it can be traced because it's a direct call chain from the decorator call. The problem has been with the variables and parameters, where we do several checks at a later point. In these cases, the user line information is lost and we can't trace back the error to the user code; the stack trace has no user frame. So to avoid printing a stack trace for such user errors, we invented the ReframeSyntaxError. That's the rule of thumb, I hope it makes sense...

[vkarak]

Looks good! Just minor corrections and some fine tuning to the docs. I could also take care of some of those.

[vkarak]

Latm! Goes in! I just made a bit of fine tuning of the docs.