Drop first heuristic as being theoretically useful

Author: IoannisPanagiotas

alpha/alpha-algo/src/main/java/org/neo4j/gds/impl/spanningtree/KSpanningTree.java

@@ -20,8 +20,6 @@
 package org.neo4j.gds.impl.spanningtree;
 
 import com.carrotsearch.hppc.BitSet;
-import org.apache.commons.lang3.mutable.MutableDouble;
-import org.apache.commons.lang3.mutable.MutableLong;
 import org.jetbrains.annotations.NotNull;
 import org.neo4j.gds.Algorithm;
 import org.neo4j.gds.api.Graph;
@@ -78,7 +76,7 @@ public SpanningTree compute() {
         prim.setTerminationFlag(getTerminationFlag());
         SpanningTree spanningTree = prim.compute();
 
-        var outputTree = combineApproach(spanningTree);
+        var outputTree = growApproach(spanningTree);
         progressTracker.endSubTask();
         return outputTree;
     }
@@ -111,97 +109,15 @@ private double init(HugeLongArray parent, HugeDoubleArray costToParent, Spanning
         return spanningTree.totalWeight();
     }
 
-    private SpanningTree cutLeafApproach(SpanningTree spanningTree) {
-        //this approach cuts a leaf at each step (remaining graph is always corrected)
-        //so we can just cut the most expensive leaf at each step
-        var priorityQueue = createPriorityQueue(graph.nodeCount(), true);
-        HugeLongArray degree = HugeLongArray.newArray(graph.nodeCount());
-        long root = startNodeId;
-        double rootCost = -1.0;
-        long rootChild = -1;
-
-        HugeLongArray parent = HugeLongArray.newArray(graph.nodeCount());
-        HugeDoubleArray costToParent = HugeDoubleArray.newArray(graph.nodeCount());
-
-        double totalCost = init(parent, costToParent, spanningTree);
-        long numberOfDeletions = spanningTree.effectiveNodeCount() - k;
-
-        progressTracker.beginSubTask(numberOfDeletions);
-        //calculate degree of each node in MST
-        for (long nodeId = 0; nodeId < graph.nodeCount(); ++nodeId) {
-            var nodeParent = parent.get(nodeId);
-            if (nodeParent != -1) {
-                degree.set(nodeParent, degree.get(nodeParent) + 1);
-                degree.set(nodeId, degree.get(nodeId) + 1);
-
-                if (nodeParent == root) { //root nodes needs special care because parent is -1
-                    rootChild = nodeId;
-                    rootCost = costToParent.get(nodeId);
-                }
-            }
-        }
-        //add all leafs in priority queue
-        for (long nodeId = 0; nodeId < graph.nodeCount(); ++nodeId) {
-            if (degree.get(nodeId) == 1) {
-                double relevantCost = (nodeId == root) ?
-                    rootCost :
-                    costToParent.get(nodeId);
-                priorityQueue.add(nodeId, relevantCost);
-            }
-        }
-
-        for (long i = 0; i < numberOfDeletions; ++i) {
-            var nextNode = priorityQueue.pop();
-            long affectedNode;
-
-            if (nextNode == root) { //the affecte node is its single child
-                affectedNode = rootChild;
-                totalCost -= rootCost;
-                clearNode(rootChild, parent, costToParent);
-                clearNode(root, parent, costToParent);
-                root = affectedNode;
-            } else { //the affected node is its paret
-                affectedNode = parent.get(nextNode);
-                totalCost -= costToParent.get(nextNode);
-                clearNode(nextNode, parent, costToParent);
-            }
-
-            degree.set(affectedNode, degree.get(affectedNode) - 1);
-            double associatedCost = -1;
-            if (degree.get(affectedNode) == 1) { //it becomes a leaf
-                if (affectedNode == root) {
-                    //if it is root, we loop at its neighbors to find its single alive child
-                    MutableDouble mutRootCost = new MutableDouble();
-                    MutableLong mutRootChild = new MutableLong();
-                    graph.forEachRelationship(root, (s, t) -> {
-                        if (parent.get(t) == s) {
-                            mutRootChild.setValue(t);
-                            mutRootCost.setValue(costToParent.get(t));
-                            return false;
-                        }
-                        return true;
-                    });
-                    rootChild = mutRootChild.longValue();
-                    rootCost = mutRootCost.doubleValue();
-                    associatedCost = rootCost;
-                } else {
-                    //otherwise we just get the info from parent
-                    associatedCost = costToParent.get(affectedNode);
-                }
-                priorityQueue.add(affectedNode, associatedCost);
-            }
-            progressTracker.logProgress();
-        }
-        progressTracker.endSubTask();
-        return new SpanningTree(root, graph.nodeCount(), k, parent, costToParent, totalCost);
-    }
 
     private SpanningTree growApproach(SpanningTree spanningTree) {
 
         //this approach grows gradually the MST found in the previous step
         //when it is about to get larger than K, we crop the current worst leaf if the new value to be added
         // is actually better
-
+        if (spanningTree.effectiveNodeCount() < k)
+            return spanningTree;
+        
         HugeLongArray outDegree = HugeLongArray.newArray(graph.nodeCount());
 
         HugeLongArray parent = HugeLongArray.newArray(graph.nodeCount());
@@ -372,21 +288,6 @@ private boolean moveMakesSense(double cost1, double cost2, DoubleUnaryOperator m
         }
     }
 
-    private SpanningTree combineApproach(SpanningTree tree) {
-        if (tree.effectiveNodeCount() < k) {
-            return tree;
-        }
-        var spanningTree1 = cutLeafApproach(tree);
-        var spanningTree2 = growApproach(tree);
-        System.out.println("Prune Leaf: " + spanningTree1.totalWeight() + " Grow Mst:" + spanningTree2.totalWeight());
-        if (spanningTree1.totalWeight() > spanningTree2.totalWeight()) {
-            return (minMax == Prim.MAX_OPERATOR) ? spanningTree1 : spanningTree2;
-        } else {
-            return (minMax == Prim.MAX_OPERATOR) ? spanningTree2 : spanningTree1;
-
-        }
-
-    }
 }
 
 

alpha/alpha-algo/src/main/java/org/neo4j/gds/impl/spanningtree/KSpanningTreeAlgorithmFactory.java

@@ -53,9 +53,7 @@ public Task progressTask(
             return Tasks.task(
                 taskName(),
                 Tasks.leaf("SpanningTree", graph.relationshipCount()),
-                Tasks.leaf("Remove relationships 1"),
-                Tasks.leaf("Remove relationships 2")
-
+                Tasks.leaf("Remove relationships")
             );
         }
 

alpha/alpha-algo/src/test/java/org/neo4j/gds/impl/spanningtree/KSpanningTreeTest.java

@@ -133,13 +133,13 @@ void shouldProduceSingleConnectedTree() {
                                     ", (c:Node)" +
                                     ", (d:Node)" +
                                     ", (e:Node)" +
-                                    ", (a)-[:TYPE {cost: 1.0}]->(b)" +
-                                    ", (b)-[:TYPE {cost: 20.0}]->(c)" +
-                                    ", (c)-[:TYPE {cost: 30.0}]->(d)" +
+                                    ", (b)-[:TYPE {cost: 1.0}]->(a)" +
+                                    ", (c)-[:TYPE {cost: 20.0}]->(b)" +
+                                    ", (d)-[:TYPE {cost: 30.0}]->(c)" +
                                     ", (d)-[:TYPE {cost: 1.0}]->(e)"
         );
         var graph = factory.build().getUnion();
-        var startNode = factory.nodeId("a");
+        var startNode = factory.nodeId("d");
 
         var k = 3;
         var spanningTree = new KSpanningTree(
@@ -172,14 +172,14 @@ void shouldProduceSingleTreeWithKMinusOneEdges(int k, double expected) {
                                     ", (d:Node)" +
                                     ", (e:Node)" +
                                     ", (f:Node)" +
-                                    ", (a)-[:TYPE {cost: 1.0}]->(b)" +
-                                    ", (b)-[:TYPE {cost: 20.0}]->(c)" +
-                                    ", (c)-[:TYPE {cost: 30.0}]->(d)" +
+                                    ", (b)-[:TYPE {cost: 1.0}]->(a)" +
+                                    ", (c)-[:TYPE {cost: 20.0}]->(b)" +
+                                    ", (d)-[:TYPE {cost: 30.0}]->(c)" +
                                     ", (d)-[:TYPE {cost: 1.0}]->(e)" +
                                     ", (e)-[:TYPE {cost: 1.0}]->(f)"
         );
         var graph = factory.build().getUnion();
-        var startNode = factory.nodeId("a");
+        var startNode = factory.nodeId("d");
 
 
         var spanningTree = new KSpanningTree(
@@ -201,52 +201,6 @@ void shouldProduceSingleTreeWithKMinusOneEdges(int k, double expected) {
         assertThat(spanningTree.totalWeight()).isEqualTo(expected);
     }
 
-    @Test
-    void worstCaseForCuttingHeaviest() {
-        var factory = GdlFactory.of("CREATE" +
-                                    "  (a:Node)" +
-                                    ", (b:Node)" +
-                                    ", (c:Node)" +
-                                    ", (d:Node)" +
-                                    ", (e:Node)" +
-                                    ", (f:Node)" +
-                                    ", (g:Node)" +
-                                    ", (h:Node)" +
-                                    ", (a)-[:TYPE {cost: 9.0}]->(b)" +
-                                    ", (b)-[:TYPE {cost: 9.0}]->(c)" +
-                                    ", (c)-[:TYPE {cost: 0.0}]->(d)" +
-                                    ", (d)-[:TYPE {cost: 10.0}]->(e)" +
-                                    ", (e)-[:TYPE {cost: 0.0}]->(f)" +
-                                    ", (f)-[:TYPE {cost: 9.0}]->(g)" +
-                                    ", (g)-[:TYPE {cost: 9.0}]->(h)"
-
-        );
-        var graph = factory.build().getUnion();
-        var startNode = factory.nodeId("a");
-
-
-        var spanningTree = new KSpanningTree(
-            graph,
-            Prim.MIN_OPERATOR,
-            startNode,
-            4,
-            ProgressTracker.NULL_TRACKER
-        ).compute();
-
-        var counter = new MutableLong(0);
-        spanningTree.forEach((__, ___, ____) -> {
-            counter.add(1);
-            return true;
-        });
-
-        assertThat(counter.getValue()).isEqualTo(4 - 1);
-        assertThat(spanningTree.totalWeight()).isEqualTo(10.0);
-        //here a 'cut-heaviest'  approach would begin by cutting edge 10.
-        //this would prune the tree into two smaller subtrees of 4 nodes each
-        //adn thus return a 18 despite the optimal being 10.
-        //this is one of the many situations where cut-heavy-fails
-
-    }
 
     @Test
     void worstCaseForPruningLeaves() {
@@ -344,16 +298,12 @@ void shouldLogProgress() {
                 "KSpanningTree :: SpanningTree 80%",
                 "KSpanningTree :: SpanningTree 100%",
                 "KSpanningTree :: SpanningTree :: Finished",
-                "KSpanningTree :: Remove relationships 1 :: Start",
-                "KSpanningTree :: Remove relationships 1 50%",
-                "KSpanningTree :: Remove relationships 1 100%",
-                "KSpanningTree :: Remove relationships 1 :: Finished",
-                "KSpanningTree :: Remove relationships 2 :: Start",
-                "KSpanningTree :: Remove relationships 2 20%",
-                "KSpanningTree :: Remove relationships 2 40%",
-                "KSpanningTree :: Remove relationships 2 60%",
-                "KSpanningTree :: Remove relationships 2 100%",
-                "KSpanningTree :: Remove relationships 2 :: Finished",
+                "KSpanningTree :: Remove relationships :: Start",
+                "KSpanningTree :: Remove relationships 20%",
+                "KSpanningTree :: Remove relationships 40%",
+                "KSpanningTree :: Remove relationships 60%",
+                "KSpanningTree :: Remove relationships 100%",
+                "KSpanningTree :: Remove relationships :: Finished",
                 "KSpanningTree :: Finished"
             );
     }