netcdf: forward 'mpi' to 'hdf5' #4672
Conversation
…cdf' Currently when we ask Spack to build `netcdf~mpi` what we get back after concretization is `netcdf~mpi ^hdf5+mpi`. This commit prevents this particular combination from being possible as it forwards the variant to hdf5. It also adds a conflict check on `netcdf+parallel-netcdf~mpi`.
|
I've tried this before and it didn't work very well, hence the workaround. Can't remember the exact issue, but it was due to a concretization bug. I believe it was caused by differences in the default MPI variant for each package. |
|
@adamjstewart Maybe it was due to bugs that have been solved in the meanwhile? I tested it manually with $ spack spec netcdf+mpi ^hdf5~mpi
Input spec
--------------------------------
netcdf+mpi
^hdf5~mpi
Normalized
--------------------------------
==> Error: Invalid spec: 'hdf5~mpi'. Package hdf5 requires variant +mpi, but spec asked for ~mpi
$ spack spec netcdf+mpi ^hdf5+mpi ^mpich
Input spec
--------------------------------
netcdf+mpi
^hdf5+mpi
^mpich
Normalized
--------------------------------
netcdf+mpi
^hdf5+mpi
^mpich
^zlib@1.2.5:
^m4
Concretized
--------------------------------
netcdf@4.4.1.1%gcc@4.8~cdmremote~dap~hdf4 maxdims=1024 maxvars=8192 +mpi~parallel-netcdf+shared arch=linux-ubuntu14-x86_64
^hdf5@1.10.1%gcc@4.8+cxx~debug+fortran+mpi+pic+shared~szip~threadsafe arch=linux-ubuntu14-x86_64
^mpich@3.2%gcc@4.8+hydra+pmi+romio~verbs arch=linux-ubuntu14-x86_64
^zlib@1.2.11%gcc@4.8+pic+shared arch=linux-ubuntu14-x86_64
^m4@1.4.18%gcc@4.8+sigsegv arch=linux-ubuntu14-x86_64
^libsigsegv@2.11%gcc@4.8 arch=linux-ubuntu14-x86_64
$ spack spec netcdf~mpi ^hdf5+mpi ^mpich
Input spec
--------------------------------
netcdf~mpi
^hdf5+mpi
^mpich
Normalized
--------------------------------
==> Error: Invalid spec: 'hdf5+mpi'. Package hdf5 requires variant ~mpi, but spec asked for +mpibut I couldn't make it fail. |
|
@adamjstewart What I get with the last commit with respect to the cases in #1553: $ spack spec netcdf ^hdf5@1.8.16
Input spec
--------------------------------
netcdf
^hdf5@1.8.16
Normalized
--------------------------------
netcdf
^hdf5@1.8.16
^zlib@1.2.5:
^m4
Concretized
--------------------------------
netcdf@4.4.1.1%gcc@4.8~cdmremote~dap~hdf4 maxdims=1024 maxvars=8192 +mpi~parallel-netcdf+shared arch=linux-ubuntu14-x86_64
^hdf5@1.8.16%gcc@4.8+cxx~debug+fortran+mpi+pic+shared~szip~threadsafe arch=linux-ubuntu14-x86_64
^openmpi@2.1.1%gcc@4.8~cuda fabrics= ~java schedulers= ~sqlite3~thread_multiple+vt arch=linux-ubuntu14-x86_64
^hwloc@1.11.7%gcc@4.8~cuda+libxml2+pci arch=linux-ubuntu14-x86_64
^libpciaccess@0.13.5%gcc@4.8 arch=linux-ubuntu14-x86_64
^libtool@2.4.6%gcc@4.8 arch=linux-ubuntu14-x86_64
^m4@1.4.18%gcc@4.8+sigsegv arch=linux-ubuntu14-x86_64
^libsigsegv@2.11%gcc@4.8 arch=linux-ubuntu14-x86_64
^pkg-config@0.29.2%gcc@4.8+internal_glib arch=linux-ubuntu14-x86_64
^util-macros@1.19.1%gcc@4.8 arch=linux-ubuntu14-x86_64
^libxml2@2.9.4%gcc@4.8~python arch=linux-ubuntu14-x86_64
^xz@5.2.3%gcc@4.8 arch=linux-ubuntu14-x86_64
^zlib@1.2.11%gcc@4.8+pic+shared arch=linux-ubuntu14-x86_64
$ spack spec netcdf+mpi ^mvapich2
Input spec
--------------------------------
netcdf+mpi
^mvapich2
Normalized
--------------------------------
netcdf+mpi
^hdf5+mpi
^mvapich2
^bison
^m4@1.4.6:
^libpciaccess
^libtool
^pkg-config@0.9.0:
^util-macros
^zlib@1.2.5:
Concretized
--------------------------------
netcdf@4.4.1.1%gcc@4.8~cdmremote~dap~hdf4 maxdims=1024 maxvars=8192 +mpi~parallel-netcdf+shared arch=linux-ubuntu14-x86_64
^hdf5@1.10.1%gcc@4.8+cxx~debug+fortran+mpi+pic+shared~szip~threadsafe arch=linux-ubuntu14-x86_64
^mvapich2@2.2%gcc@4.8 ch3_rank_bits=32 ~debug fabrics=psm process_managers= threads=multiple arch=linux-ubuntu14-x86_64
^bison@3.0.4%gcc@4.8 arch=linux-ubuntu14-x86_64
^m4@1.4.18%gcc@4.8+sigsegv arch=linux-ubuntu14-x86_64
^libsigsegv@2.11%gcc@4.8 arch=linux-ubuntu14-x86_64
^libpciaccess@0.13.5%gcc@4.8 arch=linux-ubuntu14-x86_64
^libtool@2.4.6%gcc@4.8 arch=linux-ubuntu14-x86_64
^pkg-config@0.29.2%gcc@4.8+internal_glib arch=linux-ubuntu14-x86_64
^util-macros@1.19.1%gcc@4.8 arch=linux-ubuntu14-x86_64
^zlib@1.2.11%gcc@4.8+pic+shared arch=linux-ubuntu14-x86_64 |
|
@alalazo This is identical to my original solution in #1553: 4d12c54 However, I ended up backing out that change in 28537ae because I discovered a problem. With the following packages:
hdf5:
variants: ~mpi
|
|
@adamjstewart Hmm, what do you propose to do? The current state seems really broken (asking |
|
The conflict is basically the same solution I came up with in 28537ae, except that was before I'm also fine with variant forwarding, but I don't want to be the person answering all of the questions as to why it doesn't work. Variant forwarding is a better long-term solution, and @citibeth and I have proposed a couple means of doing it automatically. But again, I've seen no progress on that front in the last 2 years and I'm starting to lose hope that we'll ever see this magical promised concretizer 🙁 |
I implemented a form of variant forwarding one year and a half ago in #391, but ended up closing it because it was not well received. If I remember correctly the main issue was the 'local' nature of that implementation, where a package could forward variants only to its direct dependencies. Honestly I would still advocate for that 😄 |
This is not a problem. You asked for a working NetCDF without MPI capability, and that's what you got. If you want to Putting in unnecessary forwarding constraints can break concretization in unpredictable ways. Suppose I have a package A that uses non-parallel NetCDF and also paralle HDF5. It would be quite natural to write in This should concretize, but it won't with this PR. If the problem is extra stuff in your DAG that you don't want... well, I do recommend looking at what Spack generates before building it, and removing any "surprises." I don't know of any other way; I don't think we can automagically elimiate "surprises" by adding ad-hoc variant forwarding in random locations. (Or more to the point, we will replace one set of solvable surprises with another set of surprises that are less tractable). |
|
I believe the "magic concretizer" we need would be the ability to encode
"suggested variants" in our `depends_on()` declarations. Such as...
"NetCDF SUGGESTS hdf5~mpi; but if you really need hdf5+mpi for some other
reason, we'll go with that." This kind of soft-suggestion would get the
intuitive behavior people want/expect, without breaking concretization in
other cases.
|
Is this made up, or do you have in mind some real use-case? |
Suppose you're building a "Linux distro;" by which I mean, a set of interrelated packages in which there is exactly one installation of each package. This can be done by creating a top-level Bundle package, and listing each item you want in that package. This is the kind of stuff we want to support in Spack Environments. It's easy to imagine that we might want to include A->netcdf~mpi and B->hdf+mpi in the same software distribution. |
|
I experienced a new wrinkle on this one. Try: Funny, since GDAL doesn't use MPI (directly). But it does use: Apparently, header files included from HDF5 (via NetCDF) result in an OpenMPI dependency in the compiled GDAL code. The upshot, as far as I can tell, is: If A->...->mpi, then A might require the MPI compiler wrappers. I have no idea what the right way is to approach this. For now, I'm building: I just want this to work, and I don't need to link to GDAL. But if I DID need to link to GDAL, I'd be up a creek now (because GDAL doesn't compile with MPI-enabled HDF5, and my stuff NEEDS MPI-enabled HDF5). |
That doesn't mean we want to have them in the same DAG for a single application/library (that's what a |
|
The idea is to build each package only once.
…On Jul 20, 2017 3:10 AM, "Massimiliano Culpo" ***@***.***> wrote:
It's easy to imagine that we might want to include A->netcdf~mpi and
B->hdf+mpi in the same software distribution.
That doesn't mean we want to have them in the same DAG for a single
application/library (that's what a conflict express). If in an
*environment* one application uses netcdf~mpi and another hdf5+mpi then
it's fine, because the two things will be concretized separately.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4672 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB1cd6zjUYRmp2CD51Zdkd-dIC5sBzjOks5sPv10gaJpZM4OM9yt>
.
|
|
I guess the only way to address @citibeth's concern is to answer the following questions:
If the answer to both of these questions is "no", then @alalazo's PR does the correct thing. If the answer to either of these questions is "yes", then @citibeth is correct in that we shouldn't force variant forwarding if it isn't required. Sound good? |
|
@adamjstewart I think there's also another question - Do you expect to see |
|
@alalazo I guess, when I run |
|
@adamjstewart In my opinion the problem is semantic: if I am allowed to do something like |
|
On Sat, Aug 5, 2017 at 2:07 PM, Adam J. Stewart ***@***.***> wrote:
I guess the only way to address @citibeth <https://github.com/citibeth>'s
concern is to answer the following questions:
1. Is it possible to build netcdf+mpi ^hdf5~mpi?
2. Is it possible to build netcdf~mpi ^hdf5+mpi?
If the answer to both of these questions is "no", then @alalazo
<https://github.com/alalazo>'s PR does the correct thing. If the answer
to either of these questions is "yes", then @citibeth
<https://github.com/citibeth> is correct in that we shouldn't force
variant forwarding if it isn't required. Sound good?
(1) No. (2) Yes.
But I also agree that this is broken from a usability standpoint. The
simplest fix I believe would be to default to `+mpi` for both `netcdf` and
`hdf5`. Spack is for HPC, after all; most users probably want to use
MPI. And it makes no sense to default them differently.
@alalazo <https://github.com/alalazo> I guess, when I run spack spec foo
~mpi, I expect to see ~mpi for every dependency, not just the top level
dependency. But I also want to be able to override that: spack spec foo
~mpi ^bar+mpi. We need a better concretizer that knows to disable MPI for
all dependencies by default. Adding depends_onprevents users from
overriding this.
I think this is a really good point. The concretizer can ALREADY do this:
just specify `+mpi` under the `all` section of `packages.yaml`.
Stepping back, we have two ways of specifying details to the concretizer:
one on the command line and one in `packages.yaml`. Unfortunately, the two
operate with semantics that differ in subtle ways. In general, I believe
the semantics in `packages.yaml` work better and have fewer "gotchas,"
including in this case. Or when virtual dependencies are involved.
@tgamblin maybe the right thing to do is to think about how to re-work the
command line syntax and semantics so it becomes essentially an override to
`packages.yaml`. This would be (semantically) equivalent to specifying ALL
variants, etc. in `packages.yaml` and then only installing with "clean"
command lines (eg: `spack install netcdf` instead of `spack install
netcdf+mpi`). I have had good success with that. So I believe that
ditching the current command-line system for one that is equivalent to
overriding `packages.yaml` would be received well. Of course, this change
would be more useful and flexible than the practice of specifying all
variants in `packages.yaml` today.
If this were to be done, there would be some way to specify variants for
the `all` package. And then one could specify `all+mpi` as @adamjstewart
is suggesting. In the meantime, you can just put `all+mpi` in
`packages.yaml` and the problem will be solved for you.
|
|
@alalazo: A quick tangent on #391. When #391 was opened @tgamblin and I were both of the opinion that forwarding should be an attribute of a variant, rather than an attribute of a dependency. That was the key issue at play. I can't speak for Todd, but I am no longer certain of that opinion. I see potential problems with both styles. If we were to go with something like #391, I see the following challenges.
There are potential solutions to this, such as searching upwards beyond parents for forwarded variants that could apply. The behavior could become counterintuitive if we're not careful.
With that said, I'm not opposed to reopening #391 and trying to address these problems. I'm not sure we'll come up with a satisfactory solution, but I'm not opposed to that approach anymore. |
I definitely agree with this. This becomes quite cumbersome: depends_on('hdf5')
depends_on('hdf5+mpi', when='+mpi')
depends_on('hdf5~mpi', when='~mpi')It also makes it impossible to build depends_on('hdf5')That is, by default, |
|
@becker33 For #391 the solutions I would have adopted to point 1 are either:
The rationale is that a package should only know about its direct dependencies, and should be able to forward a variant (or an expression involving a variant, maybe?) with a fine level of control only to the dependencies he cares about. For point 2 I agree completely. I wonder if with some meta-programming we could enable something like: class Foo(Package):
...
# Define variants 'mpi' and 'debug' here
...
# Forward them to multiple dependencies
with forwarded_variants('mpi', 'debug'):
depends_on('a')
depends_on('b')to group similar requests together. @adamjstewart With #391 this: depends_on('hdf5')
depends_on('hdf5+mpi', when='+mpi')
depends_on('hdf5~mpi', when='~mpi')would have become: depends_on('hdf5', forward='mpi') |
|
I still vote for forwarding all variants to all dependencies by default, with the ability to override them in the spec if you need to. We can handle the work once in the concretizer instead of in every single package file. |
|
@adamjstewart At a first glance, with what you propose I see no clear way to use an expression involving a variant selectively. Something like: class Foo(Package)
variant('openmp', ...)
....
depends_on('lapack', forward=negate('openmp'))
...to mean that I want to forward to |
|
@alalazo I didn't think about that case, but perhaps: depends_on('foo')
depends_on('foo+mpi', when='~mpi')
depends_on('foo~mpi', when='+mpi')would work in the rare case that this needs to be the case? (I have never looked at the concretizer so I have no idea how much work any of this would be). |
|
On Fri, Aug 11, 2017 at 9:00 AM, Adam J. Stewart ***@***.***> wrote:
I still vote for forwarding all variants to all dependencies by default,
with the ability to override them in the spec if you need to. We can handle
the work once in the concretizer instead of in every single package file.
We already have this capability with the "all" directive in
`packages.yaml`. We just need to make a way to put in on the command line
(for those who like that kind of thing).
Really... what is wrong with setting `all: +mpi`? Would that not solve the
problems in this thread?
As an aside, I think that commonly-used variants should default the same
way for all packages that use them.
|
There are pros and cons of this. The only reason I enforced all packages using |
|
On Fri, Aug 11, 2017 at 12:18 PM, Adam J. Stewart ***@***.***> wrote:
Really... what is wrong with setting all: +mpi? Would that not solve the
problems in this thread?
packages.yaml is great if you only need to build a single configuration
of every package, but many people (myself included) need to build an entire
stack of software with and without MPI (with 3 different MPIs, 4 different
compilers, etc.).
So let's figure out how do the equivalent on the command line. ???? This
should not require deep changes to the concretizer.
@tgamblin as a related issue... we currently have TWO unrelated ways to
specify variants, with TWO different kinds of semantics: on the
command-line and in `packages.yaml`. I believe that instead, we should
have JUST ONE semantics, and allow users to specify things equivalently on
the command line or in `packages.yaml`. Having dealt with both set of
semantics, I believe that the semantics currently in `packages.yaml` is
more robust, and should probably be adopted for both.
|
Currently when we ask Spack to build
netcdf~mpiwhat we get back after concretization is:that is a
netcdfthat depends onmpithroughhdf5. This PR prevents this particular combination from being possible as it forwards thempivariant tohdf5. It also adds a conflict check onnetcdf+parallel-netcdf~mpi.