From f9fdbbb43812b3a90a799b26bc2f1d8bd83738ea Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Mon, 13 May 2024 14:00:48 +1000 Subject: [PATCH] #3 Converted PSyclone to be a tool. --- source/fab/newtools/__init__.py | 2 + source/fab/newtools/psyclone.py | 60 +++++++++++++++ source/fab/newtools/tool_repository.py | 4 +- source/fab/steps/psyclone.py | 74 +++++++------------ .../psyclone/test_psyclone_system_test.py | 22 +++--- tests/unit_tests/tools/test_psyclone.py | 52 +++++++++++++ 6 files changed, 152 insertions(+), 62 deletions(-) create mode 100644 source/fab/newtools/psyclone.py create mode 100644 tests/unit_tests/tools/test_psyclone.py diff --git a/source/fab/newtools/__init__.py b/source/fab/newtools/__init__.py index 3e7ea144..e30548fc 100644 --- a/source/fab/newtools/__init__.py +++ b/source/fab/newtools/__init__.py @@ -13,6 +13,7 @@ Gfortran, Icc, Ifort) from fab.newtools.flags import Flags from fab.newtools.linker import Linker +from fab.newtools.psyclone import Psyclone from fab.newtools.preprocessor import Cpp, CppFortran, Fpp, Preprocessor from fab.newtools.tool import Tool, VendorTool # Order here is important to avoid a circular import @@ -37,6 +38,7 @@ "Ifort", "Linker", "Preprocessor", + "Psyclone", "Subversion", "Tool", "ToolBox", diff --git a/source/fab/newtools/psyclone.py b/source/fab/newtools/psyclone.py new file mode 100644 index 00000000..29158d25 --- /dev/null +++ b/source/fab/newtools/psyclone.py @@ -0,0 +1,60 @@ +############################################################################## +# (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 +############################################################################## + +"""This file the tool class for PSyclone. + +""" + +from pathlib import Path +from typing import List, Optional, Union + +from fab.newtools.categories import Categories +from fab.newtools.tool import Tool + + +class Psyclone(Tool): + '''This is the base class for `PSyclone`. + ''' + + def __init__(self): + super().__init__("psyclone", "psyclone", Categories.PSYCLONE) + + def check_available(self): + '''Checks if psyclone is available. We do this by requesting the + psyclone version. + ''' + try: + self.run("--version") + except (RuntimeError, FileNotFoundError): + return False + return True + + def process(self, api: str, + x90_file: Union[Path, str], + psy_file: Union[Path, str], + alg_file: Union[Path, str], + transformation_script: Optional[Union[Path, str]] = None, + additional_parameters: Optional[List[str]] = None, + kernel_roots: Optional[List[str]] = None, + ): + '''Create the archive with the specified name, containing the + listed members. + :param output_fpath: the output path. + :param members: the list of objects to be added to the archive. + ''' + parameters = ["-api", api, "-l", "all", + "-opsy", str(psy_file), + "-oalg", str(alg_file)] + if transformation_script: + parameters.extend(["-s", str(transformation_script)]) + if additional_parameters: + parameters.extend(additional_parameters) + if kernel_roots: + roots_with_dash_d = sum([['-d', str(k)] for k in kernel_roots], []) + parameters.extend(roots_with_dash_d) + parameters.append(str(x90_file)) + print("XX", parameters) + return self.run(additional_parameters=parameters) diff --git a/source/fab/newtools/tool_repository.py b/source/fab/newtools/tool_repository.py index 647632a7..7ff1f570 100644 --- a/source/fab/newtools/tool_repository.py +++ b/source/fab/newtools/tool_repository.py @@ -15,7 +15,7 @@ from typing import Any, Type from fab.newtools import (Ar, Categories, Cpp, CppFortran, Gcc, Gfortran, - Icc, Ifort, Linker) + Icc, Ifort, Linker, Psyclone) from fab.newtools.versioning import Fcm, Git, Subversion @@ -56,7 +56,7 @@ def __init__(self): # TODO: sort the defaults so that they actually work (since not all # tools FAB knows about are available). For now, disable Fpp: for cls in [Gcc, Icc, Gfortran, Ifort, Cpp, CppFortran, - Fcm, Git, Subversion, Ar]: + Fcm, Git, Subversion, Ar, Psyclone]: self.add_tool(cls) def add_tool(self, cls: Type[Any]): diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 69a8889a..67cfef2b 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -15,10 +15,9 @@ import warnings from itertools import chain from pathlib import Path -from typing import Dict, List, Optional, Set, Union, Tuple +from typing import Dict, List, Optional, Set, Tuple from fab.build_config import BuildConfig -from fab.tools import run_command from fab.artefacts import ArtefactsGetter, CollectionConcat, SuffixFilter from fab.parse.fortran import FortranAnalyser, AnalysedFortran @@ -32,15 +31,6 @@ logger = logging.getLogger(__name__) -def tool_available() -> bool: - """Check if the psyclone tool is available at the command line.""" - try: - run_command(['psyclone', '-h']) - except (RuntimeError, FileNotFoundError): - return False - return True - - # todo: should this be part of the psyclone step? def preprocess_x90(config, common_flags: Optional[List[str]] = None): common_flags = common_flags or [] @@ -318,37 +308,46 @@ def do_one_file(arg: Tuple[Path, MpCommonArgs]): prebuild_hash = _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-1 generated. + # There will always be one modified_alg, and 0-1 generated psy file. modified_alg: Path = x90_file.with_suffix('.f90') modified_alg = input_to_output_fpath(config=mp_payload.config, input_path=modified_alg) - generated: Path = x90_file.parent / (str(x90_file.stem) + '_psy.f90') - generated = input_to_output_fpath(config=mp_payload.config, input_path=generated) + psy_file: Path = x90_file.parent / (str(x90_file.stem) + '_psy.f90') + psy_file = input_to_output_fpath(config=mp_payload.config, input_path=psy_file) - generated.parent.mkdir(parents=True, exist_ok=True) + psy_file.parent.mkdir(parents=True, exist_ok=True) # do we already have prebuilt results for this x90 file? prebuilt_alg, prebuilt_gen = _get_prebuild_paths( - mp_payload.config.prebuild_folder, modified_alg, generated, prebuild_hash) + mp_payload.config.prebuild_folder, modified_alg, psy_file, 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) + shutil.copy2(prebuilt_gen, psy_file) log_or_dot(logger=logger, msg=msg) else: + config = mp_payload.config + psyclone = config.tool_box[Categories.PSYCLONE] try: + transformation_script = mp_payload.transformation_script + psyclone.process(api="dynamo0.3", + x90_file=x90_file, + psy_file=psy_file, + alg_file=modified_alg, + transformation_script=transformation_script, + kernel_roots=mp_payload.kernel_roots, + additional_parameters=mp_payload.cli_args) + # logger.info(f'running psyclone on {x90_file}') - run_psyclone(generated, modified_alg, x90_file, - mp_payload.kernel_roots, mp_payload.transformation_script, mp_payload.cli_args) shutil.copy2(modified_alg, prebuilt_alg) msg = f'created prebuilds for {x90_file}:\n {prebuilt_alg}' - if Path(generated).exists(): + if Path(psy_file).exists(): msg += f'\n {prebuilt_gen}' - shutil.copy2(generated, prebuilt_gen) + shutil.copy2(psy_file, prebuilt_gen) log_or_dot(logger=logger, msg=msg) except Exception as err: @@ -357,12 +356,12 @@ def do_one_file(arg: Tuple[Path, MpCommonArgs]): # do we have handwritten overrides for either of the files we just created? modified_alg = _check_override(modified_alg, mp_payload) - generated = _check_override(generated, mp_payload) + psy_file = _check_override(psy_file, mp_payload) # return the output files from psyclone result: List[Path] = [modified_alg] - if Path(generated).exists(): - result.append(generated) + if Path(psy_file).exists(): + result.append(psy_file) # we also want to return the prebuild artefact files we created, # which are just copies, in the prebuild folder, with hashes in the filenames. @@ -403,35 +402,12 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): return prebuild_hash -def _get_prebuild_paths(prebuild_folder, modified_alg, generated, prebuild_hash): +def _get_prebuild_paths(prebuild_folder, modified_alg, psy_file, prebuild_hash): prebuilt_alg = Path(prebuild_folder / f'{modified_alg.stem}.{prebuild_hash}{modified_alg.suffix}') - prebuilt_gen = Path(prebuild_folder / f'{generated.stem}.{prebuild_hash}{generated.suffix}') + prebuilt_gen = Path(prebuild_folder / f'{psy_file.stem}.{prebuild_hash}{psy_file.suffix}') return prebuilt_alg, prebuilt_gen -def run_psyclone(generated, modified_alg, x90_file, kernel_roots, - transformation_script, cli_args) -> None: - - # -d specifies "a root directory structure containing kernel source" - kernel_args: Union[List[str], list] = sum([['-d', k] for k in kernel_roots], []) - - # transformation python script - transform_options = ['-s', transformation_script] if 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, - *cli_args, - x90_file, - ] - - run_command(command) - - def _check_override(check_path: Path, mp_payload: MpCommonArgs): """ Delete the file if there's an override for it. diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 763d7dff..1cf039a1 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -17,8 +17,8 @@ from fab.steps.find_source_files import find_source_files from fab.steps.grab.folder import grab_folder from fab.steps.preprocess import preprocess_fortran -from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, psyclone, tool_available -from fab.newtools import ToolBox +from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, psyclone +from fab.newtools import ToolBox, Psyclone from fab.util import file_checksum SAMPLE_KERNEL = Path(__file__).parent / 'kernel.f90' @@ -61,7 +61,7 @@ def test_make_parsable_x90(tmp_path): unlink(parsable_x90_path) -class TestX90Analyser(object): +class TestX90Analyser(): expected_analysis_result = AnalysedX90( fpath=EXPECT_PARSABLE_X90, @@ -93,7 +93,7 @@ def test_prebuild(self, tmp_path): assert analysed_x90 == self.expected_analysis_result -class Test_analysis_for_prebuilds(object): +class Test_analysis_for_prebuilds(): def test_analyse(self, tmp_path): @@ -124,8 +124,8 @@ def test_analyse(self, tmp_path): } -@pytest.mark.skipif(not tool_available(), reason="psyclone cli tool not available") -class TestPsyclone(object): +@pytest.mark.skipif(not Psyclone().is_available, reason="psyclone cli tool not available") +class TestPsyclone(): """ Basic run of the psyclone step. @@ -185,11 +185,11 @@ def test_prebuild(self, tmp_path, config): self.steps(config) # 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.run_psyclone') as mock_run: - with config, pytest.warns(UserWarning, match="no transformation script specified"): - self.steps(config) + with mock.patch('fab.parse.x90.X90Analyser.walk_nodes') as mock_x90_walk, \ + mock.patch('fab.parse.fortran.FortranAnalyser.walk_nodes') as mock_fortran_walk, \ + mock.patch('fab.newtools.psyclone.Psyclone.process') as mock_run, \ + config, pytest.warns(UserWarning, match="no transformation script specified"): + self.steps(config) mock_x90_walk.assert_not_called() mock_fortran_walk.assert_not_called() diff --git a/tests/unit_tests/tools/test_psyclone.py b/tests/unit_tests/tools/test_psyclone.py new file mode 100644 index 00000000..437fab2a --- /dev/null +++ b/tests/unit_tests/tools/test_psyclone.py @@ -0,0 +1,52 @@ +############################################################################## +# (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 +############################################################################## + +'''Tests the PSyclone implementation. +''' + +from unittest import mock + +from fab.newtools import (Categories, Psyclone) + + +def test_psyclone_constructor(): + '''Test the psyclone constructor.''' + psyclone = Psyclone() + assert psyclone.category == Categories.PSYCLONE + assert psyclone.name == "psyclone" + assert psyclone.exec_name == "psyclone" + assert psyclone.flags == [] + + +def test_psyclone_check_available(): + '''Tests the is_available functionality.''' + psyclone = Psyclone() + with mock.patch("fab.newtools.tool.Tool.run") as tool_run: + assert psyclone.check_available() + tool_run.assert_called_once_with("--version") + + # Test behaviour if a runtime error happens: + with mock.patch("fab.newtools.tool.Tool.run", + side_effect=RuntimeError("")) as tool_run: + assert not psyclone.check_available() + + +def test_psyclone_process(): + '''Test running PSyclone.''' + psyclone = Psyclone() + with mock.patch("fab.newtools.tool.Tool.run") as tool_run: + psyclone.process(api="dynamo0.3", + x90_file="x90_file", + psy_file="psy_file", + alg_file="alg_file", + transformation_script="transformation_script", + kernel_roots=["root1", "root2"], + additional_parameters=["-c", "psyclone.cfg"]) + tool_run.assert_called_with( + additional_parameters=['-api', 'dynamo0.3', '-l', 'all', '-opsy', + 'psy_file', '-oalg', 'alg_file', '-s', + 'transformation_script', '-c', 'psyclone.cfg', + '-d', 'root1', '-d', 'root2', 'x90_file'])