Skip to content

Commit

Permalink
kill Yens and A* factories
Browse files Browse the repository at this point in the history
  • Loading branch information
@lassewesth
lassewesth committed Nov 25, 2024
1 parent 43e17b5 commit 67aa074
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 243 deletions.
1 change: 1 addition & 0 deletions algo/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,5 @@ dependencies {

testImplementation project(':centrality-algorithms')
testImplementation project(':node-embedding-algorithms')
testImplementation project(':path-finding-algorithms')
}
12 changes: 0 additions & 12 deletions algo/src/main/java/org/neo4j/gds/paths/astar/AStar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,6 @@ private AStar(Dijkstra dijkstra, TerminationFlag terminationFlag) {
this.terminationFlag = terminationFlag;
}

/**
* @deprecated Use the one with termination flag
*/
@Deprecated
public static AStar sourceTarget(
Graph graph,
ShortestPathAStarBaseConfig config,
ProgressTracker progressTracker
) {
return sourceTarget(graph, config, progressTracker, TerminationFlag.RUNNING_TRUE);
}

public static AStar sourceTarget(
Graph graph,
ShortestPathAStarBaseConfig config,
Expand Down
55 changes: 0 additions & 55 deletions algo/src/main/java/org/neo4j/gds/paths/astar/AStarFactory.java
View file

This file was deleted.

21 changes: 0 additions & 21 deletions algo/src/main/java/org/neo4j/gds/paths/dijkstra/Dijkstra.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,27 +94,6 @@ public static Dijkstra sourceTarget(
);
}

/**
* @deprecated Use the other one with termination flag
*/
@Deprecated
public static Dijkstra singleSource(
Graph graph,
long originalNodeId,
boolean trackRelationships,
Optional<HeuristicFunction> heuristicFunction,
ProgressTracker progressTracker
) {
return singleSource(
graph,
originalNodeId,
trackRelationships,
heuristicFunction,
progressTracker,
TerminationFlag.RUNNING_TRUE
);
}

/**
* Configure Dijkstra to compute all single-source shortest path.
*/
Expand Down
13 changes: 4 additions & 9 deletions algo/src/main/java/org/neo4j/gds/paths/dijkstra/DijkstraFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@
*/
package org.neo4j.gds.paths.dijkstra;

import org.jetbrains.annotations.NotNull;
import org.neo4j.gds.GraphAlgorithmFactory;
import org.neo4j.gds.api.Graph;
import org.neo4j.gds.mem.MemoryEstimation;
import org.neo4j.gds.core.utils.progress.tasks.ProgressTracker;
import org.neo4j.gds.core.utils.progress.tasks.Task;
import org.neo4j.gds.core.utils.progress.tasks.Tasks;
import org.neo4j.gds.paths.dijkstra.config.DijkstraBaseConfig;
import org.neo4j.gds.termination.TerminationFlag;

import java.util.Optional;

Expand All @@ -44,13 +44,7 @@ public String taskName() {

@Override
public Task progressTask(Graph graph, CONFIG config) {
return dijkstraProgressTask(taskName(), graph);
}


@NotNull
public static Task dijkstraProgressTask(String taskName, Graph graph) {
return Tasks.leaf(taskName, graph.relationshipCount());
return Tasks.leaf(taskName(), graph.relationshipCount());
}

public static class AllShortestPathsDijkstraFactory<T extends DijkstraBaseConfig> extends DijkstraFactory<T> {
Expand All @@ -65,7 +59,8 @@ public Dijkstra build(
configuration.sourceNode(),
false,
Optional.empty(),
progressTracker
progressTracker,
TerminationFlag.RUNNING_TRUE
);
}
}
Expand Down
64 changes: 0 additions & 64 deletions algo/src/main/java/org/neo4j/gds/paths/yens/YensFactory.java
View file

This file was deleted.

68 changes: 33 additions & 35 deletions algo/src/test/java/org/neo4j/gds/paths/astar/AStarTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,24 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.neo4j.gds.TestProgressTracker;
import org.neo4j.gds.applications.algorithms.machinery.ProgressTrackerCreator;
import org.neo4j.gds.applications.algorithms.machinery.RequestScopedDependencies;
import org.neo4j.gds.applications.algorithms.pathfinding.PathFindingAlgorithms;
import org.neo4j.gds.compat.TestLog;
import org.neo4j.gds.core.concurrency.Concurrency;
import org.neo4j.gds.core.utils.progress.EmptyTaskRegistryFactory;
import org.neo4j.gds.core.utils.progress.tasks.ProgressTracker;
import org.neo4j.gds.core.utils.warnings.EmptyUserLogRegistryFactory;
import org.neo4j.gds.extension.GdlExtension;
import org.neo4j.gds.extension.GdlGraph;
import org.neo4j.gds.extension.Inject;
import org.neo4j.gds.extension.TestGraph;
import org.neo4j.gds.logging.GdsTestLog;
import org.neo4j.gds.paths.astar.config.ShortestPathAStarStreamConfigImpl;
import org.neo4j.gds.termination.TerminationFlag;

import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.neo4j.gds.assertj.Extractors.removingThreadId;
import static org.neo4j.gds.paths.PathTestUtil.expected;

@GdlExtension
Expand Down Expand Up @@ -114,7 +114,7 @@ void sourceTarget() {
.build();

var path = AStar
.sourceTarget(graph, config, ProgressTracker.NULL_TRACKER)
.sourceTarget(graph, config, ProgressTracker.NULL_TRACKER, TerminationFlag.RUNNING_TRUE)
.compute()
.findFirst()
.get();
Expand All @@ -124,38 +124,36 @@ void sourceTarget() {

@Test
void shouldLogProgress() {
var log = new GdsTestLog();
var requestScopedDependencies = RequestScopedDependencies.builder()
.with(EmptyTaskRegistryFactory.INSTANCE)
.with(TerminationFlag.RUNNING_TRUE)
.with(EmptyUserLogRegistryFactory.INSTANCE)
.build();
var progressTrackerCreator = new ProgressTrackerCreator(log, requestScopedDependencies);
var pathFindingAlgorithms = new PathFindingAlgorithms(requestScopedDependencies, progressTrackerCreator);

var config = defaultSourceTargetConfigBuilder()
.sourceNode(graph.toOriginalNodeId("nA"))
.targetNode(graph.toOriginalNodeId("nX"))
.build();

var progressTask = new AStarFactory<>().progressTask(graph, config);
var log = new GdsTestLog();
var progressTracker = new TestProgressTracker(progressTask, log, new Concurrency(1), EmptyTaskRegistryFactory.INSTANCE);

AStar.sourceTarget(graph, config, progressTracker)
.compute()
.pathSet();

List<AtomicLong> progresses = progressTracker.getProgresses();
assertEquals(1, progresses.size());
assertEquals(9, progresses.get(0).get());

assertTrue(log.containsMessage(TestLog.INFO, "AStar :: Start"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 5%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 17%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 23%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 29%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 35%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 41%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 47%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar 52%"));
assertTrue(log.containsMessage(TestLog.INFO, "AStar :: Finished"));

// no duplicate entries in progress logger
var logMessages = log.getMessages(TestLog.INFO);
assertEquals(Set.copyOf(logMessages).size(), logMessages.size());
pathFindingAlgorithms.singlePairShortestPathAStar(graph, config).pathSet();

assertThat(log.getMessages(TestLog.INFO))
.extracting(removingThreadId())
.contains(
"AStar :: Start",
"AStar 5%",
"AStar 17%",
"AStar 23%",
"AStar 29%",
"AStar 35%",
"AStar 41%",
"AStar 47%",
"AStar 52%",
"AStar 100%",
"AStar :: Finished"
);
}

// Validated against https://www.vcalc.com/wiki/vCalc/Haversine+-+Distance
Expand Down
3 changes: 2 additions & 1 deletion algo/src/test/java/org/neo4j/gds/paths/bellmanford/BellmanFordTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.neo4j.gds.logging.GdsTestLog;
import org.neo4j.gds.paths.delta.config.AllShortestPathsDeltaStreamConfigImpl;
import org.neo4j.gds.paths.dijkstra.Dijkstra;
import org.neo4j.gds.termination.TerminationFlag;

import java.util.Optional;

Expand Down Expand Up @@ -287,7 +288,7 @@ void shouldGiveSameResultsAsDijkstra() {
.shortestPaths();

var dijkstraAlgo = Dijkstra
.singleSource(newGraph, config.sourceNode(), true, Optional.empty(), ProgressTracker.NULL_TRACKER)
.singleSource(newGraph, config.sourceNode(), true, Optional.empty(), ProgressTracker.NULL_TRACKER, TerminationFlag.RUNNING_TRUE)
.compute();

double[] bellman = new double[nodeCount];
Expand Down
3 changes: 2 additions & 1 deletion algo/src/test/java/org/neo4j/gds/paths/delta/DeltaSteppingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.neo4j.gds.logging.GdsTestLog;
import org.neo4j.gds.paths.delta.config.AllShortestPathsDeltaStreamConfigImpl;
import org.neo4j.gds.paths.dijkstra.Dijkstra;
import org.neo4j.gds.termination.TerminationFlag;

import java.util.List;
import java.util.Optional;
Expand Down Expand Up @@ -378,7 +379,7 @@ void shouldGiveSameResultsAsDijkstra() {
).compute();

var dijkstraAlgo = Dijkstra
.singleSource(newGraph, config.sourceNode(), true, Optional.empty(), ProgressTracker.NULL_TRACKER)
.singleSource(newGraph, config.sourceNode(), true, Optional.empty(), ProgressTracker.NULL_TRACKER, TerminationFlag.RUNNING_TRUE)
.compute();

double[] delta = new double[nodeCount];
Expand Down
9 changes: 6 additions & 3 deletions algo/src/test/java/org/neo4j/gds/paths/dijkstra/DijkstraTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ void singleSource() {
config.sourceNode(),
false,
Optional.empty(),
ProgressTracker.NULL_TRACKER
ProgressTracker.NULL_TRACKER,
TerminationFlag.RUNNING_TRUE
).compute()
.pathSet();

Expand Down Expand Up @@ -265,7 +266,8 @@ void singleSourceFromDisconnectedNode() {
config.sourceNode(),
false,
Optional.empty(),
ProgressTracker.NULL_TRACKER
ProgressTracker.NULL_TRACKER,
TerminationFlag.RUNNING_TRUE
)
.compute()
.pathSet();
Expand Down Expand Up @@ -380,7 +382,8 @@ void singleSource() {
config.sourceNode(),
false,
Optional.empty(),
ProgressTracker.NULL_TRACKER
ProgressTracker.NULL_TRACKER,
TerminationFlag.RUNNING_TRUE
)
.compute()
.pathSet();
Expand Down
Loading

0 comments on commit 67aa074

Please sign in to comment.