Skip to content

Commit

Permalink
No load storm safe guard for Environment Modules
Browse files Browse the repository at this point in the history 
Environment Modules v4+ safely handles automatic module load by not
reloading already loaded module. Same goes when unloading module:
already unloaded dependency will not be evaluated again.

As a result, no safe guard test is required and it should even be
avoided to get the module dependency correctly tracked.

If module dependency declaration is skipped, no relation binds the
loaded modules. Thus unloading the dependency will not lead to the auto
unload of the dependent module. Without "is-loaded" safe guard,
dependency is always declared and loaded environment is kept consistent
by auto_handling mechanism [1]

A new ModuleTool property named "supports_safe_auto_load" is introduced.
When enabled, module load safe guard code is not generated like if
depends_on is enabled.

"supports_safe_auto_load" is enabled for EnviromentModules 4.2.4+.
Prior v4 versions were not handling safely module load of module parent
name if already loaded module were not corresponding to the default
version.

[1] https://modules.readthedocs.io/en/latest/module.html#envvar-MODULES_AUTO_HANDLING
  • Loading branch information
@xdelaruelle
xdelaruelle committed Aug 18, 2024
1 parent ff8ed1e commit fc87dba
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 40 deletions.
9 changes: 7 additions & 2 deletions easybuild/tools/module_generator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -899,8 +899,13 @@ def load_module(self, mod_name, recursive_unload=None, depends_on=False, unload_

cond_tmpl = None

# Environment Modules v4+ safely handles automatic module load by not reloading already
# loaded module. No safe guard test is required and it should even be avoided to get the
# module dependency correctly tracked.
safe_auto_load = self.modules_tool.supports_safe_auto_load

if recursive_unload is None:
recursive_unload = build_option('recursive_mod_unload') or depends_on
recursive_unload = build_option('recursive_mod_unload') or depends_on or safe_auto_load

if recursive_unload:
# wrapping the 'module load' statement with an 'is-loaded or mode == unload'
Expand All @@ -911,7 +916,7 @@ def load_module(self, mod_name, recursive_unload=None, depends_on=False, unload_
# see also http://lmod.readthedocs.io/en/latest/210_load_storms.html
cond_tmpl = "[ module-info mode remove ] || %s"

if depends_on:
if depends_on or safe_auto_load:
if multi_dep_mods and len(multi_dep_mods) > 1:
parent_mod_name = os.path.dirname(mod_name)
guard = self.is_loaded(multi_dep_mods[1:])
Expand Down
4 changes: 4 additions & 0 deletions easybuild/tools/modules.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ def __init__(self, mod_paths=None, testing=False):
self.check_module_function(allow_mismatch=build_option('allow_modules_tool_mismatch'))
self.set_and_check_version()
self.supports_depends_on = False
self.supports_safe_auto_load = False

def __str__(self):
"""String representation of this ModulesTool instance."""
Expand Down Expand Up @@ -1317,6 +1318,7 @@ class EnvironmentModules(EnvironmentModulesTcl):
REQ_VERSION = '4.0.0'
DEPR_VERSION = '4.0.0' # needs to be set as EnvironmentModules inherits from EnvironmentModulesTcl
MAX_VERSION = None
REQ_VERSION_SAFE_AUTO_LOAD = '4.2.4'
VERSION_REGEXP = r'^Modules\s+Release\s+(?P<version>\d[^+\s]*)(\+\S*)?\s'

SHOW_HIDDEN_OPTION = '--all'
Expand All @@ -1343,6 +1345,8 @@ def __init__(self, *args, **kwargs):
setvar('MODULES_LIST_TERSE_OUTPUT', '', verbose=False)

super(EnvironmentModules, self).__init__(*args, **kwargs)
version = LooseVersion(self.version)
self.supports_safe_auto_load = version >= LooseVersion(self.REQ_VERSION_SAFE_AUTO_LOAD)

def check_module_function(self, allow_mismatch=False, regex=None):
"""Check whether selected module tool matches 'module' function definition."""
Expand Down
110 changes: 72 additions & 38 deletions test/framework/module_generator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,24 +266,32 @@ def test_load(self):
"""Test load part in generated module file."""

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
# default: guarded module load (which implies no recursive unloading)
expected = '\n'.join([
'',
"if { ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
if not self.modtool.supports_safe_auto_load:
# default: guarded module load (which implies no recursive unloading)
expected = '\n'.join([
'',
"if { ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
else:
expected = '\n'.join([
'',
"module load mod_name",
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name"))

# with recursive unloading: no if is-loaded guard
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
if not self.modtool.supports_safe_auto_load:
# with recursive unloading: no if is-loaded guard
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || ![ is-loaded mod_name ] } {",
" module load mod_name",
"}",
'',
])
self.assertEqual(expected, self.modgen.load_module("mod_name", recursive_unload=True))

init_config(build_options={'recursive_mod_unload': True})
Expand Down Expand Up @@ -353,13 +361,24 @@ def test_load_multi_deps(self):
res = self.modgen.load_module('Python/3.7.4', multi_dep_mods=multi_dep_mods)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
'',
"if { ![ is-loaded Python/3.7.4 ] && ![ is-loaded Python/2.7.16 ] } {",
" module load Python/3.7.4",
'}',
'',
])
if not self.modtool.supports_safe_auto_load:
expected = '\n'.join([
'',
"if { ![ is-loaded Python/3.7.4 ] && ![ is-loaded Python/2.7.16 ] } {",
" module load Python/3.7.4",
'}',
'',
])
else:
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || [ is-loaded Python/2.7.16 ] } {",
" module load Python",
'} else {',
" module load Python/3.7.4",
'}',
'',
])
else: # Lua syntax
expected = '\n'.join([
'',
Expand Down Expand Up @@ -401,14 +420,26 @@ def test_load_multi_deps(self):
res = self.modgen.load_module('foo/1.2.3', multi_dep_mods=multi_dep_mods)

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
'',
"if { ![ is-loaded foo/1.2.3 ] && ![ is-loaded foo/2.3.4 ] && " +
"![ is-loaded foo/3.4.5 ] && ![ is-loaded foo/4.5.6 ] } {",
" module load foo/1.2.3",
'}',
'',
])
if not self.modtool.supports_safe_auto_load:
expected = '\n'.join([
'',
"if { ![ is-loaded foo/1.2.3 ] && ![ is-loaded foo/2.3.4 ] && " +
"![ is-loaded foo/3.4.5 ] && ![ is-loaded foo/4.5.6 ] } {",
" module load foo/1.2.3",
'}',
'',
])
else:
expected = '\n'.join([
'',
"if { [ module-info mode remove ] || [ is-loaded foo/2.3.4 ] || [ is-loaded foo/3.4.5 ] " +
"|| [ is-loaded foo/4.5.6 ] } {",
" module load foo",
"} else {",
" module load foo/1.2.3",
'}',
'',
])
else: # Lua syntax
expected = '\n'.join([
'',
Expand Down Expand Up @@ -453,13 +484,16 @@ def test_load_multi_deps(self):
res = self.modgen.load_module('one/1.0', multi_dep_mods=['one/1.0'])

if self.MODULE_GENERATOR_CLASS == ModuleGeneratorTcl:
expected = '\n'.join([
'',
"if { ![ is-loaded one/1.0 ] } {",
" module load one/1.0",
'}',
'',
])
if not self.modtool.supports_safe_auto_load:
expected = '\n'.join([
'',
"if { ![ is-loaded one/1.0 ] } {",
" module load one/1.0",
'}',
'',
])
else:
expected = '\nmodule load one/1.0\n'
else: # Lua syntax
expected = '\n'.join([
'',
Expand Down

0 comments on commit fc87dba

Please sign in to comment.