-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mpi omp support #336
base: master
Are you sure you want to change the base?
Mpi omp support #336
Changes from 250 commits
302d58b
4068505
f24d939
075ed40
15fca77
d862762
880355a
e999cd1
fdc402c
b50b3df
b3806f1
2cf63d3
2fffe56
7a8c293
555de52
abec46d
d4ad3f0
a64c9b7
8a8781d
d967f7f
5472d7d
4c2f0d8
ccbe839
227a153
b8c64b9
e309c15
31a4877
fbb15ea
7f52d40
ca58d67
c218bcd
e14d5b3
0986b43
2e15f19
1eeaa35
a45d62b
a4a9aab
5978e80
e70885d
d2c6db0
a2ac207
3a5e86a
4ab3737
92545f8
f7cbf39
632fa8c
e0261c7
f6357a1
042e8d8
87bce71
14e4f7c
d38f020
672bb9c
8987f99
eee5043
f9fdbbb
be36931
8c790ce
ade5f67
15212ae
48c7543
664c89c
44756f7
064ea35
22a1b91
2e1b0a1
921daa8
fc668cd
47a0a70
4389374
3a015d2
202e274
3667030
860d5a3
25a1f58
2562c22
72f7ab5
53aeb62
8723a4e
358ae23
71e0b2f
b33b8b5
1109a88
8e303f5
393f98a
2464e99
cce68e9
8d9b5c5
572055a
01558a2
4d85e70
10067a1
137eb84
b95611f
5f6625c
3d58184
f2942b1
d005c47
73f91d4
6d5e82e
37f7adc
ac93579
2f633b9
03c6076
499c9b5
7570696
41d9273
56f001a
8fb2be1
71cd141
5b959c3
0f6c1a5
588f647
cd81ce3
e857c94
e024824
a1346cf
a0c2b98
169ff69
a34febc
df893d6
a2a8a54
f03bc37
7632775
e73179f
f7919ce
50f4a42
36fa57b
77b2b97
07e86d4
5c895ba
e556115
8a552f0
f0e660d
9f1ffc3
a0531a2
4ff887c
8a3b02d
10ae159
622d0bd
3f48703
ba9245d
f8c4418
6a7aef8
0b73089
7796246
412a136
8ec2227
6d1a1ee
8d4c66f
46bc4ef
ffdfd1a
4f436f3
52d5751
562d56f
ca7e477
1150318
516ec34
c17545a
19005e3
ee2fc85
5912e59
83ba6a1
7e66eca
ddd3cf2
68bc218
2bdcb8c
c02729c
a29be95
03bed8e
9ccdefd
00fa8b2
c21d34f
d8fe42b
2f069c8
9abfc1d
772e79c
c04ab9e
87b8012
523e876
538e4da
e3114c7
88cf3ea
0f1ead3
8ab8c40
674575e
c980e7e
0a61359
ff306da
c5b1b02
ccf4ad8
fd02c8c
10eb726
ea18256
6e6579a
932f401
120116c
5a24c24
dbc3d73
90adcca
150dc37
db34597
7c347c8
ff71393
5dc01f3
2b5c4bd
46706c5
d55344f
d00c187
e9333fd
a85bc7a
6c8e8df
3bb726e
286d457
dae9102
cdcc023
0bdb734
b97ec50
b1b4057
5b51aa1
19eab9f
cbe1cb8
bf81dee
196e034
3076715
7a50859
749fa2d
20fef2c
9598c09
5c6f99f
c1eb158
1524777
02fbb7b
955befc
552e4cc
b38cdb8
6826c85
e5aca3b
ccbe810
f8f8ab0
1543841
cd8ec62
13935c0
de80c44
009478f
7887d24
83836e7
a79b41d
2617322
f165e46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,8 @@ | |
|
||
from fab.artefacts import ArtefactSet, ArtefactStore | ||
from fab.constants import BUILD_OUTPUT, SOURCE_ROOT, PREBUILD | ||
from fab.metrics import send_metric, init_metrics, stop_metrics, metrics_summary | ||
from fab.metrics import (send_metric, init_metrics, stop_metrics, | ||
metrics_summary) | ||
from fab.tools.category import Category | ||
from fab.tools.tool_box import ToolBox | ||
from fab.steps.cleanup_prebuilds import CLEANUP_COUNT, cleanup_prebuilds | ||
|
@@ -41,36 +42,50 @@ class BuildConfig(): | |
""" | ||
def __init__(self, project_label: str, | ||
tool_box: ToolBox, | ||
multiprocessing: bool = True, n_procs: Optional[int] = None, | ||
mpi: bool, | ||
openmp: bool, | ||
multiprocessing: bool = True, | ||
n_procs: Optional[int] = None, | ||
reuse_artefacts: bool = False, | ||
fab_workspace: Optional[Path] = None, two_stage=False, | ||
verbose=False): | ||
fab_workspace: Optional[Path] = None, | ||
two_stage: bool = False, | ||
verbose: bool = False): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A general observation that this argument list is getting pretty unwieldy. Something to consider at a future date. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, that seems unavoidable - in a complex build systems there are a lot of things to be defined :) Now that mpi/openmp are optional, it might be better? Also, this mostly will be handled by the command line tool in the end, which should have an appropriate man page/help command. |
||
""" | ||
:param project_label: | ||
Name of the build project. The project workspace folder is created from this name, with spaces replaced | ||
by underscores. | ||
Name of the build project. The project workspace folder is | ||
created from this name, with spaces replaced by underscores. | ||
:param tool_box: The ToolBox with all tools to use in the build. | ||
:param mpi: whether the project uses MPI or not. This is used to | ||
pick a default compiler (if not explicitly set in the ToolBox), | ||
and controls PSyclone parameters. | ||
:param openmp: whether the project should use OpenMP or not. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This argument presumably has the same caveats as the previous one. Is there a way to cover this without repeating boiler plate? Either group them and have some group documentation or with suitable wording. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it doesn't. We looked at all compiolers to be reasonably expected to be used (intel-classic, intel-llvm, gnu, clang, cray, nvidia) all of which support openmp. |
||
:param multiprocessing: | ||
An option to disable multiprocessing to aid debugging. | ||
:param n_procs: | ||
The number of cores to use for multiprocessing operations. Defaults to the number of available cores. | ||
The number of cores to use for multiprocessing operations. | ||
Defaults to the number of available cores. | ||
:param reuse_artefacts: | ||
A flag to avoid reprocessing certain files on subsequent runs. | ||
WARNING: Currently unsophisticated, this flag should only be used by Fab developers. | ||
The logic behind flag will soon be improved, in a work package called "incremental build". | ||
WARNING: Currently unsophisticated, this flag should only be | ||
used by Fab developers. The logic behind flag will soon be | ||
improved, in a work package called "incremental build". | ||
:param fab_workspace: | ||
Overrides the FAB_WORKSPACE environment variable. | ||
If not set, and FAB_WORKSPACE is not set, the fab workspace defaults to *~/fab-workspace*. | ||
If not set, and FAB_WORKSPACE is not set, the fab workspace | ||
defaults to *~/fab-workspace*. | ||
:param two_stage: | ||
Compile .mod files first in a separate pass. Theoretically faster in some projects.. | ||
Compile .mod files first in a separate pass. Theoretically faster | ||
in some projects. | ||
:param verbose: | ||
DEBUG level logging. | ||
|
||
""" | ||
self._tool_box = tool_box | ||
self._mpi = mpi | ||
self._openmp = openmp | ||
self.two_stage = two_stage | ||
self.verbose = verbose | ||
compiler = tool_box[Category.FORTRAN_COMPILER] | ||
compiler = tool_box.get_tool(Category.FORTRAN_COMPILER, mpi=mpi) | ||
project_label = Template(project_label).safe_substitute( | ||
compiler=compiler.name, | ||
two_stage=f'{int(two_stage)+1}stage') | ||
|
@@ -83,7 +98,8 @@ def __init__(self, project_label: str, | |
logger.info(f"fab workspace is {fab_workspace}") | ||
|
||
self.project_workspace: Path = fab_workspace / self.project_label | ||
self.metrics_folder: Path = self.project_workspace / 'metrics' / self.project_label | ||
self.metrics_folder: Path = (self.project_workspace / 'metrics' / | ||
self.project_label) | ||
|
||
# source config | ||
self.source_root: Path = self.project_workspace / SOURCE_ROOT | ||
|
@@ -93,7 +109,8 @@ def __init__(self, project_label: str, | |
self.multiprocessing = multiprocessing | ||
|
||
# turn off multiprocessing when debugging | ||
# todo: turn off multiprocessing when running tests, as a good test runner will run using mp | ||
# todo: turn off multiprocessing when running tests, as a good test | ||
# runner will run using mp | ||
if 'pydevd' in str(sys.gettrace()): | ||
logger.info('debugger detected, running without multiprocessing') | ||
self.multiprocessing = False | ||
|
@@ -129,7 +146,8 @@ def __enter__(self): | |
self._start_time = datetime.now().replace(microsecond=0) | ||
self._run_prep() | ||
|
||
with TimerLogger(f'running {self.project_label} build steps') as build_timer: | ||
with TimerLogger(f'running {self.project_label} ' | ||
f'build steps') as build_timer: | ||
# this will return to the build script | ||
self._build_timer = build_timer | ||
return self | ||
|
@@ -138,10 +156,12 @@ def __exit__(self, exc_type, exc_val, exc_tb): | |
|
||
if not exc_type: # None if there's no error. | ||
if CLEANUP_COUNT not in self.artefact_store: | ||
logger.info("no housekeeping step was run, using a default hard cleanup") | ||
logger.info("no housekeeping step was run, using a " | ||
"default hard cleanup") | ||
cleanup_prebuilds(config=self, all_unused=True) | ||
|
||
logger.info(f"Building '{self.project_label}' took {datetime.now() - self._start_time}") | ||
logger.info(f"Building '{self.project_label}' took " | ||
f"{datetime.now() - self._start_time}") | ||
|
||
# always | ||
self._finalise_metrics(self._start_time, self._build_timer) | ||
|
@@ -164,9 +184,20 @@ def build_output(self) -> Path: | |
''' | ||
return self.project_workspace / BUILD_OUTPUT | ||
|
||
@property | ||
def mpi(self) -> bool: | ||
''':returns: whether MPI is requested or not in this config.''' | ||
return self._mpi | ||
|
||
@property | ||
def openmp(self) -> bool: | ||
''':returns: whether OpenMP is requested or not in this config.''' | ||
return self._openmp | ||
|
||
def add_current_prebuilds(self, artefacts: Iterable[Path]): | ||
""" | ||
Mark the given file paths as being current prebuilds, not to be cleaned during housekeeping. | ||
Mark the given file paths as being current prebuilds, not to be | ||
cleaned during housekeeping. | ||
|
||
""" | ||
self.artefact_store[ArtefactSet.CURRENT_PREBUILDS].update(artefacts) | ||
|
@@ -193,7 +224,8 @@ def _prep_folders(self): | |
def _init_logging(self): | ||
# add a file logger for our run | ||
self.project_workspace.mkdir(parents=True, exist_ok=True) | ||
log_file_handler = RotatingFileHandler(self.project_workspace / 'log.txt', backupCount=5, delay=True) | ||
log_file_handler = RotatingFileHandler( | ||
self.project_workspace / 'log.txt', backupCount=5, delay=True) | ||
log_file_handler.doRollover() | ||
logging.getLogger('fab').addHandler(log_file_handler) | ||
|
||
|
@@ -207,9 +239,11 @@ def _init_logging(self): | |
def _finalise_logging(self): | ||
# remove our file logger | ||
fab_logger = logging.getLogger('fab') | ||
log_file_handlers = list(by_type(fab_logger.handlers, RotatingFileHandler)) | ||
log_file_handlers = list(by_type(fab_logger.handlers, | ||
RotatingFileHandler)) | ||
if len(log_file_handlers) != 1: | ||
warnings.warn(f'expected to find 1 RotatingFileHandler for removal, found {len(log_file_handlers)}') | ||
warnings.warn(f'expected to find 1 RotatingFileHandler for ' | ||
f'removal, found {len(log_file_handlers)}') | ||
fab_logger.removeHandler(log_file_handlers[0]) | ||
|
||
def _finalise_metrics(self, start_time, steps_timer): | ||
|
@@ -249,14 +283,16 @@ def __init__(self, match: str, flags: List[str]): | |
# For source in the um folder, add an absolute include path | ||
AddFlags(match="$source/um/*", flags=['-I$source/include']), | ||
|
||
# For source in the um folder, add an include path relative to each source file. | ||
# For source in the um folder, add an include path relative to | ||
# each source file. | ||
AddFlags(match="$source/um/*", flags=['-I$relative/include']), | ||
|
||
""" | ||
self.match: str = match | ||
self.flags: List[str] = flags | ||
|
||
# todo: we don't need the project_workspace, we could just pass in the output folder | ||
# todo: we don't need the project_workspace, we could just pass in the | ||
# output folder | ||
def run(self, fpath: Path, input_flags: List[str], config): | ||
""" | ||
Check if our filter matches a given file. If it does, add our flags. | ||
|
@@ -269,12 +305,16 @@ def run(self, fpath: Path, input_flags: List[str], config): | |
Contains the folders for templating `$source` and `$output`. | ||
|
||
""" | ||
params = {'relative': fpath.parent, 'source': config.source_root, 'output': config.build_output} | ||
params = {'relative': fpath.parent, | ||
'source': config.source_root, | ||
'output': config.build_output} | ||
|
||
# does the file path match our filter? | ||
if not self.match or fnmatch(str(fpath), Template(self.match).substitute(params)): | ||
if not self.match or fnmatch(str(fpath), | ||
Template(self.match).substitute(params)): | ||
# use templating to render any relative paths in our flags | ||
add_flags = [Template(flag).substitute(params) for flag in self.flags] | ||
add_flags = [Template(flag).substitute(params) | ||
for flag in self.flags] | ||
|
||
# add our flags | ||
input_flags += add_flags | ||
|
@@ -284,15 +324,18 @@ class FlagsConfig(): | |
""" | ||
Return command-line flags for a given path. | ||
|
||
Simply allows appending flags but may evolve to also replace and remove flags. | ||
Simply allows appending flags but may evolve to also replace and | ||
remove flags. | ||
|
||
""" | ||
def __init__(self, common_flags: Optional[List[str]] = None, path_flags: Optional[List[AddFlags]] = None): | ||
def __init__(self, common_flags: Optional[List[str]] = None, | ||
path_flags: Optional[List[AddFlags]] = None): | ||
""" | ||
:param common_flags: | ||
List of flags to apply to all files. E.g `['-O2']`. | ||
:param path_flags: | ||
List of :class:`~fab.build_config.AddFlags` objects which apply flags to selected paths. | ||
List of :class:`~fab.build_config.AddFlags` objects which apply | ||
flags to selected paths. | ||
|
||
""" | ||
self.common_flags = common_flags or [] | ||
|
@@ -311,8 +354,8 @@ def flags_for_path(self, path: Path, config): | |
|
||
""" | ||
# We COULD make the user pass these template params to the constructor | ||
# but we have a design requirement to minimise the config burden on the user, | ||
# so we take care of it for them here instead. | ||
# but we have a design requirement to minimise the config burden on | ||
# the user, so we take care of it for them here instead. | ||
params = {'source': config.source_root, 'output': config.build_output} | ||
flags = [Template(i).substitute(params) for i in self.common_flags] | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is an argument for making these optional with a default to
False
but that is not an argument we need to have now. It's always possible to make an interface more permissive.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in our meeting, I've made
False
the default, and updated tests and run_configs.