From 844c7992993ef62b34b751e423d60c70dd4dbe81 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 27 May 2022 22:49:41 -0700 Subject: 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 | 37 ++++++++++++++++++-------- lib/spack/spack/solver/concretize.lp | 11 +++++--- lib/spack/spack/test/concretize_preferences.py | 2 +- 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): -- cgit v1.2.3-70-g09d2