Merge pull request #9761 from IoannisPanagiotas/pcst-initial-wrapup

Pcst initial wrapup

algo/src/main/java/org/neo4j/gds/pricesteiner/ClusterActivity.java

@@ -61,7 +61,6 @@ long numberOfActiveClusters() {
         return relevantTime.get(clusterId);
     }
 
-
     LongPredicate active() {
         return activeClusters::get;
     }

algo/src/main/java/org/neo4j/gds/pricesteiner/ClusterEventsPriorityQueue.java

@@ -48,8 +48,8 @@ void pop(){
             queue.pop();
     }
 
-    void add(long cluster, double sumOfPrizes){
-        queue.add(cluster, sumOfPrizes);
+    void add(long cluster, double remainingMoat){
+        queue.add(cluster, remainingMoat);
     }
 
 }

algo/src/main/java/org/neo4j/gds/pricesteiner/ClusterMoatPair.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.gds.pricesteiner;
+
+ class ClusterMoatPair {
+
+    private long  cluster;
+    private double totalMoat;
+
+    void assign(long cluster, double totalMoat){
+        this.cluster = cluster;
+        this.totalMoat = totalMoat;
+    }
+
+    long cluster(){
+        return cluster;
+    }
+
+    double totalMoat(){
+        return  totalMoat;
+    }
+}

algo/src/main/java/org/neo4j/gds/pricesteiner/ClusterStructure.java

@@ -81,7 +81,7 @@ void setClusterPrize(long clusterId, double prize){
         initialMoatLeft.set(clusterId, prize);
     }
 
-    double clusterPrize(long clusterId){
+    double moatLeft(long clusterId){
         return initialMoatLeft.get(clusterId);
     }
 
@@ -90,16 +90,16 @@ void setClusterPrize(long clusterId, double prize){
             return  currentMoat + slack;
     }
 
-    ClusterMoatPair sumOnEdgePart(long node, double currentMoat){
+    void sumOnEdgePart(long node, double currentMoat, ClusterMoatPair clusterMoatPair){
         double sum = 0;
-        long currentNode =node;
+        long currentNode = node;
 
         while (true){
 
             var  parentNode = parent.get(currentNode);
             double currentValue = moatAt(currentNode,currentMoat);
 
-            sum+=  currentValue;
+            sum+= currentValue;
             if (parentNode== -1){
                 break;
             }else{
@@ -113,24 +113,21 @@ ClusterMoatPair sumOnEdgePart(long node, double currentMoat){
                 }
                 sum += skippedParentSum.get(currentNode);
                 currentNode = nextParent;
-
-
             }
 
         }
-
-        return new ClusterMoatPair(currentNode,sum);
+        clusterMoatPair.assign(currentNode,sum);
     }
 
     BitSet activeOriginalNodesOfCluster(long clusterId){
-        BitSet bitSet=new BitSet(originalNodeCount);
+        BitSet bitSet = new BitSet(originalNodeCount);
 
         if (clusterId < originalNodeCount){
             bitSet.set(clusterId);
             return bitSet;
         }
 
-        HugeLongArrayStack stack= HugeLongArrayStack.newStack(originalNodeCount);
+        HugeLongArrayStack stack = HugeLongArrayStack.newStack(originalNodeCount);
         stack.push(clusterId);
 
         while (!stack.isEmpty()){
@@ -183,7 +180,6 @@ long singleActiveCluster(){
 
 
 }
- record ClusterMoatPair(long cluster, double totalMoat){}
 
 
 

algo/src/main/java/org/neo4j/gds/pricesteiner/EdgeEventsQueue.java

@@ -27,24 +27,21 @@ class EdgeEventsQueue {
 
     private final HugeObjectArray<PairingHeap> pairingHeaps;
     private final HugeLongPriorityQueue edgeEventsPriorityQueue;
-    private final ObjectArrayList<PairingHeapElement> helpingArray;
-    private long currentlyActive;
 
     EdgeEventsQueue(long nodeCount){
 
         this.pairingHeaps= HugeObjectArray.newArray(PairingHeap.class, 2*nodeCount);
         this.edgeEventsPriorityQueue = HugeLongPriorityQueue.min(2*nodeCount);
-        this.helpingArray= new ObjectArrayList<>(4096);
+        ObjectArrayList<PairingHeapElement> helpingArray = new ObjectArrayList<>(4096);
+
         for (int i=0;i<nodeCount;i++){
             pairingHeaps.set(i, new PairingHeap(helpingArray));
         }
-        currentlyActive = nodeCount;
-    }
-    long currentlyActive(){
-        return currentlyActive;
+
     }
+
     double nextEventTime(){
-        return  edgeEventsPriorityQueue.cost(edgeEventsPriorityQueue.top());
+        return edgeEventsPriorityQueue.cost(edgeEventsPriorityQueue.top());
     }
 
     long top(){
@@ -80,6 +77,7 @@ void addWithCheck(long s,  long edgePart, double w){
                edgeEventsPriorityQueue.set(s, w);
          }
      }
+
      void addWithoutCheck(long s,  long edgePart, double w){
          var pairingHeapOfs  = pairingHeaps.get(s);
          pairingHeapOfs.add(edgePart,w);
@@ -97,13 +95,11 @@ void mergeAndUpdate(long newCluster, long cluster1,long cluster2){
         deactivateCluster(cluster2);
 
         edgeEventsPriorityQueue.add(newCluster, pairingHeaps.get(newCluster).minValue());
-        currentlyActive--;
     }
 
     void deactivateCluster(long clusterId){
         // very-very bad way of removing from common heap-of-heaps
         edgeEventsPriorityQueue.set(clusterId,Double.MAX_VALUE); //ditto
-
     }
 
     void performInitialAssignment(long nodeCount){

algo/src/main/java/org/neo4j/gds/pricesteiner/GrowthPhase.java

@@ -70,6 +70,10 @@ GrowthResult grow() {
 
         progressTracker.beginSubTask("Growing");
         double moat;
+
+        ClusterMoatPair clusterMoatPairOfu = new ClusterMoatPair();
+        ClusterMoatPair clusterMoatPairOfv = new ClusterMoatPair();
+
         while (clusterStructure.numberOfActiveClusters() > 1) {
             terminationFlag.assertRunning();
             double edgeEventTime = edgeEventsQueue.nextEventTime();
@@ -92,14 +96,14 @@ GrowthResult grow() {
                     continue;
                 }
 
-                ClusterMoatPair cmu = clusterStructure.sumOnEdgePart(u,moat);
-                ClusterMoatPair cmv = clusterStructure.sumOnEdgePart(v,moat);
+                 clusterStructure.sumOnEdgePart(u,moat,clusterMoatPairOfu);
+                 clusterStructure.sumOnEdgePart(v,moat,clusterMoatPairOfv);
 
-                var uCluster = cmu.cluster();
-                var uClusterSum = cmu.totalMoat();
+                var uCluster = clusterMoatPairOfu.cluster();
+                var uClusterSum = clusterMoatPairOfu.totalMoat();
 
-                var vCluster = cmv.cluster();
-                var vClusterSum = cmv.totalMoat();
+                var vCluster = clusterMoatPairOfv.cluster();
+                var vClusterSum = clusterMoatPairOfv.totalMoat();
 
 
                 if (vCluster == uCluster) {
@@ -160,9 +164,10 @@ private void initializeEdgeParts() {
                     edgeCosts.set(edgeId, w);
                     edgeParts.set(2 * edgeId, s);
                     edgeParts.set(2 * edgeId + 1, t);
-                    edgeEventsQueue.addBothWays(s, t, edgePart1, edgePart2, w / 2);
+                    edgeEventsQueue.addBothWays(s, t, edgePart1, edgePart2, w / 2.0);
+                    return true;
                 }
-                return  s > t;
+                return false;
             });
             progressTracker.logProgress(graph.degree(u));
         }
@@ -173,7 +178,7 @@ private void initializeClusterPrizes() {
         for (long u = 0; u < graph.nodeCount(); ++u) {
             double prize = prizes.applyAsDouble(u);
             clusterStructure.setClusterPrize(u, prize);
-            clusterEventsPriorityQueue.add(u, clusterStructure.tightnessTime(u, 0));
+            clusterEventsPriorityQueue.add(u, prize);
         }
     }
 
@@ -193,13 +198,13 @@ private void mergeClusters(
             edgeEventsQueue.increaseValuesOnInactiveCluster(cluster2, moat - clusterStructure.inactiveSince(cluster2));
         }
 
-        var newCluster = clusterStructure.merge(cluster1, cluster2,moat);
+        var newCluster = clusterStructure.merge(cluster1, cluster2, moat);
 
         edgeEventsQueue.mergeAndUpdate(newCluster, cluster1, cluster2);
         clusterEventsPriorityQueue.add(newCluster, clusterStructure.tightnessTime(newCluster, moat));
 
         addToTree(edgeId);
-        edgeParts.set(2*edgeId,-edgeParts.get(2*edgeId));
+        edgeParts.set(2*edgeId, -edgeParts.get(2*edgeId)); //signal that edge id has been used
         edgeParts.set(2*edgeId+1,-edgeParts.get(2*edgeId+1));
     }
 

algo/src/main/java/org/neo4j/gds/pricesteiner/StrongPruning.java

@@ -79,7 +79,6 @@ void performPruning(){
                 }
             }
 
-
             while (currentPos < totalPos) {
                 terminationFlag.assertRunning();;
                 var nextLeaf = queue.get(currentPos++);
@@ -145,8 +144,8 @@ PrizeSteinerTreeResult resultTree(){
     private void pruneSubtree(long node, HugeLongArray helpingArray,HugeLongArray parents){
         terminationFlag.assertRunning();
         var tree = treeStructure.tree();
-        long currentPosition= 0;
-        MutableLong position=new MutableLong();
+        long currentPosition = 0;
+        MutableLong position = new MutableLong();
         helpingArray.set(position.getAndIncrement(),node);
 
         while (currentPosition < position.get()){

algo/src/main/java/org/neo4j/gds/pricesteiner/TreeProducer.java

@@ -24,7 +24,6 @@
 import org.neo4j.gds.api.Graph;
 import org.neo4j.gds.api.IdMap;
 import org.neo4j.gds.collections.ha.HugeLongArray;
-import org.neo4j.gds.core.Aggregation;
 import org.neo4j.gds.core.loading.construction.GraphFactory;
 import org.neo4j.gds.core.loading.construction.RelationshipsBuilder;
 import org.neo4j.gds.core.utils.progress.tasks.ProgressTracker;
@@ -49,11 +48,10 @@ static TreeStructure createTree(GrowthResult growthResult,long nodeCount, IdMap
 
         RelationshipsBuilder relationshipsBuilder = GraphFactory.initRelationshipsBuilder()
             .nodes(idMap)
-            .relationshipType(RelationshipType.of("_IGNORED_"))
+            .relationshipType(RelationshipType.of("TREE"))
             .orientation(Orientation.UNDIRECTED)
             .addPropertyConfig(GraphFactory.PropertyConfig.builder()
-                .propertyKey("property")
-                .aggregation(Aggregation.SUM)
+                .propertyKey("weight")
                 .build())
             .build();
 
@@ -71,11 +69,9 @@ static TreeStructure createTree(GrowthResult growthResult,long nodeCount, IdMap
         var singleTypeRelationships= relationshipsBuilder.build();
         var tree = GraphFactory.create(idMap, singleTypeRelationships);
 
-
         progressTracker.endSubTask("Tree Creation");
         return new TreeStructure(tree,degree,  idMap.nodeCount());
 
-
     }
 
 }

algo/src/test/java/org/neo4j/gds/pricesteiner/ClusterActivityTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.gds.pricesteiner;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class ClusterActivityTest {
+
+    @Test
+    void shouldWorkAsExpected(){
+        var clusterActivity =new ClusterActivity(10);
+        assertThat(clusterActivity.relevantTime(0)).isEqualTo(0);
+        assertThat(clusterActivity.numberOfActiveClusters()).isEqualTo(10);
+
+        clusterActivity.deactivateCluster(0,5);
+        assertThat(clusterActivity.relevantTime(0)).isEqualTo(5);
+        assertThat(clusterActivity.numberOfActiveClusters()).isEqualTo(9);
+
+        clusterActivity.deactivateCluster(1,5);
+        assertThat(clusterActivity.numberOfActiveClusters()).isEqualTo(8);
+
+        clusterActivity.activateCluster(11,5);
+        assertThat(clusterActivity.relevantTime(11)).isEqualTo(5);
+        assertThat(clusterActivity.numberOfActiveClusters()).isEqualTo(9);
+
+        assertThat(clusterActivity.active(0)).isFalse();
+        assertThat(clusterActivity.active(1)).isFalse();
+        assertThat(clusterActivity.active(2)).isTrue();
+        assertThat(clusterActivity.active(11)).isTrue();
+
+        clusterActivity.deactivateCluster(11,15);
+        assertThat(clusterActivity.numberOfActiveClusters()).isEqualTo(8);
+
+        assertThat(clusterActivity.relevantTime(11)).isEqualTo(15);
+    }
+
+    @Test
+    void  shouldFindSingleActiveNodeCorrectly(){
+        var clusterActivity =new ClusterActivity(4);
+        clusterActivity.deactivateCluster(0,1);
+        clusterActivity.deactivateCluster(1,1);
+        clusterActivity.deactivateCluster(3,1);
+
+        assertThat(clusterActivity.firstActiveCluster()).isEqualTo(2);
+        clusterActivity.activateCluster(0,1); //this does not happen  in practice but oh well it is a test
+        clusterActivity.deactivateCluster(2,1);
+        assertThat(clusterActivity.firstActiveCluster()).isEqualTo(0);
+
+    }
+
+}