Skip to content

Commit b0a068e

Browse files
Addressing some review comments
Co-authored-by: Florentin Dörre <florentin.dorre@neotechnology.com>
1 parent b26b4a4 commit b0a068e

File tree

2 files changed

+21
-19
lines changed

2 files changed

+21
-19
lines changed

algo/src/main/java/org/neo4j/gds/steiner/SteinerBasedDeltaStepping.java

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
public final class SteinerBasedDeltaStepping extends Algorithm<DijkstraResult> {
5959

6060
public static final int NO_BIN = Integer.MAX_VALUE;
61-
6261
private static final long NO_TERMINAL = -1;
6362
public static final int BIN_SIZE_THRESHOLD = 1000;
6463
private final Graph graph;
@@ -70,7 +69,7 @@ public final class SteinerBasedDeltaStepping extends Algorithm<DijkstraResult> {
7069
private final ExecutorService executorService;
7170
private long pathIndex;
7271
private final long numOfTerminals;
73-
private final BitSet unvisitedTerminal;
72+
private final BitSet unvisitedTerminal;
7473
private final BitSet mergedWithSource;
7574
private final LongAdder metTerminals;
7675

@@ -94,16 +93,16 @@ public final class SteinerBasedDeltaStepping extends Algorithm<DijkstraResult> {
9493
graph.nodeCount(),
9594
concurrency
9695
);
97-
this.mergedWithSource =new BitSet(graph.nodeCount());
98-
this.unvisitedTerminal= new BitSet(isTerminal.size());
96+
this.mergedWithSource = new BitSet(graph.nodeCount());
97+
this.unvisitedTerminal = new BitSet(isTerminal.size());
9998
unvisitedTerminal.or(isTerminal);
100-
this.pathIndex=0;
101-
this.metTerminals=new LongAdder();
102-
this.numOfTerminals=isTerminal.cardinality();
99+
this.pathIndex = 0;
100+
this.metTerminals = new LongAdder();
101+
this.numOfTerminals = isTerminal.cardinality();
103102

104103
}
105104

106-
private void mergeNodesOnPathToSource(long nodeId,AtomicLong frontierIndex) {
105+
private void mergeNodesOnPathToSource(long nodeId, AtomicLong frontierIndex) {
107106
long currentId = nodeId;
108107
//while not meeting merged nodes, add the current path node to the merge set
109108
//if the parent i merged, then it's path to the source has already been zeroed,
@@ -123,7 +122,7 @@ private void mergeNodesOnPathToSource(long nodeId,AtomicLong frontierIndex) {
123122
}
124123
}
125124

126-
private void relaxPhase(List<SteinerBasedDeltaTask> tasks,int currentBin,AtomicLong frontierSize){
125+
private void relaxPhase(List<SteinerBasedDeltaTask> tasks, int currentBin, AtomicLong frontierSize) {
127126
// Phase 1
128127
for (var task : tasks) {
129128
task.setPhase(Phase.RELAX);
@@ -133,7 +132,7 @@ private void relaxPhase(List<SteinerBasedDeltaTask> tasks,int currentBin,AtomicL
133132
ParallelUtil.run(tasks, executorService);
134133
}
135134

136-
private void syncPhase(List<SteinerBasedDeltaTask> tasks,int currentBin, AtomicLong frontierIndex){
135+
private void syncPhase(List<SteinerBasedDeltaTask> tasks, int currentBin, AtomicLong frontierIndex) {
137136
frontierIndex.set(0);
138137
tasks.forEach(task -> task.setPhase(Phase.SYNC));
139138

@@ -149,7 +148,12 @@ private long nextTerminal(HugeLongPriorityQueue terminalQueue) {
149148
return (terminalQueue.isEmpty()) ? NO_TERMINAL : terminalQueue.top();
150149
}
151150

152-
private boolean updateSteinerTree(long terminalId,AtomicLong frontierIndex,List<PathResult> paths, ImmutablePathResult.Builder pathResultBuilder) {
151+
private boolean updateSteinerTree(
152+
long terminalId,
153+
AtomicLong frontierIndex,
154+
List<PathResult> paths,
155+
ImmutablePathResult.Builder pathResultBuilder
156+
) {
153157
//add the new path to the solution
154158

155159
paths.add(pathResult(
@@ -187,7 +191,7 @@ private boolean ensureShortest(
187191
//find closest node to be processed afterwards
188192
double currentMinDistance = tasks
189193
.stream()
190-
.mapToDouble(SteinerBasedDeltaTask::getSmallest)
194+
.mapToDouble(SteinerBasedDeltaTask::getSmallestConsideredDistance)
191195
.min()
192196
.orElseThrow();
193197
//return true if the closet terminal is at least as close as the closest next node
@@ -210,7 +214,7 @@ private long tryToUpdateSteinerTree(
210214
List<SteinerBasedDeltaTask> tasks
211215
) {
212216

213-
//Use two simple criterias to discover if there is a terminal for which with full certainty,
217+
//We Use two simple criteria to discover if there is a terminal for which with full certainty,
214218
//we have found a shortest to it:
215219

216220
// Whenever we change from one bin to another, we find the terminal of smallest distance
@@ -236,7 +240,6 @@ private long tryToUpdateSteinerTree(
236240

237241
@Override
238242
public DijkstraResult compute() {
239-
int iteration = 0;
240243
int currentBin = 0;
241244

242245
var pathResultBuilder = ImmutablePathResult.builder()
@@ -288,7 +291,6 @@ public DijkstraResult compute() {
288291
// Phase 2
289292
syncPhase(tasks, currentBin, frontierIndex);
290293
}
291-
iteration += 1;
292294
frontierSize.set(frontierIndex.longValue());
293295
frontierIndex.set(0);
294296
}

algo/src/main/java/org/neo4j/gds/steiner/SteinerBasedDeltaTask.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class SteinerBasedDeltaTask implements Runnable {
5050
private final HugeLongPriorityQueue terminalQueue;
5151
private final ReentrantLock terminalQueueLock;
5252

53-
private double smallest;
53+
private double smallestConsideredDistance;
5454

5555
SteinerBasedDeltaTask(
5656
Graph graph,
@@ -79,15 +79,15 @@ class SteinerBasedDeltaTask implements Runnable {
7979
@Override
8080
public void run() {
8181
if (phase == SteinerBasedDeltaStepping.Phase.RELAX) {
82-
smallest = Double.MAX_VALUE;
82+
smallestConsideredDistance = Double.MAX_VALUE;
8383
relaxGlobalBin();
8484
relaxLocalBin();
8585
} else if (phase == SteinerBasedDeltaStepping.Phase.SYNC) {
8686
updateFrontier();
8787
}
8888
}
8989

90-
double getSmallest() {return smallest;}
90+
double getSmallestConsideredDistance() {return smallestConsideredDistance;}
9191

9292
void setPhase(SteinerBasedDeltaStepping.Phase phase) {
9393
this.phase = phase;
@@ -138,7 +138,7 @@ private void relaxNode(long nodeId) {
138138
graph.forEachRelationship(nodeId, 1.0, (sourceNodeId, targetNodeId, weight) -> {
139139
if (!mergedToSource.get(targetNodeId)) { //ignore merged vertices
140140
tryToUpdate(sourceNodeId, targetNodeId, weight);
141-
smallest = Math.min(weight, smallest);
141+
smallestConsideredDistance = Math.min(weight, smallestConsideredDistance);
142142
}
143143
return true;
144144
});

0 commit comments

Comments
 (0)