concretizer: add --minimal configuration/CLI option#28470
Open
concretizer: add --minimal configuration/CLI option#28470
--minimal configuration/CLI option#28470Conversation
tgamblin
force-pushed
the
minimal-concretization
branch
from
f117a76 to
60e4a72
Compare
8 tasks
tgamblin
force-pushed
the
minimal-concretization
branch
3 times, most recently
from
6a79ec9 to
e41ee22
Compare
becker33
requested changes
May 3, 2022
Member
becker33
left a comment
becker33
left a comment
There was a problem hiding this comment.
Some quick questions, but I need to give this a once-over again after it's rebased.
Comment on lines
+2051
to
+2201
| ``minimal (bool)`` | ||
| If ``True`` make minimizing nodes the top priority, even higher | ||
| than defaults from packages and preferences. |
Member
There was a problem hiding this comment.
this doesn't quite match the description you gave in the PR, it sounds like it makes minimizing builds the priority -- I think we need to make sure to document which it is properly.
if reuse is on, minimize is on, and cmake+openssl is already installed, is spack spec cmake supposed to use the old one?
lib/spack/spack/solver/asp.py
Outdated
| def __init__(self): | ||
| self.set_default_coanfiguration() | ||
|
|
||
| def set_default_coanfiguration(self): |
Member
There was a problem hiding this comment.
Suggested change
| def set_default_coanfiguration(self): | |
| def set_default_configuration(self): |
lib/spack/spack/solver/asp.py
Outdated
| continue | ||
| spack.spec.Spec.ensure_valid_variants(s) | ||
| def __init__(self): | ||
| self.set_default_coanfiguration() |
Member
There was a problem hiding this comment.
Suggested change
| self.set_default_coanfiguration() | |
| self.set_default_configuration() |
tgamblin
force-pushed
the
minimal-concretization
branch
from
e41ee22 to
bde33c7
Compare
tgamblin
force-pushed
the
minimal-concretization
branch
from
bde33c7 to
afde810
Compare
The reusing concretizer minimizes builds, but it still preserves defaults from packages and preferences while doing that. We can be more aggressive by making minimization the top priority, at the expense of "weird" concretizations. This can be advantageous: if you write your packages as explicitly as possible, then you can use that with `--minimal` to get the smallest possible package configuration (at least in terms of the number of packages in the build). Conversely, you can use minimal concretization as kind of a worst case to ensure that you have the "right" constraints on your dependencies. Example for intuition: `cmake` can optionally build without openssl, but it's enabled by default because many builds use that functionality. Using `minimal: true` will build `cmake~openssl` unless the user asks for `cmake+openssl` explicitly. - [x] add `minimal` option to `concretizer.yaml` - [x] add `--minimal` CLI option to concretizer arguments - [x] wire everything up - [x] add some tests
tgamblin
force-pushed
the
minimal-concretization
branch
from
afde810 to
3985b30
Compare
alalazo
requested changes
May 25, 2022
Comment on lines
+682
to
+684
| # Solver paramters that affect setup -- see Solver documentation | ||
| self.reuse = spack.config.get( | ||
| "concretizer:reuse", False) if reuse is None else reuse |
Member
There was a problem hiding this comment.
This is now passed in the setup method and is a list of specs.
alalazo
reviewed
May 25, 2022
Member
alalazo
left a comment
alalazo
left a comment
There was a problem hiding this comment.
This currently needs:
- An update to resolve conflicts with the recently merged #28941 (reuse argument)
spack commands --update-completion- Maybe a few lines of docs at https://spack.readthedocs.io/en/latest/build_settings.html#concretizer-options ?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This should merged after #28468, as it builds on top of it.
The reusing concretizer minimizes builds, but it still preserves defaults from packages and preferences while doing that. We can be more aggressive by making minimization the top priority, at the expense of "weird" concretizations.
This can be advantageous: if you write your packages as explicitly as possible, then you can use that with
--minimalto get the smallest possible package configuration (at least in terms of the number of packages in the build). Conversely, you can use minimal concretization as kind of a worst case to ensure that you have the "right" constraints on your dependencies.Example for intuition:
cmakecan optionally build without openssl, but it's enabled by default because many builds use that functionality. Usingminimal: truewill buildcmake~opensslunless the user asks forcmake+opensslexplicitly.Examples
Say there's one
cmakeinstalled, with hashzd4m26e:By default, we would build a fancy new one:
With
--reuse, we'd use the installedzd4m26e:--minimalalone would build a newcmakewith options that require a dependency turned off (here,~ncursesand~openssl):Using both picks
zd4m26eagain because that gets you zero builds:Using both with
cmake ~opensslwill reusencurses/53i52xr(because it doesn't add a build):I am not sure if people want this result or the one from
spack spec -l --minimal cmakeabove.--minimizeis minimizing builds, not nodes. I could see where both could be useful, but you can effectively get minimization of nodes right now by just using--minimal(see above), so I don't think we need to add another config. This is more composable.minimaloption toconcretizer.yaml--minimalCLI option to concretizer arguments