-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v0.22 concretizer yields suboptimal fresh concrete specs #44216
Comments
Bisected to 89fc9a9, which just adds new versions and deps, not immediately useful. edit: just removing version 2.0 from lcov is sufficient to have it pick up 1.16 on |
I can reproduce too. The optimization weights when
versus this output when
|
Looking into it |
Wow, I found a trivial typo in the root optimisation weight that apparently has been there since ever. I'm submitting a bugfix, but this bug is kind of rare to hit, since there must be two sub-optimal versions used in the environment and they must have the same penalty score. |
That was indeed the trigger in this case, cause it caused the old version of @jcortial-safran Thank you very much for the reproducer, that was super helpful. Can you confirm that this: diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 60165338c6..4d8a202ce3 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -1432,11 +1432,11 @@ opt_criterion(73, "deprecated versions used").
% 1. Version weight
% 2. Number of variants with a non default value, if not set
% for the root package.
-opt_criterion(70, "version weight").
+opt_criterion(70, "version badness (roots)").
#minimize{ 0@270: #true }.
#minimize{ 0@70: #true }.
#minimize {
- Weight@70+Priority
+ Weight@70+Priority,PackageNode
: attr("root", PackageNode),
version_weight(PackageNode, Weight),
build_priority(PackageNode, Priority)
@@ -1526,13 +1526,14 @@ opt_criterion(30, "non-preferred OS's").
}.
% Choose more recent versions for nodes
-opt_criterion(25, "version badness").
+opt_criterion(25, "version badness (non roots)").
#minimize{ 0@225: #true }.
#minimize{ 0@25: #true }.
#minimize{
Weight@25+Priority,node(X, Package)
: version_weight(node(X, Package), Weight),
build_priority(node(X, Package), Priority),
+ not attr("root", node(X, Package)),
not runtime(Package)
}.
solves the issue for you? |
fixes spack#44216 This fixes a bug occurring when two root specs need to select old versions, and these versions have the same penalty in the optimization. This sometimes caused an older version to be preferred to a more recent one. The issue was the omission of `PackageNode` in the optimization tuple.
Thanks for your help ! Yes it solves the issue in the example. It also improves greatmy the concretized specs in my real case. However it seems that some suboptimal specs (not in the roots, but in a dependent package, namely |
Note that cases like that might happen if giving a penalty to some nodes results in avoiding bigger penalties on other nodes. |
Yes, I agree. I need to confirm that the solution is indeed suboptimal. |
Steps to reproduce
When the
spack.yaml
file defining the environmenttest
is as follows:then the v0.22 concretizer selects the version 1.15 of
lcov
even if a version 1.16 is available.One must use a more specific constraint to actually get
lcov@=1.16
:Note that in both cases the concrete specs for
vtk@9.1
are the same (hashbher5da
). Hence it seems fair to conclude that the concretizer did not pick the "best" solution in the first case.The above case is only a minimal example that shows the issue. For larger cases, I have observed many suboptimal choices, for seemingly random packages, for instance
py-cython
,pmix
,openmpi
, and so on.That behavior did not occur using spack v0.21. For instance, using again the less constrained
spack.yaml
file for the environmenttest
:then the v0.21 concretizer selects the version 1.16 of
lcov
:Error message
No response
Information on your system
General information
spack debug report
and reported the version of Spack/Python/PlatformThe text was updated successfully, but these errors were encountered: