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
27 changes: 15 additions & 12 deletions easybuild/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -1890,7 +1890,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 @@ -2020,7 +2020,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 @@ -2082,7 +2082,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 @@ -2321,19 +2321,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)

# Transitional only in case some easyblocks still set/change cfg['parallel']
# Use _parallelLegacy to avoid deprecation warnings
# Remove for EasyBuild 5.0
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['maxparallel'])
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
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
37 changes: 35 additions & 2 deletions easybuild/framework/easyconfig/easyconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,12 @@ 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 'maxparallel' or the parallel property instead.", '5.1')
# Use a "hidden" property for now to match behavior as closely as possible
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 +149,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 @@ -499,6 +506,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 @@ -699,6 +711,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 'maxparallel' 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 @@ -1212,6 +1229,22 @@ def all_dependencies(self):

return self._all_dependencies

@property
def parallel(self):
"""Number of parallel jobs to be used for building etc."""
if self._parallel is None:
raise EasyBuildError("Parallelism in EasyConfig not set yet. "
"Need to call the easyblocks set_parallel first.")
return self._parallel

@parallel.setter
def parallel(self, value):
# Update backstorage and template value
self._parallel = value
self.template_values['parallel'] = value
# Backwards compat only. Remove with EasyBuild 5.1
self._config['_parallelLegacy'][0] = value

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
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
107 changes: 91 additions & 16 deletions test/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -2225,52 +2225,127 @@ def test_parallel(self):

handle, toy_ec2 = tempfile.mkstemp(prefix='easyblock_test_file_', suffix='.eb')
os.close(handle)
write_file(toy_ec2, toytxt + "\nparallel = 123\nmaxparallel = 67")
write_file(toy_ec2, toytxt + "\nparallel = 12\nmaxparallel = 6")

handle, toy_ec3 = tempfile.mkstemp(prefix='easyblock_test_file_', suffix='.eb')
os.close(handle)
write_file(toy_ec3, toytxt + "\nparallel = False")

handle, toy_ec4 = tempfile.mkstemp(prefix='easyblock_test_file_', suffix='.eb')
os.close(handle)
write_file(toy_ec4, toytxt + "\nmaxparallel = 6")

handle, toy_ec5 = tempfile.mkstemp(prefix='easyblock_test_file_', suffix='.eb')
os.close(handle)
write_file(toy_ec5, toytxt + "\nmaxparallel = False")

import easybuild.tools.systemtools as st
auto_parallel = 15
st.det_parallelism._default_parallelism = auto_parallel

# default: parallelism is derived from # available cores + ulimit
# Note that maxparallel has a default of 16, so we need a lower auto_paralle value here
test_eb = EasyBlock(EasyConfig(toy_ec))
test_eb.check_readiness_step()
self.assertTrue(isinstance(test_eb.cfg['parallel'], int) and test_eb.cfg['parallel'] > 0)
self.assertEqual(test_eb.cfg.parallel, auto_parallel)

auto_parallel = 128 # Don't limit by available CPU cores mock
st.det_parallelism._default_parallelism = auto_parallel

# only 'parallel' easyconfig parameter specified (no 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec1))
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
test_eb = EasyBlock(EasyConfig(toy_ec1))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], 13)
self.assertEqual(test_eb.cfg.parallel, 13)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 13)

# both 'parallel' and 'maxparallel' easyconfig parameters specified (no 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec2))
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
test_eb = EasyBlock(EasyConfig(toy_ec2))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], 67)
self.assertEqual(test_eb.cfg.parallel, 6)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 6)

# make sure 'parallel = False' is not overriden (no 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec3))
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
test_eb = EasyBlock(EasyConfig(toy_ec3))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg.parallel, False)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], False)

# only 'maxparallel' easyconfig parameter specified (no 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec4))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg.parallel, 6)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 6)

# make sure 'maxparallel = False' is treated as 1 (no 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec5))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], False)
self.assertEqual(test_eb.cfg.parallel, 1)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 1)

# only 'parallel' build option specified
init_config(build_options={'parallel': '9', 'validate': False})
init_config(build_options={'parallel': '13', 'validate': False})
test_eb = EasyBlock(EasyConfig(toy_ec))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], 9)
self.assertEqual(test_eb.cfg.parallel, 13)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 13)

# both 'parallel' build option and easyconfig parameter specified (no 'maxparallel')
test_eb = EasyBlock(EasyConfig(toy_ec1))
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
test_eb = EasyBlock(EasyConfig(toy_ec1))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], 9)
self.assertEqual(test_eb.cfg.parallel, 13)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 13)

# both 'parallel' and 'maxparallel' easyconfig parameters specified + 'parallel' build option
test_eb = EasyBlock(EasyConfig(toy_ec2))
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
test_eb = EasyBlock(EasyConfig(toy_ec2))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], 9)
self.assertEqual(test_eb.cfg.parallel, 6)

# make sure 'parallel = False' is not overriden (with 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec3))
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
test_eb = EasyBlock(EasyConfig(toy_ec3))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg.parallel, 0)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 0)

# only 'maxparallel' easyconfig parameter specified (with 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec4))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg.parallel, 6)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 6)

# make sure 'maxparallel = False' is treated as 1 (with 'parallel' build option)
test_eb = EasyBlock(EasyConfig(toy_ec5))
test_eb.check_readiness_step()
self.assertEqual(test_eb.cfg['parallel'], 0)
self.assertEqual(test_eb.cfg.parallel, 1)
with self.temporarily_allow_deprecated_behaviour(), self.mocked_stdout_stderr():
self.assertEqual(test_eb.cfg['parallel'], 1)

# Template updated correctly
test_eb.cfg['buildopts'] = '-j %(parallel)s'
self.assertEqual(test_eb.cfg['buildopts'], '-j 1')
# Might be done in an easyblock step
test_eb.cfg.parallel = 42
self.assertEqual(test_eb.cfg['buildopts'], '-j 42')
# Unaffected by build settings
test_eb.cfg.parallel = 421337
self.assertEqual(test_eb.cfg['buildopts'], '-j 421337')

# Reset mocked value
del st.det_parallelism._default_parallelism

def test_guess_start_dir(self):
"""Test guessing the start dir."""
Expand Down
14 changes: 5 additions & 9 deletions test/framework/easyconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ def test_tweaking(self):
'version = "3.14"',
'toolchain = {"name": "GCC", "version": "4.6.3"}',
'patches = %s',
'parallel = 1',
'maxparallel = 1',
'keepsymlinks = True',
]) % str(patches)
self.prep()
Expand All @@ -694,7 +694,7 @@ def test_tweaking(self):
# It should be possible to overwrite values with True/False/None as they often have special meaning
'runtest': 'False',
'hidden': 'True',
'parallel': 'None', # Good example: parallel=None means "Auto detect"
'maxparallel': 'None', # Good example: maxparallel=None means "unlimitted"
# Adding new options (added only by easyblock) should also be possible
# and in case the string "True/False/None" is really wanted it is possible to quote it first
'test_none': '"False"',
Expand All @@ -711,7 +711,7 @@ def test_tweaking(self):
self.assertEqual(eb['patches'], new_patches)
self.assertIs(eb['runtest'], False)
self.assertIs(eb['hidden'], True)
self.assertIsNone(eb['parallel'])
self.assertIsNone(eb['maxparallel'])
self.assertEqual(eb['test_none'], 'False')
self.assertEqual(eb['test_bool'], 'True')
self.assertEqual(eb['test_123'], 'None')
Expand Down Expand Up @@ -1931,8 +1931,7 @@ def test_alternative_easyconfig_parameters(self):

def test_deprecated_easyconfig_parameters(self):
"""Test handling of deprecated easyconfig parameters."""
os.environ.pop('EASYBUILD_DEPRECATED')
easybuild.tools.build_log.CURRENT_VERSION = self.orig_current_version
self.allow_deprecated_behaviour()
init_config()

test_ecs_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'easyconfigs', 'test_ecs')
Expand Down Expand Up @@ -3517,7 +3516,6 @@ def test_template_constant_dict(self):
'namelower': 'gzip',
'nameletter': 'g',
'nameletterlower': 'g',
'parallel': None,
'rpath_enabled': rpath,
'software_commit': '',
'sysroot': '',
Expand Down Expand Up @@ -3556,7 +3554,6 @@ def test_template_constant_dict(self):

res = template_constant_dict(ec)
res.pop('arch')
expected['parallel'] = 12
self.assertEqual(res, expected)

toy_ec = os.path.join(test_ecs_dir, 't', 'toy', 'toy-0.0-deps.eb')
Expand Down Expand Up @@ -3598,7 +3595,6 @@ def test_template_constant_dict(self):
'toolchain_name': 'system',
'toolchain_version': 'system',
'nameletterlower': 't',
'parallel': None,
'pymajver': '3',
'pyminver': '7',
'pyshortver': '3.7',
Expand Down Expand Up @@ -3634,7 +3630,7 @@ def test_template_constant_dict(self):
ec = EasyConfigParser(filename=test_ec).get_config_dict()

expected['module_name'] = None
for key in ('bitbucket_account', 'github_account', 'parallel', 'versionprefix'):
for key in ('bitbucket_account', 'github_account', 'versionprefix'):
del expected[key]

dep_names = [x[0] for x in ec['dependencies']]
Expand Down
Loading
Loading