From db6ea965998bfb9fd3f4bc6191ddc40a6d447846 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Nov 2023 10:40:51 +0000 Subject: [PATCH 01/13] Batch: Put SchedulerConfig into a separate file --- loki/bulk/__init__.py | 1 + loki/bulk/configure.py | 97 ++++++++++++++++++++++++++++++++++++++++++ loki/bulk/scheduler.py | 90 ++------------------------------------- 3 files changed, 102 insertions(+), 86 deletions(-) create mode 100644 loki/bulk/configure.py diff --git a/loki/bulk/__init__.py b/loki/bulk/__init__.py index 8b3ea3f87..467626ab7 100644 --- a/loki/bulk/__init__.py +++ b/loki/bulk/__init__.py @@ -7,3 +7,4 @@ from loki.bulk.scheduler import * # noqa from loki.bulk.item import * # noqa +from loki.bulk.configure import * # noqa diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py new file mode 100644 index 000000000..334411910 --- /dev/null +++ b/loki/bulk/configure.py @@ -0,0 +1,97 @@ +# (C) Copyright 2018- ECMWF. +# This software is licensed under the terms of the Apache Licence Version 2.0 +# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. +# In applying this licence, ECMWF does not waive the privileges and immunities +# granted to it by virtue of its status as an intergovernmental organisation +# nor does it submit to any jurisdiction. + +from pathlib import Path +from collections import OrderedDict + +from loki.dimension import Dimension +from loki.tools import as_tuple, CaseInsensitiveDict + + +__all__ = ['SchedulerConfig'] + + +class SchedulerConfig: + """ + Configuration object for the transformation :any:`Scheduler` that + encapsulates default behaviour and item-specific behaviour. Can + be create either from a raw dictionary or configration file. + + Parameters + ---------- + default : dict + Default options for each item + routines : dict of dicts or list of dicts + Dicts with routine-specific options. + dimensions : dict of dicts or list of dicts + Dicts with options to define :any`Dimension` objects. + disable : list of str + Subroutine names that are entirely disabled and will not be + added to either the callgraph that we traverse, nor the + visualisation. These are intended for utility routines that + pop up in many routines but can be ignored in terms of program + control flow, like ``flush`` or ``abort``. + enable_imports : bool + Disable the inclusion of module imports as scheduler dependencies. + """ + + def __init__(self, default, routines, disable=None, dimensions=None, dic2p=None, derived_types=None, + enable_imports=False): + self.default = default + if isinstance(routines, dict): + self.routines = CaseInsensitiveDict(routines) + else: + self.routines = CaseInsensitiveDict((r.name, r) for r in as_tuple(routines)) + self.disable = as_tuple(disable) + self.dimensions = dimensions + self.enable_imports = enable_imports + + if dic2p is not None: + self.dic2p = dic2p + else: + self.dic2p = {} + if derived_types is not None: + self.derived_types = derived_types + else: + self.derived_types = () + + @classmethod + def from_dict(cls, config): + default = config['default'] + if 'routine' in config: + config['routines'] = OrderedDict((r['name'], r) for r in config.get('routine', [])) + else: + config['routines'] = [] + routines = config['routines'] + disable = default.get('disable', None) + enable_imports = default.get('enable_imports', False) + + # Add any dimension definitions contained in the config dict + dimensions = {} + if 'dimension' in config: + dimensions = [Dimension(**d) for d in config['dimension']] + dimensions = {d.name: d for d in dimensions} + + dic2p = {} + if 'dic2p' in config: + dic2p = config['dic2p'] + + derived_types = () + if 'derived_types' in config: + derived_types = config['derived_types'] + + return cls(default=default, routines=routines, disable=disable, dimensions=dimensions, dic2p=dic2p, + derived_types=derived_types, enable_imports=enable_imports) + + @classmethod + def from_file(cls, path): + import toml # pylint: disable=import-outside-toplevel + # Load configuration file and process options + with Path(path).open('r') as f: + config = toml.load(f) + + return cls.from_dict(config) diff --git a/loki/bulk/scheduler.py b/loki/bulk/scheduler.py index 6821d2b55..bf40e71c3 100644 --- a/loki/bulk/scheduler.py +++ b/loki/bulk/scheduler.py @@ -7,103 +7,21 @@ from os.path import commonpath from pathlib import Path -from collections import deque, OrderedDict, defaultdict +from collections import deque, defaultdict import networkx as nx from codetiming import Timer +from loki.bulk.item import ProcedureBindingItem, SubroutineItem, GlobalVarImportItem, GenericImportItem +from loki.bulk.configure import SchedulerConfig from loki.frontend import FP, REGEX, RegexParserClass from loki.sourcefile import Sourcefile -from loki.dimension import Dimension from loki.tools import as_tuple, CaseInsensitiveDict, flatten from loki.logging import info, perf, warning, debug -from loki.bulk.item import ProcedureBindingItem, SubroutineItem, GlobalVarImportItem, GenericImportItem from loki.subroutine import Subroutine from loki.module import Module -__all__ = ['Scheduler', 'SchedulerConfig'] - - -class SchedulerConfig: - """ - Configuration object for the transformation :any:`Scheduler` that - encapsulates default behaviour and item-specific behaviour. Can - be create either from a raw dictionary or configration file. - - Parameters - ---------- - default : dict - Default options for each item - routines : dict of dicts or list of dicts - Dicts with routine-specific options. - dimensions : dict of dicts or list of dicts - Dicts with options to define :any`Dimension` objects. - disable : list of str - Subroutine names that are entirely disabled and will not be - added to either the callgraph that we traverse, nor the - visualisation. These are intended for utility routines that - pop up in many routines but can be ignored in terms of program - control flow, like ``flush`` or ``abort``. - enable_imports : bool - Disable the inclusion of module imports as scheduler dependencies. - """ - - def __init__(self, default, routines, disable=None, dimensions=None, dic2p=None, derived_types=None, - enable_imports=False): - self.default = default - if isinstance(routines, dict): - self.routines = CaseInsensitiveDict(routines) - else: - self.routines = CaseInsensitiveDict((r.name, r) for r in as_tuple(routines)) - self.disable = as_tuple(disable) - self.dimensions = dimensions - self.enable_imports = enable_imports - - if dic2p is not None: - self.dic2p = dic2p - else: - self.dic2p = {} - if derived_types is not None: - self.derived_types = derived_types - else: - self.derived_types = () - - @classmethod - def from_dict(cls, config): - default = config['default'] - if 'routine' in config: - config['routines'] = OrderedDict((r['name'], r) for r in config.get('routine', [])) - else: - config['routines'] = [] - routines = config['routines'] - disable = default.get('disable', None) - enable_imports = default.get('enable_imports', False) - - # Add any dimension definitions contained in the config dict - dimensions = {} - if 'dimension' in config: - dimensions = [Dimension(**d) for d in config['dimension']] - dimensions = {d.name: d for d in dimensions} - - dic2p = {} - if 'dic2p' in config: - dic2p = config['dic2p'] - - derived_types = () - if 'derived_types' in config: - derived_types = config['derived_types'] - - return cls(default=default, routines=routines, disable=disable, dimensions=dimensions, dic2p=dic2p, - derived_types=derived_types, enable_imports=enable_imports) - - @classmethod - def from_file(cls, path): - import toml # pylint: disable=import-outside-toplevel - # Load configuration file and process options - with Path(path).open('r') as f: - config = toml.load(f) - - return cls.from_dict(config) +__all__ = ['Scheduler'] class Scheduler: From 02e935fb36eb8993c6389d37a683b88fbe892dd7 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Nov 2023 10:41:20 +0000 Subject: [PATCH 02/13] Pylint: Silence warnings about exception rules --- .pylintrc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pylintrc b/.pylintrc index 673e445af..faef82e0f 100644 --- a/.pylintrc +++ b/.pylintrc @@ -518,5 +518,5 @@ min-public-methods=2 # Exceptions that will emit a warning when being caught. Defaults to # "BaseException, Exception". -overgeneral-exceptions=BaseException, - Exception +overgeneral-exceptions=builtin.BaseException, + builtin.Exception From d62445ab0f2e7995bb520575aff29236b5680100 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Nov 2023 15:21:44 +0000 Subject: [PATCH 03/13] Tools: Move `load_module` utility to central tools package --- loki/build/builder.py | 19 ++----------------- loki/tools/files.py | 23 ++++++++++++++++++++++- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/loki/build/builder.py b/loki/build/builder.py index ff8f5944d..9f145f667 100644 --- a/loki/build/builder.py +++ b/loki/build/builder.py @@ -5,15 +5,13 @@ # granted to it by virtue of its status as an intergovernmental organisation # nor does it submit to any jurisdiction. -import sys from pathlib import Path from collections import deque -from importlib import import_module, reload, invalidate_caches from operator import attrgetter import networkx as nx from loki.logging import default_logger -from loki.tools import as_tuple, delete +from loki.tools import as_tuple, delete, load_module from loki.build.compiler import _default_compiler from loki.build.obj import Obj from loki.build.header import Header @@ -150,20 +148,7 @@ def load_module(self, module): """ Handle import paths and load the compiled module """ - if str(self.build_dir.absolute()) not in sys.path: - sys.path.insert(0, str(self.build_dir.absolute())) - if module in sys.modules: - reload(sys.modules[module]) - return sys.modules[module] - - try: - # Attempt to load module directly - return import_module(module) - except ModuleNotFoundError: - # If module caching interferes, try again with clean caches - invalidate_caches() - return import_module(module) - + return load_module(module, path=self.build_dir.absolute()) def wrap_and_load(self, sources, modname=None, build=True, libs=None, lib_dirs=None, incl_dirs=None, diff --git a/loki/tools/files.py b/loki/tools/files.py index c627fd123..6e665dd43 100644 --- a/loki/tools/files.py +++ b/loki/tools/files.py @@ -7,6 +7,7 @@ import os import re +import sys import pickle import shutil import fnmatch @@ -14,6 +15,7 @@ from functools import wraps from hashlib import md5 from pathlib import Path +from importlib import import_module, reload, invalidate_caches from loki.logging import debug, info from loki.tools.util import as_tuple, flatten @@ -22,7 +24,7 @@ __all__ = [ 'gettempdir', 'filehash', 'delete', 'find_paths', 'find_files', - 'disk_cached' + 'disk_cached', 'load_module' ] @@ -146,3 +148,22 @@ def cached(*args, **kwargs): return res return cached return decorator + + +def load_module(module, path=None): + """ + Handle import paths and load the compiled module + """ + if path and str(path) not in sys.path: + sys.path.insert(0, str(path)) + if module in sys.modules: + reload(sys.modules[module]) + return sys.modules[module] + + try: + # Attempt to load module directly + return import_module(module) + except ModuleNotFoundError: + # If module caching interferes, try again with clean caches + invalidate_caches() + return import_module(module) From 17470fb46e3fd76ea6086eb83dbbe3adb20ad13b Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 17 Nov 2023 15:37:59 +0000 Subject: [PATCH 04/13] Scheduler: Add TransformationConfig to instantiate Trafos from dict This now allos us to encapulate individual Transformation objects, and their individual argument options, in a toml config file. --- loki/bulk/configure.py | 91 +++++++++++++++++++++++++++++++++++++---- tests/test_scheduler.py | 27 ++++++++++++ 2 files changed, 111 insertions(+), 7 deletions(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index 334411910..9a2912300 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -9,10 +9,11 @@ from collections import OrderedDict from loki.dimension import Dimension -from loki.tools import as_tuple, CaseInsensitiveDict +from loki.tools import as_tuple, CaseInsensitiveDict, load_module +from loki.logging import error -__all__ = ['SchedulerConfig'] +__all__ = ['SchedulerConfig', 'TransformationConfig'] class SchedulerConfig: @@ -39,13 +40,27 @@ class SchedulerConfig: Disable the inclusion of module imports as scheduler dependencies. """ - def __init__(self, default, routines, disable=None, dimensions=None, dic2p=None, derived_types=None, - enable_imports=False): - self.default = default + def __init__( + self, default, routines, disable=None, dimensions=None, + transformation_configs=None, dic2p=None, derived_types=None, + enable_imports=False + ): if isinstance(routines, dict): self.routines = CaseInsensitiveDict(routines) else: self.routines = CaseInsensitiveDict((r.name, r) for r in as_tuple(routines)) + + if isinstance(transformation_configs, dict): + self.transformation_configs = transformation_configs + else: + self.transformation_configs = dict((r.name, r) for r in as_tuple(transformation_configs)) + + # Instantiate Transformation objects + self.transformations = { + name: config.instantiate() for name, config in self.transformation_configs.items() + } + + self.default = default self.disable = as_tuple(disable) self.dimensions = dimensions self.enable_imports = enable_imports @@ -76,6 +91,13 @@ def from_dict(cls, config): dimensions = [Dimension(**d) for d in config['dimension']] dimensions = {d.name: d for d in dimensions} + # Create config objects for Transformation configurations + transformation_configs = config.get('transformations', {}) + transformation_configs = { + name: TransformationConfig(name=name, **cfg) + for name, cfg in transformation_configs.items() + } + dic2p = {} if 'dic2p' in config: dic2p = config['dic2p'] @@ -84,8 +106,11 @@ def from_dict(cls, config): if 'derived_types' in config: derived_types = config['derived_types'] - return cls(default=default, routines=routines, disable=disable, dimensions=dimensions, dic2p=dic2p, - derived_types=derived_types, enable_imports=enable_imports) + return cls( + default=default, routines=routines, disable=disable, dimensions=dimensions, + transformation_configs=transformation_configs, dic2p=dic2p, derived_types=derived_types, + enable_imports=enable_imports + ) @classmethod def from_file(cls, path): @@ -95,3 +120,55 @@ def from_file(cls, path): config = toml.load(f) return cls.from_dict(config) + + +class TransformationConfig: + """ + Configuration object for :any:`Transformation` instances that can + be used to create :any:`Transformation` objects from dictionaries + or a config file. + + Parameters + ---------- + name : str + Name of the transformation object + module : str + Python module from which to load the transformation class + classname : str, optional + Name of the class to look for when instantiating the transformation. + If not provided, ``name`` will be used instead. + path : str or Path, optional + Path to add to the sys.path before attempting to load the ``module`` + options : dict + Dicts of options that define the transformation behaviour. + These options will be passed as constructor arguments using + keyword-argument notation. + """ + + def __init__(self, name, module, classname=None, path=None, options=None): + self.name = name + self.module = module + self.classname = classname or self.name + self.path = path + self.options = dict(options) + + def instantiate(self): + """ + Creates instantiated :any:`Transformation` object from stored config options. + """ + # Load the module that contains the transformations + mod = load_module(self.module, path=self.path) + + # Check for and return Transformation class + if not hasattr(mod, self.classname): + raise RuntimeError('Failed to load Transformation class!') + + # Attempt to instantiate transformation from config + try: + transformation = getattr(mod, self.classname)(**self.options) + except TypeError as e: + error(f'[Loki::Transformation] Failed to instiate {self.classname} from configuration') + error(f' Options passed: {self.options}') + raise e + + return transformation diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 3c4dfd139..00b8e82f1 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -2339,3 +2339,30 @@ def test_scheduler_disable_wildcard(here, config): dirname.rmdir() except FileNotFoundError: pass + + +def test_transformation_config(config): + """ + Test the correct instantiation of :any:`Transformation` objecst from config + """ + my_config = config.copy() + my_config['transformation'] = [ + { + 'name': 'DependencyTransformation', + 'module': 'loki.transform', + 'option': + { + 'suffix': '_rick', + 'module_suffix': '_roll', + 'replace_ignore_items': False, + } + } + ] + cfg = SchedulerConfig.from_dict(my_config) + assert cfg.transformations['DependencyTransformation'] + + transformation = cfg.transformations['DependencyTransformation'] + assert isinstance(transformation, DependencyTransformation) + assert transformation.suffix == '_rick' + assert transformation.module_suffix == '_roll' + assert not transformation.replace_ignore_items From 2172508b800894a80fc52106cf3591bba0f18477 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 Nov 2023 08:49:04 +0000 Subject: [PATCH 05/13] Scheduler: Require `routines` and `dimensions` as list keys --- loki/bulk/configure.py | 13 +- tests/sources/projA/scheduler_partial.config | 6 +- tests/test_cmake.py | 6 +- tests/test_lint/test_linter.py | 26 ++-- tests/test_scheduler.py | 116 ++++++++---------- tests/test_transform_dependency.py | 7 +- tests/test_transform_hoist_variables.py | 19 ++- tests/test_transform_parametrise.py | 10 +- transformations/tests/test_argument_shape.py | 7 +- .../tests/test_global_var_offload.py | 18 +-- transformations/tests/test_pool_allocator.py | 28 ++--- transformations/tests/test_scc_cuf.py | 9 +- .../tests/test_single_column_coalesced.py | 7 +- .../tests/test_transform_derived_types.py | 7 +- .../tests/test_utility_routines.py | 6 +- 15 files changed, 117 insertions(+), 168 deletions(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index 9a2912300..a54362d0e 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -6,7 +6,6 @@ # nor does it submit to any jurisdiction. from pathlib import Path -from collections import OrderedDict from loki.dimension import Dimension from loki.tools import as_tuple, CaseInsensitiveDict, load_module @@ -77,19 +76,13 @@ def __init__( @classmethod def from_dict(cls, config): default = config['default'] - if 'routine' in config: - config['routines'] = OrderedDict((r['name'], r) for r in config.get('routine', [])) - else: - config['routines'] = [] - routines = config['routines'] + routines = config.get('routines', []) disable = default.get('disable', None) enable_imports = default.get('enable_imports', False) # Add any dimension definitions contained in the config dict - dimensions = {} - if 'dimension' in config: - dimensions = [Dimension(**d) for d in config['dimension']] - dimensions = {d.name: d for d in dimensions} + dimensions = config.get('dimensions', {}) + dimensions = {k: Dimension(**d) for k, d in dimensions.items()} # Create config objects for Transformation configurations transformation_configs = config.get('transformations', {}) diff --git a/tests/sources/projA/scheduler_partial.config b/tests/sources/projA/scheduler_partial.config index 5e44a39ba..ab0ac9aa7 100644 --- a/tests/sources/projA/scheduler_partial.config +++ b/tests/sources/projA/scheduler_partial.config @@ -11,12 +11,10 @@ strict = true # usually used to exclude files we cannot yet process. block = ['compute_l2'] -[[routine]] -name = 'compute_l1' +[routines.compute_l1] role = 'driver' expand = true -[[routine]] -name = 'another_l1' +[routines.another_l1] role = 'driver' expand = true diff --git a/tests/test_cmake.py b/tests/test_cmake.py index 33c287a88..ac31c56b5 100644 --- a/tests/test_cmake.py +++ b/tests/test_cmake.py @@ -64,9 +64,9 @@ def fixture_config(here): """ default_config = { 'default': {'role': 'kernel', 'expand': True, 'strict': True}, - 'routine': [ - {'name': 'driverB', 'role': 'driver'}, - ] + 'routines': { + 'driverB': {'role': 'driver'}, + } } filepath = here/'test_cmake_loki.config' filepath.write_text(toml.dumps(default_config)) diff --git a/tests/test_lint/test_linter.py b/tests/test_lint/test_linter.py index ab53ece30..aceb916bc 100644 --- a/tests/test_lint/test_linter.py +++ b/tests/test_lint/test_linter.py @@ -429,24 +429,24 @@ def test_linter_lint_files_glob(here, rules, counter, exclude, files, max_worker @pytest.mark.parametrize('counter,routines,files', [ - (5, [{'name': 'driverA', 'role': 'driver'}], [ + (5, {'driverA': {'role': 'driver'}}, [ 'module/driverA_mod.f90', 'module/kernelA_mod.F90', 'module/compute_l1_mod.f90', 'source/another_l1.F90', 'source/another_l2.F90' ]), - (3, [ - {'name': 'another_l1', 'role': 'driver'}, - {'name': 'compute_l1', 'role': 'driver'} - ], [ + (3, { + 'another_l1': {'role': 'driver'}, + 'compute_l1': {'role': 'driver'} + }, [ 'source/another_l1.F90', 'module/compute_l1_mod.f90', 'source/another_l2.F90', ]), - (2, [ - {'name': 'another_l1', 'role': 'driver'} - ], [ + (2, { + 'another_l1': {'role': 'driver'} + }, [ 'source/another_l1.F90', 'source/another_l2.F90' ]), @@ -477,7 +477,7 @@ def output(self, handler_reports): 'strict': False, 'block': ['compute_l2'] }, - 'routine': routines + 'routines': routines } } @@ -497,9 +497,7 @@ def output(self, handler_reports): 'expand': True, 'strict': True, }, - 'routine': [{ - 'name': 'other_routine', - }] + 'routines': {'other_routine': {}} }}, {'include': ['linter_lint_files_fix.F90']} ]) @@ -573,9 +571,7 @@ def fix_subroutine(cls, subroutine, rule_report, config): 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'other_routine', - }] + 'routines': {'other_routine': {}} }}, {'include': ['*.F90']} ]) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 00b8e82f1..ed7d5090c 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -64,7 +64,6 @@ CaseInsensitiveDict, ModuleWrapTransformation ) - pytestmark = pytest.mark.skipif(not HAVE_FP and not HAVE_OFP, reason='Fparser and OFP not available') @@ -228,17 +227,16 @@ def test_scheduler_graph_partial(here, config, frontend): """ projA = here/'sources/projA' - config['routine'] = [ - { - 'name': 'compute_l1', + config['routines'] = { + 'compute_l1': { 'role': 'driver', 'expand': True, - }, { - 'name': 'another_l1', + }, + 'another_l1': { 'role': 'driver', 'expand': True, }, - ] + } scheduler = Scheduler(paths=projA, includes=projA/'include', config=config, frontend=frontend) @@ -402,17 +400,16 @@ def test_scheduler_process(here, config, frontend): """ projA = here/'sources/projA' - config['routine'] = [ - { - 'name': 'compute_l1', + config['routines'] = { + 'compute_l1': { 'role': 'driver', 'expand': True, - }, { - 'name': 'another_l1', + }, + 'another_l1': { 'role': 'driver', 'expand': True, }, - ] + } scheduler = Scheduler(paths=projA, includes=projA/'include', config=config, frontend=frontend) @@ -447,9 +444,9 @@ def test_scheduler_process_filter(here, config, frontend): projA = here/'sources/projA' projB = here/'sources/projB' - config['routine'] = [ - {'name': 'driverE_single', 'role': 'driver', 'expand': True,}, - ] + config['routines'] = { + 'driverE_single': {'role': 'driver', 'expand': True,}, + } scheduler = Scheduler( @@ -551,14 +548,13 @@ def test_scheduler_graph_multiple_separate(here, config, frontend): projB = here/'sources/projB' configA = config.copy() - configA['routine'] = [ - { - 'name': 'kernelB', + configA['routines'] = { + 'kernelB': { 'role': 'kernel', 'ignore': ['ext_driver'], 'enrich': ['ext_driver'], }, - ] + } schedulerA = Scheduler( paths=[projA, projB], includes=projA/'include', config=configA, @@ -594,12 +590,9 @@ def test_scheduler_graph_multiple_separate(here, config, frontend): # Test second scheduler instance that holds the receiver items configB = config.copy() - configB['routine'] = [ - { - 'name': 'ext_driver', - 'role': 'kernel', - }, - ] + configB['routines'] = { + 'ext_driver': { 'role': 'kernel' }, + } schedulerB = Scheduler( paths=projB, config=configB, seed_routines=['ext_driver'], @@ -813,17 +806,17 @@ def test_scheduler_dependencies_ignore(here, frontend): configA = SchedulerConfig.from_dict({ 'default': {'role': 'kernel', 'expand': True, 'strict': True}, - 'routine': [ - {'name': 'driverB', 'role': 'driver'}, - {'name': 'kernelB', 'ignore': ['ext_driver']}, - ] + 'routines': { + 'driverB': {'role': 'driver'}, + 'kernelB': {'ignore': ['ext_driver']}, + } }) configB = SchedulerConfig.from_dict({ 'default': {'role': 'kernel', 'expand': True, 'strict': True}, - 'routine': [ - {'name': 'ext_driver', 'role': 'kernel'} - ] + 'routines': { + 'ext_driver': {'role': 'kernel'} + } }) schedulerA = Scheduler(paths=[projA, projB], includes=projA/'include', config=configA, frontend=frontend) @@ -880,10 +873,10 @@ def test_scheduler_cmake_planner(here, frontend): config = SchedulerConfig.from_dict({ 'default': {'role': 'kernel', 'expand': True, 'strict': True}, - 'routine': [ - {'name': 'driverB', 'role': 'driver'}, - {'name': 'kernelB', 'ignore': ['ext_driver']}, - ] + 'routines': { + 'driverB': {'role': 'driver'}, + 'kernelB': {'ignore': ['ext_driver']}, + } }) # Populate the scheduler @@ -1007,10 +1000,10 @@ def test_scheduler_item_children(here): """ config = SchedulerConfig.from_dict({ 'default': {'role': 'kernel', 'expand': True, 'strict': True}, - 'routine': [ - {'name': 'driver', 'role': 'driver'}, - {'name': 'another_driver', 'role': 'driver'} - ] + 'routines': { + 'driver': { 'role': 'driver'}, + 'another_driver': { 'role': 'driver'} + } }) proj_hoist = here/'sources/projHoist' @@ -1407,12 +1400,11 @@ def test_scheduler_typebound_ignore(here, config, frontend): my_config = config.copy() my_config['default']['disable'] += ['some_type%some_routine', 'header_member_routine'] - my_config['routine'] = [ - { - 'name': 'other_member', + my_config['routines'] = { + 'other_member': { 'disable': my_config['default']['disable'] + ['member_routine'] } - ] + } scheduler = Scheduler( paths=proj, seed_routines=['driver'], config=my_config, @@ -1803,13 +1795,12 @@ def test_scheduler_inline_call(here, config, frontend): my_config = config.copy() my_config['default']['enable_imports'] = True - my_config['routine'] = [ - { - 'name': 'driver', + my_config['routines'] = { + 'driver': { 'role': 'driver', 'disable': ['return_one', 'some_var', 'add_args', 'some_type'] } - ] + } scheduler = Scheduler(paths=here/'sources/projInlineCalls', config=my_config, frontend=frontend) @@ -1837,12 +1828,9 @@ def test_scheduler_import_dependencies(here, config, frontend): my_config = config.copy() my_config['default']['enable_imports'] = True - my_config['routine'] = [ - { - 'name': 'driver', - 'role': 'driver' - } - ] + my_config['routines'] = { + 'driver': { 'role': 'driver' } + } scheduler = Scheduler(paths=here/'sources/projInlineCalls', config=my_config, frontend=frontend) @@ -1895,12 +1883,9 @@ def test_scheduler_globalvarimportitem_id(here, config, frontend): my_config = config.copy() my_config['default']['enable_imports'] = True - my_config['routine'] = [ - { - 'name': 'driver', - 'role': 'driver' - } - ] + my_config['routines'] = { + 'driver': { 'role': 'driver' } + } scheduler = Scheduler(paths=here/'sources/projInlineCalls', config=my_config, frontend=frontend) importA_item = scheduler.item_map['vars_module#vara'] @@ -2346,18 +2331,17 @@ def test_transformation_config(config): Test the correct instantiation of :any:`Transformation` objecst from config """ my_config = config.copy() - my_config['transformation'] = [ - { - 'name': 'DependencyTransformation', + my_config['transformations'] = { + 'DependencyTransformation': { 'module': 'loki.transform', - 'option': + 'options': { 'suffix': '_rick', 'module_suffix': '_roll', 'replace_ignore_items': False, } } - ] + } cfg = SchedulerConfig.from_dict(my_config) assert cfg.transformations['DependencyTransformation'] diff --git a/tests/test_transform_dependency.py b/tests/test_transform_dependency.py index cbeaa3169..0b27c1553 100644 --- a/tests/test_transform_dependency.py +++ b/tests/test_transform_dependency.py @@ -32,10 +32,9 @@ def fixture_config(): 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver', - }] + 'routines': { + 'driver': {'role': 'driver'} + } } diff --git a/tests/test_transform_hoist_variables.py b/tests/test_transform_hoist_variables.py index 7038c6649..8e679258e 100644 --- a/tests/test_transform_hoist_variables.py +++ b/tests/test_transform_hoist_variables.py @@ -38,18 +38,16 @@ def fixture_config(): 'expand': True, 'strict': True, }, - 'routine': [ - { - 'name': 'driver', + 'routines': { + 'driver': { 'role': 'driver', 'expand': True, }, - { - 'name': 'another_driver', + 'another_driver': { 'role': 'driver', 'expand': True, }, - ] + } } @@ -167,7 +165,7 @@ def test_hoist_disable(here, frontend, config): """ disable = ("device1", "device2") - config['routine'].append({'name': 'kernel2', 'role': 'kernel', 'ignore': disable}) + config['routines']['kernel2'] = {'role': 'kernel', 'ignore': disable} proj = here/'sources/projHoist' scheduler = Scheduler(paths=[proj], config=config, seed_routines=['driver', 'another_driver'], frontend=frontend) @@ -399,10 +397,9 @@ def test_hoist_mixed_variable_declarations(frontend, config): 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver', - }] + 'routines': { + 'driver': {'role': 'driver'} + } } scheduler = Scheduler(paths=[basedir], config=SchedulerConfig.from_dict(config), frontend=frontend) diff --git a/tests/test_transform_parametrise.py b/tests/test_transform_parametrise.py index 4dfccb7fb..7731c654a 100644 --- a/tests/test_transform_parametrise.py +++ b/tests/test_transform_parametrise.py @@ -28,18 +28,16 @@ def fixture_config(): 'expand': True, 'strict': True, }, - 'routine': [ - { - 'name': 'driver', + 'routines': { + 'driver': { 'role': 'driver', 'expand': True, }, - { - 'name': 'another_driver', + 'another_driver': { 'role': 'driver', 'expand': True, }, - ] + } } diff --git a/transformations/tests/test_argument_shape.py b/transformations/tests/test_argument_shape.py index b9aeeca1d..63acbce52 100644 --- a/transformations/tests/test_argument_shape.py +++ b/transformations/tests/test_argument_shape.py @@ -373,10 +373,9 @@ def test_argument_shape_transformation_import(frontend, here): 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver' - }] + 'routines': { + 'driver': {'role': 'driver'} + } } header = [here/'sources/projArgShape/var_module_mod.F90'] diff --git a/transformations/tests/test_global_var_offload.py b/transformations/tests/test_global_var_offload.py index f0c4e6578..4a838f10a 100644 --- a/transformations/tests/test_global_var_offload.py +++ b/transformations/tests/test_global_var_offload.py @@ -40,12 +40,9 @@ def test_transformation_global_var_import(here, config, frontend): my_config = config.copy() my_config['default']['enable_imports'] = True - my_config['routine'] = [ - { - 'name': 'driver', - 'role': 'driver' - } - ] + my_config['routines'] = { + 'driver': {'role': 'driver'} + } scheduler = Scheduler(paths=here/'sources/projGlobalVarImports', config=my_config, frontend=frontend) scheduler.process(transformation=GlobalVarOffloadTransformation()) @@ -117,12 +114,9 @@ def test_transformation_global_var_import_derived_type(here, config, frontend): my_config = config.copy() my_config['default']['enable_imports'] = True - my_config['routine'] = [ - { - 'name': 'driver_derived_type', - 'role': 'driver' - } - ] + my_config['routines'] = { + 'driver_derived_type': {'role': 'driver'} + } scheduler = Scheduler(paths=here/'sources/projGlobalVarImports', config=my_config, frontend=frontend) scheduler.process(transformation=GlobalVarOffloadTransformation()) diff --git a/transformations/tests/test_pool_allocator.py b/transformations/tests/test_pool_allocator.py index b2364f2cd..89d1df820 100644 --- a/transformations/tests/test_pool_allocator.py +++ b/transformations/tests/test_pool_allocator.py @@ -184,10 +184,9 @@ def test_pool_allocator_temporaries(frontend, generate_driver_stack, block_dim, 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver', - }] + 'routines': { + 'driver': {'role': 'driver'} + } } if frontend == FP and not generate_driver_stack: @@ -489,10 +488,9 @@ def test_pool_allocator_temporaries_kernel_sequence(frontend, block_dim, directi 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver', - }] + 'routines': { + 'driver': {'role': 'driver'} + } } scheduler = Scheduler(paths=[basedir], config=SchedulerConfig.from_dict(config), frontend=frontend, definitions=definitions) @@ -765,11 +763,9 @@ def test_pool_allocator_temporaries_kernel_nested(frontend, block_dim, directive 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver', - 'real_kind': 'jwrb', - }] + 'routines': { + 'driver': {'role': 'driver', 'real_kind': 'jwrb'} + } } scheduler = Scheduler(paths=[basedir], config=SchedulerConfig.from_dict(config), frontend=frontend, @@ -988,9 +984,9 @@ def test_pool_allocator_more_call_checks(frontend, block_dim, caplog): 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'kernel', - }] + 'routines': { + 'kernel': {} + } } scheduler = Scheduler(paths=[basedir], config=SchedulerConfig.from_dict(config), frontend=frontend) if frontend == OMNI: diff --git a/transformations/tests/test_scc_cuf.py b/transformations/tests/test_scc_cuf.py index 0b8212b57..8e75f86ab 100644 --- a/transformations/tests/test_scc_cuf.py +++ b/transformations/tests/test_scc_cuf.py @@ -50,12 +50,9 @@ def fixture_config(): 'expand': True, 'strict': True, }, - 'routine': [ - { - 'name': 'driver', - 'role': 'driver', - }, - ] + 'routines': { + 'driver': {'role': 'driver'} + } } diff --git a/transformations/tests/test_single_column_coalesced.py b/transformations/tests/test_single_column_coalesced.py index ed69cbf2e..7cccbe50a 100644 --- a/transformations/tests/test_single_column_coalesced.py +++ b/transformations/tests/test_single_column_coalesced.py @@ -512,10 +512,9 @@ def test_scc_hoist_multiple_kernels_loops(frontend, trim_vector_sections, horizo 'expand': True, 'strict': True }, - 'routine': [{ - 'name': 'driver', - 'role': 'driver' - }] + 'routines': { + 'driver': {'role': 'driver'} + } } scheduler = Scheduler(paths=[basedir], config=SchedulerConfig.from_dict(config), frontend=frontend) diff --git a/transformations/tests/test_transform_derived_types.py b/transformations/tests/test_transform_derived_types.py index 30f3eeea9..4381abaf1 100644 --- a/transformations/tests/test_transform_derived_types.py +++ b/transformations/tests/test_transform_derived_types.py @@ -34,13 +34,12 @@ def fixture_config(): 'expand': True, 'strict': True, }, - 'routines': [ - { - 'name': 'driver', + 'routines': { + 'driver': { 'role': 'driver', 'expand': True, }, - ] + } } diff --git a/transformations/tests/test_utility_routines.py b/transformations/tests/test_utility_routines.py index 08215fcdd..369374318 100644 --- a/transformations/tests/test_utility_routines.py +++ b/transformations/tests/test_utility_routines.py @@ -26,9 +26,9 @@ def fixture_config(): 'default': { 'role': 'kernel', 'expand': True, 'strict': True, 'disable': ['dr_hook', 'abor1'] }, - 'routine': [ - {'name': 'rick_astley', 'role': 'driver'}, - ] + 'routines': { + 'rick_astley': {'role': 'driver'}, + } } return default_config From a2947530f39de74a73792fee2299893058176bd4 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 Nov 2023 10:48:16 +0000 Subject: [PATCH 06/13] Scheduler: Let TransformationConfig resolve dimension options --- loki/bulk/configure.py | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index a54362d0e..fa4c31602 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -5,6 +5,7 @@ # granted to it by virtue of its status as an intergovernmental organisation # nor does it submit to any jurisdiction. +import re from pathlib import Path from loki.dimension import Dimension @@ -44,6 +45,11 @@ def __init__( transformation_configs=None, dic2p=None, derived_types=None, enable_imports=False ): + self.default = default + self.disable = as_tuple(disable) + self.dimensions = dimensions + self.enable_imports = enable_imports + if isinstance(routines, dict): self.routines = CaseInsensitiveDict(routines) else: @@ -54,16 +60,15 @@ def __init__( else: self.transformation_configs = dict((r.name, r) for r in as_tuple(transformation_configs)) + # Resolve the dimensions for trafo configurations + for cfg in self.transformation_configs.values(): + cfg.resolve_dimensions(dimensions) + # Instantiate Transformation objects self.transformations = { name: config.instantiate() for name, config in self.transformation_configs.items() } - self.default = default - self.disable = as_tuple(disable) - self.dimensions = dimensions - self.enable_imports = enable_imports - if dic2p is not None: self.dic2p = dic2p else: @@ -138,6 +143,8 @@ class TransformationConfig: keyword-argument notation. """ + _re_dimension = re.compile(r'\%dimensions?\.(.*)\%') + def __init__(self, name, module, classname=None, path=None, options=None): self.name = name self.module = module @@ -145,6 +152,28 @@ def __init__(self, name, module, classname=None, path=None, options=None): self.path = path self.options = dict(options) + def resolve_dimensions(self, dimensions): + """ + Substitute :any:`Dimension` objects for placeholder strings. + + The format of the string replacement matches the TOML + configuration. It will attempt to replace ``%dimensions.dim_name%`` + with a :any:`Dimension` found in :param dimensions: + + Parameters + ---------- + dimensions : dict + Dict matching string to pre-configured :any:`Dimension` objects. + """ + for key, val in self.options.items(): + if not isinstance(val, str): + continue + + matches = self._re_dimension.findall(val) + matches = tuple(dimensions[m] for m in as_tuple(matches)) + if matches: + self.options[key] = matches[0] if len(matches) == 1 else matches + def instantiate(self): """ Creates instantiated :any:`Transformation` object from stored config options. From 61527e23a2b936302c1df14eddba18303fe736fb Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 Nov 2023 04:27:31 +0000 Subject: [PATCH 07/13] Scheduler: Add test for dimension-reference trafo config --- tests/test_scheduler.py | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index ed7d5090c..7e29171a2 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -61,7 +61,7 @@ SubroutineItem, ProcedureBindingItem, gettempdir, ProcedureSymbol, ProcedureType, DerivedType, TypeDef, Scalar, Array, FindInlineCalls, Import, Variable, GenericImportItem, GlobalVarImportItem, flatten, - CaseInsensitiveDict, ModuleWrapTransformation + CaseInsensitiveDict, ModuleWrapTransformation, Dimension ) pytestmark = pytest.mark.skipif(not HAVE_FP and not HAVE_OFP, reason='Fparser and OFP not available') @@ -2350,3 +2350,35 @@ def test_transformation_config(config): assert transformation.suffix == '_rick' assert transformation.module_suffix == '_roll' assert not transformation.replace_ignore_items + + +def test_transformation_config_with_dimension(config): + """ + Test instantiation of :any:`Transformation` from config with + :any:`Dimension` argument. + """ + from transformations.single_column_coalesced import SCCBaseTransformation # pylint: disable=import-outside-toplevel + + my_config = config.copy() + my_config['dimensions'] = { + 'ij': {'size': 'n', 'index': 'i'} + } + my_config['transformations'] = { + 'SCCBase': { + 'classname': 'SCCBaseTransformation', + 'module': 'transformations.single_column_coalesced', + 'options': { 'horizontal': '%dimensions.ij%', 'directive': 'openacc'} + } + } + cfg = SchedulerConfig.from_dict(my_config) + assert cfg.transformations['SCCBase'] + + transformation = cfg.transformations['SCCBase'] + # assert isinstance(transformation, SCCBaseTransformation) + # TODO: The above check can fail, even though the class is correct. + # So instead we check that the class names is correct (ouch!) + assert str(type(transformation)) == str(SCCBaseTransformation) + assert isinstance(transformation.horizontal, Dimension) + assert transformation.horizontal.size == 'n' + assert transformation.horizontal.index == 'i' + assert transformation.directive == 'openacc' From e09f2d8fdd9e3da028e88efb7cd2174ee7c925ba Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 Nov 2023 13:45:53 +0000 Subject: [PATCH 08/13] Loki-transform: Using config-based transformations for SCC-CUF Since there is more refactoring needed to replace the full incvocation logic, we stick to the ones with custom constructor arguments. --- scripts/loki_transform.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/loki_transform.py b/scripts/loki_transform.py index 8573af958..7827001d2 100644 --- a/scripts/loki_transform.py +++ b/scripts/loki_transform.py @@ -229,12 +229,9 @@ def convert( scheduler.process( SCCHoistTemporaryArraysTransformation(block_dim=block_dim) ) if mode in ['cuf-parametrise', 'cuf-hoist', 'cuf-dynamic']: - derived_types = scheduler.config.derived_types - scheduler.process( SccCufTransformation( - horizontal=horizontal, vertical=vertical, block_dim=block_dim, - transformation_type=mode.replace('cuf-', ''), - derived_types=derived_types - )) + # These transformations requires complex constructor arguments, + # so we use the file-based transformation configuration. + scheduler.process( transformation=scheduler.config.transformations[mode] ) if global_var_offload: scheduler.process(transformation=GlobalVarOffloadTransformation()) @@ -253,10 +250,13 @@ def transform_subroutine(self, routine, **kwargs): scheduler.process(transformation=TemporariesPoolAllocatorTransformation( block_dim=block_dim, directive=directive, check_bounds='scc' not in mode )) + if mode == 'cuf-parametrise': - dic2p = scheduler.config.dic2p - transformation = ParametriseTransformation(dic2p=dic2p) + # This transformation requires complex constructora arguments, + # so we use the file-based transformation configuration. + transformation = scheduler.config.transformations['ParametriseTransformation'] scheduler.process(transformation=transformation) + if mode == "cuf-hoist": vertical = scheduler.config.dimensions['vertical'] scheduler.process(transformation=HoistTemporaryArraysAnalysis(dim_vars=(vertical.size,))) From 2e381e8ab50df8727cd4d13a811c7e163918a822 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Mon, 20 Nov 2023 13:47:20 +0000 Subject: [PATCH 09/13] Scheduler: Remove custom config entries for dic2p and derived_types These were previosuly handed down to the transformation constructors, but are not configured dirctly in config. --- loki/bulk/configure.py | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index fa4c31602..07e4a4714 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -42,8 +42,7 @@ class SchedulerConfig: def __init__( self, default, routines, disable=None, dimensions=None, - transformation_configs=None, dic2p=None, derived_types=None, - enable_imports=False + transformation_configs=None, enable_imports=False ): self.default = default self.disable = as_tuple(disable) @@ -69,15 +68,6 @@ def __init__( name: config.instantiate() for name, config in self.transformation_configs.items() } - if dic2p is not None: - self.dic2p = dic2p - else: - self.dic2p = {} - if derived_types is not None: - self.derived_types = derived_types - else: - self.derived_types = () - @classmethod def from_dict(cls, config): default = config['default'] @@ -96,18 +86,9 @@ def from_dict(cls, config): for name, cfg in transformation_configs.items() } - dic2p = {} - if 'dic2p' in config: - dic2p = config['dic2p'] - - derived_types = () - if 'derived_types' in config: - derived_types = config['derived_types'] - return cls( default=default, routines=routines, disable=disable, dimensions=dimensions, - transformation_configs=transformation_configs, dic2p=dic2p, derived_types=derived_types, - enable_imports=enable_imports + transformation_configs=transformation_configs, enable_imports=enable_imports ) @classmethod From 046cd962f1b133c1b3152db0180a8eb4107f20c3 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 7 Dec 2023 14:21:12 +0000 Subject: [PATCH 10/13] Scheduler: Build custom "external" trafo for scheduler config test This avoids depending on the `transformations` sub-package. --- tests/sources/call_me_trafo.py | 20 ++++++++++++++++++++ tests/test_scheduler.py | 25 +++++++++++-------------- 2 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 tests/sources/call_me_trafo.py diff --git a/tests/sources/call_me_trafo.py b/tests/sources/call_me_trafo.py new file mode 100644 index 000000000..eb373a5a0 --- /dev/null +++ b/tests/sources/call_me_trafo.py @@ -0,0 +1,20 @@ +# (C) Copyright 2018- ECMWF. +# This software is licensed under the terms of the Apache Licence Version 2.0 +# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. +# In applying this licence, ECMWF does not waive the privileges and immunities +# granted to it by virtue of its status as an intergovernmental organisation +# nor does it submit to any jurisdiction. + +from loki import Transformation, FindNodes, CallStatement + + +class CallMeMaybeTrafo(Transformation): + """ Test transformation for dynamically loading remote transformations. """ + + def __init__(self, name='Dave', horizontal=None): + self.name = name + self.horizontal = horizontal + + def transform_subroutine(self, routine, **kwargs): + for call in FindNodes(CallStatement).visit(routine.body): + call._update(name=self.name) diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 7e29171a2..63e51bd01 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -2352,33 +2352,30 @@ def test_transformation_config(config): assert not transformation.replace_ignore_items -def test_transformation_config_with_dimension(config): +def test_transformation_config_external_with_dimension(here, config): """ Test instantiation of :any:`Transformation` from config with :any:`Dimension` argument. """ - from transformations.single_column_coalesced import SCCBaseTransformation # pylint: disable=import-outside-toplevel - my_config = config.copy() my_config['dimensions'] = { 'ij': {'size': 'n', 'index': 'i'} } my_config['transformations'] = { - 'SCCBase': { - 'classname': 'SCCBaseTransformation', - 'module': 'transformations.single_column_coalesced', - 'options': { 'horizontal': '%dimensions.ij%', 'directive': 'openacc'} + 'CallMeRick': { + 'classname': 'CallMeMaybeTrafo', + 'module': 'call_me_trafo', + 'path': str(here/'sources'), + 'options': { 'name': 'Rick', 'horizontal': '%dimensions.ij%' } } } cfg = SchedulerConfig.from_dict(my_config) - assert cfg.transformations['SCCBase'] + assert cfg.transformations['CallMeRick'] - transformation = cfg.transformations['SCCBase'] - # assert isinstance(transformation, SCCBaseTransformation) - # TODO: The above check can fail, even though the class is correct. - # So instead we check that the class names is correct (ouch!) - assert str(type(transformation)) == str(SCCBaseTransformation) + transformation = cfg.transformations['CallMeRick'] + # We don't have the type, so simply check the class name + assert type(transformation).__name__ == 'CallMeMaybeTrafo' + assert transformation.name == 'Rick' assert isinstance(transformation.horizontal, Dimension) assert transformation.horizontal.size == 'n' assert transformation.horizontal.index == 'i' - assert transformation.directive == 'openacc' From 6d28c0602b1216200487ab539426f3402d7c7106 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Thu, 7 Dec 2023 15:16:02 +0000 Subject: [PATCH 11/13] SchedulerConfig: Fix docstring and dimension regex --- loki/bulk/configure.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index 07e4a4714..ab3e41a1d 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -124,7 +124,7 @@ class TransformationConfig: keyword-argument notation. """ - _re_dimension = re.compile(r'\%dimensions?\.(.*)\%') + _re_dimension = re.compile(r'\%dimensions\.(.*?)\%') def __init__(self, name, module, classname=None, path=None, options=None): self.name = name @@ -139,7 +139,7 @@ def resolve_dimensions(self, dimensions): The format of the string replacement matches the TOML configuration. It will attempt to replace ``%dimensions.dim_name%`` - with a :any:`Dimension` found in :param dimensions: + with a :any:`Dimension` found in :data:`dimensions`: Parameters ---------- From 4954a854b7015760dfaee8e75976d466fd765e55 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 8 Dec 2023 08:17:24 +0000 Subject: [PATCH 12/13] Scheduler: Add tests for bad configuration entries --- loki/bulk/configure.py | 2 +- tests/test_scheduler.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index ab3e41a1d..394a1fbbb 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -164,7 +164,7 @@ def instantiate(self): # Check for and return Transformation class if not hasattr(mod, self.classname): - raise RuntimeError('Failed to load Transformation class!') + raise RuntimeError(f'Failed to load Transformation class: {self.classname}') # Attempt to instantiate transformation from config try: diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 63e51bd01..39832a6f7 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -2351,6 +2351,37 @@ def test_transformation_config(config): assert transformation.module_suffix == '_roll' assert not transformation.replace_ignore_items + # Test for errors when failing to instantiate a transformation + bad_config = config.copy() + bad_config['transformations'] = { + 'DependencyTrafo': { # <= typo + 'module': 'loki.transform', + 'options': {} + } + } + with pytest.raises(RuntimeError): + cfg = SchedulerConfig.from_dict(bad_config) + + worse_config = config.copy() + worse_config['transformations'] = { + 'DependencyTransform': { + 'module': 'loki.transformats', # <= typo + 'options': {} + } + } + with pytest.raises(ModuleNotFoundError): + cfg = SchedulerConfig.from_dict(worse_config) + + worst_config = config.copy() + worst_config['transformations'] = { + 'DependencyTransform': { + 'module': 'loki.transform', + 'options': {'hello': 'Dave'} + } + } + with pytest.raises(RuntimeError): + cfg = SchedulerConfig.from_dict(worst_config) + def test_transformation_config_external_with_dimension(here, config): """ From ffb6330e5f1c0a1dd31db02efd06d8eb9da25d21 Mon Sep 17 00:00:00 2001 From: Michael Lange Date: Fri, 8 Dec 2023 08:17:52 +0000 Subject: [PATCH 13/13] Scheduler: Only support new-style dict-based configurations --- loki/bulk/configure.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/loki/bulk/configure.py b/loki/bulk/configure.py index 394a1fbbb..8a3b6f89b 100644 --- a/loki/bulk/configure.py +++ b/loki/bulk/configure.py @@ -49,15 +49,8 @@ def __init__( self.dimensions = dimensions self.enable_imports = enable_imports - if isinstance(routines, dict): - self.routines = CaseInsensitiveDict(routines) - else: - self.routines = CaseInsensitiveDict((r.name, r) for r in as_tuple(routines)) - - if isinstance(transformation_configs, dict): - self.transformation_configs = transformation_configs - else: - self.transformation_configs = dict((r.name, r) for r in as_tuple(transformation_configs)) + self.routines = CaseInsensitiveDict(routines) + self.transformation_configs = transformation_configs # Resolve the dimensions for trafo configurations for cfg in self.transformation_configs.values():