Best-effort co-concretization for environments with explicit conflicts by becker33 · Pull Request #27964 · spack/spack

becker33 · 2021-12-13T18:40:05Z

Currently, environments can either be concretized fully together or fully separately. This works well for users who create environments for interoperable software and can use concretization: together. It does not allow environments with conflicting software to be concretized for maximal interoperability.

The primary use-case for this is facilities providing system software. Facilities provide multiple MPI implementations, but all software built against a given MPI ought to be interoperable.

This PR adds a concretization option together_where_possible. When this option is used, Spack will concretize specs in the environment separately, but will optimize for minimal differences in overlapping packages.

TODO:

tests
documentation
improve optimization criterion for minimizing differences
minimize appearances of a given virtual

Stretch goal:

dynamic optimization without writing to file

becker33 · 2021-12-13T18:40:59Z

@alalazo this is the PR we were discussing earlier

alalazo

I started reviewing the PR, and left a few questions. At a high level:

The spack solve output is very verbose with hundreds of optimization criteria
The association of a process ID and a spec is maybe too loose in the code, and needs to be consolidated in some way.
Having separate optimization criteria for each package poses questions on how to select priorities

I tested the solve time for a few specs and the results are great, i.e. we don't have noticeable performance losses. Solve time for PR 27694 (in seconds):

	hdf5	cp2k	trilinos
setup	3.6892	5.6508	4.7846
ground	1.3216	2.2394	1.9163
solve	2.4108	4.1936	2.8291
total	7.4647	12.1481	9.5782

Solve time for develop (in seconds):

	hdf5	cp2k	trilinos
setup	3.6545	5.4729	4.6592
ground	1.0601	1.6933	1.5260
solve	2.1859	3.7101	3.4006
total	6.9415	11.0188	9.6327

This comparison points out that the new encoding doesn't add much overhead when we fall-back to the previous case of having a single process id.

Tests run on:

Spack: 0.17.1-710-afa4221598
Python: 3.8.10
Platform: linux-ubuntu20.04-icelake
Concretizer: clingo

alalazo · 2022-01-03T14:32:54Z

lib/spack/spack/cmd/solve.py

    result = asp.solve(
        specs, dump=dump, models=models, timers=args.timers, stats=args.stats,
-        reuse=args.reuse,
+        reuse=args.reuse, multi_root=True


The solve command is cluttered because of the number of optimization criteria. A typical spack solve shows hundreds of lines of minimizations over possible dependencies - this seems a usability issue similar to lot of facts being dumped for errors.

Should be fixed in 9648b56

I see weird output. Here's what zlib looks like:

and here's hd5~mpi:

Reading the code I see that now we show by default only the criteria that are non-zero i.e. sub-optimal. Is there an easy way to show output like before in non-debug mode and add:

number of non-identical *** nodes

criteria only in debug mode?

lib/spack/spack/concretize.py

lib/spack/spack/cmd/diff.py

alalazo · 2022-01-03T14:38:38Z

lib/spack/spack/schema/env.py

                    'concretization': {
                        'type': 'string',
-                        'enum': ['together', 'separately'],
+                        'enum': ['together', 'separately', 'together_where_possible'],


Minor: wondering if we should come up with a single word or anyhow something shorter for this concretization mode.

lib/spack/spack/solver/asp.py

alalazo · 2022-01-03T15:30:10Z

lib/spack/spack/solver/asp.py

            self.gen.fact(fn.deprecated_version(pkg.name, v))

-    def spec_versions(self, spec):
+    def spec_versions(self, idx, spec):


In general I think we need to improve docstrings now that the arguments are becoming more involved conceptually. As far as I understand here idx is not arbitrary, but tied to the process ID / root ID? If there is a relation between these two arguments that needs to be respected, can't we package a spec and an idx in an object and send them around together?

The same spec could appear as part of multiple psids, since it could be a dependency of multiple roots.

lib/spack/spack/solver/asp.py

alalazo · 2022-01-04T10:22:35Z

Another data point by test-driving the PR with this simple environment:

spack:
  specs:
  - hdf5@1.13.0 ^zlib@1.2.8
  - hdf5@1.8.22
  concretization: together_where_possible

I obtain:

$ time spack -e . concretize -f
...
real	0m20,139s
user	0m19,811s
sys	0m0,309s

Compared to concretizing separately on develop:

$ time spack -e . concretize -f
==> Starting concretization pool with 2 processes
...

real	0m8,780s
user	0m16,109s
sys	0m0,335s

There is a 20% increase in user time when we have to unify zlib over 2 specs. Using a slightly more complex environment:

spack:
  specs:
  - hdf5@1.13.0 ^zlib@1.2.8
  - hdf5@1.8.21 ^zlib@1.2.11
  - hdf5@1.8.22
  concretization: together_where_possible

I have:

$ time spack -e . concretize -f
...
real	1m2,460s
user	1m1,971s
sys	0m0,461s

Compared to the separate concretization:

$ time spack -e . concretize -f
...
real	0m8,966s
user	0m24,255s
sys	0m0,514s

which is a 150% increase in user time. Furthermore, hdf5@1.8.22 is concretized using zlib@1.2.8 which I think is unexpected / sub-optimal, see screenshot:

becker33 · 2022-02-07T20:10:45Z

@alalazo I have addressed the two behavior bugs you showed. Still working on addressing other review comments.

alalazo · 2022-02-08T09:54:57Z

I tried again the same quick performance test. Here's the solve time for develop at c1b51d6 for reference (in seconds):

	hdf5 (ahqceuw)	cp2k (rhkmujf)	trilinos (7k3phbh)
setup	3.8275	5.6943	4.8123
ground	1.2049	1.7854	1.6058
solve	2.2181	4.6457	3.2920
total	7.2918	12.1834	9.7555

and here are the results for the PR at 78bb86e (merge commit, in seconds):

	hdf5 (ahqceuw)	cp2k (rhkmujf)	trilinos (7k3phbh)
setup	3.8857	5.9796	5.0564
ground	1.4768	2.2562	2.0109
solve	2.4698	4.2704	3.3387
total	7.8765 (+8%)	12.5708 (+3%)	10.4562 (+7%)

So overall just a small increase after the latest fixes, which I think is acceptable.

alalazo · 2022-02-08T10:09:13Z

I confirm that this environment:

spack:
  specs:
  - hdf5@1.13.0 ^zlib@1.2.8 ^xz@5.2.4
  - hdf5@1.8.21 ^zlib@1.2.11
  - hdf5@1.8.22
  - xz@5.2.5
  concretization: together_where_possible

now concretize as expected. I am attaching the spack.lock file as a reference.

It is concerning though that the time to reach a solution for this environment is:

real	2m34,696s
user	2m34,142s
sys	0m0,520s

if compared to separate concretization:

real	0m10,294s
user	0m27,867s
sys	0m0,571s

The concerning part is that it seems there's a great increase moving from a single spec to be unified to 2 specs. Now the user time is 154 sec. vs. 27 sec. so more than 5x. @becker33 I wonder if unifying everything in a single optimization criteria can mitigate this increase, since the search space is collapsed into a single additional dimension. I have no idea if this is just a wrong intuition or if it can be supported by any work I don't know about yet.

alalazo · 2022-02-08T10:53:58Z

Tried to construct a more complicated environment, where the new optimization criteria are more exercised:

spack:
  specs:
  - hdf5@1.13.0 ^zlib@1.2.8 ^xz@5.2.4 ^autoconf@2.71
  - hdf5@1.8.21 ^zlib@1.2.11 ^hwloc@2.6.0
  - hdf5@1.8.22
  - autoconf@2.70
  - xz@5.2.5
  concretization: together_where_possible

The overall time to solution doesn't increase much with respect to the simpler environment used before:

real	2m54,811s
user	2m54,297s
sys	0m0,482s

For reference, if we concretize separately:

real	0m10,044s
user	0m28,316s
sys	0m0,573s

Trying to concretize the e4s pipeline (as representative of a production environment) on my machine gives the following result on develop:

real	4m21,679s
user	30m46,242s
sys	0m6,934s

when trying together_where_possible:

$ time spack -e . concretize -f
Killed

real	5m31,647s
user	5m20,116s
sys	0m11,406s

as it literally hogs memory (in 5 mins. it ramped up to exceed the 32GB of RAM my machine has). I guess we need to keep an eye on memory too 😓

alalazo

I am halfway through the review. So far I have only one minor question on the output of spack solve which is probably relevant only for developers.

I tried to test drive the PR and this time the results have been always correct, so 💯 for the fixes on behavior.

I am a bit concerned by resources that this mode of concretization needs. So far I have been unable to concretize e4s (used as a representative of a production environment) on my machine due to memory requests.

alalazo · 2022-02-08T11:02:38Z

lib/spack/spack/test/concretize.py

+        import spack.solver.asp
+        specs = [spack.spec.Spec(s) for s in specs]
+        result = spack.solver.asp.solve(specs, reuse=False, multi_root=True)


Shouldn't we skip this test when using the original concretizer?

alalazo · 2022-02-08T11:03:30Z

lib/spack/spack/test/concretize.py

+        import spack.solver.asp
+
+        specs = [spack.spec.Spec(s) for s in specs]
+        result = spack.solver.asp.solve(specs, reuse=False, multi_root=True)


Same question on skipping the test for concretizer:original

Since the new criteria is above all the build criteria, we need to modify the way we display the output. Originally done by Greg in spack#27964 and cherry-picked to this branch by the co-author of the commit. Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

tgamblin · 2022-04-12T17:13:00Z

Superseded by #28941

Since the new criteria is above all the build criteria, we need to modify the way we display the output. Originally done by Greg in spack#27964 and cherry-picked to this branch by the co-author of the commit. Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com>

Currently, environments can either be concretized fully together or fully separately. This works well for users who create environments for interoperable software and can use `concretizer:unify:true`. It does not allow environments with conflicting software to be concretized for maximal interoperability. The primary use-case for this is facilities providing system software. Facilities provide multiple MPI implementations, but all software built against a given MPI ought to be interoperable. This PR adds a concretization option `concretizer:unify:when_possible`. When this option is used, Spack will concretize specs in the environment separately, but will optimize for minimal differences in overlapping packages. * Add a level of indirection to root specs This commit introduce the "literal" atom, which comes with a few different "arities". The unary "literal" contains an integer that id the ID of a spec literal. Other "literals" contain information on the requests made by literal ID. For instance zlib@1.2.11 generates the following facts: literal(0,"root","zlib"). literal(0,"node","zlib"). literal(0,"node_version_satisfies","zlib","1.2.11"). This should help with solving large environments "together where possible" since later literals can be now solved together in batches. * Add a mechanism to relax the number of literals being solved * Modify spack solve to display the new criteria Since the new criteria is above all the build criteria, we need to modify the way we display the output. Originally done by Greg in #27964 and cherry-picked to this branch by the co-author of the commit. Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com> * Inject reusable specs into the solve Instead of coupling the PyclingoDriver() object with spack.config, inject the concrete specs that can be reused. A method level function takes care of reading from the store and the buildcache. * spack solve: show output of multi-rounds * add tests for best-effort coconcretization * Enforce having at least a literal being solved Co-authored-by: Greg Becker <becker33@llnl.gov>

Currently, environments can either be concretized fully together or fully separately. This works well for users who create environments for interoperable software and can use `concretizer:unify:true`. It does not allow environments with conflicting software to be concretized for maximal interoperability. The primary use-case for this is facilities providing system software. Facilities provide multiple MPI implementations, but all software built against a given MPI ought to be interoperable. This PR adds a concretization option `concretizer:unify:when_possible`. When this option is used, Spack will concretize specs in the environment separately, but will optimize for minimal differences in overlapping packages. * Add a level of indirection to root specs This commit introduce the "literal" atom, which comes with a few different "arities". The unary "literal" contains an integer that id the ID of a spec literal. Other "literals" contain information on the requests made by literal ID. For instance zlib@1.2.11 generates the following facts: literal(0,"root","zlib"). literal(0,"node","zlib"). literal(0,"node_version_satisfies","zlib","1.2.11"). This should help with solving large environments "together where possible" since later literals can be now solved together in batches. * Add a mechanism to relax the number of literals being solved * Modify spack solve to display the new criteria Since the new criteria is above all the build criteria, we need to modify the way we display the output. Originally done by Greg in spack#27964 and cherry-picked to this branch by the co-author of the commit. Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com> * Inject reusable specs into the solve Instead of coupling the PyclingoDriver() object with spack.config, inject the concrete specs that can be reused. A method level function takes care of reading from the store and the buildcache. * spack solve: show output of multi-rounds * add tests for best-effort coconcretization * Enforce having at least a literal being solved Co-authored-by: Greg Becker <becker33@llnl.gov>

spackbot-app bot added commands dependencies environments new-variant labels Dec 13, 2021

becker33 changed the title ~~Features/concretize together conflicts~~ Best-effort co-concretization for environments with explicit conflicts Dec 13, 2021

becker33 force-pushed the features/concretize-together-conflicts branch from bfa7407 to c4bb008 Compare December 13, 2021 23:27

spackbot-app bot added the tests General test capability(ies) label Dec 14, 2021

tgamblin assigned becker33 Dec 20, 2021

becker33 force-pushed the features/concretize-together-conflicts branch from 3d93328 to bc20006 Compare December 22, 2021 00:47

becker33 requested review from alalazo and tgamblin and removed request for tgamblin December 22, 2021 02:17

becker33 assigned tgamblin and alalazo and unassigned becker33 Dec 23, 2021

alalazo reviewed Jan 3, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

alalazo mentioned this pull request Feb 7, 2022

ASP solver unable to sort variant_values_from_specs when variant can have both bool and str values #28260

Closed

3 tasks

becker33 force-pushed the features/concretize-together-conflicts branch from 296861d to 85ca2a5 Compare February 7, 2022 19:01

becker33 force-pushed the features/concretize-together-conflicts branch 3 times, most recently from 62a3406 to 264d1a8 Compare February 8, 2022 06:16

alalazo requested changes Feb 8, 2022

View reviewed changes

alalazo added a commit to alalazo/spack that referenced this pull request Mar 14, 2022

Fix unit tests cherry-picked from spack#27964

b714ac9

alalazo added a commit to alalazo/spack that referenced this pull request Mar 18, 2022

Fix unit tests cherry-picked from spack#27964

cf62ce5

alalazo added a commit to alalazo/spack that referenced this pull request Mar 18, 2022

Fix unit tests cherry-picked from spack#27964

75973be

alalazo added a commit to alalazo/spack that referenced this pull request Mar 29, 2022

Fix unit tests cherry-picked from spack#27964

fd3f2f0

alalazo added a commit to alalazo/spack that referenced this pull request Apr 2, 2022

Fix unit tests cherry-picked from spack#27964

1a6603c

tgamblin closed this Apr 12, 2022

alalazo added a commit to alalazo/spack that referenced this pull request Apr 14, 2022

Fix unit tests cherry-picked from spack#27964

83fb6b1

alalazo added a commit to alalazo/spack that referenced this pull request Apr 26, 2022

Fix unit tests cherry-picked from spack#27964

375ca14

alalazo added a commit to alalazo/spack that referenced this pull request May 9, 2022

Fix unit tests cherry-picked from spack#27964

8a06c42

alalazo added a commit to alalazo/spack that referenced this pull request May 16, 2022

Fix unit tests cherry-picked from spack#27964

53b6520

alalazo added a commit to alalazo/spack that referenced this pull request May 21, 2022

Fix unit tests cherry-picked from spack#27964

dbf88b4

alalazo added a commit to alalazo/spack that referenced this pull request May 23, 2022

Fix unit tests cherry-picked from spack#27964

d0ecf24

alalazo added a commit to alalazo/spack that referenced this pull request May 24, 2022

Fix unit tests cherry-picked from spack#27964

3bc7a21

haampie deleted the features/concretize-together-conflicts branch August 2, 2022 09:54

Conversation

becker33 commented Dec 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

becker33 commented Dec 13, 2021

Uh oh!

alalazo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalazo Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalazo commented Jan 4, 2022

Uh oh!

This comment was marked as outdated.

becker33 commented Feb 7, 2022

Uh oh!

alalazo commented Feb 8, 2022

Uh oh!

alalazo commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalazo commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalazo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgamblin commented Apr 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

becker33 commented Dec 13, 2021 •

edited

Loading

alalazo left a comment •

edited

Loading

alalazo Feb 8, 2022 •

edited

Loading

alalazo commented Feb 8, 2022 •

edited

Loading

alalazo commented Feb 8, 2022 •

edited

Loading