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

Load Recursive Modules #721

Merged
merged 3 commits into from
Jun 30, 2016

Conversation

citibeth
Copy link
Member

@citibeth citibeth commented Apr 2, 2016

Spack support for recursive modules without having to use a recursive module system.

@tgamblin
Copy link
Member

tgamblin commented Apr 4, 2016

@alalazo: how does this look to you? does it work with your module support?

@robertdfrench: how about for ORNL? I know you all wanted module prereqs.

@glennpj
Copy link
Contributor

glennpj commented Apr 4, 2016

This could be useful for development but perhaps not so much for end users. For end users, I think recursive module loading would be better done in the module files themselves. Modules can be loaded from other modules via module load directives in module files. This will recursively load the needed environment module stack. Having dependent modules loaded from module files also puts them under control of the environment module system. With that, the loaded modules can either be recursively unloaded when the top module is removed, or the directives can be fenced such that the modules are not recursively unloaded. That is an important consideration given that the environment module system will not enforce dependencies once they are loaded. The loading/unloading would best be made configuration options so that people can choose the behavior.

Regarding prereqs, some sites prefer to use prereqs rather than module load directives. So if module loading is configured to be false in spack, then prereqs would kick in. That does not address the goal of this PR though. I think prereqs and recursive module loading are both needed in the end.

@citibeth
Copy link
Member Author

citibeth commented Apr 4, 2016

Let me try to advocate for this PR:

  1. It works without any recursive module support from the environment
    modules system. Installing Environment Modules is easy. Installing Lmod
    is a royal PITA, and not all users get to choose what is installed on the
    computer they're using.
  2. It was pretty easy to implement.
  3. And it loads each module at most once (although I suppose a module
    system could be set up to do that too).
  4. Recursive module unloading has all the same problems we've seen in
    recursive Spack packge uninstallation. Right now, Spack barely knows which
    packages are there because you want them, and which are there as
    dependencies for something else. I don't see how the modules system could
    know this to make intelligent choices. In many (but not all?) cases, the
    most intelligent choices can be made by Spack, I believe.
  5. What I REALLY want is not just a blind recursive module loading, but
    only load the modules that NEED to be loaded recursively. I'm still trying
    to get a handle on what rules would apply here for my use case. (The
    problem is that Spack-build Python modules don't use RPATH). I probably
    want to provide configurable filters that choose which modules will / will
    not be loaded for any given call to "spack load --dependencies". Again,
    maybe a module system can do this, but I'm not sure.

I've already encountered a simple case of this flexibility issue: I want to
load only Python modules recursively.

module load `spack module find tcl icebin netcdf cmake@3.5.1`
module load `spack module find --dependencies tcl py-basemap py-giss`

I'm not familiar with prereqs, can you please point me to them?

Thanks!
-- Elizabeth

@alalazo
Copy link
Member

alalazo commented Apr 4, 2016

I share most of the opinions of @glennpj, in particular:

The loading/unloading would best be made configuration options so that people can choose the behavior.

To reply to @tgamblin : I don't think this PR will conflict with the work that will be done to customize module files, as it will use whatever has been generated at installation phase.

On the other hand, I hope that everything that this PR does will be configurable from modules.yaml
(in particular I would like to let the user decide if and when to use recursive loading or prerequisites) so I don't see much use for it in the long term.

@citibeth
Copy link
Member Author

citibeth commented Apr 4, 2016

I look forward to the modules.yaml capability. In the meantime, I don't think it makes sense to merge this PR (but I will definitely continue to use it).

@citibeth citibeth closed this Apr 4, 2016
@citibeth citibeth mentioned this pull request Apr 5, 2016
@citibeth
Copy link
Member Author

citibeth commented Apr 5, 2016

This could be useful for creating Spack profiles. Re-opening...

@gartung
Copy link
Member

gartung commented Apr 21, 2016

I found this useful to set up a runtime environment on OSX. The only thing I needed to add was a command to load the compiler module so that the correct system headers could be found by Cling, the interactive C++ interpreter in ROOT.

@citibeth citibeth mentioned this pull request Apr 21, 2016
@citibeth
Copy link
Member Author

One thing I realized about this approach vs. Spack profiles... in the real world, I've found that the profiles I need to set up contain a mixture of Spack-built and non-Spack-built modules. With the approach in this PR, it's easy to combine Spack-generated "module load" commands with manual "module loads".

@tgamblin
Copy link
Member

Merged!

@mathstuf
Copy link
Contributor

Is there a reason this didn't use traverse to get the dependency graph of a package?

@alalazo
Copy link
Member

alalazo commented Jun 30, 2016

@mathstuf @citibeth If it is not really urgent (like you need to fix this in the next 5 min.) I can fix any minor issue from #721 in #1135 . I am currently merging conflicts all over cmd/module and will probably ask @citibeth to confirm that I didn't break any semantic

@mathstuf
Copy link
Contributor

The API change needs to happen on #378 since it makes those API changes, but the traverse change would be nice because then I could just change it to do deptypes=('link', 'run') to make it skip builddeps :) .

tty.die("No installed packages match spec %s" % raw_spec)

if len(specs) > 1:
tty.error("Multiple matches for spec %s. Choose one:" % spec)
Copy link
Contributor

Choose a reason for hiding this comment

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

@alalazo This should be using raw_spec while you're fixing things.

@alalazo
Copy link
Member

alalazo commented Jun 30, 2016

@mathstuf done in #1135

@alalazo
Copy link
Member

alalazo commented Jul 4, 2016

@citibeth BTW :

Installing Lmod is a royal PITA

should be fixed by #1026

@citibeth citibeth deleted the efischer/160401-RecursiveModules branch October 6, 2016 14:18
matz-e pushed a commit to matz-e/spack that referenced this pull request Apr 27, 2020
* Add py-quaternion

* update NeuroM

* add plotly-helper

* Add py-cut-plane

* bump pathlib 

* add morph-tool
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants