diff --git a/cscs-checks/microbenchmarks/gpu/gpu_burn/gpu_burn_test.py b/cscs-checks/microbenchmarks/gpu/gpu_burn/gpu_burn_test.py index ac0457fd83..3b63fea20f 100644 --- a/cscs-checks/microbenchmarks/gpu/gpu_burn/gpu_burn_test.py +++ b/cscs-checks/microbenchmarks/gpu/gpu_burn/gpu_burn_test.py @@ -3,10 +3,11 @@ # # SPDX-License-Identifier: BSD-3-Clause -import os import reframe as rfm import reframe.utility.sanity as sn +import reframe.utility.osext as osext +from reframe.core.exceptions import SanityError from hpctestlib.microbenchmarks.gpu.gpu_burn import GpuBurn import cscstests.microbenchmarks.gpu.hooks as hooks @@ -24,30 +25,29 @@ class gpu_burn_check(GpuBurn): num_tasks = 0 reference = { 'dom:gpu': { - 'perf': (4115, -0.10, None, 'Gflop/s'), + 'min_perf': (4115, -0.10, None, 'Gflop/s'), }, 'daint:gpu': { - 'perf': (4115, -0.10, None, 'Gflop/s'), + 'min_perf': (4115, -0.10, None, 'Gflop/s'), }, 'arolla:cn': { - 'perf': (5861, -0.10, None, 'Gflop/s'), + 'min_perf': (5861, -0.10, None, 'Gflop/s'), }, 'tsa:cn': { - 'perf': (5861, -0.10, None, 'Gflop/s'), + 'min_perf': (5861, -0.10, None, 'Gflop/s'), }, 'ault:amda100': { - 'perf': (15000, -0.10, None, 'Gflop/s'), + 'min_perf': (15000, -0.10, None, 'Gflop/s'), }, 'ault:amdv100': { - 'perf': (5500, -0.10, None, 'Gflop/s'), + 'min_perf': (5500, -0.10, None, 'Gflop/s'), }, 'ault:intelv100': { - 'perf': (5500, -0.10, None, 'Gflop/s'), + 'min_perf': (5500, -0.10, None, 'Gflop/s'), }, 'ault:amdvega': { - 'perf': (3450, -0.10, None, 'Gflop/s'), + 'min_perf': (3450, -0.10, None, 'Gflop/s'), }, - '*': {'temp': (0, None, None, 'degC')} } maintainers = ['AJ', 'TM'] @@ -63,16 +63,25 @@ def set_num_gpus_per_node(self): hooks.set_num_gpus_per_node(self) @run_before('performance') - def report_nid_with_smallest_flops(self): - regex = r'\[(\S+)\] GPU\s+\d\(OK\): (\d+) GF/s' - rptf = os.path.join(self.stagedir, sn.evaluate(self.stdout)) - self.nids = sn.extractall(regex, rptf, 1) - self.flops = sn.extractall(regex, rptf, 2, float) + def report_slow_nodes(self): + '''Report the base perf metrics and also all the slow nodes.''' + + # Only report the nodes that don't meet the perf reference + with osext.change_dir(self.stagedir): + key = f'{self.current_partition.fullname}:min_perf' + if key in self.reference: + regex = r'\[(\S+)\] GPU\s+\d\(OK\): (\d+) GF/s' + nids = set(sn.extractall(regex, self.stdout, 1)) + + # Get the references + ref, lt, ut, *_ = self.reference[key] + + # Flag the slow nodes + for nid in nids: + try: + node_perf = self.min_perf(nid) + val = node_perf.evaluate(cache=True) + sn.assert_reference(val, ref, lt, ut).evaluate() + except SanityError: + self.perf_variables[nid] = node_perf - # Find index of smallest flops and update reference dictionary to - # include our patched units - index = self.flops.evaluate().index(min(self.flops)) - unit = f'GF/s ({self.nids[index]})' - for key, ref in self.reference.items(): - if not key.endswith(':temp'): - self.reference[key] = (*ref[:3], unit) diff --git a/cscs-checks/system/jobreport/gpu_report.py b/cscs-checks/system/jobreport/gpu_report.py index 5967ff59eb..0205ec7bf3 100644 --- a/cscs-checks/system/jobreport/gpu_report.py +++ b/cscs-checks/system/jobreport/gpu_report.py @@ -93,8 +93,12 @@ def gpu_usage_sanity(self): sn.assert_ge(sn.min(time_reported), self.burn_time) ]) + @performance_function('nodes') + def total_nodes_reported(self): + return sn.count(self.nodes_reported) + @run_before('performance') - def set_perf_patterns(self): + def set_perf_variables(self): '''The number of reported nodes can be used as a perf metric. For now, the low limit can go to zero, but this can be set to a more @@ -103,9 +107,9 @@ def set_perf_patterns(self): self.reference = { '*': { - 'nodes_reported': (self.num_tasks, self.perf_floor, 0, 'nodes') + 'nodes_reported': (self.num_tasks, self.perf_floor, 0) }, } - self.perf_patterns = { - 'nodes_reported': sn.count(self.nodes_reported) + self.perf_variables = { + 'nodes_reported': self.total_nodes_reported() } diff --git a/docs/deferrable_functions_reference.rst b/docs/deferrable_functions_reference.rst index 996820b0fe..1dab287888 100644 --- a/docs/deferrable_functions_reference.rst +++ b/docs/deferrable_functions_reference.rst @@ -7,10 +7,20 @@ Deferrable Functions Reference *Deferrable functions* are the functions whose execution may be postponed to a later time after they are called. The key characteristic of these functions is that they store their arguments when they are called, and the execution itself does not occur until the function is evaluated either explicitly or implicitly. +ReFrame provides an ample set of deferrable utilities and it also allows users to write their own deferrable functions when needed. +Please refer to ":doc:`deferrables`" for a hands-on explanation on how deferrable functions work and how to create custom deferrable functions. + + Explicit evaluation of deferrable functions ------------------------------------------- Deferrable functions may be evaluated at any time by calling :func:`evaluate` on their return value or by passing the deferred function itself to the :func:`~reframe.utility.sanity.evaluate()` free function. +These :func:`evaluate` functions take an optional :class:`bool` argument ``cache``, which can be used to cache the evaluation of the deferrable function. +Hence, if caching is enabled on a given deferrable function, any subsequent calls to :func:`evaluate` will simply return the previously cached results. + +.. versionchanged:: 3.8.0 + Support of cached evaluation is added. + Implicit evaluation of deferrable functions ------------------------------------------- @@ -48,9 +58,24 @@ Currently ReFrame provides three broad categories of deferrable functions: They include, but are not limited to, functions to iterate over regex matches in a file, extracting and converting values from regex matches, computing statistical information on series of data etc. -Users can write their own deferrable functions as well. -The page ":doc:`deferrables`" explains in detail how deferrable functions work and how users can write their own. + .. _deferrable-performance-functions: + + +-------------------------------- +Deferrable performance functions +-------------------------------- + +.. versionadded:: 3.8.0 + +Deferrable performance functions are a special type of deferrable functions which are intended for measuring a given quantity. +Therefore, this kind of deferrable functions have an associated unit that can be used to interpret the return values from these functions. +The unit of a deferrable performance function can be accessed through the public member :attr:`unit`. +Regular deferrable functions can be promoted to deferrable performance functions using the :func:`~reframe.utility.sanity.make_performance_function` utility. +Also, this utility allows to create performance functions directly from any callable. + +List of deferrable functions and utilities +------------------------------------------ .. py:decorator:: reframe.utility.sanity.deferrable(func) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 135e752632..2390e3954a 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -322,6 +322,21 @@ Built-in functions .. versionadded:: 3.7.0 +.. py:decorator:: RegressionMixin.performance_function(unit, *, perf_key=None) + + Decorate a member function as a performance function of the test. + + This decorator converts the decorated method into a performance deferrable function (see ":ref:`deferrable-performance-functions`" for more details) whose evaluation is deferred to the performance stage of the regression test. + The decorated function must take a single argument without a default value (i.e. ``self``) and any number of arguments with default values. + A test may decorate multiple member functions as performance functions, where each of the decorated functions must be provided with the units of the performance quantitites to be extracted from the test. + These performance units must be of type :class:`str`. + Any performance function may be overridden in a derived class and multiple bases may define their own performance functions. + In the event of a name conflict, the derived class will follow Python's `MRO `_ to choose the appropriate performance function. + However, defining more than one performance function with the same name in the same class is disallowed. + + The full set of performance functions of a regression test is stored under :attr:`~reframe.core.pipeline.RegressionTest.perf_variables` as key-value pairs, where, by default, the key is the name of the decorated member function, and the value is the deferred performance function itself. + Optionally, the key under which a performance function is stored in :attr:`~reframe.core.pipeline.RegressionTest.perf_variables` can be customised by passing the desired key as the ``perf_key`` argument to this decorator. + .. py:decorator:: RegressionMixin.deferrable(func) Converts the decorated method into a deferrable function. diff --git a/hpctestlib/microbenchmarks/gpu/gpu_burn/__init__.py b/hpctestlib/microbenchmarks/gpu/gpu_burn/__init__.py index 6732ffa012..1844540851 100644 --- a/hpctestlib/microbenchmarks/gpu/gpu_burn/__init__.py +++ b/hpctestlib/microbenchmarks/gpu/gpu_burn/__init__.py @@ -51,12 +51,6 @@ class GpuBurn(rfm.RegressionTest, pin_prefix=True): build_system = 'Make' executable = './gpu_burn.x' num_tasks_per_node = 1 - reference = { - '*': { - 'perf': (0, None, None, 'Gflop/s'), - 'temp': (0, None, None, 'degC') - } - } @run_before('compile') def set_gpu_build(self): @@ -83,7 +77,6 @@ def set_gpu_build(self): raise ValueError('unknown gpu_build option') @property - @deferrable def num_tasks_assigned(self): '''Total number of times the gpu burn will run. @@ -103,17 +96,27 @@ def count_successful_burns(self): r'^\s*\[[^\]]*\]\s*GPU\s*\d+\(OK\)', self.stdout) ), self.num_tasks_assigned) - @run_before('performance') - def set_perf_patterns(self): - '''Extract the minimum performance and maximum temperature recorded. + def _extract_perf_metric(self, metric, nid=None): + '''Utility to extract performance metrics.''' - The performance and temperature data are reported in Gflops/s and - deg. Celsius respectively. - ''' + if metric not in {'perf', 'temp'}: + raise ValueError( + f"unsupported value in 'metric' argument: {metric!r}" + ) + + if nid is None: + nid = r'[^\]]*' + + patt = (rf'^\s*\[{nid}\]\s*GPU\s+\d+\(\S*\):\s+(?P\S*)\s+GF\/s' + rf'\s+(?P\S*)\s+Celsius') + return sn.extractall(patt, self.stdout, metric, float) + + @performance_function('Gflop/s') + def min_perf(self, nid=None): + '''Lowest performance recorded.''' + return sn.min(self._extract_perf_metric('perf', nid)) - patt = (r'^\s*\[[^\]]*\]\s*GPU\s+\d+\(\S*\):\s+(?P\S*)\s+GF\/s' - r'\s+(?P\S*)\s+Celsius') - self.perf_patterns = { - 'perf': sn.min(sn.extractall(patt, self.stdout, 'perf', float)), - 'temp': sn.max(sn.extractall(patt, self.stdout, 'temp', float)), - } + @performance_function('degC') + def max_temp(self, nid=None): + '''Maximum temperature recorded.''' + return sn.max(self._extract_perf_metric('temp', nid)) diff --git a/reframe/core/deferrable.py b/reframe/core/deferrable.py index 1014d6bb7b..b81bd9d823 100644 --- a/reframe/core/deferrable.py +++ b/reframe/core/deferrable.py @@ -44,31 +44,36 @@ def __init__(self, fn, *args, **kwargs): # We cache the value of the last evaluation inside a tuple. # We don't cache the value directly, because it can be any. - - # NOTE: The cache for the moment is only used by - # `__rfm_json_encode__`. Enabling caching in the evaluation is a - # reasonable optimization, but might break compatibility, so it needs - # to be thought thoroughly and communicated properly in the - # documentation. self._cached = () + self._return_cached = False + + def evaluate(self, cache=False): + # Return the cached value (if any) + if self._return_cached and not cache: + return self._cached[0] + elif cache: + self._return_cached = cache - def evaluate(self): fn_args = [] for arg in self._args: fn_args.append( - arg.evaluate() if isinstance(arg, type(self)) else arg + arg.evaluate() if isinstance(arg, _DeferredExpression) else arg ) fn_kwargs = {} for k, v in self._kwargs.items(): fn_kwargs[k] = ( - v.evaluate() if isinstance(v, type(self)) else v + v.evaluate() if isinstance(v, _DeferredExpression) else v ) ret = self._fn(*fn_args, **fn_kwargs) - if isinstance(ret, type(self)): + + # Evaluate the return for as long as a deferred expression returns + # another deferred expression. + while isinstance(ret, _DeferredExpression): ret = ret.evaluate() + # Cache the results for any subsequent evaluate calls. self._cached = (ret,) return ret @@ -355,3 +360,34 @@ def __abs__(a): @deferrable def __invert__(a): return ~a + + +class _DeferredPerformanceExpression(_DeferredExpression): + '''Represents a performance function whose evaluation has been deferred. + + It extends the :class:`_DeferredExpression` class by adding the ``unit`` + attribute. This attribute represents the unit of the performance + metric to be extracted by the performance function. + ''' + + def __init__(self, fn, unit, *args, **kwargs): + super().__init__(fn, *args, **kwargs) + + if not isinstance(unit, str): + raise TypeError( + 'performance units must be a string' + ) + + self._unit = unit + + @classmethod + def construct_from_deferred_expr(cls, expr, unit): + if not isinstance(expr, _DeferredExpression): + raise TypeError("'expr' argument is not an instance of the " + "_DeferredExpression class") + + return cls(expr._fn, unit, *(expr._args), **(expr._kwargs)) + + @property + def unit(self): + return self._unit diff --git a/reframe/core/hooks.py b/reframe/core/hooks.py index e87db4d0f1..99aef67809 100644 --- a/reframe/core/hooks.py +++ b/reframe/core/hooks.py @@ -128,25 +128,6 @@ def __repr__(self): class HookRegistry: '''Global hook registry.''' - @classmethod - def create(cls, namespace): - '''Create a hook registry from a class namespace. - - Hook functions have an `_rfm_attach` attribute that specify the stages - of the pipeline where they must be attached. Dependencies will be - resolved first in the post-setup phase if not assigned elsewhere. - ''' - - local_hooks = util.OrderedSet() - for v in namespace.values(): - if hasattr(v, '_rfm_attach'): - local_hooks.add(Hook(v)) - elif hasattr(v, '_rfm_resolve_deps'): - v._rfm_attach = ['post_setup'] - local_hooks.add(Hook(v)) - - return cls(local_hooks) - def __init__(self, hooks=None): self.__hooks = util.OrderedSet() if hooks is not None: @@ -161,6 +142,20 @@ def __getattr__(self, name): def __iter__(self): return iter(self.__hooks) + def add(self, v): + '''Add value to the hook registry if it meets the conditions. + + Hook functions have an `_rfm_attach` attribute that specify the stages + of the pipeline where they must be attached. Dependencies will be + resolved first in the post-setup phase if not assigned elsewhere. + ''' + + if hasattr(v, '_rfm_attach'): + self.__hooks.add(Hook(v)) + elif hasattr(v, '_rfm_resolve_deps'): + v._rfm_attach = ['post_setup'] + self.__hooks.add(Hook(v)) + def update(self, hooks, *, denied_hooks=None): '''Update the hook registry with the hooks from another hook registry. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 76fc7bab76..d6815f14a0 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -14,9 +14,10 @@ import reframe.core.parameters as parameters import reframe.core.variables as variables import reframe.core.hooks as hooks +import reframe.utility as utils from reframe.core.exceptions import ReframeSyntaxError -from reframe.core.deferrable import deferrable +from reframe.core.deferrable import deferrable, _DeferredPerformanceExpression class RegressionTestMeta(type): @@ -30,24 +31,36 @@ class MetaNamespace(namespaces.LocalNamespace): same class body. Overriding a variable with a parameter or the other way around has an undefined behavior. A variable's value may be updated multiple times within the same class body. A parameter's - value may not be updated more than once within the same class body. + value cannot be updated more than once within the same class body. ''' def __setitem__(self, key, value): if isinstance(value, variables.TestVar): # Insert the attribute in the variable namespace - self['_rfm_local_var_space'][key] = value - value.__set_name__(self, key) + try: + self['_rfm_local_var_space'][key] = value + value.__set_name__(self, key) + except KeyError: + raise ReframeSyntaxError( + f'variable {key!r} is already declared' + ) from None - # Override the regular class attribute (if present) + # Override the regular class attribute (if present) and return self._namespace.pop(key, None) + return elif isinstance(value, parameters.TestParam): # Insert the attribute in the parameter namespace - self['_rfm_local_param_space'][key] = value + try: + self['_rfm_local_param_space'][key] = value + except KeyError: + raise ReframeSyntaxError( + f'parameter {key!r} is already declared in this class' + ) from None - # Override the regular class attribute (if present) + # Override the regular class attribute (if present) and return self._namespace.pop(key, None) + return elif key in self['_rfm_local_param_space']: raise ReframeSyntaxError( @@ -58,6 +71,33 @@ def __setitem__(self, key, value): # check from the base namespace. self._namespace[key] = value + # Register functions decorated with either @sanity_function or + # @performance_variables or @performance_function decorators. + if hasattr(value, '_rfm_sanity_fn'): + try: + super().__setitem__('_rfm_sanity', value) + except KeyError: + raise ReframeSyntaxError( + 'the @sanity_function decorator can only be used ' + 'once in the class body' + ) from None + elif hasattr(value, '_rfm_perf_key'): + try: + self['_rfm_perf_fns'][key] = value + except KeyError: + raise ReframeSyntaxError( + f'the performance function {key!r} has already been ' + f'defined in this class' + ) from None + + # Register the final methods + if hasattr(value, '_rfm_final'): + self['_rfm_final_methods'].add(key) + + # Register the hooks - if a value does not meet the conditions + # it will be simply ignored + self['_rfm_hook_registry'].add(value) + def __getitem__(self, key): '''Expose and control access to the local namespaces. @@ -190,6 +230,7 @@ def final(fn): namespace['bind'] = bind namespace['final'] = final + namespace['_rfm_final_methods'] = set() # Hook-related functionality def run_before(stage): @@ -209,6 +250,7 @@ def run_after(stage): namespace['run_before'] = run_before namespace['run_after'] = run_after namespace['require_deps'] = hooks.require_deps + namespace['_rfm_hook_registry'] = hooks.HookRegistry() # Machinery to add a sanity function def sanity_function(fn): @@ -224,6 +266,44 @@ def sanity_function(fn): namespace['sanity_function'] = sanity_function namespace['deferrable'] = deferrable + + # Machinery to add performance functions + def performance_function(units, *, perf_key=None): + '''Decorate a function to extract a performance variable. + + The ``units`` argument indicates the units of the performance + variable to be extracted. + The ``perf_key`` optional arg will be used as the name of the + performance variable. If not provided, the function name will + be used as the performance variable name. + ''' + if not isinstance(units, str): + raise TypeError('performance units must be a string') + + if perf_key and not isinstance(perf_key, str): + raise TypeError("'perf_key' must be a string") + + def _deco_wrapper(func): + if not utils.is_trivially_callable(func, non_def_args=1): + raise TypeError( + f'performance function {func.__name__!r} has more ' + f'than one argument without a default value' + ) + + @functools.wraps(func) + def _perf_fn(*args, **kwargs): + return _DeferredPerformanceExpression( + func, units, *args, **kwargs + ) + + _perf_key = perf_key if perf_key else func.__name__ + setattr(_perf_fn, '_rfm_perf_key', _perf_key) + return _perf_fn + + return _deco_wrapper + + namespace['performance_function'] = performance_function + namespace['_rfm_perf_fns'] = namespaces.LocalNamespace() return metacls.MetaNamespace(namespace) def __new__(metacls, name, bases, namespace, **kwargs): @@ -239,7 +319,7 @@ class was created or even at the instance level (e.g. doing directives = [ 'parameter', 'variable', 'bind', 'run_before', 'run_after', 'require_deps', 'required', 'deferrable', 'sanity_function', - 'final' + 'final', 'performance_function' ] for b in directives: namespace.pop(b, None) @@ -266,58 +346,50 @@ def __init__(cls, name, bases, namespace, **kwargs): # Update used names set with the local __dict__ cls._rfm_dir.update(cls.__dict__) - # Set up the hooks for the pipeline stages based on the _rfm_attach - # attribute; all dependencies will be resolved first in the post-setup - # phase if not assigned elsewhere - hook_reg = hooks.HookRegistry.create(namespace) - for base in (b for b in bases if hasattr(b, '_rfm_hook_registry')): - hook_reg.update(getattr(base, '_rfm_hook_registry'), - denied_hooks=namespace) - - cls._rfm_hook_registry = hook_reg - - # Gather all the locally defined sanity functions based on the - # _rfm_sanity_fn attribute. - - sn_fn = [v for v in namespace.values() if hasattr(v, '_rfm_sanity_fn')] - if sn_fn: - cls._rfm_sanity = sn_fn[0] - if len(sn_fn) > 1: - raise ReframeSyntaxError( - f'{cls.__qualname__!r} defines more than one sanity ' - 'function in the class body.' - ) - - else: - # Search the bases if no local sanity functions exist. - for base in (b for b in bases if hasattr(b, '_rfm_sanity')): - cls._rfm_sanity = getattr(base, '_rfm_sanity') - if cls._rfm_sanity.__name__ in namespace: - raise ReframeSyntaxError( - f'{cls.__qualname__!r} overrides the candidate ' - f'sanity function ' - f'{cls._rfm_sanity.__qualname__!r} without ' - f'defining an alternative' - ) - - break + # Update the hook registry with the bases + for base in cls._rfm_bases: + cls._rfm_hook_registry.update( + base._rfm_hook_registry, denied_hooks=namespace + ) - cls._final_methods = {v.__name__ for v in namespace.values() - if hasattr(v, '_rfm_final')} + # Search the bases if no local sanity functions exist. + if '_rfm_sanity' not in namespace: + for base in cls._rfm_bases: + if hasattr(base, '_rfm_sanity'): + cls._rfm_sanity = getattr(base, '_rfm_sanity') + if cls._rfm_sanity.__name__ in namespace: + raise ReframeSyntaxError( + f'{cls.__qualname__!r} overrides the candidate ' + f'sanity function ' + f'{cls._rfm_sanity.__qualname__!r} without ' + f'defining an alternative' + ) + + break + + # Update the performance function dict with the bases. + for base in cls._rfm_bases: + for k, v in base._rfm_perf_fns.items(): + if k not in namespace: + try: + cls._rfm_perf_fns[k] = v + except KeyError: + '''Performance function overridden by other class''' # Add the final functions from its parents - bases_w_final = [b for b in bases if hasattr(b, '_final_methods')] - cls._final_methods.update(*(b._final_methods for b in bases_w_final)) + cls._rfm_final_methods.update( + *(b._rfm_final_methods for b in cls._rfm_bases) + ) if getattr(cls, '_rfm_override_final', None): return - for v in namespace.values(): - for b in bases_w_final: - if callable(v) and v.__name__ in b._final_methods: - msg = (f"'{cls.__qualname__}.{v.__name__}' attempts to " + for b in cls._rfm_bases: + for key in b._rfm_final_methods: + if key in namespace and callable(namespace[key]): + msg = (f"'{cls.__qualname__}.{key}' attempts to " f"override final method " - f"'{b.__qualname__}.{v.__name__}'; " + f"'{b.__qualname__}.{key}'; " f"you should use the pipeline hooks instead") raise ReframeSyntaxError(msg) diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index 6590694f1f..86d3676127 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -61,7 +61,7 @@ def __repr__(self): def _raise_namespace_clash(self, name): '''Raise an error if there is a namespace clash.''' - raise ReframeSyntaxError( + raise KeyError( f'{name!r} is already present in the current namespace' ) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index f5b242f6ec..6a6d9376b3 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -12,6 +12,7 @@ import itertools import reframe.core.namespaces as namespaces +import reframe.utility as utils from reframe.core.exceptions import ReframeSyntaxError @@ -45,6 +46,18 @@ def filter_params(x): return x self.values = tuple(values) + + # Validate the filter_param argument + try: + valid = utils.is_trivially_callable(filter_params, non_def_args=1) + except TypeError: + raise TypeError( + 'the provided parameter filter is not a callable' + ) from None + else: + if not valid: + raise TypeError('filter function must take a single argument') + self.filter_params = filter_params diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 250e61d6be..e87659f4a3 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -35,7 +35,8 @@ from reframe.core.backends import getlauncher, getscheduler from reframe.core.buildsystems import BuildSystemField from reframe.core.containers import ContainerPlatformField -from reframe.core.deferrable import _DeferredExpression +from reframe.core.deferrable import (_DeferredExpression, + _DeferredPerformanceExpression) from reframe.core.exceptions import (BuildError, DependencyError, PerformanceError, PipelineError, SanityError, SkipTestError, @@ -598,8 +599,10 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. - reference = variable(typ.Tuple[object, object, object, object], - field=fields.ScopedDictField, value={}) + reference = variable(typ.Tuple[object, object, object], + typ.Dict[str, typ.Dict[ + str, typ.Tuple[object, object, object, object]] + ], field=fields.ScopedDictField, value={}) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -642,8 +645,43 @@ def pipeline_hooks(cls): #: `) as values. #: :class:`None` is also allowed. #: :default: :class:`None` - perf_patterns = variable(typ.Dict[str, _DeferredExpression], - type(None), value=None) + perf_patterns = variable(typ.Dict[str, _DeferredExpression], type(None)) + + #: The performance variables associated with the test. + #: + #: In this context, a performance variable is a key-value pair, where the + #: key is the desired variable name and the value is the deferred + #: performance expression (i.e. the result of a :ref:`deferrable + #: performance function`) that computes + #: or extracts the performance variable's value. + #: + #: By default, ReFrame will populate this field during the test's + #: instantiation with all the member functions decorated with the + #: :func:`@performance_function + #: ` decorator. + #: If no performance functions are present in the class, no performance + #: checking or reporting will be carried out. + #: + #: This mapping may be extended or replaced by other performance variables + #: that may be defined in any pipeline hook executing before the + #: performance stage. To this end, deferred performance functions can be + #: created inline using the utility + #: :func:`~reframe.utility.sanity.make_performance_function`. + #: + #: Refer to the :doc:`ReFrame Tutorials ` for concrete usage + #: examples. + #: + #: :type: A dictionary with keys of type :class:`str` and deferred + #: performance expressions as values (see + #: :ref:`deferrable-performance-functions`). + #: :default: Collection of performance variables associated to each of + #: the member functions decorated with the :func:`@performance_function + #: ` + #: decorator. + #: + #: .. versionadded:: 3.8.0 + perf_variables = variable(typ.Dict[str, _DeferredPerformanceExpression], + value={}) #: List of modules to be loaded before running this test. #: @@ -651,8 +689,8 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - modules = variable( - typ.List[str], typ.List[typ.Dict[str, object]], value=[]) + modules = variable(typ.List[str], typ.List[typ.Dict[str, object]], + value=[]) #: Environment variables to be set before running this test. #: @@ -817,7 +855,7 @@ def __new__(cls, *args, **kwargs): # Build pipeline hook registry and add the pre-init hook cls._rfm_pipeline_hooks = cls._process_hook_registry() - cls._rfm_pipeline_hooks['pre___init__'] = [cls.__pre_init__] + cls._rfm_pipeline_hooks['pre___init__'] = [obj.__pre_init__] # Attach the hooks to the pipeline stages for stage in _PIPELINE_STAGES: @@ -830,6 +868,11 @@ def __pre_init__(self): '''Initialize the test defaults from a pre-init hook.''' self.__deferred_rfm_init.evaluate() + # Build the default performance dict + if not self.perf_variables: + for fn in self._rfm_perf_fns.values(): + self.perf_variables[fn._rfm_perf_key] = fn(self) + def __init__(self): pass @@ -1663,60 +1706,109 @@ def check_performance(self): more details. ''' - if self.perf_patterns is None: - return - self._setup_perf_logging() with osext.change_dir(self._stagedir): - # Check if default reference perf values are provided and - # store all the variables tested in the performance check - has_default = False - variables = set() - for key, ref in self.reference.items(): - keyparts = key.split(self.reference.scope_separator) - system = keyparts[0] - varname = keyparts[-1] - unit = ref[3] - variables.add((varname, unit)) - if system == '*': - has_default = True - break - - if not has_default: - if not variables: - # If empty, it means that self.reference was empty, so try - # to infer their name from perf_patterns - variables = {(name, None) - for name in self.perf_patterns.keys()} - - for var in variables: - name, unit = var - ref_tuple = (0, None, None, unit) - self.reference.update({'*': {name: ref_tuple}}) - - # We first evaluate and log all performance values and then we - # check them against the reference. This way we always log them - # even if the don't meet the reference. - for tag, expr in self.perf_patterns.items(): - value = sn.evaluate(expr) - key = '%s:%s' % (self._current_partition.fullname, tag) - if key not in self.reference: - raise SanityError( - "tag `%s' not resolved in references for `%s'" % - (tag, self._current_partition.fullname)) - - self._perfvalues[key] = (value, *self.reference[key]) - self._perf_logger.log_performance(logging.INFO, tag, value, - *self.reference[key]) + if self.perf_variables or self._rfm_perf_fns: + if hasattr(self, 'perf_patterns'): + raise ReframeSyntaxError( + f"assigning a value to 'perf_pattenrs' conflicts ", + f"with using the 'performance_function' decorator ", + f"or setting a value to 'perf_variables'" + ) + # Log the performance variables + self._setup_perf_logging() + for tag, expr in self.perf_variables.items(): + try: + value = expr.evaluate() + unit = expr.unit + except Exception as e: + logging.getlogger().warning( + f'skipping evaluation of performance variable ' + f'{tag!r}: {e}' + ) + continue + + key = f'{self._current_partition.fullname}:{tag}' + try: + ref = self.reference[key] + + # If units are also provided in the reference, raise + # a warning if they match with the units provided by + # the performance function. + if len(ref) == 4: + if ref[3] != unit: + logging.getlogger().warning( + f'reference unit ({key!r}) for the ' + f'performance variable {tag!r} ' + f'does not match the unit specified ' + f'in the performance function ({unit!r}): ' + f'{unit!r} will be used' + ) + + # Pop the unit from the ref tuple (redundant) + ref = ref[:3] + except KeyError: + ref = (0, None, None) + + self._perfvalues[key] = (value, *ref, unit) + self._perf_logger.log_performance(logging.INFO, tag, value, + *ref, unit) + elif not hasattr(self, 'perf_patterns'): + return + else: + self._setup_perf_logging() + # Check if default reference perf values are provided and + # store all the variables tested in the performance check + has_default = False + variables = set() + for key, ref in self.reference.items(): + keyparts = key.split(self.reference.scope_separator) + system = keyparts[0] + varname = keyparts[-1] + unit = ref[3] + variables.add((varname, unit)) + if system == '*': + has_default = True + break + + if not has_default: + if not variables: + # If empty, it means that self.reference was empty, so + # try to infer their name from perf_patterns + variables = {(name, None) + for name in self.perf_patterns.keys()} + + for var in variables: + name, unit = var + ref_tuple = (0, None, None, unit) + self.reference.update({'*': {name: ref_tuple}}) + + # We first evaluate and log all performance values and then we + # check them against the reference. This way we always log them + # even if the don't meet the reference. + for tag, expr in self.perf_patterns.items(): + value = sn.evaluate(expr) + key = f'{self._current_partition.fullname}:{tag}' + if key not in self.reference: + raise SanityError( + f'tag {tag!r} not resolved in references for ' + f'{self._current_partition.fullname}' + ) + + self._perfvalues[key] = (value, *self.reference[key]) + self._perf_logger.log_performance(logging.INFO, tag, value, + *self.reference[key]) + + # Check the performance variables against their references. for key, values in self._perfvalues.items(): val, ref, low_thres, high_thres, *_ = values # Verify that val is a number if not isinstance(val, numbers.Number): raise SanityError( - "the value extracted for performance variable '%s' " - "is not a number: %s" % (key, val) + f'the value extracted for performance variable ' + f'{key!r} is not a number: {val}' ) tag = key.split(':')[-1] diff --git a/reframe/frontend/statistics.py b/reframe/frontend/statistics.py index ac913b92d1..35c4676090 100644 --- a/reframe/frontend/statistics.py +++ b/reframe/frontend/statistics.py @@ -171,7 +171,7 @@ def json(self, force=False): entry['result'] = 'success' entry['outputdir'] = check.outputdir - if check.perf_patterns: + if hasattr(check, 'perf_patterns'): # Record performance variables entry['perfvars'] = [] for key, ref in check.perfvalues.items(): diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 96f0140ddb..44dadba318 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -304,6 +304,34 @@ def attrs(obj): return ret +def is_trivially_callable(fn, *, non_def_args=0): + '''Check that a callable object is trivially callable. + + An object is trivially callable when it can be invoked by providing just + an expected number of non-default arguments to its call method. For + example, (non-static) member functions expect a single argument without a + default value, which will passed as ``cls`` or ``self`` during invocation + depending on whether the function is a classmethod or not, respectively. + On the other hand, member functions that are static methods are not passed + any values by default when invoked. Therefore, these functions can only be + trivially callable when their call method expects no arguments by default. + + :param fn: A callable to be tested if its trivially callable. + :param non_def_args: The number of non-default arguments the callable + ``fn`` expects when invoked. + :return: This function returns :obj:`True` if the expected number of + arguments matches the value of ``non_def_args``. Otherwise, it returns + :obj:`False`. + ''' + + if not callable(fn): + raise TypeError('argument is not a callable') + + explicit_args = [p for p in inspect.signature(fn).parameters.values() + if p.default is p.empty] + return len(explicit_args) == non_def_args + + def _is_builtin_type(cls): # NOTE: The set of types is copied from the copy.deepcopy() implementation builtin_types = (type(None), int, float, bool, complex, str, tuple, diff --git a/reframe/utility/sanity.py b/reframe/utility/sanity.py index e6047bcfa2..8cfd4607c1 100644 --- a/reframe/utility/sanity.py +++ b/reframe/utility/sanity.py @@ -14,7 +14,8 @@ import reframe.utility as util import reframe.core.warnings as warn -from reframe.core.deferrable import deferrable, _DeferredExpression +from reframe.core.deferrable import (deferrable, _DeferredExpression, + _DeferredPerformanceExpression) from reframe.core.exceptions import SanityError @@ -40,6 +41,29 @@ def _open(filename, *args, **kwargs): raise SanityError(f'{filename}: {e.strerror}') +def make_performance_function(func, unit, *args, **kwargs): + '''Convert a callable or deferred expression into a performance function. + + If ``func`` is a deferred expression, the performance function will be + built by extending this deferred expression into a deferred performance + expression. Otherwise, a new deferred performance expression will be + created from the function :func:`func`. The argument ``unit`` is the unit + associated with the deferrable performance expression, and ``*args`` and + ``**kwargs`` are the arguments to be captured by this deferred expression. + See + :doc:`deferrable functions reference ` + for further information on deferrable functions. + + .. versionadded:: 3.8.0 + ''' + if isinstance(func, _DeferredExpression): + return _DeferredPerformanceExpression.construct_from_deferred_expr( + func, unit + ) + else: + return _DeferredPerformanceExpression(func, unit, *args, **kwargs) + + # Create an alias decorator def sanity_function(func): warn.user_deprecation_warning( @@ -886,15 +910,32 @@ def defer(x): return x -def evaluate(expr): +def evaluate(expr, cache=False): '''Evaluate a deferred expression. If ``expr`` is not a deferred expression, it will be returned as is. + If ``expr`` is a deferred expression and ``cache`` is ``True``, the + results of the deferred expression will be cached and subsequent calls + to :func:`evaluate` on this deferred expression (when ``cache=False``) + will simply return the previously cached result. + + :param expr: The expression to be evaluated. + :param cache: Cache the result of this evaluation. + + .. note:: + When the ``cache`` argument is passed as ``True``, a deferred + expression will always be evaluated and its results will be re-cached. + This may replace any other results that may have been cached in + previous evaluations. .. versionadded:: 2.21 + + .. versionchanged:: 3.8.0 + The ``cache`` argument is added. ''' + if isinstance(expr, _DeferredExpression): - return expr.evaluate() + return expr.evaluate(cache=cache) else: return expr diff --git a/unittests/test_deferrable.py b/unittests/test_deferrable.py index 6149165e2e..734a927f66 100644 --- a/unittests/test_deferrable.py +++ b/unittests/test_deferrable.py @@ -16,6 +16,22 @@ def test_defer(): assert isinstance(a, _DeferredExpression) +def test_deferrable_perf(): + from reframe.core.deferrable import _DeferredPerformanceExpression as dpe + + a = sn.defer(3) + b = dpe.construct_from_deferred_expr(a, 'some_unit') + assert b.unit == 'some_unit' + + # Test wrong unit type + with pytest.raises(TypeError): + dpe(lambda x: x, 3) + + # Test not from deferred expr + with pytest.raises(TypeError): + dpe.construct_from_deferred_expr(lambda x: x, 'some_unit') + + def test_evaluate(): a = sn.defer(3) assert 3 == a.evaluate() @@ -23,6 +39,42 @@ def test_evaluate(): assert 3 == sn.evaluate(3) +def test_recursive_evaluate(): + @sn.deferrable + def c(): + @sn.deferrable + def b(): + @sn.deferrable + def a(): + return sn.defer(3) + + return a() + return b() + + assert 3 == c().evaluate() + + +def test_evaluate_cached(): + # A dummy mutable + my_list = [1] + + @sn.deferrable + def my_expr(): + return my_list[0] + + expr = my_expr() + assert expr.evaluate() == 1 + my_list = [2] + assert expr.evaluate(cache=True) == 2 + my_list = [3] + assert expr.evaluate() == 2 + + # Test that using cache=True updates the previously cached result + assert expr.evaluate(cache=True) == 3 + my_list = [4] + assert expr.evaluate() == 3 + + def test_depr_warn(monkeypatch): monkeypatch.setattr(warnings, '_RAISE_DEPRECATION_ALWAYS', True) with pytest.warns(ReframeDeprecationWarning): diff --git a/unittests/test_meta.py b/unittests/test_meta.py index 36f7318abd..7b9f12106c 100644 --- a/unittests/test_meta.py +++ b/unittests/test_meta.py @@ -53,6 +53,7 @@ class MyTest(MyMeta): sanity_function(ext) v = required final(ext) + performance_function('some_units')(ext) def __init__(self): assert not hasattr(self, 'parameter') @@ -65,6 +66,7 @@ def __init__(self): assert not hasattr(self, 'sanity_function') assert not hasattr(self, 'required') assert not hasattr(self, 'final') + assert not hasattr(self, 'performance_function') MyTest() @@ -254,3 +256,110 @@ class AllowFinalOverride(Base): def foo(self): '''Overriding foo is now allowed.''' + + +def test_callable_attributes(MyMeta): + '''Test issue #2113. + + Setting a callable without the __name__ attribute would crash the + metaclass. + ''' + + class Callable: + def __call__(self): + pass + + class Base(MyMeta): + f = Callable() + + +def test_performance_function(MyMeta): + assert hasattr(MyMeta, '_rfm_perf_fns') + + class Base(MyMeta): + @performance_function('units') + def perf_a(self): + pass + + @performance_function('units') + def perf_b(self): + pass + + def assert_perf_fn_return(self): + assert isinstance( + self.perf_a(), deferrable._DeferredPerformanceExpression + ) + + # Test the return type of the performance functions + Base().assert_perf_fn_return() + + # Test the performance function dict has been built correctly + perf_dict = {fn for fn in Base._rfm_perf_fns} + assert perf_dict == {'perf_a', 'perf_b'} + + class Derived(Base): + '''Test perf fn inheritance and perf_key argument.''' + + def perf_a(self): + '''Override perf fn with a non perf fn.''' + + @performance_function('units') + def perf_c(self): + '''Add a new perf fn.''' + + @performance_function('units', perf_key='my_perf_fn') + def perf_d(self): + '''Perf function with custom key.''' + + # Test the performance function set is correct with class inheritance + perf_dict = {fn._rfm_perf_key for fn in Derived._rfm_perf_fns.values()} + assert perf_dict == {'perf_b', 'perf_c', 'my_perf_fn'} + + # Test multiple inheritance and name conflict resolution + class ClashingBase(MyMeta): + @performance_function('units', perf_key='clash') + def perf_a(self): + return 'A' + + class Join(ClashingBase, Base): + '''Test that we follow MRO's order.''' + + class JoinAndOverride(ClashingBase, Base): + @performance_function('units') + def perf_a(self): + return 'B' + + assert Join._rfm_perf_fns['perf_a']('self').evaluate() == 'A' + assert JoinAndOverride._rfm_perf_fns['perf_a']('self').evaluate() == 'B' + + +def test_double_define_performance_function(MyMeta): + with pytest.raises(ReframeSyntaxError): + class Foo(MyMeta): + @performance_function('unit') + def foo(self): + pass + + @performance_function('unit') + def foo(self): + '''This doesn't make sense, so we raise an error''' + + +def test_performance_function_errors(MyMeta): + with pytest.raises(TypeError): + class wrong_perf_key_type(MyMeta): + @performance_function('units', perf_key=3) + def perf_fn(self): + pass + + with pytest.raises(TypeError): + class wrong_function_signature(MyMeta): + @performance_function('units') + def perf_fn(self, extra_arg): + pass + + with pytest.raises(TypeError): + class wrong_units(MyMeta): + @performance_function(5) + def perf_fn(self): + pass diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index f1a576c98a..ed2c6466d9 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -83,15 +83,27 @@ class MyTest(ExtendParams): def test_wrong_filter(): + with pytest.raises(TypeError): + class MyTest(ExtendParams): + '''Filter function is not a function''' + P1 = parameter(inherit_params=True, filter_params='not callable') + + with pytest.raises(TypeError): + class MyTest(ExtendParams): + '''Filter function takes more than 1 argument''' + P1 = parameter(inherit_params=True, filter_params=lambda x, y: []) + def bad_filter(x): raise RuntimeError('bad filter') with pytest.raises(RuntimeError): class Foo(ExtendParams): + '''Filter function raises''' P1 = parameter(inherit_params=True, filter_params=bad_filter) with pytest.raises(ReframeSyntaxError): class Foo(ExtendParams): + '''Wrong filter return type''' P1 = parameter(inherit_params=True, filter_params=lambda x: 1) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 73ede68969..fa5942aa38 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -1210,8 +1210,8 @@ def test_performance_invalid_value(dummytest, sanity_file, _run_sanity(dummytest, *dummy_gpu_exec_ctx) -def test_performance_var_evaluation(dummytest, sanity_file, - perf_file, dummy_gpu_exec_ctx): +def test_perf_patterns_evaluation(dummytest, sanity_file, + perf_file, dummy_gpu_exec_ctx): # All performance values must be evaluated, despite the first one # failing To test this, we need an extract function that will have a # side effect when evaluated, whose result we will check after calling @@ -1249,6 +1249,133 @@ def extract_perf(patt, tag): assert 'v3' in log_output +@pytest.fixture +def perftest(testsys_system, perf_file, sanity_file): + class MyTest(rfm.RunOnlyRegressionTest): + sourcesdir = None + + @sanity_function + def dummy_sanity(self): + return sn.assert_found(r'success', sanity_file) + + @performance_function('unit') + def value1(self): + pass + + @performance_function('unit') + def value2(self): + pass + + @performance_function('unit', perf_key='value3') + def value_3(self): + pass + + yield MyTest() + + +def test_validate_default_perf_variables(perftest): + assert len(perftest.perf_variables) == 3 + assert 'value1' in perftest.perf_variables + assert 'value2' in perftest.perf_variables + assert 'value3' in perftest.perf_variables + + +def test_perf_vars_without_reference(perftest, sanity_file, + perf_file, dummy_gpu_exec_ctx): + logfile = 'perf.log' + + @sn.deferrable + def extract_perf(patt, tag): + val = sn.evaluate( + sn.extractsingle(patt, perf_file, tag, float) + ) + with open(logfile, 'a') as fp: + fp.write(f'{tag}={val}') + + return val + + sanity_file.write_text('result = success\n') + perf_file.write_text('perf1 = 1.0\n' + 'perf3 = 3.3\n') + perftest.perf_variables = { + 'value1': sn.make_performance_function( + extract_perf(r'perf1 = (?P\S+)', 'v1'), 'unit' + ), + 'value3': sn.make_performance_function( + extract_perf, 'unit', r'perf3 = (?P\S+)', 'v3' + ) + } + _run_sanity(perftest, *dummy_gpu_exec_ctx) + + logfile = os.path.join(perftest.stagedir, logfile) + with open(logfile) as fp: + log_output = fp.read() + + assert 'v1' in log_output + assert 'v3' in log_output + + +def test_perf_vars_with_reference(perftest, sanity_file, + perf_file, dummy_gpu_exec_ctx): + # This test also checks that a performance function that raises an + # exception is simply skipped. + + logfile = 'perf.log' + + @sn.deferrable + def extract_perf(patt, tag): + val = sn.evaluate( + sn.extractsingle(patt, perf_file, tag, float) + ) + with open(logfile, 'a') as fp: + fp.write(f'{tag}={val}') + + return val + + def dummy_perf(x): + # Dummy function to check that a performance variable is simply + # skipped when the wrong number of arguments are passed to it. + with open(logfile, 'a') as fp: + fp.write('v2') + + return 1 + + sanity_file.write_text('result = success\n') + perf_file.write_text('perf1 = 1.0\n') + + # Make the unit in the reference different from the performance function + perftest.reference = { + '*': { + 'value1': (0, None, None, 'unit_') + } + } + perftest.perf_variables = { + 'value1': sn.make_performance_function( + extract_perf(r'perf1 = (?P\S+)', 'v1'), 'unit' + ), + 'value2': sn.make_performance_function( + dummy_perf, 'other_units', perftest, 'extra_arg' + ), + } + _run_sanity(perftest, *dummy_gpu_exec_ctx) + + logfile = os.path.join(perftest.stagedir, logfile) + with open(logfile) as fp: + log_output = fp.read() + + assert 'v1' in log_output + assert 'v2' not in log_output + + +def test_incompat_perf_syntax(perftest, sanity_file, + perf_file, dummy_gpu_exec_ctx): + sanity_file.write_text('result = success\n') + perf_file.write_text('perf1 = 1.0\n') + perftest.perf_patterns = {} + with pytest.raises(ReframeSyntaxError): + _run_sanity(perftest, *dummy_gpu_exec_ctx) + + @pytest.fixture def container_test(tmp_path): def _container_test(platform, image): diff --git a/unittests/test_utility.py b/unittests/test_utility.py index bfc7737122..801ca20db4 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1717,6 +1717,19 @@ def foo(): assert not util.is_copyable(foo()) +def test_is_trivially_callable(): + def foo(): + pass + + def bar(x, y): + pass + + assert util.is_trivially_callable(foo) + assert util.is_trivially_callable(bar, non_def_args=2) + with pytest.raises(TypeError): + util.is_trivially_callable(1) + + def test_nodelist_abbrev(): nid_nodes = [f'nid{n:03}' for n in range(5, 20)] cid_nodes = [f'cid{n:03}' for n in range(20)]