Skip to content
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

Deprecate use of parallel easyconfig parameter and fix updating the template value #4580

Open
wants to merge 11 commits into
base: 5.0.x
Choose a base branch
from
32 changes: 17 additions & 15 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ def post_init(self):
# but needs to be correct if the build is performed in the installation directory
self.log.info("Changing build dir to %s", self.installdir)
self.builddir = self.installdir
self.set_parallel()

# INIT/CLOSE LOG
def _init_log(self):
Expand Down Expand Up @@ -1978,7 +1979,7 @@ def skip_extensions_parallel(self, exts_filter):
exts_cnt = len(self.ext_instances)
cmds = [resolve_exts_filter_template(exts_filter, ext) for ext in self.ext_instances]

with ThreadPoolExecutor(max_workers=self.cfg['parallel']) as thread_pool:
with ThreadPoolExecutor(max_workers=self.cfg.parallel) as thread_pool:

# list of command to run asynchronously
async_cmds = [thread_pool.submit(run_shell_cmd, cmd, stdin=stdin, hidden=True, fail_on_error=False,
Expand Down Expand Up @@ -2108,7 +2109,7 @@ def install_extensions_parallel(self, install=True):
"""
self.log.info("Installing extensions in parallel...")

thread_pool = ThreadPoolExecutor(max_workers=self.cfg['parallel'])
thread_pool = ThreadPoolExecutor(max_workers=self.cfg.parallel)

running_exts = []
installed_ext_names = []
Expand Down Expand Up @@ -2170,7 +2171,7 @@ def update_exts_progress_bar_helper(running_exts, progress_size):

for _ in range(max_iter):

if not (exts_queue and len(running_exts) < self.cfg['parallel']):
if not (exts_queue and len(running_exts) < self.cfg.parallel):
break

# check whether extension at top of the queue is ready to install
Expand Down Expand Up @@ -2409,19 +2410,22 @@ def set_parallel(self):
"""Set 'parallel' easyconfig parameter to determine how many cores can/should be used for parallel builds."""
# set level of parallelism for build
par = build_option('parallel')
cfg_par = self.cfg['parallel']
if cfg_par is None:
if par is not None:
self.log.debug("Desired parallelism specified via 'parallel' build option: %s", par)
elif par is None:
par = cfg_par
self.log.debug("Desired parallelism specified via 'parallel' easyconfig parameter: %s", par)
else:
par = min(int(par), int(cfg_par))
self.log.debug("Desired parallelism: minimum of 'parallel' build option/easyconfig parameter: %s", par)

par = det_parallelism(par, maxpar=self.cfg['maxparallel'])
# Transitional only in case some easyblocks still set/change cfg['parallel']
# Use _parallelLegacy to avoid deprecation warnings
# Remove for EasyBuild 5.1
cfg_par = self.cfg['_parallelLegacy']
if cfg_par is not None:
if par is None:
par = cfg_par
else:
par = min(int(par), int(cfg_par))

par = det_parallelism(par, maxpar=self.cfg['max_parallel'])
self.log.info("Setting parallelism: %s" % par)
self.cfg['parallel'] = par
self.cfg.parallel = par

def remove_module_file(self):
"""Remove module file (if it exists), and check for ghost installation directory (and deal with it)."""
Expand Down Expand Up @@ -2467,8 +2471,6 @@ def check_readiness_step(self):
"""
Verify if all is ok to start build.
"""
self.set_parallel()

# check whether modules are loaded
loadedmods = self.modules_tool.loaded_modules()
if len(loadedmods) > 0:
Expand Down
2 changes: 0 additions & 2 deletions easybuild/framework/easyconfig/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,6 @@
'installopts': ['', 'Extra options for installation', BUILD],
'maxparallel': [16, 'Max degree of parallelism', BUILD],
'module_only': [False, 'Only generate module file', BUILD],
'parallel': [None, ('Degree of parallelism for e.g. make (default: based on the number of '
'cores, active cpuset and restrictions in ulimit)'), BUILD],
'patches': [[], "List of patches to apply", BUILD],
'prebuildopts': ['', 'Extra options pre-passed to build command.', BUILD],
'preconfigopts': ['', 'Extra options pre-passed to configure.', BUILD],
Expand Down
52 changes: 50 additions & 2 deletions easybuild/framework/easyconfig/easyconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,17 @@ def handle_deprecated_or_replaced_easyconfig_parameters(ec_method):
def new_ec_method(self, key, *args, **kwargs):
"""Check whether any replace easyconfig parameters are still used"""
# map deprecated parameters to their replacements, issue deprecation warning(/error)
if key in ALTERNATIVE_EASYCONFIG_PARAMETERS:
if key == 'parallel':
_log.deprecated("Easyconfig parameter 'parallel' is deprecated, "
"use 'max_parallel' or the parallel property instead.", '5.1')
# This "hidden" property allows easyblocks to continue using ec['parallel'] which contains
# the computed parallelism after the ready step but the EC parameter before that step.
# Easyblocks using `max_parallel` always get the value from the EC unmodified.
# Easyblocks should use either the parallel property or `max_parallel` such that the semantic is clear.
# In particular writes to ec['parallel'] do NOT update the %(parallel)s template.
# It can be removed when the deprecation expires.
key = '_parallelLegacy'
elif key in ALTERNATIVE_EASYCONFIG_PARAMETERS:
key = ALTERNATIVE_EASYCONFIG_PARAMETERS[key]
elif key in DEPRECATED_EASYCONFIG_PARAMETERS:
depr_key = key
Expand All @@ -144,7 +154,9 @@ def is_local_var_name(name):
"""
res = False
if name.startswith(LOCAL_VAR_PREFIX) or name.startswith('_'):
res = True
# Remove with EasyBuild 5.1
if name != '_parallelLegacy':
res = True
# __builtins__ is always defined as a 'local' variables
# single-letter local variable names are allowed (mainly for use in list comprehensions)
# in Python 2, variables defined in list comprehensions leak to the outside (no longer the case in Python 3)
Expand Down Expand Up @@ -502,6 +514,11 @@ def __init__(self, path, extra_options=None, build_specs=None, validate=True, hi
self.iterate_options = []
self.iterating = False

# Storage for parallel property. Mark as unset initially
self._parallel = None
# Legacy value, remove with EasyBuild 5.1
self._config['_parallelLegacy'] = [None, '', ('', )]

# parse easyconfig file
self.build_specs = build_specs
self.parse()
Expand Down Expand Up @@ -598,6 +615,7 @@ def copy(self, validate=None):
ec = EasyConfig(self.path, validate=validate, hidden=self.hidden, rawtxt=self.rawtxt)
# take a copy of the actual config dictionary (which already contains the extra options)
ec._config = copy.deepcopy(self._config)
ec._parallel = self._parallel # Might be already set, e.g. for extensions
# since rawtxt is defined, self.path may not get inherited, make sure it does
if self.path:
ec.path = self.path
Expand Down Expand Up @@ -702,6 +720,11 @@ def parse(self):
if missing_mandatory_keys:
raise EasyBuildError("mandatory parameters not provided in %s: %s", self.path, missing_mandatory_keys)

if 'parallel' in ec_vars:
# Replace value and issue better warning for EC params (as opposed to warnings meant for easyblocks)
self.log.deprecated("Easyconfig parameter 'parallel' is deprecated, use 'max_parallel' instead.", '5.1')
ec_vars['_parallelLegacy'] = ec_vars.pop('parallel')

# provide suggestions for typos. Local variable names are excluded from this check
possible_typos = [(key, difflib.get_close_matches(key.lower(), self._config.keys(), 1, 0.85))
for key in ec_vars if not is_local_var_name(key) and key not in self]
Expand Down Expand Up @@ -1235,6 +1258,31 @@ def all_dependencies(self):

return self._all_dependencies

@property
def is_parallel_set(self):
"""Return if the desired parallelism has been determined yet"""
return self._parallel is not None

@property
def parallel(self):
"""Number of parallel jobs to be used for building etc."""
if self._parallel is None:
raise ValueError("Parallelism in EasyConfig not set yet. Need to call the easyblocks set_parallel first.")
# This gets set when an easyblock changes ec['parallel'].
# It also gets set/updated in set_parallel to mirror the old behavior during the deprecation phase
parallelLegacy = self._config['_parallelLegacy'][0]
if parallelLegacy is not None:
return max(1, parallelLegacy)
return self._parallel

@parallel.setter
def parallel(self, value):
# Update backstorage and template value
self._parallel = max(1, value) # Also handles False
self.template_values['parallel'] = self._parallel
# Backwards compat only for easyblocks reading ec['parallel']. Remove with EasyBuild 5.1
self._config['_parallelLegacy'][0] = self._parallel

def dump(self, fp, always_overwrite=True, backup=False, explicit_toolchains=False):
"""
Dump this easyconfig to file, with the given filename.
Expand Down
2 changes: 1 addition & 1 deletion easybuild/framework/easyconfig/format/format.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
['preconfigopts', 'configopts'],
['prebuildopts', 'buildopts'],
['preinstallopts', 'installopts'],
['parallel', 'maxparallel'],
['maxparallel'],
]
LAST_PARAMS = ['exts_default_options', 'exts_list',
'sanity_check_paths', 'sanity_check_commands',
Expand Down
3 changes: 1 addition & 2 deletions easybuild/framework/easyconfig/templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
'bitbucket_account',
'github_account',
'name',
'parallel',
'version',
'versionsuffix',
'versionprefix',
Expand Down Expand Up @@ -108,7 +107,7 @@
'software_commit': "Git commit id to use for the software as specified by --software-commit command line option",
'sysroot': "Location root directory of system, prefix for standard paths like /usr/lib and /usr/include"
"as specify by the --sysroot configuration option",

'parallel': "Degree of parallelism for e.g. make",
}

# constant templates that can be used in easyconfigs
Expand Down
10 changes: 10 additions & 0 deletions easybuild/framework/extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ def __init__(self, mself, ext, extra_params=None):
self.options = resolve_template(copy.deepcopy(self.ext.get('options', {})),
self.cfg.template_values,
expect_resolved=False)
if 'parallel' in self.options:
# Replace value and issue better warning for EC params (as opposed to warnings meant for easyblocks)
self.log.deprecated("Easyconfig parameter 'parallel' is deprecated, use 'max_parallel' instead.", '5.1')
self.options['max_parallel'] = self.options.pop('parallel')

if extra_params:
self.cfg.extend_params(extra_params, overwrite=False)
Expand All @@ -149,6 +153,12 @@ def __init__(self, mself, ext, extra_params=None):
self.log.debug("Skipping unknown custom easyconfig parameter '%s' for extension %s/%s: %s",
key, name, version, value)

# If parallelism has been set already take potentially new limitation into account
if self.cfg.is_parallel_set:
max_par = self.cfg['max_parallel']
if max_par is not None and max_par < self.cfg.parallel:
self.cfg.parallel = max_par

self.sanity_check_fail_msgs = []
self.sanity_check_module_loaded = False
self.fake_mod_data = None
Expand Down
2 changes: 1 addition & 1 deletion easybuild/scripts/mk_tmpl_easyblock_for.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def build_step(self):
comp_fam = comp_map[self.toolchain.comp_family()]

# enable parallel build
par = self.cfg['parallel']
par = self.cfg.parallel
cmd = "build command --parallel %%d --compiler-family %%s" %% (par, comp_fam)
run_shell_cmd(cmd)

Expand Down
2 changes: 2 additions & 0 deletions easybuild/tools/systemtools.py
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,8 @@ def get_default_parallelism():
raise EasyBuildError("Specified level of parallelism '%s' is not an integer value: %s", par, err)

if maxpar is not None and maxpar < par:
if maxpar is False:
maxpar = 1
_log.info("Limiting parallelism from %s to %s", par, maxpar)
par = maxpar

Expand Down
Loading
Loading