concretizer: handle compiler preferences with optimization

- [x] Add support for packages.yaml and command-line compiler preferences.
- [x] Rework compiler version propagation to use optimization rather than
  hard logic constraints

4 files changed, 126 insertions, 73 deletions

diff --git a/lib/spack/spack/cmd/solve.py b/lib/spack/spack/cmd/solve.py
index fa42e87e3b..8c733fa0d8 100644
--- a/lib/spack/spack/cmd/solve.py
+++ b/lib/spack/spack/cmd/solve.py
@@ -16,7 +16,6 @@ import spack.cmd
 import spack.cmd.common.arguments as arguments
 import spack.package
 import spack.solver.asp as asp
-from spack.util.string import plural
 
 description = "concretize a specs using an ASP solver"
 section = 'developer'
diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py
index bdedfcfb3d..149e10ea26 100644
--- a/lib/spack/spack/package_prefs.py
+++ b/lib/spack/spack/package_prefs.py
@@ -50,10 +50,11 @@ class PackagePrefs(object):
        provider_spec_list.sort(key=kf)
 
     """
-    def __init__(self, pkgname, component, vpkg=None):
+    def __init__(self, pkgname, component, vpkg=None, all=True):
         self.pkgname = pkgname
         self.component = component
         self.vpkg = vpkg
+        self.all = all
 
         self._spec_order = None
 
@@ -66,7 +67,7 @@ class PackagePrefs(object):
         """
         if self._spec_order is None:
             self._spec_order = self._specs_for_pkg(
-                self.pkgname, self.component, self.vpkg)
+                self.pkgname, self.component, self.vpkg, self.all)
         spec_order = self._spec_order
 
         # integer is the index of the first spec in order that satisfies
@@ -114,12 +115,13 @@ class PackagePrefs(object):
         return []
 
     @classmethod
-    def _specs_for_pkg(cls, pkgname, component, vpkg=None):
+    def _specs_for_pkg(cls, pkgname, component, vpkg=None, all=True):
         """Given a sort order specified by the pkgname/component/second_key,
            return a list of CompilerSpecs, VersionLists, or Specs for
            that sorting list.
         """
-        pkglist = cls.order_for_package(pkgname, component, vpkg)
+        pkglist = cls.order_for_package(
+            pkgname, component, vpkg, all)
         spec_type = _spec_type(component)
         return [spec_type(s) for s in pkglist]
 
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index df27b16cd9..3b2761039e 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -141,6 +141,7 @@ class AspGenerator(object):
         self.func = AspFunctionBuilder()
         self.possible_versions = {}
         self.possible_virtuals = None
+        self.possible_compilers = []
 
     def title(self, name, char):
         self.out.write('\n')
@@ -260,7 +261,7 @@ class AspGenerator(object):
             return [self.one_of(*predicates)]
         return []
 
-    def compiler_defaults(self):
+    def available_compilers(self):
         """Facts about available compilers."""
         compilers = spack.compilers.all_compiler_specs()
 
@@ -276,28 +277,44 @@ class AspGenerator(object):
                     for v in sorted(compiler_versions[compiler])),
                 fn.compiler(compiler))
 
+    def compilers_for_default_arch(self):
+        default_arch = spack.spec.ArchSpec(spack.architecture.sys_type())
+        return [
+            compiler.spec
+            for compiler in spack.compilers.compilers_for_arch(default_arch)
+        ]
+
+    def compiler_defaults(self):
+        """Set compiler defaults, given a list of possible compilers."""
+        self.h2("Default compiler preferences")
+
+        compiler_list = self.possible_compilers.copy()
+        compiler_list = sorted(
+            compiler_list, key=lambda x: (x.name, x.version), reverse=True)
+        ppk = spack.package_prefs.PackagePrefs("all", 'compiler', all=False)
+        matches = sorted(compiler_list, key=ppk)
+
+        for i, cspec in enumerate(matches):
+            f = fn.default_compiler_preference(cspec.name, cspec.version, i)
+            self.fact(f)
+
     def package_compiler_defaults(self, pkg):
-        """Add facts about the default compiler.
+        """Facts about packages' compiler prefs."""
 
-        TODO: integrate full set of preferences into the solve (this only
-        TODO: considers the top preference)
-        """
-        # get list of all compilers
-        compiler_list = spack.compilers.all_compiler_specs()
-        if not compiler_list:
-            raise spack.compilers.NoCompilersError()
+        packages = spack.config.get("packages")
+        pkg_prefs = packages.get(pkg)
+        if not pkg_prefs or "compiler" not in pkg_prefs:
+            return
 
-        # prefer package preferences, then latest version
-        ppk = spack.package_prefs.PackagePrefs(pkg.name, 'compiler')
+        compiler_list = self.possible_compilers.copy()
         compiler_list = sorted(
             compiler_list, key=lambda x: (x.name, x.version), reverse=True)
-        compiler_list = sorted(compiler_list, key=ppk)
+        ppk = spack.package_prefs.PackagePrefs(pkg.name, 'compiler', all=False)
+        matches = sorted(compiler_list, key=ppk)
 
-        # write out default rules for this package's compilers
-        default_compiler = compiler_list[0]
-        self.fact(fn.node_compiler_default(pkg.name, default_compiler.name))
-        self.fact(fn.node_compiler_default_version(
-            pkg.name, default_compiler.name, default_compiler.version))
+        for i, cspec in enumerate(matches):
+            self.fact(fn.node_compiler_preference(
+                pkg.name, cspec.name, cspec.version, i))
 
     def pkg_rules(self, pkg):
         pkg = packagize(pkg)
@@ -422,8 +439,8 @@ class AspGenerator(object):
             arch_os = fn.arch_os_set
             arch_target = fn.arch_target_set
             variant = fn.variant_set
-            node_compiler = fn.node_compiler_set
-            node_compiler_version = fn.node_compiler_version_set
+            node_compiler = fn.node_compiler
+            node_compiler_version = fn.node_compiler_version
 
         class Body(object):
             node = fn.node
@@ -464,10 +481,28 @@ class AspGenerator(object):
         # compiler and compiler version
         if spec.compiler:
             clauses.append(f.node_compiler(spec.name, spec.compiler.name))
+            clauses.append(
+                fn.node_compiler_hard(spec.name, spec.compiler.name))
+
             if spec.compiler.concrete:
                 clauses.append(f.node_compiler_version(
                     spec.name, spec.compiler.name, spec.compiler.version))
 
+            elif spec.compiler.versions:
+                compiler_list = spack.compilers.all_compiler_specs()
+                possible_compiler_versions = [
+                    f.node_compiler_version(
+                        spec.name, spec.compiler.name, compiler.version
+                    )
+                    for compiler in compiler_list
+                    if compiler.satisfies(spec.compiler)
+                ]
+                clauses.append(self.one_of(*possible_compiler_versions))
+                for version in possible_compiler_versions:
+                    clauses.append(
+                        fn.node_compiler_version_hard(
+                            spec.name, spec.compiler.name, version))
+
         # TODO
         # external_path
         # external_module
@@ -525,11 +560,18 @@ class AspGenerator(object):
         for name in pkg_names:
             pkg = spack.repo.path.get_pkg_class(name)
             possible.update(
-                pkg.possible_dependencies(virtuals=self.possible_virtuals)
+                pkg.possible_dependencies(
+                    virtuals=self.possible_virtuals,
+                    deptype=("build", "link", "run")
+                )
             )
 
         pkgs = set(possible) | set(pkg_names)
 
+        # get possible compilers
+        self.possible_compilers = self.compilers_for_default_arch()
+
+        # read the main ASP program from concrtize.lp
         concretize_lp = pkgutil.get_data('spack.solver', 'concretize.lp')
         self.out.write(concretize_lp.decode("utf-8"))
 
@@ -537,6 +579,7 @@ class AspGenerator(object):
         self.build_version_dict(possible, specs)
 
         self.h1('General Constraints')
+        self.available_compilers()
         self.compiler_defaults()
         self.arch_defaults()
         self.virtual_providers()
@@ -628,10 +671,13 @@ class ResultParser(object):
                     for s in args]
             functions.append((name, args))
 
-        # Functions don't seem to be in particular order in output.
-        # Sort them here so that nodes are first, and so created
-        # before directives that need them (depends_on(), etc.)
-        functions.sort(key=lambda f: f[0] != "node")
+        # Functions don't seem to be in particular order in output.  Sort
+        # them here so that directives that build objects (like node and
+        # node_compiler) are called in the right order.
+        functions.sort(key=lambda f: {
+            "node": -2,
+            "node_compiler": -1,
+        }.get(f[0], 0))
 
         self._specs = {}
         for name, args in functions:
@@ -639,6 +685,7 @@ class ResultParser(object):
             if not action:
                 print("%s(%s)" % (name, ", ".join(str(a) for a in args)))
                 continue
+
             assert action and callable(action)
             action(*args)
 
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 2b8baadc5d..0d8fe1c642 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -143,45 +143,46 @@ arch_target_set(D, A) :- node(D), depends_on(P, D), arch_target_set(P, A).
 %-----------------------------------------------------------------------------
 
 % one compiler per node
-{ node_compiler(P, C) : node_compiler(P, C) } = 1 :- node(P).
-{ node_compiler_version(P, C, V) :
-  node_compiler_version(P, C, V) } = 1            :- node(P).
-
-% compiler fields are set if set to anything
-node_compiler_set(P) :- node_compiler_set(P, _).
-node_compiler_version_set(P, C) :- node_compiler_version_set(P, C, _).
-
-% avoid warnings: these are set by generated code and it's ok if they're not
-#defined node_compiler_set/2.
-#defined node_compiler_version_set/3.
-
-% if compiler value of node is set to anything, it's the value.
-node_compiler(P, C)
-  :- node(P), compiler(C), node_compiler_set(P, C).
-node_compiler_version(P, C, V)
-  :- node(P), compiler(C), compiler_version(C, V), node_compiler(P, C),
-     node_compiler_version_set(P, C, V).
-
-% node compiler versions can only be from the available compiler versions
-:- node(P), compiler(C), node_compiler(P, C), node_compiler_version(P, C, V),
-   not compiler_version(C, V).
-
-% if no compiler is set, fall back to default.
-node_compiler(P, C)
-  :- node(P), compiler(C), not node_compiler_set(P),
-     node_compiler_default(P, C).
-node_compiler_version(P, C, V)
-  :- node(P), compiler(C), compiler_version(C, V),
-     not node_compiler_version_set(P, C, V),
-     node_compiler_default_version(P, C, V).
-
-% propagate compiler, compiler version to dependencies
-node_compiler_set(D, C)
-  :- node(D), compiler(C), depends_on(P, D), node_compiler_set(P, C).
-node_compiler_version_set(D, C, V)
-  :- node(D), compiler(C), depends_on(P, D), node_compiler(D, C),
-     node_compiler_version_set(P, C, V).
-
+1 { node_compiler(P, C) : compiler(C) } 1 :- node(P).
+1 { node_compiler_version(P, C, V) : compiler_version(C, V) } 1 :- node(P).
+1 { compiler_weight(P, N) : compiler_weight(P, N) } 1 :- node(P).
+
+% dependencies imply we should try to match hard compiler constraints
+% todo: look at what to do about intersecting constraints here. we'd
+% ideally go with the "lowest" pref in the DAG
+node_compiler_match_pref(P, C) :- node_compiler_hard(P, C).
+node_compiler_match_pref(D, C)
+    :- depends_on(P, D), node_compiler_match_pref(P, C),
+       not node_compiler_hard(D, _).
+compiler_match(P, 1) :- node_compiler(P, C), node_compiler_match_pref(P, C).
+
+node_compiler_version_match_pref(P, C, V)
+    :- node_compiler_version_hard(P, C, V).
+node_compiler_version_match_pref(D, C, V)
+    :- depends_on(P, D), node_compiler_version_match_pref(P, C, V),
+       not node_compiler_version_hard(D, C, _).
+compiler_version_match(P, 1)
+    :- node_compiler_version(P, C, V),
+       node_compiler_version_match_pref(P, C, V).
+
+#defined node_compiler_hard/2.
+#defined node_compiler_version_hard/3.
+
+% compilers weighted by preference acccording to packages.yaml
+compiler_weight(P, N)
+    :- node_compiler(P, C), node_compiler_version(P, C, V),
+       node_compiler_preference(P, C, V, N).
+compiler_weight(P, N)
+    :- node_compiler(P, C), node_compiler_version(P, C, V),
+       not node_compiler_preference(P, C, _, _),
+       default_compiler_preference(C, V, N).
+compiler_weight(P, 100)
+    :- node_compiler(P, C), node_compiler_version(P, C, V),
+       not node_compiler_preference(P, C, _, _),
+       not default_compiler_preference(C, _, _).
+
+#defined node_compiler_preference/4.
+#defined default_compiler_preference/3.
 
 %-----------------------------------------------------------------------------
 % How to optimize the spec (high to low priority)
@@ -206,11 +207,15 @@ node_compiler_version_set(D, C, V)
 root(D, 2) :- root(D), node(D).
 root(D, 1) :- not root(D), node(D).
 
-% pick most preferred virtual providers
-#minimize{ N*R@3,D : provider_weight(D, N), root(P, R) }.
-
 % prefer default variants
-#minimize { N*R@2,P,V,X : variant_not_default(P, V, X, N), root(P, R) }.
+#minimize { N*R@5,P,V,X : variant_not_default(P, V, X, N), root(P, R) }.
 
 % prefer more recent versions.
-#minimize{ N@1,P,V : version_weight(P, V, N) }.
+#minimize{ N@4,P,V : version_weight(P, V, N) }.
+
+% pick most preferred virtual providers
+#minimize{ N*R@3,D : provider_weight(D, N), root(P, R) }.
+
+% compiler preferences
+#maximize{ N@2,P : compiler_match(P, N) }.
+#minimize{ N@1,P : compiler_weight(P, N) }.