Fixed longest path with zero weights

Co-authored-by: Ioannis Panagiotas <[email protected]>
neo4j · May 21, 2024 · f305ee3 · f305ee3
1 parent f3af416
commit f305ee3
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 3 deletions.
diff --git a/algo/src/main/java/org/neo4j/gds/dag/longestPath/DagLongestPath.java b/algo/src/main/java/org/neo4j/gds/dag/longestPath/DagLongestPath.java
@@ -31,14 +31,14 @@
 import org.neo4j.gds.core.concurrency.Concurrency;
 import org.neo4j.gds.core.concurrency.ExecutorServiceUtil;
 import org.neo4j.gds.core.concurrency.ParallelUtil;
-import org.neo4j.gds.termination.TerminationFlag;
 import org.neo4j.gds.core.utils.paged.ParalleLongPageCreator;
 import org.neo4j.gds.core.utils.partition.PartitionUtils;
 import org.neo4j.gds.core.utils.progress.tasks.ProgressTracker;
 import org.neo4j.gds.paths.ImmutablePathResult;
 import org.neo4j.gds.paths.PathResult;
 import org.neo4j.gds.paths.delta.TentativeDistances;
 import org.neo4j.gds.paths.dijkstra.PathFindingResult;
+import org.neo4j.gds.termination.TerminationFlag;
 
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
@@ -75,7 +75,7 @@ public DagLongestPath(
         this.nodeCount = graph.nodeCount();
         this.concurrency = concurrency;
         this.inDegrees = HugeAtomicLongArray.of(nodeCount, ParalleLongPageCreator.passThrough(this.concurrency));
-        this.parentsAndDistances = TentativeDistances.distanceAndPredecessors(nodeCount, concurrency, Double.MIN_VALUE, (a, b) -> Double.compare(a, b) < 0);
+        this.parentsAndDistances = TentativeDistances.distanceAndPredecessors(nodeCount, concurrency, -Double.MIN_VALUE, (a, b) -> Double.compare(a, b) < 0);
     }
 
     @Override

diff --git a/algo/src/test/java/org/neo4j/gds/dag/longestPath/WeightedDagLongestPathTest.java b/algo/src/test/java/org/neo4j/gds/dag/longestPath/WeightedDagLongestPathTest.java
@@ -202,4 +202,83 @@ void shouldWorkWithMultipleSources() {
             assertThat(counter).isEqualTo(7L);
         }
     }
+
+    @GdlExtension
+    @Nested
+    class GraphWithZeroWeights {
+        @GdlGraph
+        private static final String DB_QUERY =
+            "CREATE" +
+                "  (n0)" +
+                ", (n1)" +
+                ", (n2)" +
+                ", (n3)" +
+                ", (n0)-[:T {prop: 0.0}]->(n1)" +
+                ", (n1)-[:T {prop: 0.0}]->(n2)" +
+                ", (n2)-[:T {prop: 8.0}]->(n3)";
+
+        @Inject
+        private TestGraph graph;
+
+        @Test
+        void shouldWorkWithZeroWeights() {
+            IdFunction idFunction = graph::toMappedNodeId;
+
+            long[] a = new long[]{
+                idFunction.of("n0"),
+                idFunction.of("n1"),
+                idFunction.of("n2"),
+                idFunction.of("n3"),
+            };
+
+            var longestPath = new DagLongestPathFactory().build(
+                graph,
+                CONFIG,
+                ProgressTracker.NULL_TRACKER
+            );
+
+            PathFindingResult result = longestPath.compute();
+
+            long[][] EXPECTED_PATHS = new long[4][];
+            EXPECTED_PATHS[(int) a[0]] = new long[]{a[0]};
+            EXPECTED_PATHS[(int) a[1]] = new long[]{a[0], a[1]};
+            EXPECTED_PATHS[(int) a[2]] = new long[]{a[0], a[1], a[2]};
+            EXPECTED_PATHS[(int) a[3]] = new long[]{a[0], a[1], a[2], a[3]};
+            double[] EXPECTED_COSTS = new double[4];
+            EXPECTED_COSTS[(int) a[0]] = 0;
+            EXPECTED_COSTS[(int) a[1]] = 0;
+            EXPECTED_COSTS[(int) a[2]] = 0;
+            EXPECTED_COSTS[(int) a[3]] = 8;
+
+            long counter = 0;
+            for (var path : result.pathSet()) {
+                counter++;
+                int currentTargetNode = (int) path.targetNode();
+                assertThat(path.nodeIds()).isEqualTo(EXPECTED_PATHS[currentTargetNode]);
+                assertThat(path.totalCost()).isEqualTo(EXPECTED_COSTS[currentTargetNode]);
+            }
+            assertThat(counter).isEqualTo(4L);
+        }
+
+        @Test
+        void shouldLogProgress() {
+            var lpFactory = new DagLongestPathFactory<>();
+            var progressTask = lpFactory.progressTask(graph, CONFIG);
+            var log = Neo4jProxy.testLog();
+            var testTracker = new TestProgressTracker(
+                progressTask,
+                log,
+                CONFIG.concurrency(),
+                EmptyTaskRegistryFactory.INSTANCE
+            );
+
+            var lp = lpFactory.build(graph, CONFIG, testTracker);
+            lp.compute().pathSet();
+
+            String taskName = lpFactory.taskName();
+
+            assertTrue(log.containsMessage(TestLog.INFO, taskName + " :: Start"));
+            assertTrue(log.containsMessage(TestLog.INFO, taskName + " :: Finished"));
+        }
+    }
 }
diff --git a/...nding/src/test/java/org/neo4j/gds/paths/dag/longestPath/DagLongestPathStreamProcTest.java b/...nding/src/test/java/org/neo4j/gds/paths/dag/longestPath/DagLongestPathStreamProcTest.java
@@ -134,6 +134,5 @@ void testStreamWithWeights() {
                 }
             }
         );
-
     }
 }