Fix computation of max nodes

lib/spack/spack/solver/asp.py

@@ -79,9 +79,7 @@ def default_clingo_control():
     """Return a control object with the default settings used in Spack"""
     control = clingo.Control()
     control.configuration.configuration = "tweety"
-    control.configuration.solve.models = 0
     control.configuration.solver.heuristic = "Domain"
-    control.configuration.solve.parallel_mode = "1"
     control.configuration.solver.opt_strategy = "usc,one"
     return control
 
@@ -2652,7 +2650,9 @@ def depends_on(self, parent_node, dependency_node, type):
     def virtual_on_edge(self, parent_node, provider_node, virtual):
         provider = self.extract_pkg(provider_node)
         dependencies = self._specs[parent_node].edges_to_dependencies(name=provider)
-        assert len(dependencies) == 1
+        provider_spec = self._specs[provider_node]
+        dependencies = [x for x in dependencies if id(x.spec) == id(provider_spec)]
+        assert len(dependencies) == 1, f"{virtual}: {provider}"
         dependencies[0].update_virtuals((virtual,))
 
     def reorder_flags(self):

lib/spack/spack/solver/concretize.lp

@@ -15,8 +15,6 @@
 
 % Allow clingo to create nodes
 { attr("node", node(0..X-1, Package))         } :- max_nodes(Package, X), not virtual(Package).
-
-max_nodes(Package, 1) :- virtual(Package).
 { attr("virtual_node", node(0..X-1, Package)) } :- max_nodes(Package, X), virtual(Package).
 
 % Integrity constraints on DAG nodes
@@ -257,24 +255,24 @@ attr("node_version_satisfies", node(ID, Package), Constraint)
 
 % A "condition_set(PackageNode, _)" is the set of nodes on which PackageNode can require / impose conditions
 % Currently, for a given node, this is the link-run sub-DAG of PackageNode and its direct build dependencies
-condition_set(PackageNode, PackageNode, link_run) :- attr("node", PackageNode).
+condition_set(PackageNode, PackageNode, direct_link_run) :- attr("node", PackageNode).
 
-condition_set(PackageNode, PackageNode, link_run) :- provider(PackageNode, VirtualNode).
-condition_set(PackageNode, VirtualNode, link_run) :- provider(PackageNode, VirtualNode).
+condition_set(PackageNode, PackageNode, direct_link_run) :- provider(PackageNode, VirtualNode).
+condition_set(PackageNode, VirtualNode, direct_link_run) :- provider(PackageNode, VirtualNode).
 
+condition_set(PackageNode, DependencyNode, direct_build) :- condition_set(PackageNode, PackageNode, direct_link_run), attr("depends_on", PackageNode, DependencyNode, "build").
+condition_set(PackageNode, DependencyNode, direct_link_run) :- condition_set(PackageNode, PackageNode, direct_link_run), attr("depends_on", PackageNode, DependencyNode, Type), Type != "build".
 
-condition_set(PackageNode, DependencyNode, direct_build) :- condition_set(PackageNode, PackageNode, link_run), attr("depends_on", PackageNode, DependencyNode, "build").
-condition_set(PackageNode, DependencyNode, direct_link_run) :- condition_set(PackageNode, PackageNode, link_run), attr("depends_on", PackageNode, DependencyNode, Type), Type != "build".
-
+% TODO (multiple nodes): discuss if it is sensible to remove the following rule - it means that a request or imposition must be on a direct dependency
 % Add transitive link_run dependencies, but only if they are not clashing with some direct dependency
 % (otherwise we might create an unsolvable problem when the transitive dependency has requirements that
 % are in conflict with the direct dependency)
-condition_set(ID, node(DependencyID, Dependency), link_run)
-  :- condition_set(ID, PackageNode, link_run),
-     PackageNode != ID, Type != "build",
-     not condition_set(ID, node(_, Dependency), direct_build),
-     not condition_set(ID, node(_, Dependency), direct_link_run),
-     attr("depends_on", PackageNode, node(DependencyID, Dependency), Type).
+%condition_set(ID, node(DependencyID, Dependency), link_run)
+%  :- condition_set(ID, PackageNode, link_run),
+%     PackageNode != ID, Type != "build",
+%     not condition_set(ID, node(_, Dependency), direct_build),
+%     not condition_set(ID, node(_, Dependency), direct_link_run),
+%     attr("depends_on", PackageNode, node(DependencyID, Dependency), Type).
 
 condition_set(ID, VirtualNode, Type) :- condition_set(ID, PackageNode, Type), provider(PackageNode, VirtualNode).
 

lib/spack/spack/solver/counter.py

@@ -84,6 +84,7 @@ def __init__(self, specs, tests):
         self._link_run: PossibleDependencies = set()
         self._direct_build: PossibleDependencies = set()
         self._total_build: PossibleDependencies = set()
+        self._link_run_virtuals: Set[str] = set()
 
     def _compute_cache_values(self):
         self._link_run = set(
@@ -91,8 +92,9 @@ def _compute_cache_values(self):
                 *self.specs, virtuals=self._possible_virtuals, deptype=self.link_run_types
             )
         )
+        self._link_run_virtuals.update(self._possible_virtuals)
         for x in self._link_run:
-            current = spack.repo.path.get_pkg_class(x).dependencies_of_type("build")
+            current = spack.repo.PATH.get_pkg_class(x).dependencies_of_type("build")
             self._direct_build.update(current)
 
         self._total_build = set(
@@ -103,21 +105,21 @@ def _compute_cache_values(self):
         self._possible_dependencies = set(self._link_run) | set(self._total_build)
 
     def possible_packages_facts(self, gen, fn):
-        gen.h2("Maximum number of nodes (packages)")
-        for package_name in sorted(self.possible_dependencies()):
-            gen.fact(fn.max_nodes(package_name, 1))
-        gen.newline()
-        gen.h2("Maximum number of nodes (virtual packages)")
-        for package_name in sorted(self.possible_virtuals()):
+        build_tools = set(spack.repo.PATH.packages_with_tags("build-tools"))
+        gen.h2("Packages with at most a single node")
+        for package_name in sorted(self.possible_dependencies() - build_tools):
             gen.fact(fn.max_nodes(package_name, 1))
         gen.newline()
 
-        gen.h2("Build unification sets ")
-        for name in spack.repo.path.packages_with_tags("build-tools"):
-            if name not in self.possible_dependencies():
-                continue
-            gen.fact(fn.multiple_unification_sets(name))
-            gen.fact(fn.max_nodes(name, 2))
+        gen.h2("Packages with at multiple possible nodes (build-tools)")
+        for package_name in sorted(self.possible_dependencies() & build_tools):
+            gen.fact(fn.max_nodes(package_name, 2))
+            gen.fact(fn.multiple_unification_sets(package_name))
+        gen.newline()
+
+        gen.h2("Maximum number of nodes (virtual packages)")
+        for package_name in sorted(self.possible_virtuals()):
+            gen.fact(fn.max_nodes(package_name, 1))
         gen.newline()
 
         gen.h2("Possible package in link-run subDAG")
@@ -128,6 +130,7 @@ def possible_packages_facts(self, gen, fn):
 
 class FullDuplicatesCounter(MinimalDuplicatesCounter):
     def possible_packages_facts(self, gen, fn):
+        build_tools = set(spack.repo.PATH.packages_with_tags("build-tools"))
         counter = collections.Counter(
             list(self._link_run) + list(self._total_build) + list(self._direct_build)
         )
@@ -138,7 +141,7 @@ def possible_packages_facts(self, gen, fn):
         gen.newline()
 
         gen.h2("Build unification sets ")
-        for name in spack.repo.path.packages_with_tags("build-tools"):
+        for name in sorted(self.possible_dependencies() & build_tools):
             gen.fact(fn.multiple_unification_sets(name))
         gen.newline()
 
@@ -146,3 +149,11 @@ def possible_packages_facts(self, gen, fn):
         for name in sorted(self._link_run):
             gen.fact(fn.possible_in_link_run(name))
         gen.newline()
+
+        counter = collections.Counter(
+            list(self._link_run_virtuals) + list(self._possible_virtuals)
+        )
+        gen.h2("Maximum number of virtual nodes")
+        for pkg, count in sorted(counter.items(), key=lambda x: (x[1], x[0])):
+            gen.fact(fn.max_nodes(pkg, count))
+        gen.newline()