-
Notifications
You must be signed in to change notification settings - Fork 1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #6160 from Skanetrafiken/fix-stop-count-limit
Fix max-stop limit in StreetNearbyStopFinder
- Loading branch information
Showing
7 changed files
with
156 additions
and
101 deletions.
There are no files selected for viewing
34 changes: 0 additions & 34 deletions
34
application/src/main/java/org/opentripplanner/astar/strategy/MaxCountSkipEdgeStrategy.java
This file was deleted.
Oops, something went wrong.
36 changes: 36 additions & 0 deletions
36
...ication/src/main/java/org/opentripplanner/astar/strategy/MaxCountTerminationStrategy.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package org.opentripplanner.astar.strategy; | ||
|
|
||
| import java.util.function.Predicate; | ||
| import org.opentripplanner.astar.spi.AStarState; | ||
| import org.opentripplanner.astar.spi.SearchTerminationStrategy; | ||
|
|
||
| /** | ||
| * This termination strategy is used to terminate an a-star search after a number of states matching | ||
| * some criteria has been found. For example it can be used to limit a search to a maximum number of | ||
| * stops. | ||
| */ | ||
| public class MaxCountTerminationStrategy<State extends AStarState<State, ?, ?>> | ||
| implements SearchTerminationStrategy<State> { | ||
|
|
||
| private final int maxCount; | ||
| private final Predicate<State> shouldIncreaseCount; | ||
| private int count; | ||
|
|
||
| /** | ||
| * @param maxCount Terminate the search after this many matching states have been reached. | ||
| * @param shouldIncreaseCount A predicate to check if a state should increase the count or not. | ||
| */ | ||
| public MaxCountTerminationStrategy(int maxCount, Predicate<State> shouldIncreaseCount) { | ||
| this.maxCount = maxCount; | ||
| this.shouldIncreaseCount = shouldIncreaseCount; | ||
| this.count = 0; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean shouldSearchTerminate(State current) { | ||
| if (shouldIncreaseCount.test(current)) { | ||
| count++; | ||
| } | ||
| return count >= maxCount; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 0 additions & 38 deletions
38
...cation/src/test/java/org/opentripplanner/astar/strategy/MaxCountSkipEdgeStrategyTest.java
This file was deleted.
Oops, something went wrong.
29 changes: 29 additions & 0 deletions
29
...ion/src/test/java/org/opentripplanner/astar/strategy/MaxCountTerminationStrategyTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package org.opentripplanner.astar.strategy; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class MaxCountTerminationStrategyTest { | ||
|
|
||
| @Test | ||
| void countStates() { | ||
| var countAllStatesStrategy = new MaxCountTerminationStrategy<>(3, state -> true); | ||
|
|
||
| assertFalse(countAllStatesStrategy.shouldSearchTerminate(null)); | ||
| assertFalse(countAllStatesStrategy.shouldSearchTerminate(null)); | ||
| assertTrue(countAllStatesStrategy.shouldSearchTerminate(null)); | ||
| assertTrue(countAllStatesStrategy.shouldSearchTerminate(null)); | ||
| } | ||
|
|
||
| @Test | ||
| void countNoStates() { | ||
| var countNoStatesStrategy = new MaxCountTerminationStrategy<>(1, state -> false); | ||
|
|
||
| assertFalse(countNoStatesStrategy.shouldSearchTerminate(null)); | ||
| assertFalse(countNoStatesStrategy.shouldSearchTerminate(null)); | ||
| assertFalse(countNoStatesStrategy.shouldSearchTerminate(null)); | ||
| assertFalse(countNoStatesStrategy.shouldSearchTerminate(null)); | ||
| } | ||
| } |
71 changes: 71 additions & 0 deletions
71
...tripplanner/graph_builder/module/nearbystops/StreetNearbyStopFinderMultipleLinksTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package org.opentripplanner.graph_builder.module.nearbystops; | ||
|
|
||
| import static com.google.common.truth.Truth.assertThat; | ||
| import static org.opentripplanner.graph_builder.module.nearbystops.StreetNearbyStopFinderTest.assertStopAtDistance; | ||
| import static org.opentripplanner.graph_builder.module.nearbystops.StreetNearbyStopFinderTest.assertZeroDistanceStop; | ||
| import static org.opentripplanner.graph_builder.module.nearbystops.StreetNearbyStopFinderTest.sort; | ||
|
|
||
| import java.time.Duration; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.opentripplanner.framework.geometry.WgsCoordinate; | ||
| import org.opentripplanner.routing.algorithm.GraphRoutingTest; | ||
| import org.opentripplanner.routing.api.request.RouteRequest; | ||
| import org.opentripplanner.routing.api.request.request.StreetRequest; | ||
| import org.opentripplanner.street.model.vertex.TransitStopVertex; | ||
|
|
||
| class StreetNearbyStopFinderMultipleLinksTest extends GraphRoutingTest { | ||
|
|
||
| private static final WgsCoordinate origin = new WgsCoordinate(0.0, 0.0); | ||
| private TransitStopVertex stopA; | ||
| private TransitStopVertex stopB; | ||
| private TransitStopVertex stopC; | ||
|
|
||
| @BeforeEach | ||
| protected void setUp() throws Exception { | ||
| modelOf( | ||
| new Builder() { | ||
| @Override | ||
| public void build() { | ||
| var A = intersection("A", origin); | ||
| var B = intersection("B", origin.moveEastMeters(100)); | ||
| var C = intersection("C", origin.moveEastMeters(200)); | ||
|
|
||
| biStreet(A, B, 100); | ||
| biStreet(B, C, 100); | ||
|
|
||
| stopA = stop("StopA", A.toWgsCoordinate()); | ||
| stopB = stop("StopB", B.toWgsCoordinate()); | ||
| stopC = stop("StopC", C.toWgsCoordinate()); | ||
|
|
||
| biLink(A, stopA); | ||
|
|
||
| // B has many links | ||
| biLink(B, stopB); | ||
| biLink(B, stopB); | ||
| biLink(B, stopB); | ||
| biLink(B, stopB); | ||
|
|
||
| biLink(C, stopC); | ||
| } | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void testMaxStopCountRegression() { | ||
| // Max-stop-count should work correctly even though there are multiple links B <-> stopB | ||
| var durationLimit = Duration.ofMinutes(10); | ||
| var maxStopCount = 3; | ||
| var finder = new StreetNearbyStopFinder(durationLimit, maxStopCount, null); | ||
|
|
||
| var sortedNearbyStops = sort( | ||
| finder.findNearbyStops(stopA, new RouteRequest(), new StreetRequest(), false) | ||
| ); | ||
|
|
||
| assertThat(sortedNearbyStops).hasSize(3); | ||
| assertZeroDistanceStop(stopA, sortedNearbyStops.get(0)); | ||
| assertStopAtDistance(stopB, 100, sortedNearbyStops.get(1)); | ||
| assertStopAtDistance(stopC, 200, sortedNearbyStops.get(2)); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters