-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Mvapich2: Address issue with external MPI under Cray #22732
Conversation
ae0bfea
to
34a9365
Compare
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.
This looks like a good idea. Cleaned it up a bit with some idiomatic python, and it needs to be less verbose for the user.
|
3c0f29d
to
2e93ab2
Compare
@becker33 Can you re-review - I think I got everything you wanted checked out |
The package currently tries to set compilers (and ENV variables) differently if the toolchain is cray This patch uses (imo) more reasonable logic than assuming ordering of the modules. Instead, it checks if a modules looks like cray-mvapich An alternative would be to pick the modules that has 'mvapich' in the name Regardless, this is more rational than always using "modules[0]". This causes many issues when you need to use multiple modules to properly setup MPI. Fixes: Sets MPICC/MPICXX/MPIF77/MPIF90 to the spack wrappers when using cray Sets the compilers to the spack wrappers when using cray (for dependents) Adds: Meaningful messages so the user can tell what is going on and why
This is an issue with Cray and modules. Should we close it as deprecated since we now have the manifest file on Cray systems? |
Since the separate |
The
mvapich2
package has some assumptions about how to setup external packages that can be problematic.The issue is
if external_modules and 'cray' in external_modules[0]:
, which is the logic used to determine whether MPI's compilers will be set asmpicc
or spack's wrapper. With Cray, you should use spack's wrappers which point to cray's wrappers (not to mvapich's mpi compilers).The problem with the logic used is it assumes the first module will have 'cray' in the name - which is necessarily true (and I am uses spack recipes provided by Cray that are breaking this assumption).
I would like the maintainers to consider a more intuitive approach.
Given the package name is
mvapich2
, I propose search the modules for a module name containingmvapich
orcray-mvapich
. If this is the case, then almost certainly you are using the Cray toolchain and envs likeMPICC
and the spec's compilermpicc
should point tospack_cc
I've used this logic in the two areas where the issues arrises (setting up dependent packages and setting the compiler env)
This patch resolved some odd ball behavior I was observing on a Cray platform - that at first glance would be blamed on a bad package (pnetcdf in this case) - or a bug spack's external modules (there sorta is one) - which will be submitted in a separate issue.
Either way, I propose not using an undocumented assumption on how the user will list modules in their external declaration - if this patch can pass the regression tests, I'd like it considered for mainstream use!
@nithintsk @harisubramoni