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 modules needed to modify MODULEPATH to lmod module loads #29701

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

behrmann
Copy link
Contributor

Currently spack does not load hierarchy prerequisites, so that thegenerated loads scripts fail on some modules whose modules are not in MODULEPATH. This was reported in #23380.

This change walks through all dependencies of the user supplied constraints, as if --recurse-dependencies had been supplied, and adds those specs that modify the MODULEPATH and which unlock paths that are prefixes of the module paths of the user supplied specs.

This should hopefully be a partial fix for #23380. It only checks for modules that definitely modify MODULEPATH, since this is currently the only instance where I encounter this problem, so I haven't touched modules that conditionally change MODULEPATH. In terms of performance this shouldn't be much worse than --recurse-dependencies, although the module paths for the user supplied specs are generated twice (once long, once short).

Happy to hear if there is a better way to do this!

Currently spack does not load hierarchy prerequisites, so that thegenerated
loads scripts fail on some modules whose modules are not in MODULEPATH. This was
reported in spack#23380.

This change walks through all dependencies of the user supplied constraints, as
if --recurse-dependencies had been supplied, and adds those specs that modify
the MODULEPATH and which unlock paths that are prefixes of the module paths of
the user supplied specs.
@haampie
Copy link
Member

haampie commented Mar 24, 2022

Is this tangential / orthogonal to autoloading of transitive dependencies? We now generate depends_on(...) lmod statements by default.

@behrmann
Copy link
Contributor Author

behrmann commented Mar 24, 2022

I think this is orthogonal to that. I have a py-h5py built against an openmpi-using hdf5, its module file has the proper depends_on:

depends_on("hdf5/1.12.1-fqxxaoq")
depends_on("openmpi/4.1.2-whpqom5")
depends_on("py-mpi4py/3.1.2-bmlgao2")
depends_on("py-numpy/1.22.3-uxeaija")
depends_on("python/3.10.3-fb5uqec")

but this doesn't really work, since one can only module load py-h5py once module load openmpi has been loaded before, which is normally not generated if its not explicitly in the user-supplied spec.

@haampie
Copy link
Member

haampie commented Mar 24, 2022

since one can only module load py-h5py once module load openmpi has been loaded before

that's the whole point of generating depends_on statements

which is normally not generated if its not explicitly in the user-supplied spec

that sounds like a bug then. I don't understand why that module is not generated. It should generate what's required... not just what's "user provided".

@behrmann
Copy link
Contributor Author

behrmann commented Mar 24, 2022

since one can only module load py-h5py once module load openmpi has been loaded before

that's the whole point of generating depends_on statements

Well, depends_on works perfectly if the required module is in MODULEPATH, but with lmod hierarchies this is not necessarily given. Everything ends up in the Core hierarchy by default, except e.g. for things depending on mpi, which will then put modules for things depending on openmpi in a openmpi hierarchy. In the filesystem this would look something like this (somewhat shortened to make it manageable)

lmod
├── linux-debian11-x86_64
│   ├── Core
│   │   ├── openmpi
│   │   │   └── 4.1.2-whpqom5.lua
│   └── openmpi
│       └── 4.1.2-whpqom5
│           └── Core
│               ├── hdf5
│               │   └── 1.12.1-fqxxaoq.lua
│               ├── py-h5py
│               │   ├── 3.6.0-nj3gets.lua
│               │   └── 3.6.0-udkymm5.lua
│               └── py-mpi4py
│                   ├── 3.1.2-bmlgao2.lua
│                   └── 3.1.2-tspmlvd.lua
└── module-index.yaml

By default lmod/linux-debian11-x86_64/Core will be in the module path, but the openmpi hierarchy will not; it's only added to it at runtime when doing a module load openmpi/4.1.2-whpqom5, since this will add lmod/linux-debian11-x86_64/openmpi/4.1.2-whpqom5/Core.

Just doing an, e.g. module load py-h5py/3.6.0-nj3gets will fail, because lmod will not know about it without loading the appropriate openmpi module beforehand. Once this is loaded, the depends_on of the h5py module will wirk fine and pick up hdf5 and mpi4py.

Hope this makes it a bit clearer.

@haampie
Copy link
Member

haampie commented Mar 24, 2022

Ah okay, I did not realize the root package py-h5py itself was hidden behind the hierarchy. Thanks for clarifying.

@behrmann
Copy link
Contributor Author

Ah okay, I did not realize the root package py-h5py itself was hidden behind the hierarchy. Thanks for clarifying.

Yes, this patch should enable hierarchies for some things that are dependencies of the supplied specs. It will probably not work for #23380 which was about compilers (haven't tested that) and it will also not work for conditionally enabled hierarchies, because I haven't had the need for that yet so I didn't construct a case for that. It should unbreak the loads scripts for straight-forward "I need this hierarchy enabled through this module to load this other module I am interested in" cases, though. I'm new to the spack codebase and this was the most self-contained way to do this. The proper way might be to put this somewhere in the module context so that the layout can use that information and return multiple modules to the writer, but right now I don't have the time to explore that.

Do I need do something about the failing codecov?

@behrmann
Copy link
Contributor Author

My reminder for this PR just rang. It would be great if somebody could have a look at this. :)

@haampie haampie removed their assignment Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants