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.

lib/spack/spack/solver/asp.py

@@ -716,6 +716,7 @@ def __init__(self, tests=False):
         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 @@ def preferred_variants(self, pkg_name):
                     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 @@ def preferred_targets(self, pkg_name):
 
         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 @@ def target_defaults(self, specs):
                 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 @@ def target_defaults(self, specs):
 
             # 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 @@ def setup(self, driver, specs, reuse=None):
             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()

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]

lib/spack/spack/test/concretize_preferences.py

@@ -144,7 +144,7 @@ def test_preferred_target(self, mutable_mock_repo):
 
         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):