-
Notifications
You must be signed in to change notification settings - Fork 773
{perf}[gompi/2019a] Score-P 6.0 + Scalasca 2.5 + deps #6328
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
{perf}[gompi/2019a] Score-P 6.0 + Scalasca 2.5 + deps #6328
Conversation
| # Python bindings (optional) | ||
| ('Python', '2.7.14', '-bare'), | ||
| ('future', '0.16.0', '-Python-2.7.14-bare'), | ||
| ('Six', '1.11.0', '-Python-2.7.14-bare'), |
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.
@geimer By using a -bare Python here, you're effectively making this incompatible with anything that depends on a full Python, which is quite common... Isn't that a concern?
Maybe it makes sense to make the Python that gets built with GCCcore a trimmed down version of the one we have with a full toolchain, i.e. still include common dependencies like six, future, setuptools, pip, etc into it?
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.
On second though, not using some versionsuffix for a Python built with GCCcore is probably going to create issues too, especially when EasyBuild is configured with --minimal-toolchains.
Yet, create a -base or -minimal variant that includes a bunch of extensions makes more sense? See also #5072
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 There doesn't seem to be any consensus how to deal with Python on the GCCcore level, cfr. the discussion I raised two or three weeks ago on the mailing list. I'm doing something here to make progress, but the Python stuff needs to be fixed first, and then I can adjust this PR accordingly. But I'm not the one who is going to take a lead on this, as there are too many dark corners I don't understand...
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 Thinking about it, things are even worse... To support both Python2 and Python3, I would need to build two different OTF2 installations (with different dependencies). But which one should I then use to build Score-P (which doesn't need Python at all)? Whichever choice I make may be wrong, as a user may need "the other Python" for his code...
Maybe I'll just ignore the Python bindings for now and leave them disabled...
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.
Good point... Maybe you can consider building the Python bindings in a separate module, which can be loaded as needed?
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 sure our build system supports this right now. Is there any support in EB for building things twice into the same prefix but with different build dependencies (like with a list of configopts)? This may work too, but I would immediately admit that this is a rather strange feature request ;-)
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.
Wait... This won't work either due to PYTHONPATH having to point to one or the other site-packages directory...
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.
Talking to the OTF2 core devs, this currently seems to be the only option: building OTF2 twice into the same prefix with the different Python's as build dependencies, and then providing Python binding modules that simply depend on the common OTF2 installation and only set PYTHONPATH accordingly. But I don't think this is currently possible with EB...
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 in a clean way, no, but it relates closely to the way in which the ComputeCanada folks install Python packages, so we should come up with a clean way of doing that anyway...
@bartoldeman @mboisson Do you have your approach clearly documented somewhere? Maybe it's time to start implementing this...
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.
What our approach boils down to:
- Let the Python module set PYTHONPATH to a location with a file
sitecustomize.py - this file has the following contents:
import sys
import os
import site
# use prefixes from EBPYTHONPREFIXES, so they have lower priority than
# virtualenv-installed packages
if "EBPYTHONPREFIXES" in os.environ:
postfix = os.path.join('lib', 'python'+sys.version[:3], 'site-packages')
for prefix in os.environ["EBPYTHONPREFIXES"].split(os.pathsep):
sitedir = os.path.join(prefix, postfix)
if os.path.isdir(sitedir):
site.addsitedir(sitedir)
- modules that use Python set EBPYTHONPREFIXES. We now do it as follows:
modextrapaths = {
'EBPYTHONPREFIXES': [''],
}
- module have to load different python modules for builds. We now do this as follows:
prebuildopts = ['module load python/2.7 && ',
'module load python/3.5 && ',
'module load python/3.6 && ']
but that's a hack. It would be better to allow builddependencies to be a list of lists.
5. we have this for some modules that absolutely need python (for some it is optional):
modluafooter = """
depends_on("python")
"""
this basically will load the default python if none is loaded and otherwise keep whatever python module is already loaded.
Hope that helps.
| @@ -0,0 +1,34 @@ | |||
| easyblock = 'PythonPackage' | |||
|
|
|||
| name = 'Six' | |||
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 be six, cfr. http://six.readthedocs.io/: The name, “six”, comes from the fact that 2*3 equals 6.
|
@geimer This PR is getting quite big, are all of these inter-dependent? Is it worth fleshing out the |
…gs into scorep_scalasca_update_hmns_mintc
|
@geimer How do we proceed with this? Bump to |
…gs into scorep_scalasca_update_hmns_mintc
|
@boegel As outlined yesterday, I've updated this PR to the latest tool versions for
|
|
Hmm... Isn't the Travis test too strict here? IMO a "pure" bundle that only depends on other packages shouldn't be required to specify |
|
Travis test report: 2/2 runs failed - see https://travis-ci.org/easybuilders/easybuild-easyconfigs/builds/530716128 Only showing partial log for 1st failed test suite run 14970.1;
*bleep, bloop, I'm just a bot (boegelbot v20180813.01)*Please talk to my owner @boegel if you notice you me acting stupid),or submit a pull request to https://github.com/boegel/boegelbot fix the problem. |
|
@geimer You're absolutely right, we should whitelist easyconfigs that use Are you up for looking into that yourself? Note that it's a bit more involved than just checking for the use of |
| dependencies = [ | ||
| ('CubeWriter', '4.4.2'), | ||
| ('CubeLib', '4.4.3'), | ||
| ('CubeGUI', '4.4.3'), |
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.
@geimer Does it actually make sense to have these as separate installations?
If not, you could use the components support in Bundle to simply pack them together in a single installation prefix/module, much like we do with X11, see https://github.com/easybuilders/easybuild-easyconfigs/blob/master/easybuild/easyconfigs/x/X11/X11-20190311-GCCcore-8.2.0.eb
| easyblock = 'Bundle' | ||
|
|
||
| name = 'Cube' | ||
| version = '20190321' |
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.
Maybe clarify how this was picked? Is it the release date of the most recent component included in this bundle?
Does it make sense to use the version of the "main" component instead (4.4.3), which is much closer to what people would expect when installing Cube?
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 is no "main" component. The former Cube package has been split into three disjoint componts: CubeWriter, CubeLib, and CubeGUI. They now evolve somewhat independently and version numbers may diverge more over time. In this case, the three components have been released on the same date (chosen as version), but this doesn't have to be the case in the future. Dependent packages may require only some of them or in different use cases (dependency vs. builddependency, see the Score-P easyconfig), thus I would really prefer to keep them separate.
In principle, the Cube bundle isn't required at all -- I've just added it as a convenience for people who are used to the old Cube package. While we also provide a convenience bundle tarball including all components on our web page, its version is a simple count that has very little to do with the versions included (as the versions are allowed to diverge, it is hard to come up with a common version). Having said all this, I'm also OK with removing the bundle if you think it causes too much confusion. (This would also magically "fix" the failing Travis check 😉)
| sanity_check_paths = { | ||
| 'files': ['bin/cube', 'bin/cubegui-config', | ||
| ('lib/libcube4gui.a', 'lib64/libcube4gui.a'), | ||
| ('lib/libcube4gui.%s' % SHLIB_EXT, 'lib64/libcube4gui.%s' % SHLIB_EXT)], |
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.
@geimer You can drop the "lib or lib64" bit here, and just specify the paths using lib/.
Since EasyBuild v3.6.1, there's an automatic fallback to checking for lib64/x if lib/x was not found (and vice versa if lib64/x was specified), see easybuilders/easybuild-framework#2477.
It's enabled by default (and disabling it requires configuring EasyBuild with --disable-lib64-fallback-sanity-check`, which I doubt anyone does, allowing that was just a safety measure we put in place in case surprises popped up).
| sanity_check_paths = { | ||
| 'files': ['bin/cubelib-config', | ||
| ('lib/libcube4.a', 'lib64/libcube4.a'), | ||
| ('lib/libcube4.%s' % SHLIB_EXT, 'lib64/libcube4.%s' % SHLIB_EXT)], |
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.
only lib/ is fine
| sanity_check_paths = { | ||
| 'files': ['bin/cubew-config', | ||
| ('lib/libcube4w.a', 'lib64/libcube4w.a'), | ||
| ('lib/libcube4w.%s' % SHLIB_EXT, 'lib64/libcube4w.%s' % SHLIB_EXT)], |
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.
only lib/ is fine
| sanity_check_paths = { | ||
| 'files': ['bin/otf2-config', 'include/otf2/otf2.h', | ||
| ('lib/libotf2.a', 'lib64/libotf2.a'), | ||
| ('lib/libotf2.%s' % SHLIB_EXT, 'lib64/libotf2.%s' % SHLIB_EXT)], |
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.
only lib/ is fine
| ] | ||
|
|
||
| sanity_check_paths = { | ||
| 'files': ['bin/scalasca', ('lib/libpearl.replay.a', 'lib64/libpearl.replay.a')], |
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.
only lib/ is fine
| # http://apps.fz-juelich.de/unite/ | ||
| ## | ||
|
|
||
| easyblock = 'EB_Score_minus_P' |
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 can actually remove this, since it matches the software name?
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.
Wasn't there a policy change at some point in the past to make the easyblock explicit?
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.
A (long) time, we dropped the automagic fallback to trying with ConfigureMake (which was confusing since that always left you wondering which easyblock was actually being used if none was specified).
But as long as the software name matches the easyblock, there's no point in specifying it, since there's no ambiguity.
| sanity_check_paths = { | ||
| 'files': ['bin/scorep', 'include/scorep/SCOREP_User.h', | ||
| ('lib/libscorep_adapter_mpi_event.a', 'lib64/libscorep_adapter_mpi_event.a'), | ||
| ('lib/libscorep_adapter_mpi_event.%s' % SHLIB_EXT, 'lib64/libscorep_adapter_mpi_event.%s' % SHLIB_EXT)], |
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.
only lib/ is fine
|
@boegel W.r.t. the CI check: I don't have the complete picture yet when the check is desired and when it's not. If it comes down to whitelisting |
|
Test report by @geimer |
|
This PR has been pending for so long that I could incorporate yet another version bump to the packages released yesterday 😃 I think it's now (finally) ready for prime time... |
|
Test report by @boegel |
|
Test report by @boegel |
|
Going in, thanks @geimer! |
Minimal-toolchain versions (hierarchical MNS) for gompi/2018a.