diff --git a/envs/docker/Dockerfile b/envs/docker/Dockerfile index 472de4c1..55a2a0fc 100644 --- a/envs/docker/Dockerfile +++ b/envs/docker/Dockerfile @@ -1,10 +1,20 @@ +# Usage: +# docker build -t fab . +# docker run --env PYTHONPATH=/fab/source -v /home/byron/git/fab:/fab -v /home/byron:/home/byron -it fab bash + FROM ubuntu:20.04 -RUN apt update && apt install -y gcc gfortran libclang-dev python-clang python3-pip rsync +RUN apt update && apt install -y gcc gfortran libclang-dev python-clang python3-pip rsync git RUN mkdir -p ~/.local/lib/python3.8/site-packages RUN cp -vr /usr/lib/python3/dist-packages/clang ~/.local/lib/python3.8/site-packages/ -RUN pip install flake8 fparser matplotlib mypy pytest sphinx sphinx_rtd_theme +RUN pip install pytest pytest-cov pytest-mock flake8 mypy +RUN pip install sphinx sphinx_rtd_theme sphinx-autodoc-typehints +RUN pip install svn GitPython matplotlib +RUN pip install fparser psyclone==2.1.0 + +RUN mkdir /usr/share/psyclone +RUN ln -s /usr/local/share/psyclone/psyclone.cfg /usr/share/psyclone/psyclone.cfg CMD [ "python3", "--version" ] diff --git a/run_configs/lfric/atm.py b/run_configs/lfric/atm.py index eb8dc08b..e92a29fc 100755 --- a/run_configs/lfric/atm.py +++ b/run_configs/lfric/atm.py @@ -12,11 +12,12 @@ from fab.steps.grab.folder import GrabFolder from fab.steps.link import LinkExe from fab.steps.preprocess import fortran_preprocessor, c_preprocessor +from fab.steps.psyclone import Psyclone, psyclone_preprocessor from fab.steps.root_inc_files import RootIncFiles from fab.steps.find_source_files import FindSourceFiles, Exclude, Include from grab_lfric import lfric_source_config, gpl_utils_source_config -from lfric_common import Configurator, FparserWorkaround_StopConcatenation, psyclone_preprocessor, Psyclone +from lfric_common import Configurator, FparserWorkaround_StopConcatenation logger = logging.getLogger('fab') @@ -103,9 +104,14 @@ def atm_config(two_stage=False, opt='Og'): ], ), - psyclone_preprocessor(set_um_physics=True), + # todo: put this inside the psyclone step, no need for it to be separate, there's nothing required between them + psyclone_preprocessor(common_flags=['-DUM_PHYSICS', '-DRDEF_PRECISION=64', '-DUSE_XIOS', '-DCOUPLED']), - Psyclone(kernel_roots=[config.build_output]), + Psyclone( + kernel_roots=[config.build_output], + transformation_script=lfric_source / 'lfric_atm/optimisation/meto-spice/global.py', + cli_args=[], + ), # todo: do we need this one in here? FparserWorkaround_StopConcatenation(name='fparser stop bug workaround'), diff --git a/run_configs/lfric/gungho.py b/run_configs/lfric/gungho.py index 271e920f..d64eb160 100755 --- a/run_configs/lfric/gungho.py +++ b/run_configs/lfric/gungho.py @@ -15,9 +15,10 @@ from fab.steps.link import LinkExe from fab.steps.preprocess import fortran_preprocessor from fab.steps.find_source_files import FindSourceFiles, Exclude +from fab.steps.psyclone import Psyclone, psyclone_preprocessor from grab_lfric import lfric_source_config, gpl_utils_source_config -from lfric_common import Configurator, FparserWorkaround_StopConcatenation, psyclone_preprocessor, Psyclone +from lfric_common import Configurator, FparserWorkaround_StopConcatenation logger = logging.getLogger('fab') @@ -66,9 +67,13 @@ def gungho_config(two_stage=False, opt='Og'): '-DRDEF_PRECISION=64', '-DR_SOLVER_PRECISION=64', '-DR_TRAN_PRECISION=64', '-DUSE_XIOS', ]), - psyclone_preprocessor(), + psyclone_preprocessor(common_flags=['-DRDEF_PRECISION=64', '-DUSE_XIOS', '-DCOUPLED']), - Psyclone(kernel_roots=[config.build_output]), + Psyclone( + kernel_roots=[config.build_output], + transformation_script=lfric_source / 'gungho/optimisation/meto-spice/global.py', + cli_args=[], + ), FparserWorkaround_StopConcatenation(name='fparser stop bug workaround'), diff --git a/run_configs/lfric/lfric_common.py b/run_configs/lfric/lfric_common.py index 076d4c49..69cd996f 100644 --- a/run_configs/lfric/lfric_common.py +++ b/run_configs/lfric/lfric_common.py @@ -4,10 +4,7 @@ from pathlib import Path from typing import Dict -from fab.artefacts import SuffixFilter -from fab.steps import Step, check_for_errors -from fab.steps.preprocess import PreProcessor -from fab.util import log_or_dot, input_to_output_fpath +from fab.steps import Step from fab.tools import run_command logger = logging.getLogger('fab') @@ -110,78 +107,3 @@ def run(self, artefact_store, config): open(feign_config_mod_fpath, 'wt').write( open(broken_version, 'rt').read().replace(bad, good)) - - -def psyclone_preprocessor(set_um_physics=False): - um_physics = ['-DUM_PHYSICS'] if set_um_physics else [] - - return PreProcessor( - preprocessor='cpp -traditional-cpp', - - source=SuffixFilter('all_source', '.x90'), - output_collection='preprocessed_x90', - - output_suffix='.x90', - name='preprocess x90', - common_flags=[ - '-P', - '-DRDEF_PRECISION=64', '-DUSE_XIOS', '-DCOUPLED', - *um_physics, - ], - ) - - -class Psyclone(Step): - - def __init__(self, name=None, kernel_roots=None): - super().__init__(name=name or 'psyclone') - self.kernel_roots = kernel_roots or [] - - def run(self, artefact_store: Dict, config): - super().run(artefact_store=artefact_store, config=config) - - results = self.run_mp(artefact_store['preprocessed_x90'], self.do_one_file) - check_for_errors(results, caller_label=self.name) - - successes = list(filter(lambda r: not isinstance(r, Exception), results)) - logger.info(f"success with {len(successes)} files") - artefact_store['psyclone_output'] = [] - for files in successes: - artefact_store['psyclone_output'].extend(files) - - def do_one_file(self, x90_file): - log_or_dot(logger=logger, msg=str(x90_file)) - - generated = x90_file.parent / (str(x90_file.stem) + '_psy.f90') - modified_alg = x90_file.with_suffix('.f90') - - # generate into the build output, not the source - generated = input_to_output_fpath(config=self._config, input_path=generated) - modified_alg = input_to_output_fpath(config=self._config, input_path=modified_alg) - generated.parent.mkdir(parents=True, exist_ok=True) - - # -d specifies "a root directory structure containing kernel source" - kernel_options = sum([['-d', k] for k in self.kernel_roots], []) - - command = [ - 'psyclone', '-api', 'dynamo0.3', - '-l', 'all', - *kernel_options, - '-opsy', generated, # filename of generated PSy code - '-oalg', modified_alg, # filename of transformed algorithm code - x90_file, - ] - - if self._config.reuse_artefacts and Path(modified_alg).exists(): - pass - else: - try: - run_command(command) - except Exception as err: - logger.error(err) - return err - - result = [modified_alg] - if Path(generated).exists(): - result.append(generated) - return result diff --git a/run_configs/lfric/mesh_tools.py b/run_configs/lfric/mesh_tools.py index a8d4393d..df5af3b7 100755 --- a/run_configs/lfric/mesh_tools.py +++ b/run_configs/lfric/mesh_tools.py @@ -9,8 +9,9 @@ from fab.steps.link import LinkExe from fab.steps.preprocess import fortran_preprocessor from fab.steps.find_source_files import FindSourceFiles, Exclude +from fab.steps.psyclone import Psyclone, psyclone_preprocessor -from lfric_common import Configurator, psyclone_preprocessor, Psyclone, FparserWorkaround_StopConcatenation +from lfric_common import Configurator, FparserWorkaround_StopConcatenation from grab_lfric import lfric_source_config, gpl_utils_source_config @@ -44,7 +45,7 @@ def mesh_tools_config(two_stage=False, opt='Og'): fortran_preprocessor(preprocessor='cpp -traditional-cpp', common_flags=['-P']), - psyclone_preprocessor(), + psyclone_preprocessor(common_flags=['-DRDEF_PRECISION=64', '-DUSE_XIOS', '-DCOUPLED']), Psyclone(kernel_roots=[config.build_output]), diff --git a/run_configs/um/build_um.py b/run_configs/um/build_um.py index a5ce34a8..25249cfb 100755 --- a/run_configs/um/build_um.py +++ b/run_configs/um/build_um.py @@ -13,10 +13,12 @@ import re import warnings from argparse import ArgumentParser +from pathlib import Path from fab.artefacts import CollectionGetter from fab.build_config import AddFlags, BuildConfig from fab.constants import PRAGMAD_C +from fab.parse.fortran import FortranParserWorkaround from fab.steps import Step from fab.steps.analyse import Analyse from fab.steps.archive_objects import ArchiveObjects @@ -128,7 +130,19 @@ def um_atmos_safe_config(revision, two_stage=False): ], ), - Analyse(root_symbol='um_main'), + Analyse( + root_symbol='um_main', + + # depending on environment, fparser2 can fail to parse this file but it does compile. + special_measure_analysis_results=[ + FortranParserWorkaround( + fpath=Path(config.build_output / "casim/lookup.f90"), + symbol_defs={'lookup'}, + symbol_deps={'mphys_die', 'variable_precision', 'mphys_switches', 'mphys_parameters', 'special', + 'passive_fields', 'casim_moments_mod', 'yomhook', 'parkind1'}, + ) + ] + ), CompileC(compiler='gcc', common_flags=['-c', '-std=c99']), diff --git a/setup.py b/setup.py index 92d33a27..ce1ec0c5 100644 --- a/setup.py +++ b/setup.py @@ -20,9 +20,10 @@ else: raise RuntimeError('Cannot determine package version.') + tests = ['pytest', 'pytest-cov', 'pytest-mock', 'flake8', 'mypy'] docs = ['sphinx', 'sphinx_rtd_theme', 'sphinx-autodoc-typehints'] -features = ['GitPython', 'matplotlib'] +features = ['GitPython', 'matplotlib', 'jinja2', 'psyclone==2.1.0'] setuptools.setup( name='sci-fab', diff --git a/source/fab/artefacts.py b/source/fab/artefacts.py index c604b641..7953f947 100644 --- a/source/fab/artefacts.py +++ b/source/fab/artefacts.py @@ -16,7 +16,6 @@ from typing import Iterable, Union, Dict, List from fab.constants import BUILD_TREES - from fab.dep_tree import filter_source_tree, AnalysedDependent from fab.util import suffix_filter @@ -86,6 +85,7 @@ def __init__(self, collections: Iterable[Union[str, ArtefactsGetter]]): # todo: ensure the labelled values are iterables def __call__(self, artefact_store: Dict): super().__call__(artefact_store) + # todo: this should be a set, in case a file appears in multiple collections result = [] for collection in self.collections: if isinstance(collection, str): diff --git a/source/fab/build_config.py b/source/fab/build_config.py index 5c4e012f..b525ca08 100644 --- a/source/fab/build_config.py +++ b/source/fab/build_config.py @@ -119,8 +119,6 @@ def add_current_prebuilds(self, artefacts: Iterable[Path]): Mark the given file paths as being current prebuilds, not to be cleaned during housekeeping. """ - if not self._artefact_store.get(CURRENT_PREBUILDS): - self.init_artefact_store() self._artefact_store[CURRENT_PREBUILDS].update(artefacts) def run(self): @@ -151,16 +149,16 @@ def run(self): self._finalise_logging() def _run_prep(self): + self._init_logging() + logger.info('') logger.info('------------------------------------------------------------') logger.info(f'running {self.project_label}') logger.info('------------------------------------------------------------') logger.info('') - self.build_output.mkdir(parents=True, exist_ok=True) - self.prebuild_folder.mkdir(parents=True, exist_ok=True) + self._prep_output_folders() - self._init_logging() init_metrics(metrics_folder=self.metrics_folder) # note: initialising here gives a new set of artefacts each run @@ -172,6 +170,10 @@ def _run_prep(self): logger.info("no housekeeping specified, adding a default hard cleanup") self.steps.append(CleanupPrebuilds(all_unused=True)) + def _prep_output_folders(self): + self.build_output.mkdir(parents=True, exist_ok=True) + self.prebuild_folder.mkdir(parents=True, exist_ok=True) + def _init_logging(self): # add a file logger for our run self.project_workspace.mkdir(parents=True, exist_ok=True) diff --git a/source/fab/dep_tree.py b/source/fab/dep_tree.py index a8619295..8f9ba9e1 100644 --- a/source/fab/dep_tree.py +++ b/source/fab/dep_tree.py @@ -9,8 +9,8 @@ """ # todo: we've since adopted the term "source tree", so we should probably rename this module to match. -import logging from abc import ABC +import logging from pathlib import Path from typing import Set, Dict, Iterable, List, Union, Optional, Any @@ -87,13 +87,15 @@ def to_dict(self) -> Dict[str, Any]: @classmethod def from_dict(cls, d): - return cls( + result = cls( fpath=Path(d["fpath"]), file_hash=d["file_hash"], symbol_defs=set(d["symbol_defs"]), symbol_deps=set(d["symbol_deps"]), file_deps=set(map(Path, d["file_deps"])), ) + assert result.file_hash is not None + return result def extract_sub_tree(source_tree: Dict[Path, AnalysedDependent], root: Path, verbose=False)\ diff --git a/source/fab/parse/__init__.py b/source/fab/parse/__init__.py index e30417e3..b07838b5 100644 --- a/source/fab/parse/__init__.py +++ b/source/fab/parse/__init__.py @@ -47,7 +47,8 @@ def file_hash(self): return self._file_hash def __eq__(self, other): - return vars(self) == vars(other) and type(self) == type(other) + # todo: better to use self.field_names() instead of vars(self) in order to evaluate any lazy attributes? + return vars(self) == vars(other) # persistence def to_dict(self) -> Dict[str, Any]: @@ -104,13 +105,13 @@ def __repr__(self): return f'{self.__class__.__name__}({params})' # We need to be hashable before we can go into a set, which is useful for our subclasses. - # Note, the result will change with each Python invocation. + # Note, the numerical result will change with each Python invocation. def __hash__(self): # Build up a list of things to hash, from our attributes. # We use self.field_names() rather than vars(self) because we want to evaluate any lazy attributes. # We turn dicts and sets into sorted tuples for hashing. - # todo: There's a good reason dicts and sets aren't hashable, so we should be sure we're happy doing this. - # Discuss. + # todo: There's a good reason dicts and sets aren't supposed to be hashable. + # Please see https://github.com/metomi/fab/issues/229 things = set() for field_name in self.field_names(): thing = getattr(self, field_name) @@ -139,3 +140,8 @@ def __init__(self, fpath: Union[str, Path]): """ super().__init__(fpath=fpath) + + @classmethod + def from_dict(cls, d): + # todo: load & save should be implemented here and used by the calling code, to save reanalysis. + raise NotImplementedError diff --git a/source/fab/parse/c.py b/source/fab/parse/c.py index 76ddf417..24c26045 100644 --- a/source/fab/parse/c.py +++ b/source/fab/parse/c.py @@ -11,13 +11,14 @@ from pathlib import Path from typing import List, Optional, Union, Tuple +from fab.dep_tree import AnalysedDependent + try: import clang # type: ignore import clang.cindex # type: ignore except ImportError: clang = None -from fab.dep_tree import AnalysedDependent from fab.util import log_or_dot, file_checksum logger = logging.getLogger(__name__) @@ -109,15 +110,17 @@ def run(self, fpath: Path) \ msg = 'clang not available, C analysis disabled' warnings.warn(msg, ImportWarning) return ImportWarning(msg), None - log_or_dot(logger, f"analysing {fpath}") # do we already have analysis results for this file? # todo: dupe - probably best in a parser base class file_hash = file_checksum(fpath).file_hash analysis_fpath = Path(self._config.prebuild_folder / f'{fpath.stem}.{file_hash}.an') if analysis_fpath.exists(): + log_or_dot(logger, f"found analysis prebuild for {fpath}") return AnalysedC.load(analysis_fpath), analysis_fpath + log_or_dot(logger, f"analysing {fpath}") + analysed_file = AnalysedC(fpath=fpath, file_hash=file_hash) # parse the file diff --git a/source/fab/parse/fortran.py b/source/fab/parse/fortran.py index 72fe3bd0..44366756 100644 --- a/source/fab/parse/fortran.py +++ b/source/fab/parse/fortran.py @@ -11,6 +11,7 @@ from pathlib import Path from typing import Union, Optional, Iterable, Dict, Any, Set +from fab.dep_tree import AnalysedDependent from fparser.two.Fortran2003 import ( # type: ignore Use_Stmt, Module_Stmt, Program_Stmt, Subroutine_Stmt, Function_Stmt, Language_Binding_Spec, Char_Literal_Constant, Interface_Block, Name, Comment, Module, Call_Stmt, Derived_Type_Def, Derived_Type_Stmt, @@ -20,7 +21,6 @@ from fparser.two.Fortran2008 import ( # type: ignore Type_Declaration_Stmt, Attr_Spec_List, Entity_Decl_List) -from fab.dep_tree import AnalysedDependent from fab.parse.fortran_common import iter_content, _has_ancestor_type, _typed_child, FortranAnalyserBase from fab.util import file_checksum, string_checksum diff --git a/source/fab/parse/fortran_common.py b/source/fab/parse/fortran_common.py index 4c84e122..5803185b 100644 --- a/source/fab/parse/fortran_common.py +++ b/source/fab/parse/fortran_common.py @@ -17,15 +17,14 @@ from fparser.two.utils import FortranSyntaxError # type: ignore from fab import FabException -from fab.parse import EmptySourceFile from fab.dep_tree import AnalysedDependent +from fab.parse import EmptySourceFile from fab.util import log_or_dot, file_checksum logger = logging.getLogger(__name__) -# todo: a nicer recursion pattern? def iter_content(obj): """ Return a generator which yields every node in the tree. @@ -105,14 +104,14 @@ def run(self, fpath: Path) \ Returns the analysis data and the result file where it was stored/loaded. """ - log_or_dot(logger, f"analysing {fpath}") - # calculate the prebuild filename file_hash = file_checksum(fpath).file_hash analysis_fpath = self._get_analysis_fpath(fpath, file_hash) # do we already have analysis results for this file? if analysis_fpath.exists(): + log_or_dot(logger, f"found analysis prebuild for {fpath}") + # Load the result file into whatever result class we use. loaded_result = self.result_class.load(analysis_fpath) if loaded_result: @@ -122,6 +121,8 @@ def run(self, fpath: Path) \ loaded_result.fpath = fpath return loaded_result, analysis_fpath + log_or_dot(logger, f"analysing {fpath}") + # parse the file, get a node tree node_tree = self._parse_file(fpath=fpath) if isinstance(node_tree, Exception): diff --git a/source/fab/parse/x90.py b/source/fab/parse/x90.py index 7692bde1..902c01fe 100644 --- a/source/fab/parse/x90.py +++ b/source/fab/parse/x90.py @@ -4,7 +4,7 @@ # which you should have received as part of this distribution # ############################################################################## from pathlib import Path -from typing import Union, Optional, Dict, Any +from typing import Iterable, Set, Union, Optional, Dict, Any from fparser.two.Fortran2003 import Use_Stmt, Call_Stmt, Name, Only_List, Actual_Arg_Spec_List, Part_Ref # type: ignore @@ -20,15 +20,12 @@ class AnalysedX90(AnalysedFile): """ def __init__(self, fpath: Union[str, Path], file_hash: int, # todo: the fortran version doesn't include the remaining args - update this too, for simplicity. - kernel_deps: Optional[Dict[str, str]] = None): + kernel_deps: Optional[Iterable[str]] = None): """ :param fpath: The path of the x90 file. :param file_hash: The checksum of the x90 file. - - todo: not as param - :param kernel_deps: Kernel symbols used by the x90 file. @@ -36,12 +33,12 @@ def __init__(self, fpath: Union[str, Path], file_hash: int, super().__init__(fpath=fpath, file_hash=file_hash) # Maps used kernel metadata (type def names) to the modules they're found in - self.kernel_deps: Dict[str, str] = kernel_deps or {} + self.kernel_deps: Set[str] = set(kernel_deps or {}) def to_dict(self) -> Dict[str, Any]: result = super().to_dict() result.update({ - "kernel_deps": self.kernel_deps, + "kernel_deps": sorted(self.kernel_deps), }) return result @@ -50,7 +47,7 @@ def from_dict(cls, d): result = cls( fpath=Path(d["fpath"]), file_hash=d["file_hash"], - kernel_deps=d["kernel_deps"], + kernel_deps=set(d["kernel_deps"]), ) assert result.file_hash is not None return result @@ -115,14 +112,13 @@ def _process_call_statement(self, symbol_deps: Dict[str, str], analysed_file, ob if called_name.string == "invoke": arg_list = _typed_child(obj, Actual_Arg_Spec_List) if not arg_list: - logger.info(f'No arg list passed to invoke: {obj.string}') + logger.debug(f'No arg list passed to invoke: {obj.string}') return args = by_type(arg_list.children, Part_Ref) for arg in args: arg_name = _typed_child(arg, Name) arg_name = arg_name.string if arg_name in symbol_deps: - in_mod = symbol_deps[arg_name] - analysed_file.kernel_deps[arg_name] = in_mod + analysed_file.kernel_deps.add(arg_name) else: logger.debug(f"arg '{arg_name}' to invoke() was not used, presumed built-in kernel") diff --git a/source/fab/steps/__init__.py b/source/fab/steps/__init__.py index 7b0d7bd4..3e3c121e 100644 --- a/source/fab/steps/__init__.py +++ b/source/fab/steps/__init__.py @@ -22,13 +22,13 @@ class Step(ABC): """ - def __init__(self, name): + def __init__(self, name=None): """ :param name: Human friendly name for logger output. Steps generally provide a sensible default. """ - self.name = name + self.name = name or self.__class__.__name__ # runtime self._config = None diff --git a/source/fab/steps/analyse.py b/source/fab/steps/analyse.py index 0585ea64..b8039122 100644 --- a/source/fab/steps/analyse.py +++ b/source/fab/steps/analyse.py @@ -40,16 +40,15 @@ from pathlib import Path from typing import Dict, List, Iterable, Set, Optional, Union +from fab.artefacts import ArtefactsGetter, CollectionConcat, SuffixFilter +from fab.constants import BUILD_TREES +from fab.dep_tree import AnalysedDependent, extract_sub_tree, validate_dependencies +from fab.mo import add_mo_commented_file_deps +from fab.parse import AnalysedFile, EmptySourceFile from fab.parse.c import CAnalyser - from fab.parse.fortran import FortranParserWorkaround, FortranAnalyser - -from fab.constants import BUILD_TREES, CURRENT_PREBUILDS -from fab.dep_tree import extract_sub_tree, validate_dependencies, AnalysedDependent -from fab.mo import add_mo_commented_file_deps from fab.steps import Step from fab.util import TimerLogger, by_type -from fab.artefacts import ArtefactsGetter, CollectionConcat, SuffixFilter logger = logging.getLogger(__name__) @@ -58,7 +57,7 @@ 'preprocessed_c', 'preprocessed_fortran', - # todo: this is lfric stuff so might be better placed with the lfric run configs + # todo: this is lfric stuff so might be better placed elsewhere SuffixFilter('psyclone_output', '.f90'), 'preprocessed_psyclone', 'configurator_output', @@ -168,7 +167,7 @@ def run(self, artefact_store: Dict, config): self._add_manual_results(analysed_files) # analyse - project_source_tree, symbols = self._analyse_dependencies(analysed_files) + project_source_tree, symbol_table = self._analyse_dependencies(analysed_files) # add the file dependencies for MO FCM's "DEPENDS ON:" commented file deps (being removed soon) with TimerLogger("adding MO FCM 'DEPENDS ON:' file dependency comments"): @@ -178,33 +177,36 @@ def run(self, artefact_store: Dict, config): # extract "build trees" for executables. if self.root_symbols: - build_trees = self._extract_build_trees(project_source_tree, symbols) + build_trees = self._extract_build_trees(project_source_tree, symbol_table) else: build_trees = {None: project_source_tree} # throw in any extra source we need, which Fab can't automatically detect for build_tree in build_trees.values(): - self._add_unreferenced_deps(symbols, project_source_tree, build_tree) + self._add_unreferenced_deps(symbol_table, project_source_tree, build_tree) validate_dependencies(build_tree) artefact_store[BUILD_TREES] = build_trees def _analyse_dependencies(self, analysed_files: Iterable[AnalysedDependent]): """ - Turn symbol deps into file deps and build a source dependency tree for the entire source. + Build a source dependency tree for the entire source. """ with TimerLogger("converting symbol dependencies to file dependencies"): # map symbols to the files they're in - symbols: Dict[str, Path] = self._gen_symbol_table(analysed_files) + symbol_table: Dict[str, Path] = self._gen_symbol_table(analysed_files) # fill in the file deps attribute in the analysed file objects - self._gen_file_deps(analysed_files, symbols) + self._gen_file_deps(analysed_files, symbol_table) + # build the tree + # the nodes refer to other nodes via the file dependencies we just made, which are keys into this dict source_tree: Dict[Path, AnalysedDependent] = {a.fpath: a for a in analysed_files} - return source_tree, symbols - def _extract_build_trees(self, project_source_tree, symbols): + return source_tree, symbol_table + + def _extract_build_trees(self, project_source_tree, symbol_table): """ Find the subset of files needed to build each root symbol (executable). @@ -215,9 +217,9 @@ def _extract_build_trees(self, project_source_tree, symbols): build_trees = {} for root in self.root_symbols: with TimerLogger(f"extracting build tree for root '{root}'"): - build_tree = extract_sub_tree(project_source_tree, symbols[root], verbose=False) + build_tree = extract_sub_tree(project_source_tree, symbol_table[root], verbose=False) - logger.info(f"target source tree size {len(build_tree)} (target '{symbols[root]}')") + logger.info(f"target source tree size {len(build_tree)} (target '{symbol_table[root]}')") build_trees[root] = build_tree return build_trees @@ -236,6 +238,10 @@ def _parse_files(self, files: List[Path]) -> Set[AnalysedDependent]: fortran_results = self.run_mp(items=fortran_files, func=self.fortran_analyser.run) fortran_analyses, fortran_artefacts = zip(*fortran_results) if fortran_results else (tuple(), tuple()) + # warn about naughty fortran usage + if self.fortran_analyser.depends_on_comment_found: + warnings.warn("deprecated 'DEPENDS ON:' comment found in fortran code") + # c c_files = set(filter(lambda f: f.suffix == '.c', files)) with TimerLogger(f"analysing {len(c_files)} preprocessed c files"): @@ -257,13 +263,12 @@ def _parse_files(self, files: List[Path]) -> Set[AnalysedDependent]: # record the artefacts as being current artefacts = by_type(fortran_artefacts + c_artefacts, Path) - self._config._artefact_store[CURRENT_PREBUILDS].update(artefacts) # slightly naughty access. - - # warn about naughty fortran usage - if self.fortran_analyser.depends_on_comment_found: - warnings.warn("deprecated 'DEPENDS ON:' comment found in fortran code") + self._config.add_current_prebuilds(artefacts) - return set(by_type(analyses, AnalysedDependent)) + # ignore empty files + analysed_files = by_type(analyses, AnalysedFile) + non_empty = {af for af in analysed_files if not isinstance(af, EmptySourceFile)} + return non_empty def _add_manual_results(self, analysed_files: Set[AnalysedDependent]): # add manual analysis results for files which could not be parsed @@ -328,8 +333,7 @@ def _gen_file_deps(self, analysed_files: Iterable[AnalysedDependent], symbols: D if deps_not_found: logger.info(f"{len(deps_not_found)} deps not found") - def _add_unreferenced_deps(self, - symbols: Dict[str, Path], + def _add_unreferenced_deps(self, symbol_table: Dict[str, Path], all_analysed_files: Dict[Path, AnalysedDependent], build_tree: Dict[Path, AnalysedDependent]): """ @@ -345,7 +349,7 @@ def _add_unreferenced_deps(self, for symbol_dep in self.unreferenced_deps: # what file is the symbol in? - analysed_fpath = symbols.get(symbol_dep) + analysed_fpath = symbol_table.get(symbol_dep) if not analysed_fpath: warnings.warn(f"no file found for unreferenced dependency {symbol_dep}") continue diff --git a/source/fab/steps/c_pragma_injector.py b/source/fab/steps/c_pragma_injector.py index 1ed5f9bc..00284f3b 100644 --- a/source/fab/steps/c_pragma_injector.py +++ b/source/fab/steps/c_pragma_injector.py @@ -12,7 +12,6 @@ from typing import Dict, Pattern, Optional, Match from fab import FabException - from fab.constants import PRAGMAD_C from fab.steps import Step from fab.artefacts import ArtefactsGetter, SuffixFilter diff --git a/source/fab/steps/compile_c.py b/source/fab/steps/compile_c.py index 17e7eadf..b19361f3 100644 --- a/source/fab/steps/compile_c.py +++ b/source/fab/steps/compile_c.py @@ -15,16 +15,14 @@ from typing import List, Dict, Optional from fab import FabException - -from fab.parse.c import AnalysedC - from fab.artefacts import ArtefactsGetter, FilterBuildTrees from fab.build_config import FlagsConfig from fab.constants import OBJECT_FILES from fab.metrics import send_metric +from fab.parse.c import AnalysedC from fab.steps import check_for_errors, Step -from fab.util import CompiledFile, log_or_dot, Timer, by_type from fab.tools import flags_checksum, run_command, get_tool, get_compiler_version +from fab.util import CompiledFile, log_or_dot, Timer, by_type logger = logging.getLogger(__name__) @@ -139,7 +137,7 @@ def _compile_file(self, analysed_file: AnalysedC): command.append(str(analysed_file.fpath)) command.extend(['-o', str(obj_file_prebuild)]) - log_or_dot(logger, 'CompileC running command: ' + ' '.join(command)) + log_or_dot(logger, f'CompileC compiling {analysed_file.fpath}') try: run_command(command) except Exception as err: diff --git a/source/fab/steps/compile_fortran.py b/source/fab/steps/compile_fortran.py index ddaa5a20..972d4b38 100644 --- a/source/fab/steps/compile_fortran.py +++ b/source/fab/steps/compile_fortran.py @@ -16,12 +16,11 @@ from pathlib import Path from typing import List, Set, Dict, Tuple, Optional, Union -from fab.parse.fortran import AnalysedFortran - from fab.artefacts import ArtefactsGetter, FilterBuildTrees from fab.build_config import FlagsConfig from fab.constants import OBJECT_FILES from fab.metrics import send_metric +from fab.parse.fortran import AnalysedFortran from fab.steps import check_for_errors, Step from fab.tools import COMPILERS, remove_managed_flags, flags_checksum, run_command, get_tool, get_compiler_version from fab.util import CompiledFile, log_or_dot_finish, log_or_dot, Timer, by_type, \ @@ -88,7 +87,7 @@ def __init__(self, compiler: Optional[str] = None, common_flags: Optional[List[s self.source_getter = source or DEFAULT_SOURCE_GETTER self.two_stage_flag = two_stage_flag - # runtime + # runtime, for child processes to read self._stage = None self._mod_hashes: Dict[str, int] = {} @@ -274,6 +273,7 @@ def process_file(self, analysed_file: AnalysedFortran) \ def _get_obj_combo_hash(self, analysed_file, flags): # get a combo hash of things which matter to the object file we define + # todo: don't just silently use 0 for a missing dep hash mod_deps_hashes = {mod_dep: self._mod_hashes.get(mod_dep, 0) for mod_dep in analysed_file.module_deps} try: obj_combo_hash = sum([ @@ -334,8 +334,6 @@ def compile_file(self, analysed_file, flags, output_fpath): command.append(analysed_file.fpath.name) command.extend(['-o', str(output_fpath)]) - log_or_dot(logger, 'CompileFortran running command: ' + ' '.join(command)) - run_command(command, cwd=analysed_file.fpath.parent) # todo: probably better to record both mod and obj metrics diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py new file mode 100644 index 00000000..2161d18e --- /dev/null +++ b/source/fab/steps/psyclone.py @@ -0,0 +1,435 @@ +# ############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +# ############################################################################## +""" +A preprocessor and code generation step for PSyclone. +https://github.com/stfc/PSyclone + +""" +from dataclasses import dataclass +import logging +import re +import shutil +import warnings +from itertools import chain +from pathlib import Path +from typing import Dict, List, Optional, Set + +from fab.tools import run_command + +from fab.artefacts import ArtefactsGetter, CollectionConcat, SuffixFilter +from fab.parse.fortran import FortranAnalyser, AnalysedFortran +from fab.parse.x90 import X90Analyser, AnalysedX90 +from fab.steps import Step, check_for_errors +from fab.steps.preprocess import PreProcessor +from fab.util import log_or_dot, input_to_output_fpath, file_checksum, file_walk, TimerLogger, \ + string_checksum, suffix_filter, by_type, log_or_dot_finish + +logger = logging.getLogger(__name__) + + +# todo: should this be part of the psyclone step? +def psyclone_preprocessor(common_flags: Optional[List[str]] = None): + common_flags = common_flags or [] + + return PreProcessor( + # todo: use env vars and param + preprocessor='cpp -traditional-cpp', + + source=SuffixFilter('all_source', '.X90'), + output_collection='preprocessed_x90', + + output_suffix='.x90', + name='preprocess x90', + common_flags=common_flags + ['-P'], + ) + + +@dataclass +class MpPayload: + """ + Runtime data for child processes to read. + + Contains data used to calculate the prebuild hash. + + """ + analysed_x90: Dict[Path, AnalysedX90] + all_kernel_hashes: Dict[str, int] + transformation_script_hash: int = 0 + + +DEFAULT_SOURCE_GETTER = CollectionConcat([ + 'preprocessed_x90', # any X90 we've preprocessed this run + SuffixFilter('all_source', '.x90'), # any already preprocessed x90 we pulled in +]) + + +class Psyclone(Step): + """ + Psyclone runner step. + + This step stores prebuilt results to speed up subsequent builds. + To generate the prebuild hashes, it analyses the X90 and kernel files, storing prebuilt results for these also. + + Kernel files are just normal Fortran, and the standard Fortran analyser is used to analyse them + + """ + def __init__(self, name=None, kernel_roots=None, + transformation_script: Optional[Path] = None, + cli_args: Optional[List[str]] = None, + source_getter: Optional[ArtefactsGetter] = None): + super().__init__(name=name or 'psyclone') + self.kernel_roots = kernel_roots or [] + self.transformation_script = transformation_script + + # "the gross switch which turns off MPI usage is a command-line argument" + self.cli_args: List[str] = cli_args or [] + + self.source_getter = source_getter or DEFAULT_SOURCE_GETTER + + @classmethod + def tool_available(cls) -> bool: + """Check if tje psyclone tool is available at the command line.""" + try: + run_command(['psyclone', '-h']) + except (RuntimeError, FileNotFoundError): + return False + return True + + def run(self, artefact_store: Dict, config): + super().run(artefact_store=artefact_store, config=config) + x90s = self.source_getter(artefact_store) + + # get the data for child processes to calculate prebuild hashes + mp_payload = self.analysis_for_prebuilds(x90s) + + # run psyclone. + # for every file, we get back a list of its output files plus a list of the prebuild copies. + mp_arg = [(x90, mp_payload) for x90 in x90s] + with TimerLogger(f"running psyclone on {len(x90s)} x90 files"): + results = self.run_mp(mp_arg, self.do_one_file) + log_or_dot_finish(logger) + outputs, prebuilds = zip(*results) if results else ((), ()) + check_for_errors(outputs, caller_label=self.name) + + # flatten the list of lists we got back from run_mp + output_files: List[Path] = list(chain(*by_type(outputs, List))) + prebuild_files: List[Path] = list(chain(*by_type(prebuilds, List))) + + # record the output files in the artefact store for further processing + artefact_store['psyclone_output'] = output_files + outputs_str = "\n".join(map(str, output_files)) + logger.debug(f'psyclone outputs:\n{outputs_str}\n') + + # mark the prebuild files as being current so the cleanup step doesn't delete them + config.add_current_prebuilds(prebuild_files) + prebuilds_str = "\n".join(map(str, prebuild_files)) + logger.debug(f'psyclone prebuilds:\n{prebuilds_str}\n') + + # todo: delete any psy layer files which have hand-written overrides, in a given overrides folder + # is this called psykal? + # assert False + + # todo: test that we can run this step before or after the analysis step + def analysis_for_prebuilds(self, x90s) -> MpPayload: + """ + Analysis for PSyclone prebuilds. + + In order to build reusable psyclone results, we need to know everything that goes into making one. + Then we can hash it all, and check for changes in subsequent builds. + We'll build up this data in a payload object, to be passed to the child processes. + + Changes which must trigger reprocessing of an x90 file: + - x90 source: + - kernel metadata used by the x90 + - transformation script + - cli args + + Later: + - the psyclone version, to cover changes to built-in kernels + + Kernels: + + Kernel metadata are type definitions passed to invoke(). + For example, this x90 code depends on the kernel `compute_total_mass_kernel_type`. + .. code-block:: fortran + + call invoke( name = "compute_dry_mass", & + compute_total_mass_kernel_type(dry_mass, rho, chi, panel_id, qr), & + sum_X(total_dry, dry_mass)) + + We can see this kernel in a use statement at the top of the x90. + .. code-block:: fortran + + use compute_total_mass_kernel_mod, only: compute_total_mass_kernel_type + + Some kernels, such as `setval_c`, are + `PSyclone built-ins `_. + They will not appear in use statements and can be ignored. + + The Psyclone and Analyse steps both use the generic Fortran analyser, which recognises Psyclone kernel metadata. + The Analysis step must come after this step because it needs to analyse the fortran we create. + + """ + # hash the transformation script + if self.transformation_script: + transformation_script_hash = file_checksum(self.transformation_script).file_hash + else: + warnings.warn('no transformation script specified') + transformation_script_hash = 0 + + # analyse the x90s + analysed_x90 = self._analyse_x90s(x90s) + + # Analyse the kernel files, hashing the psyclone kernel metadata. + # We only need the hashes right now but they all need analysing anyway, and we don't want to parse twice. + # We pass them through the general fortran analyser, which currently recognises kernel metadata. + # todo: We'd like to separate that from the general fortran analyser at some point, to reduce coupling. + all_kernel_hashes = self._analyse_kernels(self.kernel_roots) + + return MpPayload( + transformation_script_hash=transformation_script_hash, + analysed_x90=analysed_x90, + all_kernel_hashes=all_kernel_hashes + ) + + def _analyse_x90s(self, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: + # Analyse parsable versions of the x90s, finding kernel dependencies. + + # make parsable - todo: fast enough not to require prebuilds? + with TimerLogger(f"converting {len(x90s)} x90s into parsable fortran"): + parsable_x90s = self.run_mp(items=x90s, func=make_parsable_x90) + + # parse + x90_analyser = X90Analyser() + x90_analyser._config = self._config + with TimerLogger(f"analysing {len(parsable_x90s)} parsable x90 files"): + x90_results = self.run_mp(items=parsable_x90s, func=x90_analyser.run) + log_or_dot_finish(logger) + x90_analyses, x90_artefacts = zip(*x90_results) if x90_results else ((), ()) + check_for_errors(results=x90_analyses) + + # mark the analysis results files (i.e. prebuilds) as being current, so the cleanup knows not to delete them + prebuild_files = list(by_type(x90_artefacts, Path)) + self._config.add_current_prebuilds(prebuild_files) + + # record the analysis results against the original x90 filenames (not the parsable versions we analysed) + analysed_x90 = by_type(x90_analyses, AnalysedX90) + analysed_x90 = {result.fpath.with_suffix('.x90'): result for result in analysed_x90} + + # make the hashes from the original x90s, not the parsable versions which have invoke names removed. + for p, r in analysed_x90.items(): + analysed_x90[p]._file_hash = file_checksum(p).file_hash + + return analysed_x90 + + def _analyse_kernels(self, kernel_roots) -> Dict[str, int]: + # We want to hash the kernel metadata (type defs). + # Ignore the prebuild folder. Todo: test the prebuild folder is ignored, in case someone breaks this. + file_lists = [file_walk(root, ignore_folders=[self._config.prebuild_folder]) for root in kernel_roots] + all_kernel_files: Set[Path] = set(*chain(file_lists)) + kernel_files: List[Path] = suffix_filter(all_kernel_files, ['.f90']) + + # We use the normal Fortran analyser, which records psyclone kernel metadata. + # todo: We'd like to separate that from the general fortran analyser at some point, to reduce coupling. + # The Analyse step also uses the same fortran analyser. It stores its results so they won't be analysed twice. + fortran_analyser = FortranAnalyser() + fortran_analyser._config = self._config + with TimerLogger(f"analysing {len(kernel_files)} potential psyclone kernel files"): + fortran_results = self.run_mp(items=kernel_files, func=fortran_analyser.run) + log_or_dot_finish(logger) + fortran_analyses, fortran_artefacts = zip(*fortran_results) if fortran_results else (tuple(), tuple()) + + errors: List[Exception] = list(by_type(fortran_analyses, Exception)) + if errors: + errs_str = '\n\n'.join(map(str, errors)) + logger.error(f"There were {len(errors)} errors while parsing kernels:\n\n{errs_str}") + + # mark the analysis results files (i.e. prebuilds) as being current, so the cleanup knows not to delete them + prebuild_files = list(by_type(fortran_artefacts, Path)) + self._config.add_current_prebuilds(prebuild_files) + + analysed_fortran: List[AnalysedFortran] = list(by_type(fortran_analyses, AnalysedFortran)) + + # gather all kernel hashes into one big lump + all_kernel_hashes: Dict[str, int] = {} + for af in analysed_fortran: + assert set(af.psyclone_kernels).isdisjoint(all_kernel_hashes), \ + f"duplicate kernel name(s): {set(af.psyclone_kernels) & set(all_kernel_hashes)}" + all_kernel_hashes.update(af.psyclone_kernels) + + return all_kernel_hashes + + def do_one_file(self, arg): + x90_file, mp_payload = arg + prebuild_hash = self._gen_prebuild_hash(x90_file, mp_payload) + + # These are the filenames we expect to be output for this x90 input file. + # There will always be one modified_alg, and 0+ generated. + modified_alg = x90_file.with_suffix('.f90') + modified_alg = input_to_output_fpath(config=self._config, input_path=modified_alg) + generated = x90_file.parent / (str(x90_file.stem) + '_psy.f90') + generated = input_to_output_fpath(config=self._config, input_path=generated) + + generated.parent.mkdir(parents=True, exist_ok=True) + + # todo: do we have handwritten overrides? + + # do we already have prebuilt results for this x90 file? + prebuilt_alg, prebuilt_gen = self._get_prebuild_paths(modified_alg, generated, prebuild_hash) + if prebuilt_alg.exists(): + # todo: error handling in here + msg = f'found prebuilds for {x90_file}:\n {prebuilt_alg}' + shutil.copy2(prebuilt_alg, modified_alg) + if prebuilt_gen.exists(): + msg += f'\n {prebuilt_gen}' + shutil.copy2(prebuilt_gen, generated) + log_or_dot(logger=logger, msg=msg) + + else: + try: + # logger.info(f'running psyclone on {x90_file}') + self.run_psyclone(generated, modified_alg, x90_file) + + shutil.copy2(modified_alg, prebuilt_alg) + msg = f'created prebuilds for {x90_file}:\n {prebuilt_alg}' + if Path(generated).exists(): + msg += f'\n {prebuilt_gen}' + shutil.copy2(generated, prebuilt_gen) + log_or_dot(logger=logger, msg=msg) + + except Exception as err: + logger.error(err) + return err, None + + # return the output files from psyclone + result: List[Path] = [modified_alg] + if Path(generated).exists(): + result.append(generated) + + # we also want to return the prebuild artefact files we created, + # which are just copies, in the prebuild folder, with hashes in the filenames. + prebuild_result: List[Path] = [prebuilt_alg, prebuilt_gen] + + return result, prebuild_result + + def _gen_prebuild_hash(self, x90_file: Path, mp_payload: MpPayload): + """ + Calculate the prebuild hash for this x90 file, based on all the things which should trigger reprocessing. + + """ + # We've analysed (a parsable version of) this x90. + analysis_result = mp_payload.analysed_x90[x90_file] # type: ignore + + # include the hashes of kernels used by this x90 + kernel_deps_hashes = { + mp_payload.all_kernel_hashes[kernel_name] for kernel_name in analysis_result.kernel_deps} # type: ignore + + # hash everything which should trigger re-processing + # todo: hash the psyclone version in case the built-in kernels change? + prebuild_hash = sum([ + + # the hash of the x90 (not of the parsable version, so includes invoke names) + analysis_result.file_hash, + + # the hashes of the kernels used by this x90 + sum(kernel_deps_hashes), + + # + mp_payload.transformation_script_hash, + + # command-line arguments + string_checksum(str(self.cli_args)), + ]) + + return prebuild_hash + + def _get_prebuild_paths(self, modified_alg, generated, prebuild_hash): + prebuilt_alg = Path(self._config.prebuild_folder / f'{modified_alg.stem}.{prebuild_hash}{modified_alg.suffix}') + prebuilt_gen = Path(self._config.prebuild_folder / f'{generated.stem}.{prebuild_hash}{generated.suffix}') + return prebuilt_alg, prebuilt_gen + + def run_psyclone(self, generated, modified_alg, x90_file): + + # -d specifies "a root directory structure containing kernel source" + kernel_args = sum([['-d', k] for k in self.kernel_roots], []) + + # transformation python script + transform_options = ['-s', self.transformation_script] if self.transformation_script else [] + + command = [ + 'psyclone', '-api', 'dynamo0.3', + '-l', 'all', + *kernel_args, + '-opsy', generated, # filename of generated PSy code + '-oalg', modified_alg, # filename of transformed algorithm code + *transform_options, + *self.cli_args, + x90_file, + ] + + run_command(command) + + +# regex to convert an x90 into parsable fortran, so it can be analysed using a third party tool + +WHITE = r'[\s&]+' +OPT_WHITE = r'[\s&]*' + +SQ_STRING = "'[^']*'" +DQ_STRING = '"[^"]*"' +STRING = f'({SQ_STRING}|{DQ_STRING})' + +NAME_KEYWORD = 'name' + OPT_WHITE + '=' + OPT_WHITE + STRING + OPT_WHITE + ',' + OPT_WHITE +NAMED_INVOKE = 'call' + WHITE + 'invoke' + OPT_WHITE + r'\(' + OPT_WHITE + NAME_KEYWORD + +_x90_compliance_pattern = None + + +# todo: In the future, we'd like to extend fparser to handle the leading invoke keywords. (Lots of effort.) +def make_parsable_x90(x90_path: Path) -> Path: + """ + Take out the leading name keyword in calls to invoke(), making temporary, parsable fortran from x90s. + + If present it looks like this:: + + call invoke( name = "compute_dry_mass", ... + + Returns the path of the parsable file. + + This function is not slow so we're not creating prebuilds for this work. + + """ + global _x90_compliance_pattern + if not _x90_compliance_pattern: + _x90_compliance_pattern = re.compile(pattern=NAMED_INVOKE) + + # src = open(x90_path, 'rt').read() + + # Before we remove the name keywords to invoke, we must remove any comment lines. + # This is the simplest way to avoid producing bad fortran when the name keyword is followed by a comment line. + # I.e. The comment line doesn't have an "&", so we get "call invoke(!" with no "&", which is a syntax error. + src_lines = open(x90_path, 'rt').readlines() + no_comment_lines = [line for line in src_lines if not line.lstrip().startswith('!')] + src = ''.join(no_comment_lines) + + replaced = [] + + def repl(matchobj): + # matchobj[0] contains the entire matching string, from "call" to the "," after the name keyword. + # matchobj[1] contains the single group in the search pattern, which is defined in STRING. + name = matchobj[1].replace('"', '').replace("'", "") + replaced.append(name) + return 'call invoke(' + + out = _x90_compliance_pattern.sub(repl=repl, string=src) + + out_path = x90_path.with_suffix('.parsable_x90') + open(out_path, 'wt').write(out) + + logger.debug(f'names removed from {str(x90_path)}: {replaced}') + + return out_path diff --git a/source/fab/tools.py b/source/fab/tools.py index 6a89e3a4..46c874e6 100644 --- a/source/fab/tools.py +++ b/source/fab/tools.py @@ -7,12 +7,15 @@ Known command line tools whose flags we wish to manage. """ +import logging from pathlib import Path import subprocess import warnings from typing import Dict, List, Optional, Tuple, Union -from fab.util import logger, string_checksum +from fab.util import string_checksum + +logger = logging.getLogger(__name__) class Compiler(object): @@ -91,7 +94,8 @@ def run_command(command: List[str], env=None, cwd: Optional[Union[Path, str]] = If True, capture and return stdout. If False, the command will print its output directly to the console. """ - logger.debug(f'run_command: {command}') + dbg_msg = ' '.join(map(str, command)) + logger.debug(f'run_command: {dbg_msg}') res = subprocess.run(command, capture_output=capture_output, env=env, cwd=cwd) if res.returncode != 0: msg = f'Command failed:\n{command}' diff --git a/source/fab/util.py b/source/fab/util.py index e4555a73..7fe51cb8 100644 --- a/source/fab/util.py +++ b/source/fab/util.py @@ -87,10 +87,9 @@ def file_walk(path: Union[str, Path], ignore_folders: Optional[List[Path]] = Non The prebuild folder can contain multiple versions of a single, generated fortran file, created by multiple runs of the build config. The prebuild folder stores these copies for when they're next - needed, when they are copied out and reused. We often don't want to include this folder when + needed, when they are copied out and reused. We usually won't want to include this folder when searching for source code to analyse. - - To meet these needs, this function will not traverse *into* the given folders, if provided. + To meet these needs, this function will not traverse into the given folders, if provided. """ path = Path(path) @@ -194,17 +193,31 @@ def input_to_output_fpath(config, input_path: Path): :param input_path: The path to transform from input to output folders. + Note: This function can also handle paths which are not in the project workspace at all. + This can happen when pointing the FindFiles step elsewhere, for example. + In that case, the entire path will be made relative to the source folder instead of its anchor. + """ build_output = config.build_output - # perhaps it's already in the output folder? todo: can use Path.is_relative_to from Python 3.9 + # perhaps it's already in the output folder? try: input_path.relative_to(build_output) return input_path except ValueError: pass - rel_path = input_path.relative_to(config.source_root) - return build_output / rel_path + + # try to convert it from the project source folder to the output folder + try: + rel_path = input_path.relative_to(config.source_root) + return build_output / rel_path + except ValueError: + pass + + # It's neither in the project source folder nor the output folder. + # This can happen if we're pointing the FindFiles step elsewhere. + # We'll just have to convert the entire path to be inside the output folder. + return build_output / '/'.join(input_path.parts[1:]) def suffix_filter(fpaths: Iterable[Path], suffixes: Iterable[str]): @@ -248,7 +261,7 @@ def get_fab_workspace() -> Path: return fab_workspace -def get_prebuild_file_groups(prebuild_files) -> Dict[str, Set]: +def get_prebuild_file_groups(prebuild_files: Iterable[Path]) -> Dict[str, Set]: """ Group prebuild filenames by originating artefact. @@ -270,16 +283,17 @@ def get_prebuild_file_groups(prebuild_files) -> Dict[str, Set]: return pbf_groups -def common_arg_parser(): +def common_arg_parser() -> ArgumentParser: """ A helper function returning an argument parser with common, useful arguments controlling command line tools. - More arguments can be added as needed by the calling code. + More arguments can be added. The caller must call `parse_args` on the returned parser. """ # consider adding preprocessor, linker, optimisation, two-stage arg_parser = ArgumentParser() arg_parser.add_argument('--compiler', default=None) arg_parser.add_argument('--two-stage', action='store_true') + arg_parser.add_argument('--verbose', action='store_true') return arg_parser diff --git a/system-tests-new/FortranDependencies/test_FortranDependencies.py b/system-tests-new/FortranDependencies/test_FortranDependencies.py index 50f803b3..e3e651e3 100644 --- a/system-tests-new/FortranDependencies/test_FortranDependencies.py +++ b/system-tests-new/FortranDependencies/test_FortranDependencies.py @@ -8,11 +8,11 @@ from fab.build_config import BuildConfig from fab.constants import EXECUTABLES +from fab.parse.fortran import AnalysedFortran from fab.steps.analyse import Analyse from fab.steps.compile_c import CompileC from fab.steps.compile_fortran import CompileFortran from fab.steps.find_source_files import FindSourceFiles -from fab.parse.fortran import AnalysedFortran from fab.steps.grab.folder import GrabFolder from fab.steps.link import LinkExe from fab.steps.preprocess import fortran_preprocessor diff --git a/system-tests-new/incremental/project-source/my_mod.F90 b/system-tests-new/incremental_fortran/project-source/my_mod.F90 similarity index 100% rename from system-tests-new/incremental/project-source/my_mod.F90 rename to system-tests-new/incremental_fortran/project-source/my_mod.F90 diff --git a/system-tests-new/incremental/project-source/my_prog.F90 b/system-tests-new/incremental_fortran/project-source/my_prog.F90 similarity index 100% rename from system-tests-new/incremental/project-source/my_prog.F90 rename to system-tests-new/incremental_fortran/project-source/my_prog.F90 diff --git a/system-tests-new/incremental/test_incremental.py b/system-tests-new/incremental_fortran/test_incremental_fortran.py similarity index 100% rename from system-tests-new/incremental/test_incremental.py rename to system-tests-new/incremental_fortran/test_incremental_fortran.py diff --git a/system-tests-new/psyclone/algorithm.x90 b/system-tests-new/psyclone/algorithm.x90 new file mode 100644 index 00000000..6464f3aa --- /dev/null +++ b/system-tests-new/psyclone/algorithm.x90 @@ -0,0 +1,90 @@ + +module algorithm_mod + + use kernel_mod, only : kernel_one_type, kernel_two_type + + implicit none + + private + + public :: my_subroutine_one + +contains + + subroutine my_subroutine_one(a, b) + + implicit none + + integer :: a + integer :: b + + ! one line + call invoke( name = "name a", kernel_one_type( a, b ), kernel_two_type( a, b ), built_in() ) + ! no spaces + call invoke(name="name b",kernel_one_type(a,b),kernel_two_type(a,b),built_in()) + + ! arg lines + call invoke( name = "name c", & + ! a comment line after the name keyword was causing a syntax error, as there was no & + kernel_one_type( a, b ), & + kernel_two_type( a, b ), & + built_in() ) + ! no spaces + call invoke(name="name d",& + kernel_one_type(a,b),& + kernel_two_type(a,b),& + built_in()) + + ! arg arg lines + call invoke( & + name = "name e", & + kernel_one_type( a, & + b ), & + kernel_two_type( a, & + b ), & + built_in() ) + ! no spaces + call invoke(& + name="name f",& + kernel_one_type(a,& + b),& + kernel_two_type(a,& + b),& + built_in()) + + end subroutine my_subroutine_one + + subroutine my_subroutine_two(a, b) + + implicit none + + integer :: a + integer :: b + + ! like subroutine_one, but without the name keyword + call invoke(kernel_one_type( a, b ), kernel_two_type( a, b ), built_in() ) + call invoke(kernel_one_type(a,b),kernel_two_type(a,b),built_in()) + + call invoke( kernel_one_type( a, b ), & + kernel_two_type( a, b ), & + built_in() ) + call invoke(kernel_one_type(a,b),& + kernel_two_type(a,b),& + built_in()) + + call invoke( & + kernel_one_type( a, & + b ), & + kernel_two_type( a, & + b ), & + built_in() ) + call invoke(& + kernel_one_type(a,& + b),& + kernel_two_type(a,& + b),& + built_in()) + + end subroutine my_subroutine_two + +end module algorithm_mod diff --git a/system-tests-new/psyclone/expect.parsable_x90 b/system-tests-new/psyclone/expect.parsable_x90 new file mode 100644 index 00000000..726b691a --- /dev/null +++ b/system-tests-new/psyclone/expect.parsable_x90 @@ -0,0 +1,76 @@ + +module algorithm_mod + + use kernel_mod, only : kernel_one_type, kernel_two_type + + implicit none + + private + + public :: my_subroutine_one + +contains + + subroutine my_subroutine_one(a, b) + + implicit none + + integer :: a + integer :: b + + call invoke(kernel_one_type( a, b ), kernel_two_type( a, b ), built_in() ) + call invoke(kernel_one_type(a,b),kernel_two_type(a,b),built_in()) + + call invoke(kernel_one_type( a, b ), & + kernel_two_type( a, b ), & + built_in() ) + call invoke(kernel_one_type(a,b),& + kernel_two_type(a,b),& + built_in()) + + call invoke(kernel_one_type( a, & + b ), & + kernel_two_type( a, & + b ), & + built_in() ) + call invoke(kernel_one_type(a,& + b),& + kernel_two_type(a,& + b),& + built_in()) + + end subroutine my_subroutine_one + + subroutine my_subroutine_two(a, b) + + implicit none + + integer :: a + integer :: b + + call invoke(kernel_one_type( a, b ), kernel_two_type( a, b ), built_in() ) + call invoke(kernel_one_type(a,b),kernel_two_type(a,b),built_in()) + + call invoke( kernel_one_type( a, b ), & + kernel_two_type( a, b ), & + built_in() ) + call invoke(kernel_one_type(a,b),& + kernel_two_type(a,b),& + built_in()) + + call invoke( & + kernel_one_type( a, & + b ), & + kernel_two_type( a, & + b ), & + built_in() ) + call invoke(& + kernel_one_type(a,& + b),& + kernel_two_type(a,& + b),& + built_in()) + + end subroutine my_subroutine_two + +end module algorithm_mod diff --git a/system-tests-new/psyclone/kernel.f90 b/system-tests-new/psyclone/kernel.f90 new file mode 100644 index 00000000..0bc80753 --- /dev/null +++ b/system-tests-new/psyclone/kernel.f90 @@ -0,0 +1,56 @@ +! four kernels, only two of which are used by the x90 + +module kernel_mod + +use some_other_kernel_mod, only : kernel_type + +implicit none + +private + +!> The type declaration for the kernel. Contains the metadata needed by the Psy layer +type, public, extends(kernel_type) :: kernel_one_type + private + type(arg_type) :: meta_args(2) = (/ & + arg_type(GH_FIELD, GH_REAL, GH_WRITE, Wtheta), & + arg_type(GH_FIELD, GH_REAL, GH_READ, W2) & + /) + integer :: operates_on = CELL_COLUMN +contains + procedure, nopass :: kernel_one_code +end type + +type, public, extends(kernel_type) :: kernel_two_type + private + type(arg_type) :: meta_args(2) = (/ & + arg_type(GH_FIELD, GH_REAL, GH_WRITE, Wtheta), & + arg_type(GH_FIELD, GH_REAL, GH_READ, W2) & + /) + integer :: operates_on = CELL_COLUMN +contains + procedure, nopass :: kernel_two_code +end type + +type, public, extends(kernel_type) :: kernel_three_type + private + type(arg_type) :: meta_args(2) = (/ & + arg_type(GH_FIELD, GH_REAL, GH_WRITE, Wtheta), & + arg_type(GH_FIELD, GH_REAL, GH_READ, W2) & + /) + integer :: operates_on = CELL_COLUMN +contains + procedure, nopass :: kernel_two_code +end type + +type, public, extends(kernel_type) :: kernel_four_type + private + type(arg_type) :: meta_args(2) = (/ & + arg_type(GH_FIELD, GH_REAL, GH_WRITE, Wtheta), & + arg_type(GH_FIELD, GH_REAL, GH_READ, W2) & + /) + integer :: operates_on = CELL_COLUMN +contains + procedure, nopass :: kernel_two_code +end type + +end module kernel_mod diff --git a/system-tests-new/psyclone/skeleton/algorithm/algorithm_mod.x90 b/system-tests-new/psyclone/skeleton/algorithm/algorithm_mod.x90 new file mode 100644 index 00000000..62b412bf --- /dev/null +++ b/system-tests-new/psyclone/skeleton/algorithm/algorithm_mod.x90 @@ -0,0 +1,57 @@ +!----------------------------------------------------------------------------- +! (c) Crown copyright 2017 Met Office. All rights reserved. +! The file LICENCE, distributed with this code, contains details of the terms +! under which the code may be used. +!----------------------------------------------------------------------------- + +!>@brief Barebones algorithm to help the development of miniapps +module algorithm_mod + + use constants_mod, only: i_def,r_def + use log_mod, only: log_event, & + LOG_LEVEL_INFO + use mesh_mod, only: mesh_type + use field_mod, only: field_type + use finite_element_config_mod, only: element_order + use fs_continuity_mod, only: W2 + use function_space_collection_mod, only: function_space_collection + use operator_mod, only: operator_type + use my_kernel_mod, only: my_kernel_type + use skeleton_constants_mod, only: get_div + + implicit none + + private + + public :: skeleton_alg + +contains + + !> @details An algorithm for developing miniapps + !> @param[inout] field_1 A prognostic field object + subroutine skeleton_alg(field_1) + + implicit none + + ! Prognostic fields + type( field_type ), intent( inout ) :: field_1 + + ! Diagnostic fields + type( field_type ) :: field_2 + + real(r_def) :: s + type(mesh_type), pointer :: mesh => null() + type( operator_type ), pointer :: divergence => null() + + ! Set the new field to a constant value and compute the divergence of it + divergence => get_div() + s = 2.0_r_def + call invoke( name = "Compute divergence", & + setval_c(field_2, s ), & + setval_c(field_1, 0.0_r_def), & + my_kernel_type(field_1, field_2, divergence) ) + + + end subroutine skeleton_alg + +end module algorithm_mod diff --git a/system-tests-new/psyclone/skeleton/kernel/my_kernel_mod.F90 b/system-tests-new/psyclone/skeleton/kernel/my_kernel_mod.F90 new file mode 100644 index 00000000..de9cbc1d --- /dev/null +++ b/system-tests-new/psyclone/skeleton/kernel/my_kernel_mod.F90 @@ -0,0 +1,89 @@ + +module my_kernel_mod + use argument_mod, only : arg_type, & + GH_FIELD, GH_OPERATOR, & + GH_REAL, GH_READ, GH_INC, & + ANY_SPACE_1, ANY_SPACE_2, & + CELL_COLUMN + use constants_mod, only : i_def, r_single, r_double + use kernel_mod, only : kernel_type + + implicit none + + private + + type, public, extends(kernel_type) :: my_kernel_type + private + type(arg_type) :: meta_args(3) = (/ & + arg_type(GH_FIELD, GH_REAL, GH_INC, ANY_SPACE_1), & + arg_type(GH_FIELD, GH_REAL, GH_READ, ANY_SPACE_2), & + arg_type(GH_OPERATOR, GH_REAL, GH_READ, ANY_SPACE_1, ANY_SPACE_2) & + /) + integer :: operates_on = CELL_COLUMN + end type + + public :: matrix_vector_code + + interface matrix_vector_code + module procedure & + matrix_vector_code_r_single, & + matrix_vector_code_r_double + end interface + +contains + + subroutine matrix_vector_code_r_single(cell, & + nlayers, & + lhs, x, & + ncell_3d, & + matrix, & + ndf1, undf1, map1, & + ndf2, undf2, map2) + + implicit none + + ! Arguments + integer(kind=i_def), intent(in) :: cell, nlayers, ncell_3d + integer(kind=i_def), intent(in) :: undf1, ndf1 + integer(kind=i_def), intent(in) :: undf2, ndf2 + integer(kind=i_def), dimension(ndf1), intent(in) :: map1 + integer(kind=i_def), dimension(ndf2), intent(in) :: map2 + real(kind=r_single), dimension(undf2), intent(in) :: x + real(kind=r_single), dimension(undf1), intent(inout) :: lhs + real(kind=r_single), dimension(ndf1,ndf2,ncell_3d), intent(in) :: matrix + + ! Internal variables + integer(kind=i_def) :: df, k, ik + real(kind=r_single), dimension(ndf2) :: x_e + real(kind=r_single), dimension(ndf1) :: lhs_e + + end subroutine matrix_vector_code_r_single + + subroutine matrix_vector_code_r_double(cell, & + nlayers, & + lhs, x, & + ncell_3d, & + matrix, & + ndf1, undf1, map1, & + ndf2, undf2, map2) + + implicit none + + ! Arguments + integer(kind=i_def), intent(in) :: cell, nlayers, ncell_3d + integer(kind=i_def), intent(in) :: undf1, ndf1 + integer(kind=i_def), intent(in) :: undf2, ndf2 + integer(kind=i_def), dimension(ndf1), intent(in) :: map1 + integer(kind=i_def), dimension(ndf2), intent(in) :: map2 + real(kind=r_double), dimension(undf2), intent(in) :: x + real(kind=r_double), dimension(undf1), intent(inout) :: lhs + real(kind=r_double), dimension(ndf1,ndf2,ncell_3d), intent(in) :: matrix + + ! Internal variables + integer(kind=i_def) :: df, k, ik + real(kind=r_double), dimension(ndf2) :: x_e + real(kind=r_double), dimension(ndf1) :: lhs_e + + end subroutine matrix_vector_code_r_double + +end module my_kernel_mod diff --git a/system-tests-new/psyclone/skeleton/skeleton.f90 b/system-tests-new/psyclone/skeleton/skeleton.f90 new file mode 100644 index 00000000..61ad43bb --- /dev/null +++ b/system-tests-new/psyclone/skeleton/skeleton.f90 @@ -0,0 +1,10 @@ + +program skeleton + + use algorithm_mod, only : algorithm + + implicit none + + call skeleton_alg() + +end program skeleton diff --git a/system-tests-new/psyclone/test_psyclone.py b/system-tests-new/psyclone/test_psyclone.py new file mode 100644 index 00000000..e3bf4ff0 --- /dev/null +++ b/system-tests-new/psyclone/test_psyclone.py @@ -0,0 +1,181 @@ +# ############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +# ############################################################################## +import filecmp +import shutil +from os import unlink +from pathlib import Path +from typing import Tuple +from unittest import mock + +import pytest + +from fab.build_config import BuildConfig +from fab.parse.x90 import X90Analyser, AnalysedX90 +from fab.steps.find_source_files import FindSourceFiles +from fab.steps.grab.folder import GrabFolder +from fab.steps.preprocess import fortran_preprocessor +from fab.steps.psyclone import make_parsable_x90, Psyclone, psyclone_preprocessor, MpPayload +from fab.util import file_checksum + +SAMPLE_KERNEL = Path(__file__).parent / 'kernel.f90' + +# this x90 has "name=" keywords and is not parsable fortran +SAMPLE_X90 = Path(__file__).parent / 'algorithm.x90' + +# this is the sanitised version, with the name keywords removed, so it is parsable fortran +EXPECT_PARSABLE_X90 = Path(__file__).parent / 'expect.parsable_x90' + +# the name keywords which are removed from the x90 +NAME_KEYWORDS = ['name a', 'name b', 'name c', 'name d', 'name e', 'name f'] + + +# todo: Tidy up the test data in here. There are two sample projects, one not even in its own subfolder. +# Make the skeleton sample call more than one kernel. +# Make the skeleton sample include a big X90 and a little x90. + + +def test_make_parsable_x90(tmp_path): + # turn x90 into parsable fortran by removing the name keyword from calls to invoke + grab_x90_path = SAMPLE_X90 + input_x90_path = tmp_path / grab_x90_path.name + shutil.copy(grab_x90_path, input_x90_path) + + parsable_x90_path = make_parsable_x90(input_x90_path) + + # the point of this function is to make the file parsable by fparser + x90_analyser = X90Analyser() + x90_analyser._config = BuildConfig('proj', fab_workspace=tmp_path) + x90_analyser._config._prep_output_folders() + x90_analyser.run(parsable_x90_path) + + # ensure the files are as expected + assert filecmp.cmp(parsable_x90_path, EXPECT_PARSABLE_X90) + + # make_parsable_x90() puts its output file next to the source. + # Because we're reading sample code from our Fab git repos, + # we don't want to leave this test output in our working copies, so delete it. + # Otherwise, it'll appear in the output from `git status`. + unlink(parsable_x90_path) + + +class TestX90Analyser(object): + + expected_analysis_result = AnalysedX90( + fpath=EXPECT_PARSABLE_X90, + file_hash=3906123776, + kernel_deps={'kernel_one_type', 'kernel_two_type'}) + + def run(self, tmp_path) -> Tuple[AnalysedX90, Path]: + parsable_x90_path = self.expected_analysis_result.fpath + x90_analyser = X90Analyser() + x90_analyser._config = BuildConfig('proj', fab_workspace=tmp_path) + x90_analyser._config._prep_output_folders() + return x90_analyser.run(parsable_x90_path) # type: ignore + + def test_vanilla(self, tmp_path): + analysed_x90, _ = self.run(tmp_path) + assert analysed_x90 == self.expected_analysis_result + + def test_prebuild(self, tmp_path): + self.run(tmp_path) + + # Run it a second time, ensure it's not re-processed and still gives the correct result + with mock.patch('fab.parse.x90.X90Analyser.walk_nodes') as mock_walk: + analysed_x90, _ = self.run(tmp_path) + mock_walk.assert_not_called() + assert analysed_x90 == self.expected_analysis_result + + +class Test_analysis_for_prebuilds(object): + + @pytest.fixture + def psyclone_step(self, tmp_path) -> Psyclone: + psyclone_step = Psyclone(kernel_roots=[Path(__file__).parent], transformation_script=__file__) + psyclone_step._config = BuildConfig('proj', fab_workspace=tmp_path) + psyclone_step._config._prep_output_folders() + return psyclone_step + + def test_analyse(self, psyclone_step): + + mp_payload: MpPayload = psyclone_step.analysis_for_prebuilds(x90s=[SAMPLE_X90]) + + # transformation_script_hash + assert mp_payload.transformation_script_hash == file_checksum(__file__).file_hash + + # analysed_x90 + assert mp_payload.analysed_x90 == { + SAMPLE_X90: AnalysedX90( + fpath=SAMPLE_X90.with_suffix('.parsable_x90'), + file_hash=file_checksum(SAMPLE_X90).file_hash, + kernel_deps={'kernel_one_type', 'kernel_two_type'})} + + # all_kernel_hashes + assert mp_payload.all_kernel_hashes == { + 'kernel_one_type': 2915127408, + 'kernel_two_type': 3793991362, + 'kernel_three_type': 319981435, + 'kernel_four_type': 1427207736, + } + + +@pytest.mark.skipif(not Psyclone.tool_available(), reason="psyclone cli tool not available") +class TestPsyclone(object): + """ + Basic run of the psyclone step. + + """ + @pytest.fixture + def config(self, tmp_path): + here = Path(__file__).parent + + config = BuildConfig('proj', fab_workspace=tmp_path, verbose=True, multiprocessing=False) + config.steps = [ + GrabFolder(here / 'skeleton'), + FindSourceFiles(), + fortran_preprocessor(preprocessor='cpp -traditional-cpp', common_flags=['-P']), + + psyclone_preprocessor(), + # todo: it's easy to forget that we need to find the f90 not the F90. + # it manifests as an error, a missing kernel hash. + # Perhaps add validation, warn if it's not in the build_output folder? + Psyclone(kernel_roots=[config.build_output / 'kernel']), + ] + + return config + + def test_run(self, config): + # if these files exist after the run then we know: + # a) the expected files were created + # b) the prebuilds were protected from automatic cleanup + expect_files = [ + # there should be an f90 and a _psy.f90 built from the x90 + config.build_output / 'algorithm/algorithm_mod.f90', + config.build_output / 'algorithm/algorithm_mod_psy.f90', + + # Expect these prebuild files + # todo: the kernal hash differs between fpp and cpp, perhaps just use wildcards. + config.prebuild_folder / 'algorithm_mod.1602753696.an', # x90 analysis result + config.prebuild_folder / 'my_kernel_mod.4187107526.an', # kernel analysis results + config.prebuild_folder / 'algorithm_mod.5088673431.f90', # prebuild + config.prebuild_folder / 'algorithm_mod_psy.5088673431.f90', # prebuild + ] + + assert all(not f.exists() for f in expect_files) + config.run() + assert all(f.exists() for f in expect_files) + + def test_prebuild(self, tmp_path, config): + config.run() + + # make sure no work gets done the second time round + with mock.patch('fab.parse.x90.X90Analyser.walk_nodes') as mock_x90_walk: + with mock.patch('fab.parse.fortran.FortranAnalyser.walk_nodes') as mock_fortran_walk: + with mock.patch('fab.steps.psyclone.Psyclone.run_psyclone') as mock_run: + config.run() + + mock_x90_walk.assert_not_called() + mock_fortran_walk.assert_not_called() + mock_run.assert_not_called() diff --git a/tests/integration_tests/test_nothing_lol.py b/tests/integration_tests/test_nothing_lol.py deleted file mode 100644 index 83d6893a..00000000 --- a/tests/integration_tests/test_nothing_lol.py +++ /dev/null @@ -1,10 +0,0 @@ -# ############################################################################## -# (c) Crown copyright Met Office. All rights reserved. -# For further details please refer to the file COPYRIGHT -# which you should have received as part of this distribution -# ############################################################################## - -class TestNothingLol(object): - - def test_nothing(self): - assert 'We still need this folder in this pr, until the mypy config change is merged. How silly :)' diff --git a/tests/unit_tests/parse/c/test_c_analyser.py b/tests/unit_tests/parse/c/test_c_analyser.py index e511c374..2691b6ec 100644 --- a/tests/unit_tests/parse/c/test_c_analyser.py +++ b/tests/unit_tests/parse/c/test_c_analyser.py @@ -8,9 +8,9 @@ from unittest.mock import Mock import clang # type: ignore -from fab.parse.c import CAnalyser, AnalysedC from fab.build_config import BuildConfig +from fab.parse.c import CAnalyser, AnalysedC def test_simple_result(tmp_path): @@ -26,7 +26,6 @@ def test_simple_result(tmp_path): file_hash=1429445462, symbol_deps={'usr_var', 'usr_func'}, symbol_defs={'func_decl', 'func_def', 'var_def', 'var_extern_def', 'main'}, - file_deps=set(), ) assert analysis == expected assert artefact == c_analyser._config.prebuild_folder / f'test_c_analyser.{analysis.file_hash}.an' diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.py b/tests/unit_tests/parse/fortran/test_fortran_analyser.py index a29f897a..25a1de20 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.py +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.py @@ -7,17 +7,15 @@ from tempfile import NamedTemporaryFile from unittest import mock -import pytest -from fab.parse.fortran_common import iter_content - -from fab.parse import EmptySourceFile - -from fab.parse.fortran import AnalysedFortran, FortranAnalyser from fparser.common.readfortran import FortranFileReader # type: ignore from fparser.two.Fortran2008 import Type_Declaration_Stmt # type: ignore from fparser.two.parser import ParserFactory # type: ignore +import pytest from fab.build_config import BuildConfig +from fab.parse import EmptySourceFile +from fab.parse.fortran import FortranAnalyser, AnalysedFortran +from fab.parse.fortran_common import iter_content # todo: test function binding diff --git a/tests/unit_tests/parse/test_x90.py b/tests/unit_tests/parse/test_x90.py index fb81fe6c..4cf4aecf 100644 --- a/tests/unit_tests/parse/test_x90.py +++ b/tests/unit_tests/parse/test_x90.py @@ -16,31 +16,20 @@ class TestAnalysedX90(object): def analysed_x90(self): return AnalysedX90( fpath=Path('foo.f90'), file_hash=123, - kernel_deps={ - 'kernel_one_type': 'kernel_module_one', - 'kernel_two_type': 'kernel_module_two', - } - ) + kernel_deps={'kernel_one_type', 'kernel_two_type'}) @pytest.fixture def different_kernel_deps(self): return AnalysedX90( fpath=Path('foo.f90'), file_hash=123, - kernel_deps={ - 'kernel_three_type': 'kernel_module_three', - 'kernel_four_type': 'kernel_module_four', - } - ) + kernel_deps={'kernel_three_type', 'kernel_four_type'}) @pytest.fixture def as_dict(self): return { 'fpath': 'foo.f90', 'file_hash': 123, - 'kernel_deps': { - 'kernel_one_type': 'kernel_module_one', - 'kernel_two_type': 'kernel_module_two', - }, + 'kernel_deps': ['kernel_one_type', 'kernel_two_type'], } def test_to_dict(self, analysed_x90, as_dict): diff --git a/tests/unit_tests/steps/test_analyse.py b/tests/unit_tests/steps/test_analyse.py index b9629b9c..57ddab54 100644 --- a/tests/unit_tests/steps/test_analyse.py +++ b/tests/unit_tests/steps/test_analyse.py @@ -2,10 +2,10 @@ from unittest import mock import pytest -from fab.parse.fortran import FortranParserWorkaround, AnalysedFortran from fab.build_config import BuildConfig from fab.dep_tree import AnalysedDependent +from fab.parse.fortran import FortranParserWorkaround, AnalysedFortran from fab.steps.analyse import Analyse from fab.util import HashedFile @@ -30,7 +30,6 @@ def analysed_files(self): def test_vanilla(self, analysed_files): analyser = Analyse(root_symbol=None) - result = analyser._gen_symbol_table(analysed_files=analysed_files) assert result == { @@ -43,7 +42,6 @@ def test_vanilla(self, analysed_files): def test_duplicate_symbol(self, analysed_files): # duplicate a symbol from the first file in the second file analysed_files[1].symbol_defs.add('foo_1') - analyser = Analyse(root_symbol=None) with pytest.warns(UserWarning): @@ -79,13 +77,14 @@ def test_vanilla(self, analyser): assert analysed_files[0].file_deps == {symbols['dep1_mod'], symbols['dep2']} +# todo: this is fortran-ey, move it? class Test_add_unreferenced_deps(object): def test_vanilla(self): analyser = Analyse(root_symbol=None) # we analysed the source folder and found these symbols - symbols = { + symbol_table = { "root": Path("root.f90"), "root_dep": Path("root_dep.f90"), "util": Path("util.f90"), @@ -94,8 +93,8 @@ def test_vanilla(self): # we extracted the build tree build_tree = { - Path('root.f90'): AnalysedDependent(fpath=Path(), file_hash=0), - Path('root_dep.f90'): AnalysedDependent(fpath=Path(), file_hash=0), + Path('root.f90'): AnalysedFortran(fpath=Path(), file_hash=0), + Path('root_dep.f90'): AnalysedFortran(fpath=Path(), file_hash=0), } # we want to force this symbol into the build (because it's not used via modules) @@ -104,11 +103,12 @@ def test_vanilla(self): # the stuff to add to the build tree will be found in here all_analysed_files = { # root.f90 and root_util.f90 would also be in here but the test doesn't need them - Path('util.f90'): AnalysedDependent(fpath=Path('util.f90'), file_deps={Path('util_dep.f90')}, file_hash=0), - Path('util_dep.f90'): AnalysedDependent(fpath=Path('util_dep.f90'), file_hash=0), + Path('util.f90'): AnalysedFortran(fpath=Path('util.f90'), file_deps={Path('util_dep.f90')}, file_hash=0), + Path('util_dep.f90'): AnalysedFortran(fpath=Path('util_dep.f90'), file_hash=0), } - analyser._add_unreferenced_deps(symbols=symbols, all_analysed_files=all_analysed_files, build_tree=build_tree) + analyser._add_unreferenced_deps( + symbol_table=symbol_table, all_analysed_files=all_analysed_files, build_tree=build_tree) assert Path('util.f90') in build_tree assert Path('util_dep.f90') in build_tree @@ -121,6 +121,9 @@ def test_vanilla(self): class Test_parse_files(object): + # todo: test the correct artefacts are marked as current for the cleanup step + # todo: this method should be tested a bit more thoroughly + def test_exceptions(self, tmp_path): # make sure parse exceptions do not stop the build with mock.patch('fab.steps.Step.run_mp', return_value=[(Exception('foo'), None)]): @@ -131,6 +134,7 @@ def test_exceptions(self, tmp_path): class Test_add_manual_results(object): + # test user-specified analysis results, for when fparser fails to parse a valid file. def test_vanilla(self): # test normal usage of manual analysis results diff --git a/tests/unit_tests/steps/test_compile_fortran.py b/tests/unit_tests/steps/test_compile_fortran.py index 033eaecf..12a85bed 100644 --- a/tests/unit_tests/steps/test_compile_fortran.py +++ b/tests/unit_tests/steps/test_compile_fortran.py @@ -4,10 +4,10 @@ from unittest.mock import call import pytest -from fab.parse.fortran import AnalysedFortran from fab.build_config import BuildConfig from fab.constants import BUILD_TREES, OBJECT_FILES +from fab.parse.fortran import AnalysedFortran from fab.steps.compile_fortran import CompileFortran, get_mod_hashes from fab.util import CompiledFile diff --git a/tests/unit_tests/steps/test_link.py b/tests/unit_tests/steps/test_link.py index caa5f437..a59aa98e 100644 --- a/tests/unit_tests/steps/test_link.py +++ b/tests/unit_tests/steps/test_link.py @@ -7,7 +7,6 @@ from unittest import mock from fab.constants import OBJECT_FILES - from fab.steps.link import LinkExe diff --git a/tests/unit_tests/steps/test_psyclone.py b/tests/unit_tests/steps/test_psyclone.py new file mode 100644 index 00000000..7ce5ec15 --- /dev/null +++ b/tests/unit_tests/steps/test_psyclone.py @@ -0,0 +1,84 @@ +# ############################################################################## +# (c) Crown copyright Met Office. All rights reserved. +# For further details please refer to the file COPYRIGHT +# which you should have received as part of this distribution +# ############################################################################## +from pathlib import Path +from typing import Tuple + +import pytest + +from fab.build_config import BuildConfig +from fab.parse.x90 import AnalysedX90 +from fab.steps.psyclone import MpPayload, Psyclone + + +class Test_gen_prebuild_hash(object): + """ + Tests for the prebuild hashing calculation. + + """ + @pytest.fixture + def data(self, tmp_path) -> Tuple[Psyclone, MpPayload, Path, int]: + config = BuildConfig('proj', fab_workspace=tmp_path) + config._prep_output_folders() + + psyclone_step = Psyclone(kernel_roots=[Path(__file__).parent]) + psyclone_step._config = config + + transformation_script_hash = 123 + + x90_file = Path('foo.x90') + analysed_x90 = { + x90_file: AnalysedX90( + fpath=x90_file, + file_hash=234, + kernel_deps={'kernel1', 'kernel2'}) + } + + all_kernel_hashes = { + 'kernel1': 345, + 'kernel2': 456, + } + + expect_hash = 223133615 + + mp_payload = MpPayload( + transformation_script_hash=transformation_script_hash, + analysed_x90=analysed_x90, + all_kernel_hashes=all_kernel_hashes, + ) + return psyclone_step, mp_payload, x90_file, expect_hash + + def test_vanilla(self, data): + psyclone_step, mp_payload, x90_file, expect_hash = data + result = psyclone_step._gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + assert result == expect_hash + + def test_file_hash(self, data): + # changing the file hash should change the hash + psyclone_step, mp_payload, x90_file, expect_hash = data + mp_payload.analysed_x90[x90_file]._file_hash += 1 + result = psyclone_step._gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + assert result == expect_hash + 1 + + def test_kernal_deps(self, data): + # changing a kernel deps hash should change the hash + psyclone_step, mp_payload, x90_file, expect_hash = data + mp_payload.all_kernel_hashes['kernel1'] += 1 + result = psyclone_step._gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + assert result == expect_hash + 1 + + def test_trans_script(self, data): + # changing the transformation script should change the hash + psyclone_step, mp_payload, x90_file, expect_hash = data + mp_payload.transformation_script_hash += 1 + result = psyclone_step._gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + assert result == expect_hash + 1 + + def test_cli_args(self, data): + # changing the cli args should change the hash + psyclone_step, mp_payload, x90_file, expect_hash = data + psyclone_step.cli_args = ['--foo'] + result = psyclone_step._gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + assert result != expect_hash diff --git a/tests/unit_tests/test_artefacts.py b/tests/unit_tests/test_artefacts.py index 64e71369..bdd0dce2 100644 --- a/tests/unit_tests/test_artefacts.py +++ b/tests/unit_tests/test_artefacts.py @@ -3,9 +3,8 @@ import pytest -from fab.constants import BUILD_TREES - from fab.artefacts import FilterBuildTrees +from fab.constants import BUILD_TREES class TestFilterBuildTrees(object): diff --git a/tests/unit_tests/test_build_config.py b/tests/unit_tests/test_build_config.py index 55deb004..1106533c 100644 --- a/tests/unit_tests/test_build_config.py +++ b/tests/unit_tests/test_build_config.py @@ -6,6 +6,7 @@ from textwrap import dedent from unittest import mock + from fab.build_config import BuildConfig from fab.parse.fortran import AnalysedFortran from fab.steps.cleanup_prebuilds import CleanupPrebuilds @@ -21,7 +22,7 @@ def test_error_newlines(self, tmp_path): # We're testing the general reporting mechanism here, once they get back to the top, # that the newlines *within* the tool error are displayed correctly. - mock_source = {None: [AnalysedFortran('foo.f', file_hash=0)]} + mock_source = {None: [AnalysedFortran('foo.f90', file_hash=0)]} with mock.patch('fab.steps.compile_fortran.get_compiler_version', return_value='1.2.3'): with mock.patch('fab.steps.compile_fortran.DEFAULT_SOURCE_GETTER', return_value=mock_source): diff --git a/tests/unit_tests/test_tools.py b/tests/unit_tests/test_tools.py index 82af0cb5..8edcb437 100644 --- a/tests/unit_tests/test_tools.py +++ b/tests/unit_tests/test_tools.py @@ -181,7 +181,7 @@ class Test_run_command(object): def test_no_error(self): mock_result = mock.Mock(returncode=0) with mock.patch('fab.tools.subprocess.run', return_value=mock_result): - run_command(None) + run_command([]) def test_error(self): mock_result = mock.Mock(returncode=1) @@ -189,5 +189,5 @@ def test_error(self): mock_result.stderr.decode = mock.Mock(return_value=mocked_error_message) with mock.patch('fab.tools.subprocess.run', return_value=mock_result): with pytest.raises(RuntimeError) as err: - run_command(None) + run_command([]) assert mocked_error_message in str(err.value) diff --git a/tests/unit_tests/test_util.py b/tests/unit_tests/test_util.py index b61bb80d..e5f2ad65 100644 --- a/tests/unit_tests/test_util.py +++ b/tests/unit_tests/test_util.py @@ -1,9 +1,10 @@ from pathlib import Path +from unittest import mock import pytest from fab.artefacts import CollectionConcat, SuffixFilter -from fab.util import suffix_filter, file_walk +from fab.util import input_to_output_fpath, suffix_filter, file_walk @pytest.fixture @@ -71,3 +72,25 @@ def test_ignore(self, files, tmp_path): result = list(file_walk(tmp_path / 'foo', ignore_folders=[pbf.parent])) assert result == [f] + + +class Test_input_to_output_fpath(object): + + @pytest.fixture + def config(self): + return mock.Mock(source_root=Path('/proj/source'), build_output=Path('/proj/build_output')) + + def test_vanilla(self, config): + input_path = Path('/proj/source/folder/file.txt') + result = input_to_output_fpath(config, input_path) + assert result == Path(config.build_output / 'folder/file.txt') + + def test_already_output(self, config): + input_path = Path('/proj/build_output/folder/file.txt') + result = input_to_output_fpath(config, input_path) + assert result == input_path + + def test_outside_project(self, config): + input_path = Path('/other/folder/file.txt') + result = input_to_output_fpath(config, input_path) + assert result == Path(config.build_output / 'other/folder/file.txt')