ASP-based solver: rescale target weights so that 0 is always the best score (#31226)

fixes #30997

Instead of giving a penalty of 30 to all nodes when preferences
are not package specific, give a penalty of 100 to all targets
of a node where we have package specific preferences, if the target
is not explicitly preferred.

2 files changed, 15 insertions, 49 deletions

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 3b475c2c77..7e712f2c63 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -717,7 +717,7 @@ class SpackSolverSetup(object):
         self.variant_values_from_specs = set()
         self.version_constraints = set()
         self.target_constraints = set()
-        self.default_targets = {}
+        self.default_targets = []
         self.compiler_version_constraints = set()
         self.post_facts = []
 
@@ -1178,29 +1178,19 @@ class SpackSolverSetup(object):
         if not self.target_specs_cache:
             self.target_specs_cache = [
                 spack.spec.Spec('target={0}'.format(target_name))
-                for target_name in archspec.cpu.TARGETS
+                for _, target_name in self.default_targets
             ]
 
-        target_specs = self.target_specs_cache
-        preferred_targets = [x for x in target_specs if key_fn(x) < 0]
+        package_targets = self.target_specs_cache[:]
+        package_targets.sort(key=key_fn)
 
-        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
+        offset = 0
+        best_default = self.default_targets[0][1]
+        for i, preferred in enumerate(package_targets):
+            if str(preferred.architecture.target) == best_default and i != 0:
+                offset = 100
+            self.gen.fact(fn.target_weight(
+                pkg_name, str(preferred.architecture.target), i + offset
             ))
 
     def flag_defaults(self):
@@ -1604,11 +1594,12 @@ class SpackSolverSetup(object):
             # these are stored to be generated as facts later offset by the
             # number of preferred targets
             if target.name in best_targets:
-                self.default_targets[target.name] = i
+                self.default_targets.append((i, target.name))
                 i += 1
             else:
-                self.default_targets[target.name] = 100
+                self.default_targets.append((100, target.name))
 
+            self.default_targets = list(sorted(set(self.default_targets)))
             self.gen.newline()
 
     def virtual_providers(self):
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 6f767e317c..3b264d7bfd 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -807,27 +807,6 @@ target_compatible(Descendent, Ancestor)
 #defined target_satisfies/2.
 #defined target_parent/2.
 
-% 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, Package, Weight),
-     node(Package),
-     not derive_target_from_parent(_, Package),
-     not package_target_weight(Target, Package, _).
-
-% TODO: Need to account for the case of more than one parent
-% TODO: each of which sets different targets
-target_weight(Target, Dependency, Weight)
-  :- depends_on(Package, Dependency),
-     derive_target_from_parent(Package, Dependency),
-     target_weight(Target, Package, Weight).
-
-target_weight(Target, Package, Weight)
-  :- package_target_weight(Target, Package, Weight).
-
 % can't use targets on node if the compiler for the node doesn't support them
 error(2, "{0} compiler '{2}@{3}' incompatible with 'target={1}'", Package, Target, Compiler, Version)
   :- node_target(Package, Target),
@@ -844,11 +823,7 @@ node_target(Package, Target)
 node_target_weight(Package, Weight)
  :- node(Package),
     node_target(Package, Target),
-    target_weight(Target, Package, Weight).
-
-derive_target_from_parent(Parent, Package)
-  :- depends_on(Parent, Package),
-     not package_target_weight(_, Package, _).
+    target_weight(Package, Target, Weight).
 
 % compatibility rules for targets among nodes
 node_target_match(Parent, Dependency)