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

add generic SystemMPI easyblock to generate module for existing MPI library installation #1106

Merged
merged 64 commits into from
Oct 11, 2017
Merged
Show file tree
Hide file tree
Changes from 61 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
c2fcd86
First go at a system OpenMPI easyblock
Feb 10, 2017
b3bdaae
Switch computer
Feb 13, 2017
902b9cc
Make extract_compiler_version available to SystemMPI
Feb 14, 2017
8f789ce
Update outdated comments
Feb 15, 2017
da237ca
Fix variable typo
Feb 15, 2017
d3d4c5f
Correctly update version in make_module_extend_modpath
Feb 15, 2017
02520ce
Correct syntax problem
Feb 15, 2017
3309b16
Add support for impi
Feb 16, 2017
6d380fe
Address review comments
Feb 16, 2017
f89a9e4
Add OpenMPI to travis
Feb 16, 2017
78f6115
Allow tests on Travis to pass
Feb 16, 2017
5e7eaa5
Allow tests on Travis to pass
Feb 16, 2017
7fc6265
Typo
Feb 16, 2017
b9ced9d
Add another cheat to get Travis to pass his tests
Feb 16, 2017
1877b18
Stop cheating on tests
Feb 16, 2017
c52c92d
Add special case to tests for SystemMPI
Feb 16, 2017
44026e6
Roll back toolchain addition in tests
Feb 16, 2017
7b7f704
Add special case for dummy to module-only test
Feb 16, 2017
abacb35
Fix bugs in impi path
Feb 16, 2017
cfa57f6
Need to go one deeper in search for impi root due to symlink
Feb 16, 2017
212e53a
systemcompiler is not good enough to resolve compilers not in the def…
Feb 16, 2017
bcab0fa
Update systemcompiler.py
Feb 16, 2017
c51a0f7
Add option to systemcompiler to attempt to find the path extensions f…
Feb 17, 2017
4f76ca0
Merge remote-tracking branch 'origin/systemmpi' into systemmpi
Feb 17, 2017
7850024
Add author
Feb 17, 2017
eae2b8e
Implement path extensions for systemmpi
Feb 17, 2017
f4a3ea6
Implement path extensions for systemmpi
Feb 17, 2017
5a88443
Make sure systemcompiler finds the correct install paths for version …
Feb 17, 2017
b0ee25e
Try to get system compiler to match the correct version of intel comp…
Feb 17, 2017
f144400
Fix compiler version detection for intel
Feb 17, 2017
7b525b0
Fix dodgy variable name
Feb 17, 2017
60b86ce
Allow picking up of the licence for intel compilers
Feb 17, 2017
3d5263c
Make sure 201* is treated as a number
Feb 17, 2017
4096508
Support older version of impi
Feb 23, 2017
8e32528
Address comments
Mar 1, 2017
ccee79b
Use the GCC init as the bundle causes errors because of unexpected va…
Mar 1, 2017
89e13be
Change order in extra_options to avoid the error in __init__
Mar 1, 2017
637a815
Change order in extra_options to avoid the error in __init__
Mar 1, 2017
5f17da8
Add arguments to Bundle.make_module_extra
Mar 1, 2017
b62ac7c
Fix error with make_module_extra when not having __init__ from the bu…
Mar 1, 2017
8d0dd01
Ignore the intelbase prepare_step, just use the bundle one
Mar 1, 2017
e3985f9
Try to overcome make_module_extra error
Mar 1, 2017
53b3c34
Try to overcome make_module_extra error by passing the arguments down
Mar 1, 2017
e88e2a1
Try to overcome make_module_extra error by passing the arguments down
Mar 1, 2017
0078d2f
Try to overcome make_module_extra error by passing the arguments down
Mar 1, 2017
0250ce3
Try to overcome make_module_extra error by passing the arguments down
Mar 1, 2017
6eaf360
Add missing LooseVersion
Mar 1, 2017
2f5a09d
Add missing LooseVersion
Mar 1, 2017
4a3cc32
Address comments for systemcompiler.py
Sep 15, 2017
61ba4e5
Merge branch 'develop' into systemmpi
Sep 15, 2017
58cb72a
Address comments for systemmpi.py
Sep 15, 2017
fddb19b
Allow either icc or gcc to sit underneath impi
Sep 15, 2017
a1facb1
Add `*args, **kwargs` to prepare_step(s)
Sep 15, 2017
570cf47
Add `*args, **kwargs` to prepare_step(s)...in full this time
Sep 15, 2017
3a1496a
Add `*args, **kwargs` to prepare_step(s)...in full this time
Sep 15, 2017
951a974
Revert silly changes
Sep 15, 2017
7594102
Fix attribute bug
Sep 15, 2017
477002f
pass in post_install_step to avoid problems with symlinks created by …
boegel Sep 30, 2017
c90a799
Merge pull request #10 from boegel/systemmpi
Sep 30, 2017
f35d52a
fix error reporting for missing MPI compiler wrapper for impi
boegel Sep 30, 2017
2206203
Merge pull request #11 from boegel/systemmpi
Sep 30, 2017
71cec42
pass down *args and **kwargs in make_module_extra rather than working…
boegel Oct 11, 2017
aa2e77e
improve error reporting for missing compiler, fix error reporting for…
boegel Oct 11, 2017
a43637a
Merge pull request #12 from boegel/systemmpi
Oct 11, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ addons:
apt:
packages:
- tcl8.5
before_install:
# For testing systemmpi easyblock we need an OpenMPI instance
- sudo apt-get update && sudo apt-get install -y libopenmpi-dev openmpi-bin
install:
# install easybuild-framework (and dependencies)
# prefer clone & easy_install over easy_install directly in order to obtain information of what was installed exactly
Expand Down
2 changes: 1 addition & 1 deletion easybuild/easyblocks/generic/bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def install_step(self):

def make_module_extra(self):
"""Set extra stuff in module file, e.g. $EBROOT*, $EBVERSION*, etc."""
return super(Bundle, self).make_module_extra(altroot=self.altroot, altversion=self.altversion)
return EasyBlock.make_module_extra(self, altroot=self.altroot, altversion=self.altversion)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a better way of fixing the issue you bumped into, see ocaisa#12


def sanity_check_step(self, *args, **kwargs):
"""
Expand Down
180 changes: 153 additions & 27 deletions easybuild/easyblocks/generic/systemcompiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,64 @@

@author Bernd Mohr (Juelich Supercomputing Centre)
@author Kenneth Hoste (Ghent University)
@author Alan O'Cais (Juelich Supercomputing Centre)
"""
import os
import re
from vsc.utils import fancylogger
from distutils.version import LooseVersion

from easybuild.easyblocks.generic.bundle import Bundle
from easybuild.tools.filetools import read_file, which
from easybuild.tools.run import run_cmd
from easybuild.easyblocks.icc import EB_icc
from easybuild.easyblocks.ifort import EB_ifort
from easybuild.easyblocks.gcc import EB_GCC
from easybuild.framework.easyconfig.easyconfig import ActiveMNS
from easybuild.framework.easyconfig import CUSTOM
from easybuild.tools.build_log import EasyBuildError
from easybuild.tools.filetools import read_file, resolve_path, which
from easybuild.tools.run import run_cmd

_log = fancylogger.getLogger('easyblocks.generic.systemcompiler')

def extract_compiler_version(compiler_name):
"""Extract compiler version for provided compiler_name."""
# look for 3-4 digit version number, surrounded by spaces
# examples:
# gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-11)
# Intel(R) C Intel(R) 64 Compiler XE for applications running on Intel(R) 64, Version 15.0.1.133 Build 20141023
version_regex = re.compile(r'\s([0-9]+(?:\.[0-9]+){1,3})\s', re.M)
if compiler_name == 'gcc':
out, _ = run_cmd("gcc --version", simple=False)
res = version_regex.search(out)
if res is None:
raise EasyBuildError("Could not extract GCC version from %s", out)
compiler_version = res.group(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure res is not None to avoid a nasty traceback, spit out a meaningful error instead

elif compiler_name in ['icc', 'ifort']:
# A fully resolved icc/ifort (without symlinks) includes the release version in the path
# e.g. .../composer_xe_2015.3.187/bin/intel64/icc
# Match the last incidence of _ since we don't know what might be in the path, then split it up on /
out = (resolve_path(which(compiler_name)).split("_")[-1]).split("/")
compiler_version = out[0]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, shouldn't we do a sanity check on this to make sure what we end up with looks like a version? e.g. check for only digits and .s?

# Check what we have looks like a version number (the regex we use requires spaces around the version number)
if version_regex.search(" " + compiler_version + " ") is None:
typical_install_path = '.../composer_xe_2015.3.187/bin/intel64/icc'
raise EasyBuildError(
"Derived Intel compiler version %s doesn't look correct, is compiler installed in a path like %s?",
typical_install_path, compiler_version
)
else:
raise EasyBuildError("Unknown compiler %s", compiler_name)

if compiler_version:
_log.debug("Extracted compiler version '%s' for %s from: %s", compiler_version, compiler_name, out)
else:
raise EasyBuildError("Failed to extract compiler version for %s using regex pattern '%s' from: %s",
compiler_name, version_regex.pattern, out)

class SystemCompiler(Bundle):
return compiler_version

# No need to inherit from EB_icc since EB_ifort already inherits from that
class SystemCompiler(Bundle, EB_GCC, EB_ifort):
"""
Support for generating a module file for the system compiler with specified name.

Expand All @@ -48,46 +94,49 @@ class SystemCompiler(Bundle):
if an actual version is specified, it is checked against the derived version of the system compiler that was found.
"""

def extract_compiler_version(self, txt):
"""Extract compiler version from provided string."""
# look for 3-4 digit version number, surrounded by spaces
# examples:
# gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-11)
# Intel(R) C Intel(R) 64 Compiler XE for applications running on Intel(R) 64, Version 15.0.1.133 Build 20141023
version_regex = re.compile(r'\s([0-9]+(?:\.[0-9]+){1,3})\s', re.M)
res = version_regex.search(txt)
if res:
self.compiler_version = res.group(1)
self.log.debug("Extracted compiler version '%s' from: %s", self.compiler_version, txt)
else:
raise EasyBuildError("Failed to extract compiler version using regex pattern '%s' from: %s",
version_regex.pattern, txt)
@staticmethod
def extra_options():
"""Add custom easyconfig parameters for SystemCompiler easyblock."""
# Gather extra_vars from inherited classes, order matters here to make this work without problems in __init__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing docstring:

def extra_options():
    """Add custom easyconfig parameters for SystemCompiler easyblock."""
    # Gather ...

extra_vars = EB_GCC.extra_options()
extra_vars.update(EB_icc.extra_options())
extra_vars.update(EB_ifort.extra_options())
extra_vars.update(Bundle.extra_options())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change accordingly if you list Bundle first

# Add an option to add all module path extensions to the resultant easyconfig
# This is useful if you are importing a compiler from a non-default path
extra_vars.update({
'generate_standalone_module': [
False,
"Add known path/library extensions and environment variables for the compiler to the final module",
CUSTOM
],
})
return extra_vars

def __init__(self, *args, **kwargs):
"""Extra initialization: determine system compiler version and prefix."""
super(SystemCompiler, self).__init__(*args, **kwargs)

# Determine compiler path (real path, with resolved symlinks)
compiler_name = self.cfg['name'].lower()
if compiler_name == 'gcccore':
compiler_name = 'gcc'
path_to_compiler = which(compiler_name)
if path_to_compiler:
path_to_compiler = os.path.realpath(path_to_compiler)
path_to_compiler = resolve_path(path_to_compiler)
self.log.info("Found path to compiler '%s' (with symlinks resolved): %s", compiler_name, path_to_compiler)
else:
raise EasyBuildError("%s not found in $PATH", compiler_name)

# Determine compiler version and installation prefix
if compiler_name == 'gcc':
out, _ = run_cmd("gcc --version", simple=False)
self.extract_compiler_version(out)
# Determine compiler version
self.compiler_version = extract_compiler_version(compiler_name)

# Determine installation prefix
if compiler_name == 'gcc':
# strip off 'bin/gcc'
self.compiler_prefix = os.path.dirname(os.path.dirname(path_to_compiler))

elif compiler_name in ['icc', 'ifort']:
out, _ = run_cmd("%s -V" % compiler_name, simple=False)
self.extract_compiler_version(out)

intelvars_fn = path_to_compiler + 'vars.sh'
if os.path.isfile(intelvars_fn):
self.log.debug("Trying to determine compiler install prefix from %s", intelvars_fn)
Expand All @@ -103,12 +152,27 @@ def __init__(self, *args, **kwargs):
# strip off 'bin/intel*/icc'
self.compiler_prefix = os.path.dirname(os.path.dirname(os.path.dirname(path_to_compiler)))

# For versions 2016+ of Intel compilers they changed the installation path so must shave off 2 more
# directories from result of the above
if LooseVersion(self.compiler_version) >= LooseVersion('2016'):
self.compiler_prefix = os.path.dirname(os.path.dirname(self.compiler_prefix))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the above as fallback rather than overriding it; also, use LooseVersion when comparing versions:

elif LooseVersion(self.compiler_version) >= LooseVersion('2016'):
    # for versions 2016+ of Intel compilers they changed the installation path
    self.compiler_prefix = os.path.dirname(os.path.dirname(self.compiler_prefix))
else:
    # strip off 'bin/intel*/icc'
    self.compiler_prefix = os.path.dirname(os.path.dirname(os.path.dirname(path_to_compiler)))


else:
raise EasyBuildError("Unknown system compiler %s" % self.cfg['name'])

if not os.path.exists(self.compiler_prefix):
raise EasyBuildError("Path derived for system compiler (%s) does not exist: %s!",
compiler_name, self.compiler_prefix)
self.log.debug("Derived version/install prefix for system compiler %s: %s, %s",
compiler_name, self.compiler_version, self.compiler_prefix)

if self.compiler_prefix in ['/usr', '/usr/local']:
if self.cfg['generate_standalone_module']:
# Force off adding paths to module since unloading such a module would be a potential shell killer
self.cfg['generate_standalone_module'] = False
self.log.warning("Disabling option 'generate_standalone_module' since installation prefix is %s",
self.compiler_prefix)

# If EasyConfig specified "real" version (not 'system' which means 'derive automatically'), check it
if self.cfg['version'] == 'system':
self.log.info("Found specified version '%s', going with derived compiler version '%s'",
Expand All @@ -127,15 +191,41 @@ def __init__(self, *args, **kwargs):
self.orig_version = self.cfg['version']
self.orig_installdir = self.installdir

def prepare_step(self, *args, **kwargs):
"""Do the bundle prepare step to ensure any deps are loaded at a minimum."""
if self.cfg['generate_standalone_module']:
if self.cfg['name'] in ['GCC', 'GCCcore']:
EB_GCC.prepare_step(self, *args, **kwargs)
elif self.cfg['name'] in ['icc']:
EB_icc.prepare_step(self, *args, **kwargs)
elif self.cfg['name'] in ['ifort']:
EB_ifort.prepare_step(self, *args, **kwargs)
else:
raise EasyBuildError("I don't know how to do the prepare_step for %s", self.cfg['name'])
else:
Bundle.prepare_step(self, *args, **kwargs)

def make_installdir(self, dontcreate=None):
"""Custom implementation of make installdir: do nothing, do not touch system compiler directories and files."""
pass

def make_module_req_guess(self):
"""
A dictionary of possible directories to look for. Return empty dict for a system compiler.
A dictionary of possible directories to look for. Return known dict for the system compiler, or empty dict if
generate_standalone_module parameter is False
"""
return {}
if self.cfg['generate_standalone_module']:
if self.cfg['name'] in ['GCC','GCCcore']:
guesses = EB_GCC.make_module_req_guess(self)
elif self.cfg['name'] in ['icc']:
guesses = EB_icc.make_module_req_guess(self)
elif self.cfg['name'] in ['ifort']:
guesses = EB_ifort.make_module_req_guess(self)
else:
raise EasyBuildError("I don't know how to generate module var guesses for %s", self.cfg['name'])
else:
guesses = {}
return guesses

def make_module_step(self, fake=False):
"""
Expand Down Expand Up @@ -168,3 +258,39 @@ def make_module_extend_modpath(self):
# Reset to actual compiler version (e.g., "4.8.2")
self.cfg['version'] = self.compiler_version
return res

def make_module_extra(self, *args, **kwargs):
"""Add any additional module text."""
if self.cfg['generate_standalone_module']:
if self.cfg['name'] in ['GCC','GCCcore']:
extras = EB_GCC.make_module_extra(self, *args, **kwargs)
elif self.cfg['name'] in ['icc']:
extras = EB_icc.make_module_extra(self, *args, **kwargs)
elif self.cfg['name'] in ['ifort']:
extras = EB_ifort.make_module_extra(self, *args, **kwargs)
else:
raise EasyBuildError("I don't know how to generate extra module text for %s", self.cfg['name'])
else:
extras = super(SystemCompiler, self).make_module_extra(*args, **kwargs)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@boegel For a system icc this seems to kick an error:

File "/tmp/eb-JeUdnR/included-easyblocks/easybuild/easyblocks/generic/systemcompiler.py", line 290, in sanity_check_step
    fake_mod_data = self.load_fake_module(purge=True)
  File "/usr/local/software/jureca/Stages/2017a/software/EasyBuild/3.3.1/lib/python2.7/site-packages/easybuild_framework-3.3.1-py2.7.egg/easybuild/framework/easyblock.py", line 1251, in load_fake_module
    fake_mod_path = self.make_module_step(fake=True)
  File "/tmp/eb-JeUdnR/included-easyblocks/easybuild/easyblocks/generic/systemcompiler.py", line 240, in make_module_step
    res = super(SystemCompiler, self).make_module_step(fake=fake)
  File "/usr/local/software/jureca/Stages/2017a/software/EasyBuild/3.3.1/lib/python2.7/site-packages/easybuild_framework-3.3.1-py2.7.egg/easybuild/framework/easyblock.py", line 2195, in make_module_step
    txt += self.make_module_extra()
  File "/tmp/eb-JeUdnR/included-easyblocks/easybuild/easyblocks/generic/systemcompiler.py", line 274, in make_module_extra
    extras = super(SystemCompiler, self).make_module_extra(*args, **kwargs)
  File "/usr/local/software/jureca/Stages/2017a/software/EasyBuild/3.3.1/lib/python2.7/site-packages/easybuild_easyblocks-3.3.1-py2.7.egg/easybuild/easyblocks/generic/bundle.py", line 165, in make_module_extra
    return super(Bundle, self).make_module_extra(altroot=self.altroot, altversion=self.altversion)
TypeError: make_module_extra() got an unexpected keyword argument 'altroot'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error only occurs when generate_standalone_module = False, otherwise it works

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I be doing nothing if generate_standalone_module = False to avoid this problem?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not using the customised bundle.py easyblock from this PR which does EasyBlock.make_module_extra(...) rather than super(Bundle, self).make_module_extra(...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into this same problem when testing the SystemMPI easyblock, and it was fixed when also using the updated bundle.py.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, confirmed, works with the bundle.py in the PR

return extras

def post_install_step(self, *args, **kwargs):
"""Do nothing."""
pass

def cleanup_step(self):
"""Do nothing."""
pass

def permissions_step(self):
"""Do nothing."""
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to redefine these either if we derive from Bundle? the sanity_check_step even looks like a blatant copy of part of what Bundle does?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are required because they are not inherited from bundle (which doesn't define them)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ok


def sanity_check_step(self, *args, **kwargs):
"""
Nothing is being installed, so just being able to load the (fake) module is sufficient
"""
self.log.info("Testing loading of module '%s' by means of sanity check" % self.full_mod_name)
fake_mod_data = self.load_fake_module(purge=True)
self.log.debug("Cleaning up after testing loading of module")
self.clean_up_fake_module(fake_mod_data)
Loading