Correctly assign community volumes for weighted graphs

Co-authored-by: Veselin Nikolov <veselin.nikolov@neotechnology.com>

Author: IoannisPanagiotas

algo/src/main/java/org/neo4j/gds/leiden/Leiden.java

@@ -277,9 +277,9 @@ private void updateModularity(
         boolean localPhaseConverged,
         int iteration
     ) {
-        //will calculate modularity only if:
-        // -    the local phase has not converged (i.e., no swaps done)
-        //- or we terminate in the first iteration (i.e., given seeding is optimal, graph is empty etc)
+        // Will calculate modularity only if:
+        // - the local phase has not converged (i.e., no swaps done)
+        // - or we terminate in the first iteration (i.e., given seeding is optimal, graph is empty, etc)
         boolean shouldCalculateModularity = !localPhaseConverged || iteration == 0;
 
         if (shouldCalculateModularity) {
@@ -329,7 +329,7 @@ private double initVolumes(
         rootGraph.forEachNode(nodeId -> {
             long communityId = initialCommunities.get(nodeId);
             progressTracker.logProgress();
-            communityVolumes.addTo(communityId, rootGraph.degree(nodeId));
+            communityVolumes.addTo(communityId, nodeVolumes.get(nodeId));
             return true;
         });
         progressTracker.endSubTask("Initialization");

algo/src/main/java/org/neo4j/gds/leiden/LocalMovePhase.java

@@ -19,7 +19,6 @@
  */
 package org.neo4j.gds.leiden;
 
-import org.apache.commons.lang3.mutable.MutableDouble;
 import org.neo4j.gds.api.Graph;
 import org.neo4j.gds.core.concurrency.RunWithConcurrency;
 import org.neo4j.gds.core.utils.mem.MemoryEstimation;
@@ -103,10 +102,9 @@ private LocalMovePhase(
     }
 
     /**
-     *
      * @return The new community count.
      */
-    public long run() {
+    public void run() {
         HugeAtomicDoubleArray atomicCommunityVolumes = HugeAtomicDoubleArray.newArray(graph.nodeCount());
         graph.forEachNode(v -> {
             atomicCommunityVolumes.set(v, communityVolumes.get(v));
@@ -147,15 +145,11 @@ public long run() {
             swaps += task.swaps;
         }
 
-        MutableDouble aliveCommunities = new MutableDouble(graph.nodeCount());
         graph.forEachNode(v -> {
             communityVolumes.set(v, atomicCommunityVolumes.get(v));
-            if (Double.compare(communityVolumes.get(v), 0.0) == 0) {
-                aliveCommunities.decrement();
-            }
             return true;
         });
-        return aliveCommunities.longValue();
+
     }
-    
+
 }

algo/src/main/java/org/neo4j/gds/leiden/LocalMoveTask.java

@@ -129,8 +129,11 @@ private void moveNodeToNewCommunity(
         long oldCommunityId = currentCommunities.get(nodeId);
         currentCommunities.set(nodeId, newCommunityId);
         communityVolumes.getAndAdd(newCommunityId, currentNodeVolume);
-        communityVolumes.getAndAdd(oldCommunityId, -currentNodeVolume);
-
+        //do a atomic update to never go into negatives etc
+        communityVolumes.update(oldCommunityId, (oldValue) -> {
+            var diff = oldValue - currentNodeVolume;
+            return Math.max(diff, 0.0);
+        });
         swaps++;
 
     }

algo/src/test/java/org/neo4j/gds/leiden/LeidenWeightedCliqueTest.java

@@ -0,0 +1,90 @@
+/*
+ * 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.leiden;
+
+import org.junit.jupiter.api.Test;
+import org.neo4j.gds.Orientation;
+import org.neo4j.gds.core.utils.progress.tasks.ProgressTracker;
+import org.neo4j.gds.extension.GdlExtension;
+import org.neo4j.gds.extension.GdlGraph;
+import org.neo4j.gds.extension.IdFunction;
+import org.neo4j.gds.extension.Inject;
+import org.neo4j.gds.extension.TestGraph;
+
+import java.util.Arrays;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.neo4j.gds.core.ProcedureConstants.TOLERANCE_DEFAULT;
+
+@GdlExtension
+class LeidenWeightedCliqueTest {
+
+    @GdlGraph(orientation = Orientation.UNDIRECTED)
+    private static final String DB_CYPHER =
+        "CREATE" +
+        "(a : Node), "+
+        "(b : Node), "+
+        "(c : Node), "+
+        "(d : Node), "+
+        "(e : Node), "+
+
+        "(a)-[:R {w: 0.1}]->(b),"+
+        "(a)-[:R {w: 0.1}]->(c),"+
+        "(a)-[:R {w: 0.1}]->(d),"+
+        "(a)-[:R {w: 0.1}]->(e),"+
+
+        "(b)-[:R {w: 0.1}]->(c),"+
+        "(b)-[:R {w: 0.1}]->(d),"+
+        "(b)-[:R {w: 0.1}]->(e),"+
+
+        "(c)-[:R {w: 0.1}]->(d),"+
+        "(c)-[:R {w: 0.1}]->(e),"+
+
+        "(d)-[:R {w: 0.1}]->(e)";
+
+    @Inject
+    private TestGraph graph;
+
+    @Inject
+    private IdFunction idFunction;
+
+    @Test
+    void weightedLeiden() {
+        var maxLevels = 10;
+
+        Leiden leiden = new Leiden(
+            graph,
+            maxLevels,
+            1.0,
+            0.01,
+            true,
+            19L,
+            null,
+            TOLERANCE_DEFAULT,
+            4,
+            ProgressTracker.NULL_TRACKER
+
+        );
+        var leidenResult = leiden.compute();
+        assertThat(Arrays.stream(leidenResult.communities().toArray()).distinct().count()).isEqualTo(1);
+
+    }
+
+}