I was also thinking that maintaining a view by default:

1. Should be done when concretize_together is true
2. Should NOT be done when concretize_together is false

Of course, any explicit configuration for views should override the default. I wonder what is the stance of other people on this.

@becker33

@scheibelp

This PR adds a boolean entry in spack.yaml:

spack:
    concretize_together: true

If set to true all the specs in the environment are concretized to be self-consistent, meaning there will be a single configuration for each package and a single provider for each virtual dependency. The default value is false to maintain backward compatibility with the behavior before this PR.

This issue was actually thought through and discussed since the beginning of Spack Environments. In the end, we implemented what we did, rather than the full plan, in order to get SOMETHING functional out the door --- and because the concretizer at the time did not support concretize_together in a lightweight manner.

The full plan is basically to allow concretize_together() on a sets of packages WITHIN an environment. I don't know what the syntax would be for this. But the general idea is... suppose you have packages A through G in your environment. The current scheme would concretize all of them separately. But if you wanted them all concretized together, you would say (again, recognizing that the final syntax is undefined):

concretize_together(A,B,C,D,E,F,G)

Or you could do (for example):

concretize_together(A), concretize_together(B), concretize_together(C,D,E,F,G)

The last case is actually the most likely, IMHO. In my experience, a Spack Environment for a particular project requires (a) a bunch of required dependencies for the project, and (b) a bunch of ancillary command-line tools that are used to create input files, view output files, do source code control, etc. Packages in (a) should be concretized together, and packages in (b) should not.

In other words, my preferred environment would look something like this:

emacs
git
nco
concretize_together(libA, libB, libC, libD)

To add a detail on this: right now we create views by default in Spack environments. What I propose is to limit this default only to cases where we know that the concretized specs are surely self-consistent and without clashes.

Three counterpoints to this:

1. A simple rule like this no longer makes sense once we generalize concretize_together() sets.
2. We already have precedence rules to resolve clashes --- which are not a serious problem in real life anyway.
3. The issue of clashes is exactly the same, whether you render your environment as a view or as a list of modules to load, or a as an (as-of-yet unimplemented) singe module.

This issue was actually thought through and discussed since the beginning of Spack Environments. In the end, we implemented what we did, rather than the full plan, in order to get SOMETHING functional out the door --- and because the concretizer at the time did not support concretize_together in a lightweight manner.

Is the full plan written somewhere? I didn't participate in the initial discussions and I thought that what was planned was merged already. Also, the concretizer didn't change in the meanwhile - I just wrote a function around it 🙂

From my point of view there are two major cases in which environments can be useful:

1. Deployment of a software stack
2. Development of one or several libraries / applications

Point 1. can somehow be addressed by the current state of environments and support for that use case will be completed by #11057. Point 2. is instead kind of unusable, or at least very inconvenient, as people might need to over-constraint specs in order for them to be concretized in a self-consistent manner. This PR makes point 2. usable.

Please add functionality to allow arbitrary subsets within the environment to be concretized together. That is much more useful than an all-or-nothing choice.

That, in my opinion, can be added later - on top of #11057. Because basically the subset and syntax you're looking for is exactly the one introduced by @becker33 in that PR. We could allow having a boolean key concretize_together at the same level of the matrix key, and the one appearing here can be overridden by that.

I don't see the need to oblige people to use the more complicated (and more powerful) syntax in #11057 if they just need to express a simple list of specs that needs to be concretized together. Going to your example, that would look like:

spack:
  specs:
  - matrix:
    - ['git', 'emacs', 'nco']
  - matrix:
    - ['libA', 'libB', 'libC', 'libD']
    concretize_together: true

but if I just need the four libraries concretized together, I'd rather say something like:

spack:
  specs:
  - 'libA'
  - 'libB'
  - 'libC' 
  - 'libD'
  concretize_together: true

rather than:

spack:
  specs:
  - matrix:
    - ['libA', 'libB', 'libC', 'libD']
    concretize_together: true

In the end I don't think that your request to extend the feature should be a blocker for this PR, as what you ask for can be implemented later. But I'll let @scheibelp and @becker33 judge that.

Side note: I still fail to see why I would want to have packages with conflicting dependencies (i.e. not concretised together) in the same environment, so take my comments with some amount of salt.

Why couldn't we (in the future) extend the concretise_together setting to take either a bool (all or nothing) or a list of tuples. Where each tuple would contain each set of (partial) specs that should be concretised together?

something like:

    concretise_together: [ ([email protected], [email protected]), 
                                          ([email protected], jdk@8),
                                          (py-matplotlib@2:, [email protected]:) ]

The only thing that does not trivially fit into this scheme is autogenerated matrices of software, but in this case the full freedom to choose arbitrary combinations is probably not needed anyways. IOW each combination of the matrix is probably supposed to be concretised together anyways.

For the default settings: I agree with @citibeth that a subtle connection between concretise_together and maintain_view is bad because it surprises the user. But if concretise_together is anticipated to produce problems like "We just ran out of inodes" it might be a good idea. From my experience I didn't run into that problem, but then again I only was on comparatively and small clusters.

In real life, I think most environments will be concretized together in one
batch. That is the case we need to make easy.

That is my thought exactly, but currently it is not the default behaviour, @alalazo's change would do that (on the cost of potential future compatibility problems).

@citibeth

Regarding #11372 (comment)

To install A and B both in the same environment, you will have to concretize them separately. Since L is a library, it is linked to F and G via RPATH

@tgamblin suggested that once the concretizer is updated to allow for multiple instances of dependencies based on dependency type (e.g. that two run dependencies can use conflicting link dependencies), that this issue will be handled automatically.

Given that, I think concretize_together is sufficiently expressive to handle the case you discuss (eventually).

@scheibelp @alalazo @becker33

@tgamblin suggested that once the concretizer is updated to allow for multiple instances of dependencies based on dependency type (e.g. that two run dependencies can use conflicting link dependencies), that this issue will be handled automatically.

Given that, I think concretize_together is sufficiently expressive to handle the case you discuss (eventually).

Short answer: I agree. Long answer:

I'm reluctant to break things, or leave them broken, on the assumption that the new concretizer will make it all well. That's why Spack still can't build Python3 stacks without a lot of fiddling. However, there already exist other ways to get everything to concretize the same way, even with concretize_together=False (put entries in the environment's spack.yaml). Therefore, we are not breaking anything by giving up on controllable concretization groups; nor will the new concretizer enable any new functionality on this front. But I agree, it will be a convenience.

This brings up the next question: will we get rid of concretize_together=True when the new concretizer comes along?

And finally... if we go this route, then we seem to be agreeing that environments are not just a bunch of specs, but specs that are meant to be concretized together (a la Linux distro); the only real exception being that non-top-level packages in the DAG could have multiple instances when needed.

If this is the case, then #11057 needs to be significantly reworked, because it breaks this fundamental assumption/feature of environments. If we do plan on merging #11057 in its current form --- which implicitly involves subsets of packages concretized together and separately from others --- then we might as well go whole hog and make it an explicit feature of environments.

In other words, I see a fundamental conflict between #11372 and #11057. We go either one way or the other. And right now, it looks like we're leaning toward #11372 and away from #11057. Any thoughts / comments?

Can anybody in the thread test drive again this PR and give some feedback? The only change with respect to its previous state is that concretize_together is now become concretization and accepts either the string together or the string separately (default).

==> Error: /usr/local/pace-apps/spack/root/0.12/78577c0/var/spack/environments/base_gcc/spack.yaml:7: Additional properties are not allowed ('concretize_together' was unexpected)

Then I tried (not fully aware of the syntax)

  concretization: ['separately']

Iterating

==> Error: /usr/local/pace-apps/spack/root/0.12/78577c0/var/spack/environments/base_gcc/spack.yaml:18: ['separately'] is not one of ['together', 'separately']

Then

  concretization: 'separately'

  concretization: 'together'

appear to do their jobs in the respective environments.

@kmanalo That's expected behavior: in the schema the value is supposed to be a string not a list. I agree that the default error message might be confusing in that case.

Thanks!

Great to see this got merged. Hope to test this soon. I don't see any documentation for this new option at https://spack.readthedocs.io/en/latest/spack.html. When will that appear?

@bartlettroscoe There's a brief mention of the new option here and here. Let me know what you think of that.

Looks good. I look forward to trying this out (but I think that means a complete upgrade of Spack so I am a little nervous).

I was looking for the term "concretize" instead of "concretization" so I missed this reference.

Alas, I can't even try this due to #13248 when attempting an upgrade. :-(