-
Notifications
You must be signed in to change notification settings - Fork 285
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
Conversation
self.extract_ompi_version(out) | ||
|
||
# extract the installation prefix | ||
self.mpi_prefix, _ = run_cmd("ompi_info --path prefix|awk '{print $2}'", simple=False) |
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.
Very happy if someone would suggest some regex that gives me the same end result (@geimer)
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.
@ocaisa You mean something like res = re.search('^\s+Open MPI: (.*)$', output_of_ompi_info)
and then res.group(1)
?
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.
Thanks @geimer, that gives me enough to help me figure out how to extract the rest of the info I'll need too
The failing tests are because there is no |
Include checks on compiler in toolchain and underneath MPI Add any relevant envvars in the environment to the module
@boegel Ready for review |
# 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 |
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.
Update this comment!
@boegel Remaining error is simply that there is no |
I saw this PR so I'm jumping in to share the solution I've made some time ago to address the same issue. Basically, instead of creating a dedicated easyblock, I just create an 'empty' easyconfig file named OpenMPI/system, ie it creates a module named The advantage is that it is automatically recognized as an OpenMPI toolchain by EasyBuild, so there's no need for any kind of magic to properly set MPICC, MPICXX internally. EasyBuild will automatically set them to the right 'OpenMPI' values. Same thing for software that needs to know the type of the toolchain in the easyblock (eg OpenFOAM). It works transparently. See my 'implementation' in those files:
This allowed me to use EasyBuild to compile OpenFOAM on my Ubuntu workstation using the system compilers. I believe it will work smoothly with the other compilers/MPIs. |
@besserox What you have should be fine for a subset of what I'm interested in. What I'm concerned about is more the case where there are various compilers and MPI implementations on the system and I want to chose a particular one to use as a toolchain. To do this I need to know exactly where it is installed and if there are any environment variables set that affect it's behaviour (which is frequently true for a tuned MPI instance). |
if res: | ||
compiler_version = res.group(1) | ||
debug_msg = "Extracted compiler version '%s' for %s from: %s", compiler_version, compiler_name, out | ||
return compiler_version, debug_msg |
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.
don't pass down the debug log message, just log here; also, we try to avoid 'inline' return
statements
_log = fancylogger.getLogger('easyblocks.generic.systemcompiler')
...
def extract_compiler_version(compiler_name):
...
if res:
compiler_version = res.group(1)
_log.debug("Extracted compiler version '%s' for %s from: %s", compiler_version, compiler_name, out)
else:
raise EasyBuildError("...")
return compiler_version
self.orig_installdir = self.installdir | ||
|
||
def make_installdir(self, dontcreate=None): | ||
"""Custom implementation of make installdir: do nothing, do not touch system compiler directories and files.""" |
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.
s/compiler/MPI/g
|
||
def make_module_req_guess(self): | ||
""" | ||
A dictionary of possible directories to look for. Return empty dict for a system compiler. |
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.
s/compiler/MPI/g
return res | ||
|
||
def make_module_extra(self): | ||
"""Add all the build environment variables.""" |
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.
fix docstring (nothing is being built?)
if res: | ||
setting = res.group(1) | ||
self.log.debug("Extracted OpenMPI setting %s: '%s' from search text", search, setting) | ||
return setting |
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.
avoid inline return
please
"""Extra initialization: determine system MPI version, prefix and any associated envvars.""" | ||
super(SystemMPI, self).__init__(*args, **kwargs) | ||
|
||
# Determine MPI path (real path, with resolved symlinks) to ensure it exists |
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.
comment is in wrong place?
mpi_name = self.cfg['name'].lower() | ||
|
||
# Check we have MPI wrappers | ||
if mpi_name != 'impi': |
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.
use positive logic:
if mpi_name == 'impi':
mpi_c_wrapper = 'mpiicc'
else:
mpi_c_wrapper = 'mpicc'
|
||
# Extract any OpenMPI environment variables in the current environment and ensure they are added to the | ||
# final module | ||
raw_env = os.environ |
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.
no need to introduce raw_env
, just use os.environ
on line below?
if self.cfg['version'] == 'system': | ||
self.log.info("Found specified version '%s', going with derived MPI version '%s'", | ||
self.cfg['version'], self.mpi_version) | ||
elif self.cfg['version'] != self.mpi_version: |
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.
use positive logic, use log.info
to inform that specified version matches found version
c_compiler_name = self.toolchain.COMPILER_CC | ||
compiler_version = get_software_version(self.toolchain.COMPILER_MODULE_NAME[0]) | ||
|
||
if self.mpi_c_compiler != c_compiler_name and self.c_compiler_version != compiler_version: |
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.
s/and/or/g
?
@ocaisa To ensure that OpenMPI is available in Travis, see https://github.com/hpcugent/hanythingondemand/blob/master/.travis.yml#L3 |
# The prefix in the the mpiicc script can be used to extract the explicit version | ||
contents_of_mpiicc, _ = read_file(path_to_mpi_c_wrapper) | ||
prefix_regex = re.compile(r'(?<=compilers_and_libraries_)(.*)(?=/linux/mpi)', re.M) | ||
self.mpi_version = prefix_regex.search(contents_of_mpiicc) |
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.
double-check to make sure the version was indeed found (i.e. not None
)
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've implemented that further down for the generic case
# If I_MPI_ROOT is defined, let's use that | ||
raw_env = os.environ | ||
if raw_env['I_MPI_ROOT']: | ||
self.mpi_prefix = raw_env['I_MPI_ROOT'] |
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.
no need to introduce raw_env
, and you can do this in one go:
self.mpi_prefix = os.getenv('I_MPI_ROOT', os.path.dirname(os.path.dirname(mpi_c_wrapper)))
In any case: also check that the directory you come up with actually exists?
self.mpi_envvars += dict((key, value) for key, value in raw_env.iteritems() | ||
if key.startswith("MPI") and key.endswith("PROFILE")) | ||
|
||
# Extract the C compiler used underneath OpenMPI |
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.
not OpenMPI here, but Intel MPI
|
||
# Extract the C compiler used underneath OpenMPI | ||
compile_info, _ = run_cmd("%s -compile-info", simple=False) | ||
self.mpi_c_compiler = compile_info.group(0) |
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.
compile_info
is just a string, so no .group(0)
?
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.
Sorry, still working on this, bit of a pain to test
@ocaisa That makes sense |
super(SystemMPI, self).__init__(*args, **kwargs) | ||
|
||
# If we are testing on Travis let's cheat (since it has been set up to have OpenMPI installed) | ||
if self.cfg['name'] == 'foo': |
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.
hmm, no :)
we should find another way for this...
and install path. | ||
""" | ||
# First let's verify that the toolchain and the compilers under MPI match | ||
if self.travis: |
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.
Are we reimplementing https://github.com/auchenberg/volkswagen ?
This kind of code should never make it into easyblocks; instead, we should modify the tests to test specific easyblocks under controlled circumstances. See how we handle systemcompiler.py
in test/easyblocks/init_easyblocks.py
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.
Ok, ok
Error from tests is
Is that a recent addition? Any hints on a fix @boegel |
@ocaisa For future reference: the |
Works for GCC/OpenMPI with easyconfigs from easybuilders/easybuild-easyconfigs#4136 |
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) |
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.
@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'
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.
This error only occurs when generate_standalone_module = False
, otherwise it works
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.
Should I be doing nothing if generate_standalone_module = False
to avoid this problem?
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.
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(...)
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 ran into this same problem when testing the SystemMPI
easyblock, and it was fixed when also using the updated bundle.py
.
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.
Yes, confirmed, works with the bundle.py
in the PR
Looks good to go, tested with easybuilders/easybuild-easyconfigs#4136 for GCC/OpenMPI and Intel compilers/MPI (2017), works as expected. |
pass in post_install_step to avoid problems with symlinks created by GCC easyblock
path_to_mpi_c_wrapper = which(mpi_c_wrapper) | ||
if not path_to_mpi_c_wrapper: | ||
raise EasyBuildError("Could not find suitable MPI wrapper to extract version for impi", | ||
mpi_c_wrapper) |
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.
this extra argument shouldn't be here, see ocaisa#11
fix error reporting for missing MPI compiler wrapper for impi
… weird derived Intel compiler version
@@ -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) |
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.
there's a better way of fixing the issue you bumped into, see ocaisa#12
tweaks & fixes for SystemCompiler & SystemMPI easyblocks
Tested with easybuilders/easybuild-easyconfigs#4136, good to go, thanks for your efforts and patience @ocaisa! |
Allow the use of a (recognised) MPI implementation from the system as part of a toolchain.
Current easyblock works for OpenMPI but requires some additional checks to ensure a consistent toolchain stack.