Skip to content

Commit e4ebc9b

Browse files
authored
Merge pull request #3211 from stfc/1540_do_indirect_imports_by_name
(towards #1540) Resolve indirect imports when requested by name
2 parents 6a32322 + ec50127 commit e4ebc9b

File tree

9 files changed

+184
-49
lines changed

9 files changed

+184
-49
lines changed

.github/workflows/nemo_v5_tests.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ jobs:
5555
outputs:
5656
bench_gfortran_omp_cpu: ${{ steps.bench_gfortran_omp_cpu.outputs.time }}
5757
bench_nvfortran_omp_offload: ${{ steps.bench_nvfortran_omp_offload.outputs.time }}
58+
bench_nvfortran_omp_offload_build: ${{ steps.bench_nvfortran_omp_offload.outputs.build_time }}
5859
orca1_nvfortran_omp_offload: ${{ steps.orca1_nvfortran_omp_offload.outputs.time }}
5960
orca2_nvfortran_omp_offload: ${{ steps.orca2_nvfortran_omp_offload.outputs.time }}
6061
bench_nvfortran_omp_offload_async: ${{ steps.bench_nvfortran_omp_offload_async.outputs.time }}
@@ -266,7 +267,9 @@ jobs:
266267
unset ENABLE_PROFILING
267268
export FCFLAGS="-i4 -Mr8 -O3 -mp=gpu -gpu=mem:managed"
268269
rm -rf tests/${TEST_DIR}
270+
export BUILD_START="${SECONDS}"
269271
./makenemo -r BENCH -m linux_spack -n ${TEST_DIR} -j ${NUM_PARALLEL} -v 1
272+
export BUILD_ELAPSED=$((${SECONDS}-${BUILD_START}))
270273
271274
# Run non-reproducible test
272275
cd $NEMO_DIR/tests/${TEST_DIR}/EXP00
@@ -276,6 +279,7 @@ jobs:
276279
OMP_NUM_THREADS=4 mpirun -np 1 ./nemo
277280
export VAR_TIME=$(grep "local proces" timing.output | head -n 1 | awk '{print $4}' | tr -d s)
278281
echo "time=${VAR_TIME}" >> "${GITHUB_OUTPUT}"
282+
echo "build_time=${BUILD_ELAPSED}" >> "${GITHUB_OUTPUT}"
279283
280284
- name: NEMO 5.0 nvidia OpenMP for GPUs (UKMO ORCA1 - managed memory)
281285
id: orca1_nvfortran_omp_offload
@@ -512,6 +516,13 @@ jobs:
512516
elapsed_time: '"${{needs.run_if_on_mirror.outputs.bench_nvfortran_omp_offload}}"',
513517
'"$COMMON_FIELDS"'
514518
},
519+
{
520+
ci_test: "NEMOv5 OpenMP for GPU (BENCH) build time",
521+
nemo_version: "NEMO 5.0-RC MO patch",
522+
compiler:"nvhpc-'"$NVFORTRAN_VERSION"'",
523+
elapsed_time: '"${{needs.run_if_on_mirror.outputs.bench_nvfortran_omp_offload_build}}"',
524+
'"$COMMON_FIELDS"'
525+
},
515526
{
516527
ci_test: "NEMOv5 OpenMP for GPU (ORCA1)",
517528
nemo_version: "NEMO 5.0-RC MO patch",

changelog

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
15) PR #3211 towards #1540. Resolve indirect imports when requested
2+
by name.
3+
14
14) PR #3194 towards #3183. Adds a new get_all_accessed_symbols
25
method to Node to reduce the cost of certain transformations.
3-
6+
47
13) PR #3204 towards #3203. Fix make calls using netcdf target.
58

69
12) PR #3182 for #2772. Add support for creating StructureReferences
@@ -10,7 +13,7 @@
1013
interfaces.
1114

1215
10) PR #3191 for #3189. Ensures that Fortran '==' is converted into
13-
Sympy Eq(...).
16+
Sympy Eq(...).
1417

1518
9) PR #2944 for #2909. Add OWNED_DOF and OWNED_CELL_COLUMN values in the
1619
operates_on LFRic metadata.

doc/developer_guide/module_manager.rst

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ of `ModuleManager`.
172172

173173
.. testcode ::
174174
175-
mod_manager = ModuleManager.get(cache_active=True)
175+
ModuleManager.get().cache_active = True
176176
177177
178178
Most of the time in the PSyIR generation is currently spent in the
@@ -200,8 +200,9 @@ a path can be provided to the module manager.
200200

201201
.. testcode ::
202202
203-
mod_manager = ModuleManager.get(cache_active=True,
204-
cache_path="/tmp/my_cache_path")
203+
mod_manager = ModuleManager.get()
204+
mod_manager.cache_active = True
205+
mod_manager.cache_path = "/tmp/my_cache_path"
205206
206207
A cache file name will then be created based on the hashsum of each
207208
source code file. The combination of the provided `cache_path` and

src/psyclone/generator.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
import traceback
5353
import importlib
5454
import shutil
55-
from typing import Union, Callable, List, Tuple
55+
from typing import Union, Callable, List, Tuple, Iterable
5656
import logging
5757

5858
from fparser.api import get_reader
@@ -164,6 +164,12 @@ def load_script(
164164

165165
if hasattr(recipe_module, "RESOLVE_IMPORTS"):
166166
imports_to_resolve = recipe_module.RESOLVE_IMPORTS
167+
# If the imports_to_resolve has the list of explicit filenames, respect
168+
# these while resolving the imports.
169+
# TODO #1540: We still don't transfer the imports_to_resolve=True to
170+
# the ModuleManager for performance reasons (but we could).
171+
if isinstance(imports_to_resolve, Iterable):
172+
ModuleManager.get().resolve_indirect_imports = imports_to_resolve
167173
else:
168174
imports_to_resolve = []
169175

@@ -601,10 +607,9 @@ def main(arguments):
601607
"specify the output destination of each psykal layer.")
602608
sys.exit(1)
603609

604-
# This has to be before the Config.get, because otherwise that creates a
605-
# ModuleManager Singleton without caching
606-
mod_manager = ModuleManager.get(cache_active=args.enable_cache)
607-
# Set the ignore patterns:
610+
# Set ModuleManager properties from flags
611+
mod_manager = ModuleManager.get()
612+
mod_manager.cache_active = args.enable_cache
608613
for pattern in args.modman_file_ignore:
609614
mod_manager.add_ignore_file(pattern)
610615

src/psyclone/parse/file_info.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
import hashlib
4343
import os
4444
import pickle
45-
from typing import Optional
45+
from typing import Optional, Union, Iterable
4646

4747
from fparser.two import Fortran2003
4848
from fparser.two.parser import ParserFactory
@@ -94,12 +94,15 @@ class FileInfo:
9494
This allows using, e.g., `~/.cache/psyclone` as a cache
9595
directory for all cached files.
9696
See _get_filepath_cache() for more information.
97+
:param resolve_imports: whether to resolve imports. It can be a list
98+
of module names to provide finer control.
9799
98100
"""
99101
def __init__(self,
100102
filepath: str,
101103
cache_active: Optional[bool] = False,
102-
cache_path: Optional[str] = None
104+
cache_path: Optional[str] = None,
105+
resolve_imports: Union[bool, Iterable[str]] = False
103106
):
104107

105108
# Full path to file
@@ -144,6 +147,10 @@ def __init__(self,
144147
# is requested.
145148
self._cache_data_save: _CacheFileInfo = None
146149

150+
# Whether to resolve imports. It can be a list of module names to
151+
# provide finer control.
152+
self._resolve_imports: Union[bool, Iterable[str]] = resolve_imports
153+
147154
def _get_cache_filepath(self):
148155
"""Return the filepath of the cache.
149156
@@ -547,7 +554,9 @@ def get_psyir(
547554

548555
# We generate PSyIR from the fparser tree
549556
_, filename = os.path.split(self.filename)
550-
processor = self._processor = Fparser2Reader()
557+
processor = Fparser2Reader(
558+
resolve_modules=self._resolve_imports
559+
)
551560
self._psyir_node = processor.generate_psyir(fparse_tree, filename)
552561

553562
# TODO #2786: Uncomment if psyir nodes are serializable

src/psyclone/parse/module_manager.py

Lines changed: 79 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,54 +74,34 @@ def get(cache_active: Optional[bool] = None,
7474
cache_path: Optional[str] = None):
7575
'''Static function that if necessary creates and returns the singleton
7676
ModuleManager instance.
77-
78-
:param use_caching: If `True`, a file-based caching of the fparser
79-
tree will be used. This can significantly accelerate obtaining
80-
a PSyIR from a source file.
81-
For parallel builds, parallel race conditions to the cache file
82-
can happen, but this shouldn't lead to wrong results. However,
83-
that's untested so far.
84-
85-
:param cache_path: If set, the cache file will be stored in the given
86-
path (directory) using a hashsum of the source code to create
87-
a unique cache file name. If `None`, a cache file will be created
88-
in the same directory as the source file with a new
89-
file ending `.psycache`.
90-
9177
'''
9278
if not ModuleManager._instance:
93-
ModuleManager._instance = ModuleManager(cache_active, cache_path)
79+
ModuleManager._instance = ModuleManager()
9480

9581
return ModuleManager._instance
9682

9783
# ------------------------------------------------------------------------
98-
def __init__(
99-
self,
100-
cache_active: Optional[bool] = None,
101-
cache_path: Optional[str] = None
102-
):
84+
def __init__(self):
10385
"""
10486
Set up the module manager. Module manager is actually a singleton
10587
and should not be created directly. Use `ModuleManager.get()`
10688
instead.
10789
108-
:param cache_active: Whether to use (`True`) or
109-
disable (`False`) caching
110-
:param cache_path: Path to the cache directory. If `None`, the
111-
cache file will be created in the same directory as the source
112-
file with a new file ending `.psycache`.
11390
"""
11491

11592
if ModuleManager._instance is not None:
11693
raise InternalError("You need to use 'ModuleManager.get()' "
11794
"to get the singleton instance.")
11895

11996
# Disable caching by default
120-
self._cache_active = (
121-
cache_active if cache_active is not None else False)
97+
self._cache_active = False
12298

12399
# Path to cache
124-
self._cache_path: Optional[str] = cache_path
100+
self._cache_path: Optional[str] = None
101+
102+
# Whether to resolve imports while inspecting another import.
103+
# It can be a list of specific module names for finer control.
104+
self._resolve_indirect_imports: Union[bool, Iterable[str]] = False
125105

126106
self._visited_files: dict[str, FileInfo] = {}
127107

@@ -159,6 +139,75 @@ def __init__(
159139
self._module_pattern = re.compile(r"^\s*module\s+([a-z]\S*)\s*$",
160140
flags=re.IGNORECASE | re.MULTILINE)
161141

142+
@property
143+
def cache_active(self) -> bool:
144+
'''
145+
:returns: whether the parsed files will be cached.
146+
'''
147+
return self._cache_active
148+
149+
@cache_active.setter
150+
def cache_active(self, value: bool):
151+
'''
152+
:param value: specify whether the parsed files will be cached.
153+
154+
:raises TypeError: if the provided value is not a bool.
155+
'''
156+
if not isinstance(value, bool):
157+
raise TypeError(
158+
f"'cache_active' must be a bool, but found {type(value)}")
159+
self._cache_active = value
160+
161+
@property
162+
def cache_path(self) -> Optional[str]:
163+
'''
164+
:returns: the path where the cache file will be stored, if it is None
165+
the file will be created in the same directory as the source file.
166+
'''
167+
return self._cache_path
168+
169+
@cache_path.setter
170+
def cache_path(self, value: Optional[str]):
171+
'''
172+
:param value: specify the path where the cache file will be stored, if
173+
None the file will be created in the same directory as the source
174+
file.
175+
176+
:raises TypeError: if the provided value is not a str.
177+
'''
178+
if value is not None and not isinstance(value, str):
179+
raise TypeError(
180+
f"'cache_path' must be a str, but found {type(value)}")
181+
self._cache_path = value
182+
183+
@property
184+
def resolve_indirect_imports(self) -> Union[bool, Iterable[str]]:
185+
'''
186+
:returns: whether indirect imports will be imported (if found). This
187+
can be a list of module names to provide finer control.
188+
'''
189+
return self._resolve_indirect_imports
190+
191+
@resolve_indirect_imports.setter
192+
def resolve_indirect_imports(self, value: Union[bool, Iterable[str]]):
193+
'''
194+
:param value: specify whether indirect imports will be imported (if
195+
found). This can be a list of module names for finer control.
196+
197+
:raises TypeError: if the provided value is not bool or Iterable[str].
198+
'''
199+
if not isinstance(value, (Iterable, bool)):
200+
raise TypeError(
201+
f"'resolve_indirect_imports' must be a boolean or an Iterable,"
202+
f" but found {type(value)}")
203+
if isinstance(value, Iterable):
204+
for x in value:
205+
if not isinstance(x, str):
206+
raise TypeError(
207+
f"'resolve_indirect_imports' must be an Iterable of "
208+
f"str, but found an item of {type(x)}")
209+
self._resolve_indirect_imports = value
210+
162211
# ------------------------------------------------------------------------
163212
def add_search_path(self, directories, recursive=True):
164213
'''If the directory is not already contained in the search path,
@@ -219,7 +268,8 @@ def _add_all_files_from_dir(self, directory: str) -> list[FileInfo]:
219268
FileInfo(
220269
full_path,
221270
cache_active=self._cache_active,
222-
cache_path=self._cache_path
271+
cache_path=self._cache_path,
272+
resolve_imports=self._resolve_indirect_imports
223273
)
224274
new_files.append(self._visited_files[full_path])
225275
return new_files

src/psyclone/tests/generator_test.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,20 +1078,25 @@ def trans(psyir):
10781078

10791079

10801080
@pytest.mark.parametrize(
1081-
"idx, value, output",
1082-
[("0", "False", "result = a + b"),
1083-
("1", "True", "result = 1 + 1"),
1084-
("2", "[\"module1\"]", "result = 1 + b"),
1085-
("3", "[\"module2\"]", "result = a + 1"),
1081+
"idx, value, output", [
1082+
("0", "False", "result = a + b + c"),
1083+
# Indirect import is not resolved
1084+
("1", "True", "result = 1 + 1 + c"),
1085+
("2", "[\"module1\"]", "result = 1 + b + c"),
1086+
("3", "[\"module2\"]", "result = a + 1 + c"),
1087+
# Indirect import resolved by name
1088+
("4", "[\"module1\",\"module3\"]", "result = 1 + b + 1"),
10861089
# Now change both with case insensitive names
1087-
("4", "[\"mOdule1\",\"moduLe2\"]", "result = 1 + 1")])
1090+
("5", "[\"mOdule1\",\"moduLe2\"]", "result = 1 + 1 + c")
1091+
])
10881092
def test_code_transformation_resolve_imports(tmpdir, capsys, monkeypatch,
10891093
idx, value, output):
10901094
''' Test that applying recipes in the code-transformation mode follows the
10911095
selected list of module names when generating the tree. '''
10921096

10931097
module1 = '''
10941098
module module1
1099+
use module3
10951100
integer :: a
10961101
end module module1
10971102
'''
@@ -1100,14 +1105,19 @@ def test_code_transformation_resolve_imports(tmpdir, capsys, monkeypatch,
11001105
integer :: b
11011106
end module module2
11021107
'''
1108+
module3 = '''
1109+
module module3
1110+
integer :: c
1111+
end module module3
1112+
'''
11031113
code = '''
11041114
module test
11051115
use module1
11061116
use module2
11071117
real :: result
11081118
contains
11091119
subroutine mytest()
1110-
result = a + b
1120+
result = a + b + c
11111121
end subroutine mytest
11121122
end module test
11131123
'''
@@ -1127,13 +1137,15 @@ def trans(psyir):
11271137
recipe_name = f"replace_integers_{idx}.py"
11281138
for filename, content in [("module1.f90", module1),
11291139
("module2.f90", module2),
1140+
("module3.f90", module3),
11301141
("code.f90", code),
11311142
(recipe_name, recipe)]:
11321143
with open(tmpdir.join(filename), "w", encoding='utf-8') as my_file:
11331144
my_file.write(content)
11341145

11351146
# Execute the recipe (no -I needed as we have everything at the same place)
11361147
monkeypatch.chdir(tmpdir)
1148+
ModuleManager._instance = None
11371149
main(["code.f90", "-s", recipe_name])
11381150
captured = capsys.readouterr()
11391151

0 commit comments

Comments
 (0)