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.

Sign up for GitHub

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

Enhance generic Bundle easyblock to transfer module requirements of components #3472

Merged
merged 3 commits into from
Nov 9, 2024
Merged

Enhance generic Bundle easyblock to transfer module requirements of components #3472

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b13db46
Enhance generic Bundle EasyBlock to transfer module requirements of c…
Thyre Oct 7, 2024
eda3ca7
Bundle: make implementation more efficient
Thyre Nov 8, 2024
e696150
Bundle: Update authors
Thyre Nov 8, 2024
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
45 changes: 38 additions & 7 deletions easybuild/easyblocks/generic/bundle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
@author: Pieter De Baets (Ghent University)
@author: Jens Timmerman (Ghent University)
@author: Jasper Grimm (University of York)
@author: Jan Andre Reuter (Juelich Supercomputing Centre)
"""
import copy
import os
Expand Down Expand Up @@ -71,8 +72,8 @@ def __init__(self, *args, **kwargs):
self.altroot = None
self.altversion = None

# list of EasyConfig instances for components
self.comp_cfgs = []
# list of EasyConfig instances and their EasyBlocks for components
self.comp_instances = []
Copy link
Member

Choose a reason for hiding this comment

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

@Thyre renaming this will have some fallout, see the clang_aomp.py where self.comp_cfgs is used...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, we should keep the old one, yeah. With the PR reverted, I revise the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... I missed that EasyBlock since it is unused. However, there is only a single usage of self.comp_cfgs.
When I open a new PR for this, I'll update the clang_aomp.py EasyBlock as well. Haven't found other usages.


# list of EasyConfig instances of components for which to run sanity checks
self.comp_cfgs_sanity_check = []
Expand Down Expand Up @@ -198,7 +199,7 @@ def __init__(self, *args, **kwargs):
if comp_cfg['patches']:
self.cfg.update('patches', comp_cfg['patches'])

self.comp_cfgs.append(comp_cfg)
self.comp_instances.append((comp_cfg, comp_cfg.easyblock(comp_cfg)))
Copy link
Contributor Author

@Thyre Thyre Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@boegel, @lexming This may be the issue causing infinite loop. Changing this to:

-self.comp_instances.append((comp_cfg, comp_cfg.easyblock(comp_cfg)))
+self.comp_instances.append((comp_cfg, comp_cfg.easyblock))

certainly improves the situation.
I'll continue checking if the test passes locally now.

Copy link
Contributor Author

@Thyre Thyre Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The test suite is now stuck for QuantumESPRESSO. That's a huge progress (since we were previously stuck even for the first few EasyConfigs with A), but still not a complete fix.

Copy link
Contributor Author

@Thyre Thyre Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The QuantumESPRESSO check is not stuck. Instead, the logging files are not closed and all (?) tests after that are logged as well, causing around 11GB logs on my system. This also happens with develop as far as I can tell.

total 11G
drwx------  2 jreuter jreuter  800 12. Nov 20:44 .
drwxrwxrwt 24 root    root     540 12. Nov 20:36 ..
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-5.3.0-20241112.203827.pyJFM.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-5.4.0-20241112.203827.FyduX.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-5.4.0-20241112.203827.hzkuy.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.0-20241112.203827.FzleV.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.1-20241112.203827.FZgWV.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.2.1-20241112.203827.xMxaO.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.2-20241112.203827.fABEs.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.3-20241112.203827.kaPJh.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.3-20241112.203827.nZhTm.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.4.1-20241112.203827.juhTP.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.5-20241112.203827.pfDnY.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.5-20241112.203827.VBbnI.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.6-20241112.203827.bRTCU.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.6-20241112.203827.ewZNc.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.6-20241112.203827.IzdWy.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.6-20241112.203827.PgGZc.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.6-20241112.203827.vwIEO.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.7-20241112.203827.KUUtU.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.7-20241112.203827.piJmd.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.7-20241112.203827.rQAjF.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.7-20241112.203827.sbJUP.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.7-20241112.203827.ZBRpm.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.7-20241112.203827.ZSOpP.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.8-20241112.203827.cKErR.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.8-20241112.203827.wwDpe.log
-rw-r--r--  1 jreuter jreuter 299M 12. Nov 20:45 easybuild-QuantumESPRESSO-6.8-20241112.203827.XATBJ.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.0-20241112.203827.KuImR.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.0-20241112.203827.SXBiL.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.1-20241112.203827.BWcVK.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.1-20241112.203827.tpQDP.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.2-20241112.203827.GrtNk.log
-rw-r--r--  1 jreuter jreuter 298M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.2-20241112.203827.MArZe.log
-rw-r--r--  1 jreuter jreuter 283M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.2-20241112.203828.qrsQj.log
-rw-r--r--  1 jreuter jreuter 283M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.3.1-20241112.203828.gMpKu.log
-rw-r--r--  1 jreuter jreuter 283M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.3.1-20241112.203828.kpFyU.log
-rw-r--r--  1 jreuter jreuter 283M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.3.1-20241112.203828.lrPYN.log
-rw-r--r--  1 jreuter jreuter 283M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.3-20241112.203828.TOcVA.log
-rw-r--r--  1 jreuter jreuter 283M 12. Nov 20:45 easybuild-QuantumESPRESSO-7.3-20241112.203828.wnnQg.log

Could this be related to the way the EasyBlock is written?


self.cfg.update('checksums', checksums_patches)

Expand All @@ -217,7 +218,7 @@ def check_checksums(self):
"""
checksum_issues = super(Bundle, self).check_checksums()

for comp in self.comp_cfgs:
for comp, _ in self.comp_instances:
checksum_issues.extend(self.check_checksums_for(comp, sub="of component %s" % comp['name']))

return checksum_issues
Expand Down Expand Up @@ -247,14 +248,12 @@ def build_step(self):
def install_step(self):
"""Install components, if specified."""
comp_cnt = len(self.cfg['components'])
for idx, cfg in enumerate(self.comp_cfgs):
for idx, (cfg, comp) in enumerate(self.comp_instances):

print_msg("installing bundle component %s v%s (%d/%d)..." %
(cfg['name'], cfg['version'], idx + 1, comp_cnt))
self.log.info("Installing component %s v%s using easyblock %s", cfg['name'], cfg['version'], cfg.easyblock)

comp = cfg.easyblock(cfg)

# correct build/install dirs
comp.builddir = self.builddir
comp.install_subdir, comp.installdir = self.install_subdir, self.installdir
Expand Down Expand Up @@ -324,6 +323,38 @@ def install_step(self):
# close log for this component
comp.close_log()

def make_module_req_guess(self):
"""
Set module requirements from all comppnents, e.g. $PATH, etc.
During the install step, we only set these requirements temporarily.
Later on when building the module, those paths are not considered.
Therefore, iterate through all the components again and gather
the requirements.

Do not remove duplicates or check for existance of folders,
as this is done in the generic EasyBlock while creating
the modulefile already.
"""
# Start with the paths from the generic EasyBlock.
# If not added here, they might be missing entirely and fail sanity checks.
final_reqs = super(Bundle, self).make_module_req_guess()

for cfg, comp in self.comp_instances:
self.log.info("Gathering module paths for component %s v%s", cfg['name'], cfg['version'])
reqs = comp.make_module_req_guess()
Copy link
Member

Choose a reason for hiding this comment

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

What if one of the component itself is a Bundle instance, that may result in an infinite loop I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... a Bundle instance should gather the requirements of the components. If one of the components is also a Bundle, we should collect all of them recursively.

An infinite loop can be achieved if one EasyBlock tries to call this function via super. Can we prevent that?

Copy link
Contributor

Choose a reason for hiding this comment

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

@boegel I don't see why there would be an infinite loop here, the list of components is finite and equal to whatever the easyconfig defines. Even if there is a bundle in the component list, it is also defined in the easyconfig.
This is not like looping over dependencies, with multiple layers of depth.

Copy link
Contributor

Choose a reason for hiding this comment

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

Having a Bundle as component of another Bundle is explicitly forbidden. See

easybuild-easyblocks/easybuild/easyblocks/generic/bundle.py

Line 142 in 28b2084

raise EasyBuildError("The Bundle easyblock can not be used to install components in a bundle")

So this is not the cause of the issue (unless some unit test does some weird stuff and hijacks that check in Bundle)

Copy link
Contributor

Choose a reason for hiding this comment

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

Apparently there is a general bug in this check, as it does not cover easyblocks based on Bundle, like PythonBundle or OpenSSL_wrapper. So that is one thing that can improve.
However, it is already impossible to make a nested bundle in practice because the resulting construct always errors out. If the bundle component has sources, it results in error about "bundles cannot have sources"; and if it does not have sources it results in error about "missing sources".


try:
for key, value in sorted(reqs.items()):
if isinstance(reqs, string_type):
value = [value]
final_reqs.setdefault(key, [])
final_reqs[key] += value
except AttributeError:
raise EasyBuildError("Cannot process module requirements of bundle component %s v%s",
cfg['name'], cfg['version'])

return final_reqs

def make_module_extra(self, *args, **kwargs):
"""Set extra stuff in module file, e.g. $EBROOT*, $EBVERSION*, etc."""
if not self.altroot and not self.altversion:
Expand Down
Loading