-
Notifications
You must be signed in to change notification settings - Fork 203
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 Generation module naming scheme #3547
base: develop
Are you sure you want to change the base?
Conversation
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.
@eylenth The main thing missing here is a test that covers this new module naming scheme.
Are you up for looking into that?
We already have similar tests in test/framework/module_generator.py
, see for example test_hierarchical_mns
.
It would be good to have a test in place first, before we start rewording generation_mns.py
...
elif ec['toolchain']['version'] == '8.3.0': | ||
release_date = '2019b' | ||
elif ec['toolchain']['version'] == '10.2.0': | ||
release_date = '2020b' |
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.
As you mentioned, we can do a lot better here, although we would need to rely on easyconfig files (for the foss
toolchain, in this case) being available then...
Perhaps it's not too terrible after all to hardcode this here, but then we need to maintain it when new common toolchain versions are defined, of course.
If we do so, this should be improved so we have a constant on top that defines the relation between GCC
/GCCcore
version and (common) toolchain version, rather than copy-pasting things and leaving gaps (like not covering 2020a
).
I could give it a try to write tests in |
I have asked a python developer from another team at our concern to look into the unit tests |
refactoring and unittest generation mns
I've added a lookup table to map toolchain versions to a specific generation. The entries in the table are based on the easyconfigs that are available in the easybuild-easyconfig repo. Based on the data on this page (https://docs.easybuild.io/en/latest/Common-toolchains.html#overview-of-common-toolchains), I updated the lookup table, but as you can see, it is still incomplete ('TBD' value for those versions that I couldn't determine the generation for). I'm a bit out of my depth here when it comes to completing the lookup table. Can you suggest something/someone to fill out the missing parts? Any suggestion on how to keep this lookup table configurable, so that users can add versions from their custom easyconfigs? Maybe an ENV that points to a dictionary that the lookup table should be updated with? To make sure that the lookup table is always in sync with the easybuild-easyconfig repository (so that all used versions are mapped to a generation), I'd like to add a test that checks/enforces this. But since this test evaluates a link between two separate repositories, I'm not sure where this test should go. |
@tsoenen I think the better way forward here is to not have a lookup table at all. That avoids having to maintain it, and it also allows for this module naming scheme to work with custom toolchains that may not be included in the central easyconfigs repository. The alternative is to let EasyBuild figure out itself what the correct mapping is for the current "active" toolchain. That'll require that the corresponding easyconfigs for the toolchain are available to EasyBuild, but that's more or less the case already anyway for other reasons. I promised @eylenth to look into this, but I haven't found time yet for that. |
@boegel Any updates on this? How should we proceed? |
@tsoenen My apologies for not getting back to this yet, it's been difficult to find enough time... There's a couple of things that should happen here:
The latter should basically boil down to leveraging the >>> from easybuild.tools.options import set_up_configuration
>>> set_up_configuration()
>>> from easybuild.framework.easyconfig.easyconfig import get_toolchain_hierarchy
>>> get_toolchain_hierarchy({'name': 'foss', 'version': '2021a'})
[{'name': 'GCCcore', 'version': '10.3.0'}, {'name': 'GCC', 'version': '10.3.0'}, {'name': 'gompi', 'version': '2021a'}, {'name': 'foss', 'version': '2021a'}] Is this something you're up for looking into yourself? |
@boegel Thanks for the feedback. I'll take a stab at it and reach out if I'm blocked. |
@boegel One question related to the use of
gives
How do I go from there to e.g. |
I would generate the look-up table dynamically using the top-level toolchains |
@ocaisa: thanks for the response. This gets me a bit further, but not all the way there. For example, this is part of the dynamic lookup table:
|
You are destined to run into these issues because not everything fits neatly into the proposed naming scheme. What happens if the same GCC version is used in two generations? |
I did a quick scan of the easyconfig files in the test directory, and almost all of them can be mapped on a GCC or GCCcore version (those that can't be mapped are GCC and GCCcore seem to release more versions than there are generations. Does it occur that the same GCC version is used in multiple generations? If we have a one to one mapping between GCC / GCCcore versions and generations (e.g. by mapping 'in between' versions on the most recent generation of its older versions), it seems like we are almost there. Do you think this is feasible (I'm a little out of my depth here)? |
That's correct, we add easyconfigs for every GCC release, but they may not end up actually being used in a (common) toolchain as base compiler.
No, every generation only has a single GCC(core) version; see https://docs.easybuild.io/en/latest/Common-toolchains.html#component-versions-in-foss-toolchain.
Can you give a concrete example of what you mean?
I'm not sure, I'm starting to think that this module naming scheme can only work for a subset of toolchains, not everything. One other option is to just opt-out in case you run into something you can't map, with a meaningful error message? |
@boegel Thx for the feedback
GCC
Does this imply that the error message should list all the toolchains for which the generated mapping can't provide a generation, allowing the user to provide a hardcoded mapping for those toolchains in a file, which can then be attached when calling the namingscheme code? |
Hey @boegel , when you have time, could you provide some feedback on my last message? |
dynamic lookup table for generation mns
self.lookup_table = {} | ||
|
||
# Get all generations | ||
foss_filenames = search_easyconfigs("^foss-20[0-9]{2}[a-z]\.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.
invalid escape sequence '.'
Hi @boegel and @ocaisa. I've continued with this effort, and updated the code so that the mapping between generations and toolchains happens dynamically. The objective of this PR is to introduce a new module naming scheme that is based on the foss-generation that the toolchain belongs to (e.g. the full path could be something like
Since
|
I was thinking about this a bit and I wonder if there is not a simpler solution for your use case. For quite a few years now, there has been a This approach would remove the need for a lookup table (but kind of export the problem to Lmod). You still have legacy stuff, before we had GCCcore, but that is going back in history a long time. Unfortunately this doesn't help your recursion error, and I can't see an easy way out of that. I was wondering if you can figure out a way to temporarily change the return value of ActiveMNS? |
|
Here a simplified version that groups modules based on GCCcore (and includes the recursion fix):
I think we would still struggle to accept this in EB as it would require clever external modules to make sure the MODULEPATH is set up correctly. Also, it is effectively implementing a standard hierarchy without the MPI level...I'd probably just do that instead. |
Here's a version that leverages HierarchicalMNS and just exlcudes adding an MPI layer:
You can try this out with
|
@ocaisa Thanks for the feedback!
I'm not quite following this. I've made it possible for the user to extend the lookup table with custom mappings, to overcome issues with external easyconfigs (But I'm not sure that is the problem you are referencing). This is the current version that I'm working with:
I have noticed that setting the attribute on I also noticed that the mapping part / lookup table is absent in the code that you propose, where that is specifically our objective: to obtain a module naming scheme that uses generations. Is there a reason why it is absent in your proposal (e.g. to keep things more generic)? |
Getting up to speed with main repo
Removed circular reference when building dynamic mapping
hound fixes
My latest version is available in the PR. afaik it covers all the issues that were discussed throughout the thread. It does differ from the version that @ocaisa proposed, which didn't completely cover what we are trying to achieve (explicitly using the generation namings) - unless I'm missing something. Can you guys provide a judgement on the current version? |
self.lookup_table[('foss', generation)] = generation | ||
|
||
# Force config to point to other MNS | ||
ConfigurationVariables()._FrozenDict__dict['module_naming_scheme'] = 'GenerationModuleNamingScheme' |
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 don't understand this, this points to the same MNS, not a different one.
Determine short module name, i.e. the name under which modules will be exposed to users. | ||
Examples: GCC/4.8.3, OpenMPI/1.6.5, OpenBLAS/0.2.9, HPL/2.1, Python/2.7.5 | ||
""" | ||
return os.path.join(ec['name'], self.det_full_version(ec)) |
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 don't believe that you can drop the toolchain/version suffix here without exposing yourself to lots of problems. How does this take into account that you can have, for example, OpenMPI with other compilers? Looking at a concrete example:
OpenMPI-4.0.3-GCC-9.3.0.eb
OpenMPI-4.0.3-gcccuda-2020a.eb
OpenMPI-4.0.3-iccifort-2020.1.217.eb
all of these map to the same module file but are clearly not the same, this lack of uniqueness is a big problem as EB will see them all as installed once the module file is created.
It seems that the MNS is by design focussed on foss
alone which means it is heavily exposed to problems that may occur when mixing different toolchains (for example fortran module incompatability between Intel/GCC). Even at foss
level the MNS relies on there being no shadowing of software with different toolchains (for example, Python with GCCcore
and the same version with GCC
), this is currently true in recent releases but there is no guarantee that sites themselves respect this.
@ocaisa Thanks for the feedback. I understand the uniqueness conflict. We are only using foss in our setup, so it doesn't occur for us. We understand that it can't become part of the easybuild repo like this, and won't be pursuing this PR anymore. Thx for the feedback throughout the process |
I am reopening this PR. I had a talk with @boegel and there could be a workaround to continue with this PR. |
@ocaisa This may be useful in the context of EESSI, where we eventually want a better way of grouping compatible modules... |
@eylenth: Tests failed in GitHub Actions, see https://github.com/easybuilders/easybuild-framework/actions/runs/6730938397
bleep, bloop, I'm just a bot (boegelbot v20200716.01) |
I have created an new customized module naming scheme: GenerationModuleNamingScheme.
it is an implementation of different generation modules using release dates.
An example:
GCCcore-10.2.0, GCC-10.2.0 and foss-2020b will be installed in one specific modulepath: releases/2020b
GCCcore-7.3.0, GCC-7.3.0-2.30, foss-2018b will be installed in another modulepath: releases/2018b
A module available looks like this:
I have had a meeting with @boegel , and he suggested to create a pull request for this
But we need an optimization in the det_module_subdir class, to get rid of the ugly "if else" structure
Kenneth told us that a table lookup can be implemented.