Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fastisochrones): Fix the max visited nodes bug for fast-isochrones #1538

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ RELEASING:
- update spring-boot from 2.7.10 to 2.7.12 ([#1474](https://github.com/GIScience/openrouteservice/issues/1474))
- Upgrade org.geotools.gt-epsg-hsql to version 29.1. ([#1479](https://github.com/GIScience/openrouteservice/issues/1479))
- various style and low level code problems ([#1489](https://github.com/GIScience/openrouteservice/pull/1489))
- Fix the max visited nodes bug for fast-isochrones ([#1538](https://github.com/GIScience/openrouteservice/pull/1538))

## [7.1.0] - 2023-06-13
### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ private void calculateBorderNodeDistances(BorderNodeDistanceStorage borderNodeDi
DijkstraOneToManyAlgorithm algorithm = new DijkstraOneToManyAlgorithm(graph, weighting, TraversalMode.NODE_BASED);
algorithm.setEdgeFilter(edgeFilterSequence);
algorithm.prepare(new int[]{borderNode}, cellBorderNodes);
algorithm.setMaxVisitedNodes(getMaxCellNodesNumber() * 20);
SPTEntry[] targets = algorithm.calcPaths(borderNode, cellBorderNodes);
int[] ids = new int[targets.length - 1];
double[] distances = new double[targets.length - 1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public MatrixResult compute(MatrixLocations srcData, MatrixLocations dstData, in
for (int srcIndex = 0; srcIndex < srcData.size(); srcIndex++)
pathMetricsExtractor.setEmptyValues(srcIndex, dstData, times, distances, weights);
} else {
DijkstraOneToManyAlgorithm algorithm = new DijkstraOneToManyAlgorithm(graph, weighting, TraversalMode.NODE_BASED);
DijkstraOneToManyAlgorithm algorithm = new DijkstraOneToManyAlgorithm(graph, weighting, TraversalMode.NODE_BASED, true);
//TODO Refactoring : Check whether this access filter is unnecessary
algorithm.setEdgeFilter(AccessFilter.allEdges(this.encoder.getAccessEnc()));
algorithm.prepare(srcData.getNodeIds(), dstData.getNodeIds());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ public String toString() {
}

protected boolean isMaxVisitedNodesExceeded() {
if (getVisitedNodes() > maxVisitedNodes)
throw new MaxVisitedNodesExceededException();
return false;
return maxVisitedNodes < getVisitedNodes();
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
/* This file is part of Openrouteservice.
*
* Openrouteservice is free software; you can redistribute it and/or modify it under the terms of the
* GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1
* Openrouteservice is free software; you can redistribute it and/or modify it under the terms of the
* GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1
* of the License, or (at your option) any later version.

* This library 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.
* This library 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 Lesser General Public License for more details.

* You should have received a copy of the GNU Lesser General Public License along with this library;
* if not, see <https://www.gnu.org/licenses/>.
* You should have received a copy of the GNU Lesser General Public License along with this library;
* if not, see <https://www.gnu.org/licenses/>.
*/
package org.heigit.ors.routing.algorithms;

Expand All @@ -22,6 +22,7 @@
import com.graphhopper.util.EdgeExplorer;
import com.graphhopper.util.EdgeIterator;
import com.graphhopper.util.Parameters;
import org.heigit.ors.exceptions.MaxVisitedNodesExceededException;

import java.util.PriorityQueue;

Expand All @@ -35,10 +36,17 @@ public class DijkstraOneToManyAlgorithm extends AbstractOneToManyRoutingAlgorith
private IntObjectMap<SPTEntry> targets;
private int targetsCount = 0;

private boolean failOnMaxVisitedNodesExceeded = false;

public DijkstraOneToManyAlgorithm(Graph graph, Weighting weighting, TraversalMode tMode) {
this(graph, weighting, tMode, false);
}

public DijkstraOneToManyAlgorithm(Graph graph, Weighting weighting, TraversalMode tMode, boolean failOnMaxVisitedNodesExceeded) {
super(graph, weighting, tMode);
int size = Math.min(Math.max(200, graph.getNodes() / 10), 2000);
initCollections(size);
this.failOnMaxVisitedNodesExceeded = failOnMaxVisitedNodesExceeded;
}

protected void initCollections(int size) {
Expand Down Expand Up @@ -106,7 +114,12 @@ protected void runAlgo() {
EdgeExplorer explorer = outEdgeExplorer;
while (true) {
visitedNodes++;
if (isMaxVisitedNodesExceeded() || finished())
if (this.failOnMaxVisitedNodesExceeded && isMaxVisitedNodesExceeded())
// Fail only if necessary for the matrix api endpoint
throw new MaxVisitedNodesExceededException();
else if (isMaxVisitedNodesExceeded() || finished())
// Do not fail but quit the search if the max visited nodes are exceeded
// Important for the fast-isochrones cell nodes calculation
break;

int startNode = currEdge.adjNode;
Expand Down