Best effort co-concretization (iterative algorithm) #28941
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spackbot-app
bot
added
commands
environments
gitlab
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
93ae2cd
to
253a6ab
Compare
|
@becker33 @tgamblin e4s-pr-generate took just 2 mins and a single core using |
3 tasks
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
253a6ab
to
5edd66d
Compare
becker33
requested changes
Feb 17, 2022
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
57864b7
to
c1b872b
Compare
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
5aa3301
to
d0a2c0f
Compare
spackbot-app
bot
added
the
documentation
becker33
requested changes
Feb 23, 2022
Comment on lines
+1420
to
+1704
| # The algorithm is greedy, and it might decide to solve the "best" | ||
| # spec early in which case reuse is suboptimal. In this case the most | ||
| # recent version of libdwarf is selected and concretized to libelf@0.8.13 | ||
| (['libdwarf@20111030^libelf@0.8.10', | ||
| 'libdwarf@20130207^libelf@0.8.12', | ||
| 'libdwarf@20130729'], 'libelf@0.8.12', 1), |
There was a problem hiding this comment.
Could we improve this by having both root literals (the literals we already use) and dependency literals (explicitly mentioned dependencies of roots) and minimizing the unsolved dependency literals as well? I think that would cause us to concretize the most specific specs first, which would increase our odds of a more minimal build.
There was a problem hiding this comment.
Adding a comment, following a discussion with @becker33 yesterday. We can do something very similar to this by adding weights during the setup phase for the literals that are solved. Then we can minimize the sum of the weights for reuse (currently weights are uniform and equal to 1 for each literal spec).
We can experiment with different ways of computing the weights in a separate PR.
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
c1f062f
to
7cebbff
Compare
|
@becker33 Pending unrelated errors in pipelines I think this is done. |
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
7cebbff
to
9e81c86
Compare
becker33
previously approved these changes
Feb 28, 2022
|
@becker33 Is this PR waiting further review before being merged? |
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
9e81c86
to
d2a1e49
Compare
|
@becker33 It's a mystery to me how @eugeneswalker (or more likely Github on his behalf) could dismiss your review with a commit that doesn't exist anymore, but can you please ✔️ again? I rebased the PR to double check that everything is working correctly with #28673 in |
What da!?! I definitely didn't mean to do this, didn't even know about this PR. If anyone knows how this could happen... |
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
d2a1e49
to
543eb60
Compare
This change injects the reusable specs in the SpecBuilder and avoids retrieving them from either the DB or the buildcache (making the underlying abstractions less coupled). It also fixes a bug where we were "reusing" only the root specs and not the other nodes of the DAG.
alalazo
force-pushed
the
features/iterative_coconcretization
branch
from
3792d93
to
08bc55e
Compare
becker33
approved these changes
May 24, 2022
Spack v0.18.0 release
automation
moved this from Review in progress
to Reviewer approved
May 24, 2022
Separate Concretization for Build Dependencies
automation
moved this from Review in progress
to Reviewer approved
May 24, 2022
Separate Concretization for Build Dependencies
automation
moved this from Reviewer approved
to Done
May 24, 2022
iarspider
pushed a commit
to iarspider/spack
that referenced
this pull request
May 30, 2022
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>
bhatiaharsh
pushed a commit
to bhatiaharsh/spack
that referenced
this pull request
Aug 8, 2022
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use case description copied from #27964
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.
Implementation differences
The algorithm used here is greedy, since specs are computed in multiple rounds where clingo concretizes together as many input specs as possible. The gist of the algorithm is:
https://github.com/alalazo/spack/blob/f6420eaf6eb5e10a3e4d0b0a1a6c20e4404f92c1/lib/spack/spack/solver/asp.py#L2092-L2135
To relax the requirements on input specs and allow for not solving some of them we add a new indirection:
https://github.com/alalazo/spack/blob/f6420eaf6eb5e10a3e4d0b0a1a6c20e4404f92c1/lib/spack/spack/solver/asp.py#L1787-L1802
and we give clingo the choice to solve for the input literal or not. All the input specs computed in previous rounds are then reused in later rounds to ensure the environment is as contained as possible.
Comparison with #27964
When trying out the encoding in #27964 for production environments we saw that there were huge requests:
The solve-time can very likely be improved a lot by removing the symmetry of the new encoding with some artificial rules. The memory requests during the grounding phase instead will necessarily grow linearly with the number of PSIDs needed to solve the environment. That number is not known a-priori. It is capped by the number of specs in an environment (which is computationally unfeasible) but practically is usually much less. For instance,
e4shas 119 specs and needs 3 process spaces to concretize.This PR instead takes a greedy approach and perform N solves each with a single process space. It might give a different (and "suboptimal" i.e. with a number of subprocesses greater than strictly needed) result if compared with the encoding in #27964 but it is complete, i.e. if there is at least a solution the algorithm will not give a false negative. On the bright side, due to its greedy nature, it's much faster when solving for environments (
e4sis solved in 3 rounds in ~1 min.).