From 844c7992993ef62b34b751e423d60c70dd4dbe81 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 27 May 2022 22:49:41 -0700 Subject: [PATCH] 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 ++++-- .../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 7a8117976bf99..1b3eb8b1a86f1 100644 --- a/lib/spack/spack/solver/asp.py +++ b/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() diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index b9b3141499790..5ee2eeb73115c 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 525bd0701f0eb..28bb13ee22748 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/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):