target optimization: re-norm optimization scale so that 0 is best. (#29926)

referred targets are currently the only minimization criteria for Spack for which we allow
negative values. That means Spack may be incentivized to add nodes to the DAG if they
match the preferred target.

This PR re-norms the minimization criteria so that preferred targets are weighted from 0,
and default target weights are offset by the number of preferred targets per-package to
calculate node_target_weight.

Also fixes a bug in the test for preferred targets that was making the test easier to pass
than it should be.

3 files changed, 34 insertions, 16 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 7a8117976b..1b3eb8b1a8 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -716,6 +716,7 @@ class SpackSolverSetup(object):
         self.variant_values_from_specs = set()
         self.version_constraints = set()
         self.target_constraints = set()
+        self.default_targets = {}
         self.compiler_version_constraints = set()
         self.post_facts = []
 
@@ -1164,7 +1165,7 @@ class SpackSolverSetup(object):
                     pkg_name, variant.name, value
                 ))
 
-    def preferred_targets(self, pkg_name):
+    def target_preferences(self, pkg_name):
         key_fn = spack.package_prefs.PackagePrefs(pkg_name, 'target')
 
         if not self.target_specs_cache:
@@ -1175,13 +1176,25 @@ class SpackSolverSetup(object):
 
         target_specs = self.target_specs_cache
         preferred_targets = [x for x in target_specs if key_fn(x) < 0]
-        if not preferred_targets:
-            return
 
-        preferred = preferred_targets[0]
-        self.gen.fact(fn.package_target_weight(
-            str(preferred.architecture.target), pkg_name, -30
-        ))
+        for i, preferred in enumerate(preferred_targets):
+            self.gen.fact(fn.package_target_weight(
+                str(preferred.architecture.target), pkg_name, i
+            ))
+
+        # generate weights for non-preferred targets on a per-package basis
+        default_targets = {
+            name: weight for
+            name, weight in self.default_targets.items()
+            if not any(preferred.architecture.target.name == name
+                       for preferred in preferred_targets)
+        }
+
+        num_preferred = len(preferred_targets)
+        for name, weight in default_targets.items():
+            self.gen.fact(fn.default_target_weight(
+                name, pkg_name, weight + num_preferred + 30
+            ))
 
     def flag_defaults(self):
         self.gen.h2("Compiler flag defaults")
@@ -1572,7 +1585,7 @@ class SpackSolverSetup(object):
                 compiler.name, compiler.version, uarch.family.name
             ))
 
-        i = 0
+        i = 0  # TODO compute per-target offset?
         for target in candidate_targets:
             self.gen.fact(fn.target(target.name))
             self.gen.fact(fn.target_family(target.name, target.family.name))
@@ -1581,11 +1594,13 @@ class SpackSolverSetup(object):
 
             # prefer best possible targets; weight others poorly so
             # they're not used unless set explicitly
+            # these are stored to be generated as facts later offset by the
+            # number of preferred targets
             if target.name in best_targets:
-                self.gen.fact(fn.default_target_weight(target.name, i))
+                self.default_targets[target.name] = i
                 i += 1
             else:
-                self.gen.fact(fn.default_target_weight(target.name, 100))
+                self.default_targets[target.name] = 100
 
             self.gen.newline()
 
@@ -1862,7 +1877,7 @@ class SpackSolverSetup(object):
             self.pkg_rules(pkg, tests=self.tests)
             self.gen.h2('Package preferences: %s' % pkg)
             self.preferred_variants(pkg)
-            self.preferred_targets(pkg)
+            self.target_preferences(pkg)
 
         # Inject dev_path from environment
         env = ev.active_environment()
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index b9b3141499..5ee2eeb731 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -779,10 +779,13 @@ target_compatible(Descendent, Ancestor)
 #defined target_satisfies/2.
 #defined target_parent/2.
 
-% The target weight is either the default target weight
-% or a more specific per-package weight if set
+% If the package does not have any specific weight for this
+% target, offset the default weights by the number of specific
+% weights and use that. We additionally offset by 30 to ensure
+% preferences are propagated even against large numbers of
+% otherwise "better" matches.
 target_weight(Target, Package, Weight)
-  :- default_target_weight(Target, Weight),
+  :- default_target_weight(Target, Package, Weight),
      node(Package),
      not derive_target_from_parent(_, Package),
      not package_target_weight(Target, Package, _).
@@ -1238,7 +1241,7 @@ opt_criterion(1, "non-preferred targets").
 %-----------------
 #heuristic version(Package, Version) : version_declared(Package, Version, 0), node(Package). [10, true]
 #heuristic version_weight(Package, 0) : version_declared(Package, Version, 0), node(Package). [10, true]
-#heuristic node_target(Package, Target) : default_target_weight(Target, 0), node(Package). [10, true]
+#heuristic node_target(Package, Target) : package_target_weight(Target, Package, 0), node(Package). [10, true]
 #heuristic node_target_weight(Package, 0) : node(Package). [10, true]
 #heuristic variant_value(Package, Variant, Value) : variant_default_value(Package, Variant, Value), node(Package). [10, true]
 #heuristic provider(Package, Virtual) : possible_provider_weight(Package, Virtual, 0, _), virtual_node(Virtual). [10, true]
diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py
index 525bd0701f..28bb13ee22 100644
--- a/lib/spack/spack/test/concretize_preferences.py
+++ b/lib/spack/spack/test/concretize_preferences.py
@@ -144,7 +144,7 @@ class TestConcretizePreferences(object):
 
         update_packages('mpileaks', 'target', [default])
         spec = concretize('mpileaks')
-        assert str(spec['mpich'].target) == default
+        assert str(spec['mpileaks'].target) == default
         assert str(spec['mpich'].target) == default
 
     def test_preferred_versions(self):