From e309c15261ce270b04cbcbf11c247e2517a757cc Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 11:19:16 +1000 Subject: [PATCH 01/16] Changed the transformation_script parameter of function psyclone to accept a function that can return file-specific transformation scripts --- source/fab/steps/psyclone.py | 36 +++++----- .../psyclone/test_psyclone_system_test.py | 70 ++++++++++++++++--- 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index d7b1cdba..27439be1 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -15,7 +15,7 @@ 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, Union, Tuple, Callable from fab.build_config import BuildConfig from fab.tools import run_command @@ -75,13 +75,12 @@ class MpCommonArgs: analysed_x90: Dict[Path, AnalysedX90] kernel_roots: List[Path] - transformation_script: Path + transformation_script: Optional[Callable[[Path],Path]] cli_args: List[str] all_kernel_hashes: Dict[str, int] overrides_folder: Optional[Path] override_files: List[str] # filenames (not paths) of hand crafted overrides - transformation_script_hash: int = 0 DEFAULT_SOURCE_GETTER = CollectionConcat([ @@ -92,7 +91,7 @@ class MpCommonArgs: @step def psyclone(config, kernel_roots: Optional[List[Path]] = None, - transformation_script: Optional[Path] = None, + transformation_script: Optional[Callable[[Path],Path]] = None, cli_args: Optional[List[str]] = None, source_getter: Optional[ArtefactsGetter] = None, overrides_folder: Optional[Path] = None): @@ -114,7 +113,7 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, :param kernel_roots: Folders containing kernel files. Must be part of the analysed source code. :param transformation_script: - The Python transformation script. + The function to get Python transformation script. It takes in a file path, and returns the path of the transformation script or none. :param cli_args: Passed through to the psyclone cli tool. :param source_getter: @@ -168,7 +167,7 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, def _generate_mp_payload(config, prebuild_analyses, overrides_folder, kernel_roots, transformation_script, cli_args): - transformation_script_hash, analysed_x90, all_kernel_hashes = prebuild_analyses + analysed_x90, all_kernel_hashes = prebuild_analyses override_files: List[str] = [] if overrides_folder: @@ -176,7 +175,6 @@ def _generate_mp_payload(config, prebuild_analyses, overrides_folder, kernel_roo return MpCommonArgs( config=config, - transformation_script_hash=transformation_script_hash, kernel_roots=kernel_roots, transformation_script=transformation_script, cli_args=cli_args, @@ -229,12 +227,9 @@ def _analysis_for_prebuilds(config, x90s, transformation_script, kernel_roots) - The Analysis step must come after this step because it needs to analyse the fortran we create. """ - # hash the transformation script - if transformation_script: - transformation_script_hash = file_checksum(transformation_script).file_hash - else: + # give warning if there is no transformation script + if not transformation_script: warnings.warn('no transformation script specified') - transformation_script_hash = 0 # analyse the x90s analysed_x90 = _analyse_x90s(config, x90s) @@ -245,7 +240,7 @@ def _analysis_for_prebuilds(config, x90s, transformation_script, kernel_roots) - # todo: We'd like to separate that from the general fortran analyser at some point, to reduce coupling. all_kernel_hashes = _analyse_kernels(config, kernel_roots) - return transformation_script_hash, analysed_x90, all_kernel_hashes + return analysed_x90, all_kernel_hashes def _analyse_x90s(config, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: @@ -387,6 +382,12 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): kernel_deps_hashes = { mp_payload.all_kernel_hashes[kernel_name] for kernel_name in analysis_result.kernel_deps} # type: ignore + # calculate the transformation script hash for this file + if mp_payload.transformation_script and mp_payload.transformation_script(fpath=x90_file): + transformation_script_hash = file_checksum(mp_payload.transformation_script(fpath=x90_file)).file_hash + else: + transformation_script_hash = 0 + # hash everything which should trigger re-processing # todo: hash the psyclone version in case the built-in kernels change? prebuild_hash = sum([ @@ -397,8 +398,8 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): # the hashes of the kernels used by this x90 sum(kernel_deps_hashes), - # - mp_payload.transformation_script_hash, + # the hash of the transformation script for this x90 + transformation_script_hash, # command-line arguments string_checksum(str(mp_payload.cli_args)), @@ -419,7 +420,10 @@ def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation 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 [] + if transformation_script and transformation_script(fpath=x90_file): + transform_options = ['-s', transformation_script(fpath=x90_file)] + else: + transform_options = [] command = [ 'psyclone', '-api', 'dynamo0.3', diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 20cd7761..c6036ae1 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.util import file_checksum +from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, psyclone, tool_available, run_psyclone, _gen_prebuild_hash, MpCommonArgs +from fab.util import file_checksum, string_checksum SAMPLE_KERNEL = Path(__file__).parent / 'kernel.f90' @@ -96,16 +96,38 @@ class Test_analysis_for_prebuilds(object): def test_analyse(self, tmp_path): + # Transformation_script function is supplied by LFRic or other apps, and is not inside Fab. + # Here a dummy function is created for mocking. + def dummy_transformation_script(fpath): + pass + with BuildConfig('proj', fab_workspace=tmp_path) as config: - transformation_script_hash, analysed_x90, all_kernel_hashes = \ + # the script is just hashed later, so any one will do - use this file! + mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) + analysed_x90, all_kernel_hashes = \ _analysis_for_prebuilds(config, x90s=[SAMPLE_X90], kernel_roots=[Path(__file__).parent], - # the script is just hashed, so any one will do - use this file! - transformation_script=Path(__file__)) - - # transformation_script_hash - assert transformation_script_hash == file_checksum(__file__).file_hash + transformation_script=mock_transformation_script, + ) + test_mpcommonargs = MpCommonArgs(config=config, + kernel_roots=[Path(__file__).parent], + transformation_script=mock_transformation_script, + cli_args=[], + analysed_x90=analysed_x90, + all_kernel_hashes=all_kernel_hashes, + overrides_folder=None, + override_files=[], + ) + prebuild_hash = _gen_prebuild_hash(x90_file=SAMPLE_X90, mp_payload=test_mpcommonargs) + + # test transformation_script_hash with prebuild_hash + # transformation_script_hash is not returned and so is tested with prebuild_hash + assert prebuild_hash == sum([analysed_x90[SAMPLE_X90].file_hash, + sum({all_kernel_hashes[kernel_name] for kernel_name in analysed_x90[SAMPLE_X90].kernel_deps}), + file_checksum(__file__).file_hash, # transformation_script_hash + string_checksum(str([])) + ]) # analysed_x90 assert analysed_x90 == { @@ -192,3 +214,35 @@ def test_prebuild(self, tmp_path, config): mock_x90_walk.assert_not_called() mock_fortran_walk.assert_not_called() mock_run.assert_not_called() + +class TestTransformationScript(object): + """ + Check whether transformation script is called with x90 file twice + and whether transformation script is passed to psyclone after '-s'. + + """ + def test_transformation_script(self): + def dummy_transformation_script(fpath): + pass + mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) + with mock.patch('fab.steps.psyclone.run_command') as mock_run_command: + mock_transformation_script.return_value = Path(__file__) + run_psyclone(generated=Path(__file__), + modified_alg=Path(__file__), + x90_file=Path(__file__), + kernel_roots=[], + transformation_script=mock_transformation_script, + cli_args=[], + ) + + # check whether x90 is passed to transformation_script + mock_transformation_script.assert_called_with(Path(__file__)) + assert mock_transformation_script.call_count==2 + # check transformation_script is passed to psyclone command with '-s' + mock_run_command.assert_called_with(['psyclone', '-api', 'dynamo0.3', + '-l', 'all', + '-opsy', Path(__file__), + '-oalg', Path(__file__), + '-s', Path(__file__), + Path(__file__), + ]) From 0986b43932bb226389fe9674d3817193c1c7c5dd Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 14:20:55 +1000 Subject: [PATCH 02/16] Removed fpath= for input transformation_script function to pass mypy test for Python 3.7; Moved transformation_script_hash test to unit test from system test --- source/fab/steps/psyclone.py | 18 +++++----- .../psyclone/test_psyclone_system_test.py | 34 +++---------------- .../steps/test_psyclone_unit_test.py | 23 +++++++++---- 3 files changed, 31 insertions(+), 44 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 27439be1..5e329a14 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -383,10 +383,11 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): mp_payload.all_kernel_hashes[kernel_name] for kernel_name in analysis_result.kernel_deps} # type: ignore # calculate the transformation script hash for this file - if mp_payload.transformation_script and mp_payload.transformation_script(fpath=x90_file): - transformation_script_hash = file_checksum(mp_payload.transformation_script(fpath=x90_file)).file_hash - else: - transformation_script_hash = 0 + transformation_script_hash = 0 + if mp_payload.transformation_script: + transformation_script_return_path = mp_payload.transformation_script(x90_file) + if transformation_script_return_path: + transformation_script_hash = file_checksum(transformation_script_return_path).file_hash # hash everything which should trigger re-processing # todo: hash the psyclone version in case the built-in kernels change? @@ -420,10 +421,11 @@ def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation kernel_args: Union[List[str], list] = sum([['-d', k] for k in kernel_roots], []) # transformation python script - if transformation_script and transformation_script(fpath=x90_file): - transform_options = ['-s', transformation_script(fpath=x90_file)] - else: - transform_options = [] + transform_options = [] + if transformation_script: + transformation_script_return_path = transformation_script(x90_file) + if transformation_script_return_path: + transform_options = ['-s', transformation_script_return_path] command = [ 'psyclone', '-api', 'dynamo0.3', diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index c6036ae1..cd03e0e2 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, run_psyclone, _gen_prebuild_hash, MpCommonArgs -from fab.util import file_checksum, string_checksum +from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, psyclone, tool_available, run_psyclone +from fab.util import file_checksum SAMPLE_KERNEL = Path(__file__).parent / 'kernel.f90' @@ -95,40 +95,14 @@ def test_prebuild(self, tmp_path): class Test_analysis_for_prebuilds(object): def test_analyse(self, tmp_path): - - # Transformation_script function is supplied by LFRic or other apps, and is not inside Fab. - # Here a dummy function is created for mocking. - def dummy_transformation_script(fpath): - pass - with BuildConfig('proj', fab_workspace=tmp_path) as config: - # the script is just hashed later, so any one will do - use this file! - mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) analysed_x90, all_kernel_hashes = \ _analysis_for_prebuilds(config, x90s=[SAMPLE_X90], kernel_roots=[Path(__file__).parent], - transformation_script=mock_transformation_script, + transformation_script=None, ) - test_mpcommonargs = MpCommonArgs(config=config, - kernel_roots=[Path(__file__).parent], - transformation_script=mock_transformation_script, - cli_args=[], - analysed_x90=analysed_x90, - all_kernel_hashes=all_kernel_hashes, - overrides_folder=None, - override_files=[], - ) - prebuild_hash = _gen_prebuild_hash(x90_file=SAMPLE_X90, mp_payload=test_mpcommonargs) - - # test transformation_script_hash with prebuild_hash - # transformation_script_hash is not returned and so is tested with prebuild_hash - assert prebuild_hash == sum([analysed_x90[SAMPLE_X90].file_hash, - sum({all_kernel_hashes[kernel_name] for kernel_name in analysed_x90[SAMPLE_X90].kernel_deps}), - file_checksum(__file__).file_hash, # transformation_script_hash - string_checksum(str([])) - ]) - + # analysed_x90 assert analysed_x90 == { SAMPLE_X90: AnalysedX90( diff --git a/tests/unit_tests/steps/test_psyclone_unit_test.py b/tests/unit_tests/steps/test_psyclone_unit_test.py index d2c3da8e..532941f2 100644 --- a/tests/unit_tests/steps/test_psyclone_unit_test.py +++ b/tests/unit_tests/steps/test_psyclone_unit_test.py @@ -11,6 +11,7 @@ from fab.parse.x90 import AnalysedX90 from fab.steps.psyclone import _check_override, _gen_prebuild_hash, MpCommonArgs +from fab.util import file_checksum class Test_gen_prebuild_hash(object): @@ -34,15 +35,24 @@ def data(self, tmp_path) -> Tuple[MpCommonArgs, Path, int]: 'kernel2': 456, } - expect_hash = 223133615 + # Transformation_script function is supplied by LFRic or other apps, and is not inside Fab. + # Here a dummy function is created for mocking. + def dummy_transformation_script(fpath): + pass + # the script is just hashed later, so any one will do - use this file! + mock_transformation_script = mock.create_autospec(dummy_transformation_script, + return_value=Path(__file__)) + + expect_hash = 223133492 + file_checksum(__file__).file_hash # add the transformation_script_hash mp_payload = MpCommonArgs( analysed_x90=analysed_x90, all_kernel_hashes=all_kernel_hashes, - transformation_script_hash=123, cli_args=[], - config=None, kernel_roots=None, transformation_script=None, # type: ignore[arg-type] - overrides_folder=None, override_files=None, # type: ignore[arg-type] + config=None, kernel_roots=None, + transformation_script=mock_transformation_script, # type: ignore[arg-type] + overrides_folder=None, + override_files=None, # type: ignore[arg-type] ) return mp_payload, x90_file, expect_hash @@ -68,9 +78,10 @@ def test_kernal_deps(self, data): def test_trans_script(self, data): # changing the transformation script should change the hash mp_payload, x90_file, expect_hash = data - mp_payload.transformation_script_hash += 1 + mp_payload.transformation_script = None result = _gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) - assert result == expect_hash + 1 + # transformation_script_hash = 0 + assert result == expect_hash - file_checksum(__file__).file_hash def test_cli_args(self, data): # changing the cli args should change the hash From 2e15f192c524480cec6f2ae57f660e611bd14970 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 14:29:56 +1000 Subject: [PATCH 03/16] Fix mypy typing check errors for psyclone unit test --- source/fab/steps/psyclone.py | 2 +- tests/unit_tests/steps/test_psyclone_unit_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 5e329a14..9c32a69e 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -71,7 +71,7 @@ class MpCommonArgs: Contains data used to calculate the prebuild hash. """ - config: BuildConfig + config: Optional[BuildConfig] analysed_x90: Dict[Path, AnalysedX90] kernel_roots: List[Path] diff --git a/tests/unit_tests/steps/test_psyclone_unit_test.py b/tests/unit_tests/steps/test_psyclone_unit_test.py index 532941f2..ba2e4135 100644 --- a/tests/unit_tests/steps/test_psyclone_unit_test.py +++ b/tests/unit_tests/steps/test_psyclone_unit_test.py @@ -49,7 +49,8 @@ def dummy_transformation_script(fpath): analysed_x90=analysed_x90, all_kernel_hashes=all_kernel_hashes, cli_args=[], - config=None, kernel_roots=None, + config=None, + kernel_roots=[], transformation_script=mock_transformation_script, # type: ignore[arg-type] overrides_folder=None, override_files=None, # type: ignore[arg-type] From 1eeaa35d7566a9447a054049468b56e5815fffab Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 14:38:16 +1000 Subject: [PATCH 04/16] Fix config typing issue with mypy in psyclone unit test --- source/fab/steps/psyclone.py | 2 +- tests/unit_tests/steps/test_psyclone_unit_test.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 9c32a69e..5e329a14 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -71,7 +71,7 @@ class MpCommonArgs: Contains data used to calculate the prebuild hash. """ - config: Optional[BuildConfig] + config: BuildConfig analysed_x90: Dict[Path, AnalysedX90] kernel_roots: List[Path] diff --git a/tests/unit_tests/steps/test_psyclone_unit_test.py b/tests/unit_tests/steps/test_psyclone_unit_test.py index ba2e4135..5760defb 100644 --- a/tests/unit_tests/steps/test_psyclone_unit_test.py +++ b/tests/unit_tests/steps/test_psyclone_unit_test.py @@ -12,6 +12,7 @@ from fab.parse.x90 import AnalysedX90 from fab.steps.psyclone import _check_override, _gen_prebuild_hash, MpCommonArgs from fab.util import file_checksum +from fab.build_config import BuildConfig class Test_gen_prebuild_hash(object): @@ -49,7 +50,7 @@ def dummy_transformation_script(fpath): analysed_x90=analysed_x90, all_kernel_hashes=all_kernel_hashes, cli_args=[], - config=None, + config=BuildConfig('proj', fab_workspace=tmp_path), kernel_roots=[], transformation_script=mock_transformation_script, # type: ignore[arg-type] overrides_folder=None, From a45d62be590c1b8b0173ab10376b93ee4624dce4 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 14:50:03 +1000 Subject: [PATCH 05/16] Fix flake8 issues; Revert Config mypy typing fix --- source/fab/steps/psyclone.py | 7 +++-- .../psyclone/test_psyclone_system_test.py | 30 ++++++++++--------- .../steps/test_psyclone_unit_test.py | 17 +++++------ 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 5e329a14..480dd558 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -75,7 +75,7 @@ class MpCommonArgs: analysed_x90: Dict[Path, AnalysedX90] kernel_roots: List[Path] - transformation_script: Optional[Callable[[Path],Path]] + transformation_script: Optional[Callable[[Path], Path]] cli_args: List[str] all_kernel_hashes: Dict[str, int] @@ -91,7 +91,7 @@ class MpCommonArgs: @step def psyclone(config, kernel_roots: Optional[List[Path]] = None, - transformation_script: Optional[Callable[[Path],Path]] = None, + transformation_script: Optional[Callable[[Path], Path]] = None, cli_args: Optional[List[str]] = None, source_getter: Optional[ArtefactsGetter] = None, overrides_folder: Optional[Path] = None): @@ -113,7 +113,8 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, :param kernel_roots: Folders containing kernel files. Must be part of the analysed source code. :param transformation_script: - The function to get Python transformation script. It takes in a file path, and returns the path of the transformation script or none. + The function to get Python transformation script. + It takes in a file path, and returns the path of the transformation script or none. :param cli_args: Passed through to the psyclone cli tool. :param source_getter: diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index cd03e0e2..e7f19a13 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -17,7 +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, run_psyclone +from fab.steps.psyclone import _analysis_for_prebuilds, make_parsable_x90, preprocess_x90, \ + psyclone, tool_available, run_psyclone from fab.util import file_checksum SAMPLE_KERNEL = Path(__file__).parent / 'kernel.f90' @@ -102,7 +103,7 @@ def test_analyse(self, tmp_path): kernel_roots=[Path(__file__).parent], transformation_script=None, ) - + # analysed_x90 assert analysed_x90 == { SAMPLE_X90: AnalysedX90( @@ -189,34 +190,35 @@ def test_prebuild(self, tmp_path, config): mock_fortran_walk.assert_not_called() mock_run.assert_not_called() + class TestTransformationScript(object): """ - Check whether transformation script is called with x90 file twice + Check whether transformation script is called with x90 file twice and whether transformation script is passed to psyclone after '-s'. """ def test_transformation_script(self): def dummy_transformation_script(fpath): pass - mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) + mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) with mock.patch('fab.steps.psyclone.run_command') as mock_run_command: mock_transformation_script.return_value = Path(__file__) run_psyclone(generated=Path(__file__), - modified_alg=Path(__file__), - x90_file=Path(__file__), - kernel_roots=[], - transformation_script=mock_transformation_script, + modified_alg=Path(__file__), + x90_file=Path(__file__), + kernel_roots=[], + transformation_script=mock_transformation_script, cli_args=[], - ) + ) - # check whether x90 is passed to transformation_script + # check whether x90 is passed to transformation_script mock_transformation_script.assert_called_with(Path(__file__)) - assert mock_transformation_script.call_count==2 - # check transformation_script is passed to psyclone command with '-s' + assert mock_transformation_script.call_count == 2 + # check transformation_script is passed to psyclone command with '-s' mock_run_command.assert_called_with(['psyclone', '-api', 'dynamo0.3', '-l', 'all', - '-opsy', Path(__file__), + '-opsy', Path(__file__), '-oalg', Path(__file__), '-s', Path(__file__), Path(__file__), - ]) + ]) diff --git a/tests/unit_tests/steps/test_psyclone_unit_test.py b/tests/unit_tests/steps/test_psyclone_unit_test.py index 5760defb..2a76b99c 100644 --- a/tests/unit_tests/steps/test_psyclone_unit_test.py +++ b/tests/unit_tests/steps/test_psyclone_unit_test.py @@ -12,7 +12,6 @@ from fab.parse.x90 import AnalysedX90 from fab.steps.psyclone import _check_override, _gen_prebuild_hash, MpCommonArgs from fab.util import file_checksum -from fab.build_config import BuildConfig class Test_gen_prebuild_hash(object): @@ -36,24 +35,24 @@ def data(self, tmp_path) -> Tuple[MpCommonArgs, Path, int]: 'kernel2': 456, } - # Transformation_script function is supplied by LFRic or other apps, and is not inside Fab. + # Transformation_script function is supplied by LFRic or other apps, and is not inside Fab. # Here a dummy function is created for mocking. def dummy_transformation_script(fpath): pass # the script is just hashed later, so any one will do - use this file! - mock_transformation_script = mock.create_autospec(dummy_transformation_script, + mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) - expect_hash = 223133492 + file_checksum(__file__).file_hash # add the transformation_script_hash + expect_hash = 223133492 + file_checksum(__file__).file_hash # add the transformation_script_hash mp_payload = MpCommonArgs( analysed_x90=analysed_x90, all_kernel_hashes=all_kernel_hashes, cli_args=[], - config=BuildConfig('proj', fab_workspace=tmp_path), - kernel_roots=[], - transformation_script=mock_transformation_script, # type: ignore[arg-type] - overrides_folder=None, + config=None, # type: ignore[arg-type] + kernel_roots=[], + transformation_script=mock_transformation_script, + overrides_folder=None, override_files=None, # type: ignore[arg-type] ) return mp_payload, x90_file, expect_hash @@ -83,7 +82,7 @@ def test_trans_script(self, data): mp_payload.transformation_script = None result = _gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) # transformation_script_hash = 0 - assert result == expect_hash - file_checksum(__file__).file_hash + assert result == expect_hash - file_checksum(__file__).file_hash def test_cli_args(self, data): # changing the cli args should change the hash From a4a9aab128a68d83d79ea1f8439e3a91139f88e0 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 14:56:04 +1000 Subject: [PATCH 06/16] Add comment to ignore typing check for fpath parameter of input transformation_script function --- source/fab/steps/psyclone.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 480dd558..91a182db 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -386,7 +386,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): # calculate the transformation script hash for this file transformation_script_hash = 0 if mp_payload.transformation_script: - transformation_script_return_path = mp_payload.transformation_script(x90_file) + transformation_script_return_path = mp_payload.transformation_script(fpath=x90_file) # type: ignore[call-arg] if transformation_script_return_path: transformation_script_hash = file_checksum(transformation_script_return_path).file_hash @@ -424,7 +424,7 @@ def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation # transformation python script transform_options = [] if transformation_script: - transformation_script_return_path = transformation_script(x90_file) + transformation_script_return_path = transformation_script(fpath=x90_file) # type: ignore[call-arg] if transformation_script_return_path: transform_options = ['-s', transformation_script_return_path] From 5978e8082c8a4530bafacde7b1e2f82bdfb294d5 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 14:59:49 +1000 Subject: [PATCH 07/16] Fix assert check after transformation_script function is changed from being called twice to once --- tests/system_tests/psyclone/test_psyclone_system_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index e7f19a13..422e615e 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -193,7 +193,7 @@ def test_prebuild(self, tmp_path, config): class TestTransformationScript(object): """ - Check whether transformation script is called with x90 file twice + Check whether transformation script is called with x90 file once and whether transformation script is passed to psyclone after '-s'. """ @@ -212,8 +212,7 @@ def dummy_transformation_script(fpath): ) # check whether x90 is passed to transformation_script - mock_transformation_script.assert_called_with(Path(__file__)) - assert mock_transformation_script.call_count == 2 + mock_transformation_script.assert_called_once_with(Path(__file__)) # check transformation_script is passed to psyclone command with '-s' mock_run_command.assert_called_with(['psyclone', '-api', 'dynamo0.3', '-l', 'all', From e70885d33f2a3bfee084e9af71e7a403aaeba48c Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 15:09:31 +1000 Subject: [PATCH 08/16] Filter out 'no transformation script' warning for psyclone system test --- tests/system_tests/psyclone/test_psyclone_system_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 422e615e..2cf68421 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -96,7 +96,8 @@ def test_prebuild(self, tmp_path): class Test_analysis_for_prebuilds(object): def test_analyse(self, tmp_path): - with BuildConfig('proj', fab_workspace=tmp_path) as config: + with BuildConfig('proj', fab_workspace=tmp_path) as config, \ + pytest.warns(UserWarning, match="no transformation script specified"): analysed_x90, all_kernel_hashes = \ _analysis_for_prebuilds(config, x90s=[SAMPLE_X90], From d2c6db0ab9a6f191532a601f9d3b2f3a81d6981d Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Tue, 30 Apr 2024 18:19:46 +1000 Subject: [PATCH 09/16] Replace 'ignore' typing of fpath of transformation_script with removing keyword argument --- source/fab/steps/psyclone.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 91a182db..480dd558 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -386,7 +386,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): # calculate the transformation script hash for this file transformation_script_hash = 0 if mp_payload.transformation_script: - transformation_script_return_path = mp_payload.transformation_script(fpath=x90_file) # type: ignore[call-arg] + transformation_script_return_path = mp_payload.transformation_script(x90_file) if transformation_script_return_path: transformation_script_hash = file_checksum(transformation_script_return_path).file_hash @@ -424,7 +424,7 @@ def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation # transformation python script transform_options = [] if transformation_script: - transformation_script_return_path = transformation_script(fpath=x90_file) # type: ignore[call-arg] + transformation_script_return_path = transformation_script(x90_file) if transformation_script_return_path: transform_options = ['-s', transformation_script_return_path] From 3a5e86a585699fce942f07ea8b92896e857887b9 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Fri, 3 May 2024 18:23:18 +1000 Subject: [PATCH 10/16] 1. Updated transformation_script description; 2. Modified mock_transformation_script; 3.Removed redundant _analysis_for_prebuilds --- source/fab/steps/psyclone.py | 111 +++++++----------- .../psyclone/test_psyclone_system_test.py | 19 +-- .../steps/test_psyclone_unit_test.py | 10 +- 3 files changed, 54 insertions(+), 86 deletions(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 480dd558..b9140bea 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -114,7 +114,8 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, Folders containing kernel files. Must be part of the analysed source code. :param transformation_script: The function to get Python transformation script. - It takes in a file path, and returns the path of the transformation script or none. + It takes in a file path, and returns the path of the transformation script or None. + If no function is given or the function returns None, no script will be applied and PSyclone still runs. :param cli_args: Passed through to the psyclone cli tool. :param source_getter: @@ -134,10 +135,15 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, x90s = source_getter(config.artefact_store) - # get the data for child processes to calculate prebuild hashes - prebuild_analyses = _analysis_for_prebuilds(config, x90s, transformation_script, kernel_roots) + # analyse the x90s + analysed_x90 = _analyse_x90s(config, x90s) + + # analyse the kernel files, + all_kernel_hashes = _analyse_kernels(config, kernel_roots) + + # get the data in a payload object for child processes to calculate prebuild hashes mp_payload = _generate_mp_payload( - config, prebuild_analyses, overrides_folder, kernel_roots, transformation_script, cli_args) + config, analysed_x90, all_kernel_hashes, overrides_folder, kernel_roots, transformation_script, cli_args) # run psyclone. # for every file, we get back a list of its output files plus a list of the prebuild copies. @@ -167,9 +173,8 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, # assert False -def _generate_mp_payload(config, prebuild_analyses, overrides_folder, kernel_roots, transformation_script, cli_args): - analysed_x90, all_kernel_hashes = prebuild_analyses - +def _generate_mp_payload(config, analysed_x90, all_kernel_hashes, overrides_folder, kernel_roots, + transformation_script, cli_args): override_files: List[str] = [] if overrides_folder: override_files = [f.name for f in file_walk(overrides_folder)] @@ -186,64 +191,6 @@ def _generate_mp_payload(config, prebuild_analyses, overrides_folder, kernel_roo ) -# todo: test that we can run this step before or after the analysis step -def _analysis_for_prebuilds(config, x90s, transformation_script, kernel_roots) -> Tuple: - """ - 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. - - """ - # give warning if there is no transformation script - if not transformation_script: - warnings.warn('no transformation script specified') - - # analyse the x90s - analysed_x90 = _analyse_x90s(config, 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 = _analyse_kernels(config, kernel_roots) - - return analysed_x90, all_kernel_hashes - - def _analyse_x90s(config, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: # Analyse parsable versions of the x90s, finding kernel dependencies. @@ -276,7 +223,31 @@ def _analyse_x90s(config, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: def _analyse_kernels(config, kernel_roots) -> Dict[str, int]: - # We want to hash the kernel metadata (type defs). + """ + We want to hash the kernel metadata (type defs). + + 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. + + """ # Ignore the prebuild folder. Todo: test the prebuild folder is ignored, in case someone breaks this. file_lists = [list(file_walk(root, ignore_folders=[config.prebuild_folder])) for root in kernel_roots] all_kernel_files: Set[Path] = set(sum(file_lists, [])) @@ -375,6 +346,12 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): """ Calculate the prebuild hash for this x90 file, based on all the things which should trigger reprocessing. + Changes which must trigger reprocessing of an x90 file: + - x90 source: + - kernel metadata used by the x90 + - transformation script + - cli args + """ # We've analysed (a parsable version of) this x90. analysis_result = mp_payload.analysed_x90[x90_file] # type: ignore @@ -389,6 +366,8 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): transformation_script_return_path = mp_payload.transformation_script(x90_file) if transformation_script_return_path: transformation_script_hash = file_checksum(transformation_script_return_path).file_hash + if transformation_script_hash == 0: + warnings.warn('no transformation script specified') # hash everything which should trigger re-processing # todo: hash the psyclone version in case the built-in kernels change? diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 2cf68421..c638c6e5 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -17,7 +17,7 @@ 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, \ +from fab.steps.psyclone import _analyse_x90s, _analyse_kernels, make_parsable_x90, preprocess_x90, \ psyclone, tool_available, run_psyclone from fab.util import file_checksum @@ -93,17 +93,12 @@ def test_prebuild(self, tmp_path): assert analysed_x90 == self.expected_analysis_result -class Test_analysis_for_prebuilds(object): +class Test_analysis_for_x90s_and_kernels(object): def test_analyse(self, tmp_path): - with BuildConfig('proj', fab_workspace=tmp_path) as config, \ - pytest.warns(UserWarning, match="no transformation script specified"): - analysed_x90, all_kernel_hashes = \ - _analysis_for_prebuilds(config, - x90s=[SAMPLE_X90], - kernel_roots=[Path(__file__).parent], - transformation_script=None, - ) + with BuildConfig('proj', fab_workspace=tmp_path) as config: + analysed_x90 = _analyse_x90s(config, x90s=[SAMPLE_X90]) + all_kernel_hashes = _analyse_kernels(config, kernel_roots=[Path(__file__).parent]) # analysed_x90 assert analysed_x90 == { @@ -199,9 +194,7 @@ class TestTransformationScript(object): """ def test_transformation_script(self): - def dummy_transformation_script(fpath): - pass - mock_transformation_script = mock.create_autospec(dummy_transformation_script, return_value=Path(__file__)) + mock_transformation_script = mock.Mock(return_value=__file__) with mock.patch('fab.steps.psyclone.run_command') as mock_run_command: mock_transformation_script.return_value = Path(__file__) run_psyclone(generated=Path(__file__), diff --git a/tests/unit_tests/steps/test_psyclone_unit_test.py b/tests/unit_tests/steps/test_psyclone_unit_test.py index 2a76b99c..13980c0d 100644 --- a/tests/unit_tests/steps/test_psyclone_unit_test.py +++ b/tests/unit_tests/steps/test_psyclone_unit_test.py @@ -35,13 +35,8 @@ def data(self, tmp_path) -> Tuple[MpCommonArgs, Path, int]: 'kernel2': 456, } - # Transformation_script function is supplied by LFRic or other apps, and is not inside Fab. - # Here a dummy function is created for mocking. - def dummy_transformation_script(fpath): - pass # the script is just hashed later, so any one will do - use this file! - mock_transformation_script = mock.create_autospec(dummy_transformation_script, - return_value=Path(__file__)) + mock_transformation_script = mock.Mock(return_value=__file__) expect_hash = 223133492 + file_checksum(__file__).file_hash # add the transformation_script_hash @@ -80,7 +75,8 @@ def test_trans_script(self, data): # changing the transformation script should change the hash mp_payload, x90_file, expect_hash = data mp_payload.transformation_script = None - result = _gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) + with pytest.warns(UserWarning, match="no transformation script specified"): + result = _gen_prebuild_hash(x90_file=x90_file, mp_payload=mp_payload) # transformation_script_hash = 0 assert result == expect_hash - file_checksum(__file__).file_hash From 4ab3737e9a8489fb62bfd767dc02c1fd636d3cf2 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Fri, 3 May 2024 18:52:11 +1000 Subject: [PATCH 11/16] Updated lfric/atm.py and lfric/gungho.py examples to pass in transformation_script functions --- run_configs/lfric/atm.py | 38 ++++++++++++++++++++++++++++++++++--- run_configs/lfric/gungho.py | 35 ++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/run_configs/lfric/atm.py b/run_configs/lfric/atm.py index 220ed009..544bb2f1 100755 --- a/run_configs/lfric/atm.py +++ b/run_configs/lfric/atm.py @@ -16,11 +16,11 @@ from grab_lfric import lfric_source_config, gpl_utils_source_config from lfric_common import configurator, fparser_workaround_stop_concatenation +from fnmatch import fnmatch +from string import Template logger = logging.getLogger('fab') -# todo: optimisation path stuff - def file_filtering(config): """Based on lfric_atm/fcm-make/extract.cfg""" @@ -163,6 +163,38 @@ def file_filtering(config): ] +def get_transformation_script(fpath): + ''':returns: the transformation script to be used by PSyclone. + :rtype: Path + + ''' + params = {'relative': fpath.parent, 'source': lfric_source_config.source_root, + 'output': lfric_source_config.build_output} + global_transformation_script = '$source/lfric/lfric_atm/optimisation/meto-spice/global.py' + local_transformation_script = None + if global_transformation_script: + if local_transformation_script: + # global defined, local defined + for key_match in local_transformation_script: + if fnmatch(str(fpath), Template(key_match).substitute(params)): + # use templating to render any relative paths + return Template(local_transformation_script[key_match]).substitute(params) + return Template(global_transformation_script).substitute(params) + else: + # global defined, local not defined + return Template(global_transformation_script).substitute(params) + elif local_transformation_script: + # global not defined, local defined + for key_match in local_transformation_script: + if fnmatch(str(fpath), Template(key_match).substitute(params)): + # use templating to render any relative paths + return Template(local_transformation_script[key_match]).substitute(params) + return "" + else: + # global not defined, local not defined + return "" + + if __name__ == '__main__': lfric_source = lfric_source_config.source_root / 'lfric' gpl_utils_source = gpl_utils_source_config.source_root / 'gpl_utils' @@ -239,7 +271,7 @@ def file_filtering(config): psyclone( state, kernel_roots=[state.build_output / 'lfric' / 'kernel'], - transformation_script=lfric_source / 'lfric_atm/optimisation/meto-spice/global.py', + transformation_script=get_transformation_script, cli_args=[], ) diff --git a/run_configs/lfric/gungho.py b/run_configs/lfric/gungho.py index 39782cf6..c9fd9ef6 100755 --- a/run_configs/lfric/gungho.py +++ b/run_configs/lfric/gungho.py @@ -18,11 +18,42 @@ from grab_lfric import lfric_source_config, gpl_utils_source_config from lfric_common import configurator, fparser_workaround_stop_concatenation +from fnmatch import fnmatch +from string import Template logger = logging.getLogger('fab') -# todo: optimisation path stuff +def get_transformation_script(fpath): + ''':returns: the transformation script to be used by PSyclone. + :rtype: Path + + ''' + params = {'relative': fpath.parent, 'source': lfric_source_config.source_root, + 'output': lfric_source_config.build_output} + global_transformation_script = '$source/lfric/miniapps/gungho_model/optimisation/meto-spice/global.py' + local_transformation_script = None + if global_transformation_script: + if local_transformation_script: + # global defined, local defined + for key_match in local_transformation_script: + if fnmatch(str(fpath), Template(key_match).substitute(params)): + # use templating to render any relative paths + return Template(local_transformation_script[key_match]).substitute(params) + return Template(global_transformation_script).substitute(params) + else: + # global defined, local not defined + return Template(global_transformation_script).substitute(params) + elif local_transformation_script: + # global not defined, local defined + for key_match in local_transformation_script: + if fnmatch(str(fpath), Template(key_match).substitute(params)): + # use templating to render any relative paths + return Template(local_transformation_script[key_match]).substitute(params) + return "" + else: + # global not defined, local not defined + return "" if __name__ == '__main__': @@ -65,7 +96,7 @@ psyclone( state, kernel_roots=[state.build_output], - transformation_script=lfric_source / 'miniapps/gungho_model/optimisation/meto-spice/global.py', + transformation_script=get_transformation_script, cli_args=[], ) From 92545f8899f08136b67857e7c35a689aaaf5a040 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Fri, 3 May 2024 19:38:35 +1000 Subject: [PATCH 12/16] Added description for the psyclone step to instructions on writing a config --- docs/source/writing_config.rst | 56 ++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/docs/source/writing_config.rst b/docs/source/writing_config.rst index aaab8471..68d86800 100644 --- a/docs/source/writing_config.rst +++ b/docs/source/writing_config.rst @@ -132,6 +132,52 @@ Preprocessed files are created in the `'build_output'` folder, inside the projec After the fortran_preprocessor step, there will be a collection called ``"preprocessed_fortran"``, in the artefact store. +PSyclone +======== + +If you want to use PSyclone to do code transformation and pre-processing (see https://github.com/stfc/PSyclone), +you must run :func:`~fab.steps.psyclone.preprocess_x90` and :func:`~fab.steps.psyclone.psyclone`, +before you run the :func:`~fab.steps.analyse.analyse` step below. + +* For :func:`~fab.steps.psyclone.preprocess_x90`: + You can pass in `common_flags` list as an argument. +* For :func:`~fab.steps.psyclone.psyclone`: + You can pass in kernel file roots to `kernel_roots`, a function to get transformation script to + `transformation_script` (see examples in :ref:`~fab.run_configs.lfric.gungho.py` and + :ref:`~fab.run_configs.lfric.atm.py`), command-line arguments to `cli_args`, + override for input files to `source_getter`, and folders containing override files to `overrides_folder` + + +.. code-block:: + :linenos: + :caption: build_it.py + :emphasize-lines: 8,18,19 + + #!/usr/bin/env python3 + from logging import getLogger + + from fab.build_config import BuildConfig + 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 psyclone, preprocess_x90 + + logger = getLogger('fab') + + if __name__ == '__main__': + + with BuildConfig(project_label=' Date: Thu, 9 May 2024 15:39:48 +1000 Subject: [PATCH 13/16] Modified the documentation for writing a config with PSyclone --- docs/source/writing_config.rst | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/source/writing_config.rst b/docs/source/writing_config.rst index 68d86800..ed028b22 100644 --- a/docs/source/writing_config.rst +++ b/docs/source/writing_config.rst @@ -64,7 +64,7 @@ A grab step will copy files from a folder or remote repo into a folder called if __name__ == '__main__': - with BuildConfig(project_label='` environment variable if __name__ == '__main__': - with BuildConfig(project_label=' Date: Fri, 10 May 2024 11:19:17 +1000 Subject: [PATCH 14/16] Add config as a parameter for run_psyclone for the transformation_script to use;Updated the related functions and tests; Changed the logic of the transformation_script examples --- run_configs/lfric/atm.py | 35 +++++-------------- run_configs/lfric/gungho.py | 35 +++++-------------- source/fab/steps/psyclone.py | 15 ++++---- .../psyclone/test_psyclone_system_test.py | 3 +- 4 files changed, 28 insertions(+), 60 deletions(-) diff --git a/run_configs/lfric/atm.py b/run_configs/lfric/atm.py index 544bb2f1..2e1be991 100755 --- a/run_configs/lfric/atm.py +++ b/run_configs/lfric/atm.py @@ -16,8 +16,6 @@ from grab_lfric import lfric_source_config, gpl_utils_source_config from lfric_common import configurator, fparser_workaround_stop_concatenation -from fnmatch import fnmatch -from string import Template logger = logging.getLogger('fab') @@ -163,35 +161,20 @@ def file_filtering(config): ] -def get_transformation_script(fpath): +def get_transformation_script(fpath, config): ''':returns: the transformation script to be used by PSyclone. :rtype: Path ''' - params = {'relative': fpath.parent, 'source': lfric_source_config.source_root, - 'output': lfric_source_config.build_output} - global_transformation_script = '$source/lfric/lfric_atm/optimisation/meto-spice/global.py' - local_transformation_script = None - if global_transformation_script: - if local_transformation_script: - # global defined, local defined - for key_match in local_transformation_script: - if fnmatch(str(fpath), Template(key_match).substitute(params)): - # use templating to render any relative paths - return Template(local_transformation_script[key_match]).substitute(params) - return Template(global_transformation_script).substitute(params) - else: - # global defined, local not defined - return Template(global_transformation_script).substitute(params) - elif local_transformation_script: - # global not defined, local defined - for key_match in local_transformation_script: - if fnmatch(str(fpath), Template(key_match).substitute(params)): - # use templating to render any relative paths - return Template(local_transformation_script[key_match]).substitute(params) - return "" + global_transformation_script = config.source_root / 'lfric' / 'lfric_atm' / 'optimisation' / \ + 'meto-spice' / 'global.py' + local_transformation_script = config.source_root / 'lfric' / 'lfric_atm' / 'optimisation' / \ + 'meto-spice' / (fpath.relative_to(config.source_root).with_suffix('.py')) + if local_transformation_script: + return local_transformation_script + elif global_transformation_script: + return global_transformation_script else: - # global not defined, local not defined return "" diff --git a/run_configs/lfric/gungho.py b/run_configs/lfric/gungho.py index c9fd9ef6..db72d63a 100755 --- a/run_configs/lfric/gungho.py +++ b/run_configs/lfric/gungho.py @@ -18,41 +18,24 @@ from grab_lfric import lfric_source_config, gpl_utils_source_config from lfric_common import configurator, fparser_workaround_stop_concatenation -from fnmatch import fnmatch -from string import Template logger = logging.getLogger('fab') -def get_transformation_script(fpath): +def get_transformation_script(fpath, config): ''':returns: the transformation script to be used by PSyclone. :rtype: Path ''' - params = {'relative': fpath.parent, 'source': lfric_source_config.source_root, - 'output': lfric_source_config.build_output} - global_transformation_script = '$source/lfric/miniapps/gungho_model/optimisation/meto-spice/global.py' - local_transformation_script = None - if global_transformation_script: - if local_transformation_script: - # global defined, local defined - for key_match in local_transformation_script: - if fnmatch(str(fpath), Template(key_match).substitute(params)): - # use templating to render any relative paths - return Template(local_transformation_script[key_match]).substitute(params) - return Template(global_transformation_script).substitute(params) - else: - # global defined, local not defined - return Template(global_transformation_script).substitute(params) - elif local_transformation_script: - # global not defined, local defined - for key_match in local_transformation_script: - if fnmatch(str(fpath), Template(key_match).substitute(params)): - # use templating to render any relative paths - return Template(local_transformation_script[key_match]).substitute(params) - return "" + global_transformation_script = config.source_root / 'lfric' / 'miniapps' / 'gungho_model' / 'optimisation' / \ + 'meto-spice' / 'global.py' + local_transformation_script = config.source_root / 'lfric' / 'miniapps' / 'gungho_model' / 'optimisation' / \ + 'meto-spice' / (fpath.relative_to(config.source_root).with_suffix('.py')) + if local_transformation_script: + return local_transformation_script + elif global_transformation_script: + return global_transformation_script else: - # global not defined, local not defined return "" diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index b9140bea..b6671bc5 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -75,7 +75,7 @@ class MpCommonArgs: analysed_x90: Dict[Path, AnalysedX90] kernel_roots: List[Path] - transformation_script: Optional[Callable[[Path], Path]] + transformation_script: Optional[Callable[[Path, BuildConfig], Path]] cli_args: List[str] all_kernel_hashes: Dict[str, int] @@ -91,7 +91,7 @@ class MpCommonArgs: @step def psyclone(config, kernel_roots: Optional[List[Path]] = None, - transformation_script: Optional[Callable[[Path], Path]] = None, + transformation_script: Optional[Callable[[Path, BuildConfig], Path]] = None, cli_args: Optional[List[str]] = None, source_getter: Optional[ArtefactsGetter] = None, overrides_folder: Optional[Path] = None): @@ -114,7 +114,7 @@ def psyclone(config, kernel_roots: Optional[List[Path]] = None, Folders containing kernel files. Must be part of the analysed source code. :param transformation_script: The function to get Python transformation script. - It takes in a file path, and returns the path of the transformation script or None. + It takes in a file path and the config object, and returns the path of the transformation script or None. If no function is given or the function returns None, no script will be applied and PSyclone still runs. :param cli_args: Passed through to the psyclone cli tool. @@ -313,7 +313,8 @@ def do_one_file(arg: Tuple[Path, MpCommonArgs]): try: # 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) + mp_payload.kernel_roots, mp_payload.transformation_script, + mp_payload.cli_args, mp_payload.config) shutil.copy2(modified_alg, prebuilt_alg) msg = f'created prebuilds for {x90_file}:\n {prebuilt_alg}' @@ -363,7 +364,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): # calculate the transformation script hash for this file transformation_script_hash = 0 if mp_payload.transformation_script: - transformation_script_return_path = mp_payload.transformation_script(x90_file) + transformation_script_return_path = mp_payload.transformation_script(x90_file, mp_payload.config) if transformation_script_return_path: transformation_script_hash = file_checksum(transformation_script_return_path).file_hash if transformation_script_hash == 0: @@ -395,7 +396,7 @@ def _get_prebuild_paths(prebuild_folder, modified_alg, generated, prebuild_hash) return prebuilt_alg, prebuilt_gen -def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation_script, cli_args): +def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation_script, cli_args, config): # -d specifies "a root directory structure containing kernel source" kernel_args: Union[List[str], list] = sum([['-d', k] for k in kernel_roots], []) @@ -403,7 +404,7 @@ def run_psyclone(generated, modified_alg, x90_file, kernel_roots, transformation # transformation python script transform_options = [] if transformation_script: - transformation_script_return_path = transformation_script(x90_file) + transformation_script_return_path = transformation_script(x90_file, config) if transformation_script_return_path: transform_options = ['-s', transformation_script_return_path] diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index c638c6e5..3d1f6e21 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -203,10 +203,11 @@ def test_transformation_script(self): kernel_roots=[], transformation_script=mock_transformation_script, cli_args=[], + config=None, # type: ignore[arg-type] ) # check whether x90 is passed to transformation_script - mock_transformation_script.assert_called_once_with(Path(__file__)) + mock_transformation_script.assert_called_once_with(Path(__file__), None) # check transformation_script is passed to psyclone command with '-s' mock_run_command.assert_called_with(['psyclone', '-api', 'dynamo0.3', '-l', 'all', From ade5f6732b09a5cfee8432a521c782629eae53b4 Mon Sep 17 00:00:00 2001 From: Junwei Lyu Date: Mon, 13 May 2024 15:08:05 +1000 Subject: [PATCH 15/16] Modified the get_optimisation_script function examples and updated the doc formatting --- docs/source/writing_config.rst | 11 +++++++---- run_configs/lfric/atm.py | 14 ++++++-------- run_configs/lfric/gungho.py | 14 ++++++-------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/docs/source/writing_config.rst b/docs/source/writing_config.rst index ed028b22..02928a93 100644 --- a/docs/source/writing_config.rst +++ b/docs/source/writing_config.rst @@ -142,10 +142,13 @@ before you run the :func:`~fab.steps.analyse.analyse` step below. * For :func:`~fab.steps.psyclone.preprocess_x90`: You can pass in `common_flags` list as an argument. * For :func:`~fab.steps.psyclone.psyclone`: - You can pass in kernel file roots to `kernel_roots`, a function to get transformation script to - `transformation_script` (see examples in ``~fab.run_configs.lfric.gungho.py`` and - ``~fab.run_configs.lfric.atm.py``), command-line arguments to `cli_args`, - override for input files to `source_getter`, and folders containing override files to `overrides_folder` + You can pass in + * kernel file roots to `kernel_roots`, + * a function to get transformation script to `transformation_script` + (see examples in ``~fab.run_configs.lfric.gungho.py`` and ``~fab.run_configs.lfric.atm.py``), + * command-line arguments to `cli_args`, + * override for input files to `source_getter`, + * folders containing override files to `overrides_folder`. .. code-block:: diff --git a/run_configs/lfric/atm.py b/run_configs/lfric/atm.py index 2e1be991..1d3dac66 100755 --- a/run_configs/lfric/atm.py +++ b/run_configs/lfric/atm.py @@ -166,16 +166,14 @@ def get_transformation_script(fpath, config): :rtype: Path ''' - global_transformation_script = config.source_root / 'lfric' / 'lfric_atm' / 'optimisation' / \ - 'meto-spice' / 'global.py' - local_transformation_script = config.source_root / 'lfric' / 'lfric_atm' / 'optimisation' / \ - 'meto-spice' / (fpath.relative_to(config.source_root).with_suffix('.py')) - if local_transformation_script: + optimisation_path = config.source_root / 'lfric' / 'lfric_atm' / 'optimisation' / 'meto-spice' + local_transformation_script = optimisation_path / (fpath.relative_to(config.source_root).with_suffix('.py')) + if local_transformation_script.exists(): return local_transformation_script - elif global_transformation_script: + global_transformation_script = optimisation_path / 'global.py' + if global_transformation_script.exists(): return global_transformation_script - else: - return "" + return "" if __name__ == '__main__': diff --git a/run_configs/lfric/gungho.py b/run_configs/lfric/gungho.py index db72d63a..e8789af6 100755 --- a/run_configs/lfric/gungho.py +++ b/run_configs/lfric/gungho.py @@ -27,16 +27,14 @@ def get_transformation_script(fpath, config): :rtype: Path ''' - global_transformation_script = config.source_root / 'lfric' / 'miniapps' / 'gungho_model' / 'optimisation' / \ - 'meto-spice' / 'global.py' - local_transformation_script = config.source_root / 'lfric' / 'miniapps' / 'gungho_model' / 'optimisation' / \ - 'meto-spice' / (fpath.relative_to(config.source_root).with_suffix('.py')) - if local_transformation_script: + optimisation_path = config.source_root / 'lfric' / 'miniapps' / 'gungho_model' / 'optimisation' / 'meto-spice' + local_transformation_script = optimisation_path / (fpath.relative_to(config.source_root).with_suffix('.py')) + if local_transformation_script.exists(): return local_transformation_script - elif global_transformation_script: + global_transformation_script = optimisation_path / 'global.py' + if global_transformation_script.exists(): return global_transformation_script - else: - return "" + return "" if __name__ == '__main__': From 25a1f58faf6ec6fec757d61c6b4f639db205a537 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Wed, 15 May 2024 10:14:14 +1000 Subject: [PATCH 16/16] Fixed minor errors in documentation. --- docs/source/writing_config.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/source/writing_config.rst b/docs/source/writing_config.rst index 02928a93..54a742aa 100644 --- a/docs/source/writing_config.rst +++ b/docs/source/writing_config.rst @@ -142,10 +142,11 @@ before you run the :func:`~fab.steps.analyse.analyse` step below. * For :func:`~fab.steps.psyclone.preprocess_x90`: You can pass in `common_flags` list as an argument. * For :func:`~fab.steps.psyclone.psyclone`: - You can pass in + You can pass in: + * kernel file roots to `kernel_roots`, * a function to get transformation script to `transformation_script` - (see examples in ``~fab.run_configs.lfric.gungho.py`` and ``~fab.run_configs.lfric.atm.py``), + (see examples in ``~fab.run_configs.lfric.gungho.py`` and ``~fab.run_configs.lfric.atm.py``), * command-line arguments to `cli_args`, * override for input files to `source_getter`, * folders containing override files to `overrides_folder`.