From 5f980acc9f6ba5724bf16f8460201c03b65056c5 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Sun, 20 Oct 2024 18:50:16 +0200 Subject: [PATCH 01/20] refactor:Rename 'transit' to 'transitData' in SearchContext (Raptor) --- .../raptor/configure/RaptorConfig.java | 4 ++-- .../rangeraptor/context/SearchContext.java | 20 +++++++++---------- .../path/configure/PathConfig.java | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java b/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java index cc488448304..e5c9d90a586 100644 --- a/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java +++ b/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java @@ -146,7 +146,7 @@ private RangeRaptorWorker createWorker( return new DefaultRangeRaptorWorker<>( workerState, routingStrategy, - ctx.transit(), + ctx.transitData(), ctx.slackProvider(), ctxLeg.accessPaths(), ctx.calculator(), @@ -159,7 +159,7 @@ private RangeRaptorWorker createWorker( private RangeRaptor createRangeRaptor(SearchContext ctx, RangeRaptorWorker worker) { return new RangeRaptor<>( worker, - ctx.transit(), + ctx.transitData(), ctx.legs().getFirst().accessPaths(), ctx.roundTracker(), ctx.calculator(), diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java index e322881d786..e197bad8461 100644 --- a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/context/SearchContext.java @@ -59,7 +59,7 @@ public class SearchContext { /** * the transit data role needed for routing */ - protected final RaptorTransitDataProvider transit; + protected final RaptorTransitDataProvider transitData; private final RaptorTransitCalculator calculator; private final RaptorTuningParameters tuningParameters; @@ -75,10 +75,10 @@ public class SearchContext { /** Lazy initialized */ private RaptorCostCalculator costCalculator = null; - public SearchContext( + SearchContext( RaptorRequest request, RaptorTuningParameters tuningParameters, - RaptorTransitDataProvider transit, + RaptorTransitDataProvider transitData, AccessPaths accessPaths, List viaConnections, EgressPaths egressPaths, @@ -86,7 +86,7 @@ public SearchContext( ) { this.request = request; this.tuningParameters = tuningParameters; - this.transit = transit; + this.transitData = transitData; this.calculator = createCalculator(request, tuningParameters); this.roundTracker = @@ -133,8 +133,8 @@ public MultiCriteriaRequest multiCriteria() { return request.multiCriteria(); } - public RaptorTransitDataProvider transit() { - return transit; + public RaptorTransitDataProvider transitData() { + return transitData; } public RaptorTransitCalculator calculator() { @@ -150,7 +150,7 @@ public SlackProvider slackProvider() { } public RaptorSlackProvider raptorSlackProvider() { - return transit.slackProvider(); + return transitData.slackProvider(); } /** @@ -167,7 +167,7 @@ public ToIntFunction boardSlackProvider() { @Nullable public RaptorCostCalculator costCalculator() { if (costCalculator == null) { - this.costCalculator = transit.multiCriteriaCostCalculator(); + this.costCalculator = transitData.multiCriteriaCostCalculator(); } return costCalculator; } @@ -187,7 +187,7 @@ public IntPredicate acceptC2AtDestination() { /** Number of stops in transit graph. */ public int nStops() { - return transit.numberOfStops(); + return transitData.numberOfStops(); } /** Calculate the maximum number of rounds to perform. */ @@ -224,7 +224,7 @@ public boolean useConstrainedTransfers() { /* private methods */ public RaptorStopNameResolver stopNameResolver() { - return transit.stopNameResolver(); + return transitData.stopNameResolver(); } public TimeBasedBoardingSupport createTimeBasedBoardingSupport() { diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java index 43d504ced7d..24c4d8a3bed 100644 --- a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/path/configure/PathConfig.java @@ -102,7 +102,7 @@ private PathMapper createPathMapper(boolean includeCost) { ctx.raptorSlackProvider(), includeCost ? ctx.costCalculator() : null, ctx.stopNameResolver(), - ctx.transit().transferConstraintsSearch(), + ctx.transitData().transferConstraintsSearch(), ctx.lifeCycle() ); } From ff070c2b48db95f5d785441f34c07973de587a06 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 21 Oct 2024 11:44:18 +0200 Subject: [PATCH 02/20] refactor: Delete unused class ViaRangeRaptorDynamicSearch --- .../service/ViaRangeRaptorDynamicSearch.java | 298 ------------------ 1 file changed, 298 deletions(-) delete mode 100644 application/src/main/java/org/opentripplanner/raptor/service/ViaRangeRaptorDynamicSearch.java diff --git a/application/src/main/java/org/opentripplanner/raptor/service/ViaRangeRaptorDynamicSearch.java b/application/src/main/java/org/opentripplanner/raptor/service/ViaRangeRaptorDynamicSearch.java deleted file mode 100644 index 4476e40464f..00000000000 --- a/application/src/main/java/org/opentripplanner/raptor/service/ViaRangeRaptorDynamicSearch.java +++ /dev/null @@ -1,298 +0,0 @@ -package org.opentripplanner.raptor.service; - -import static org.opentripplanner.raptor.api.model.SearchDirection.FORWARD; -import static org.opentripplanner.raptor.api.model.SearchDirection.REVERSE; -import static org.opentripplanner.raptor.api.request.RaptorProfile.MULTI_CRITERIA; -import static org.opentripplanner.raptor.service.HeuristicToRunResolver.resolveHeuristicToRunBasedOnOptimizationsAndSearchParameters; - -import java.util.Collections; -import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; -import java.util.stream.Collectors; -import javax.annotation.Nullable; -import org.opentripplanner.framework.application.OTPRequestTimeoutException; -import org.opentripplanner.raptor.RaptorService; -import org.opentripplanner.raptor.api.model.RaptorTripSchedule; -import org.opentripplanner.raptor.api.request.RaptorRequest; -import org.opentripplanner.raptor.api.request.SearchParams; -import org.opentripplanner.raptor.api.request.SearchParamsBuilder; -import org.opentripplanner.raptor.api.response.RaptorResponse; -import org.opentripplanner.raptor.configure.RaptorConfig; -import org.opentripplanner.raptor.rangeraptor.RangeRaptor; -import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; -import org.opentripplanner.raptor.rangeraptor.transit.RaptorSearchWindowCalculator; -import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * This search helps the {@link RaptorService} to configure - * heuristics and set dynamic search parameters like EDT, LAT and raptor-search-window. - *

- * If possible the forward and reverse heuristics will be run in parallel. - *

- * Depending on which optimization is enabled and which search parameters are set a forward and/or a - * reverse "single-iteration" raptor search is performed and heuristics are collected. This is used - * to configure the "main" multi-iteration RangeRaptor search. - */ -public class ViaRangeRaptorDynamicSearch { - - private static final Logger LOG = LoggerFactory.getLogger(ViaRangeRaptorDynamicSearch.class); - - private final RaptorConfig config; - private final RaptorTransitDataProvider transitData; - private final RaptorRequest originalRequest; - private final RaptorSearchWindowCalculator dynamicSearchWindowCalculator; - - private final HeuristicSearchTask fwdHeuristics; - private final HeuristicSearchTask revHeuristics; - - public ViaRangeRaptorDynamicSearch( - RaptorConfig config, - RaptorTransitDataProvider transitData, - RaptorRequest originalRequest - ) { - this.config = config; - this.transitData = transitData; - this.originalRequest = originalRequest; - this.dynamicSearchWindowCalculator = - config.searchWindowCalculator().withSearchParams(originalRequest.searchParams()); - - this.fwdHeuristics = new HeuristicSearchTask<>(FORWARD, "Forward", config, transitData); - this.revHeuristics = new HeuristicSearchTask<>(REVERSE, "Reverse", config, transitData); - } - - public RaptorResponse route() { - try { - enableHeuristicSearchBasedOnOptimizationsAndSearchParameters(); - - // Run heuristics, if no destination is reached - runHeuristics(); - - // Set search-window and other dynamic calculated parameters - var dynamicRequest = requestWithDynamicSearchParams(originalRequest); - - return createAndRunDynamicRRWorker(dynamicRequest); - } catch (DestinationNotReachedException e) { - return new RaptorResponse<>( - Collections.emptyList(), - null, - // If a trip exists(forward heuristics succeed), but is outside the calculated - // search-window, then set the search-window params as if the request was - // performed. This enables the client to page to the next window - requestWithDynamicSearchParams(originalRequest), - false - ); - } - } - - /** - * Only exposed for testing purposes - */ - @Nullable - public Heuristics getDestinationHeuristics() { - if (!originalRequest.useDestinationPruning()) { - return null; - } - LOG.debug("RangeRaptor - Destination pruning enabled."); - return revHeuristics.result(); - } - - /** - * Create and prepare heuristic search (both FORWARD and REVERSE) based on optimizations and input - * search parameters. This is done for Standard and Multi-criteria profiles only. - */ - private void enableHeuristicSearchBasedOnOptimizationsAndSearchParameters() { - // We delegate this to a static method to be able to write unit test on this logic - resolveHeuristicToRunBasedOnOptimizationsAndSearchParameters( - originalRequest, - fwdHeuristics::enable, - revHeuristics::enable - ); - } - - /** - * Run standard "singe-iteration" raptor search to calculate heuristics - this should be really - * fast to run compared with a (multi-criteria) range-raptor search. - * - * @throws DestinationNotReachedException if destination is not reached. - */ - private void runHeuristics() { - if (isItPossibleToRunHeuristicsInParallel()) { - runHeuristicsInParallel(); - } else { - runHeuristicsSequentially(); - } - fwdHeuristics.debugCompareResult(revHeuristics); - } - - private RaptorResponse createAndRunDynamicRRWorker(RaptorRequest request) { - LOG.debug("Main request: {}", request); - RangeRaptor rangeRaptorRouter; - - // Create worker - if (request.profile().is(MULTI_CRITERIA)) { - rangeRaptorRouter = - config.createRangeRaptorWithMcWorker(transitData, request, getDestinationHeuristics()); - } else { - rangeRaptorRouter = config.createRangeRaptorWithStdWorker(transitData, request); - } - - // Route - var result = rangeRaptorRouter.route(); - - // create and return response - return new RaptorResponse<>( - result.extractPaths(), - new DefaultStopArrivals(result), - request, - // This method is not run unless the heuristic reached the destination - true - ); - } - - private boolean isItPossibleToRunHeuristicsInParallel() { - SearchParams s = originalRequest.searchParams(); - return ( - config.isMultiThreaded() && - originalRequest.runInParallel() && - s.isEarliestDepartureTimeSet() && - s.isLatestArrivalTimeSet() && - fwdHeuristics.isEnabled() && - revHeuristics.isEnabled() - ); - } - - /** - * @throws DestinationNotReachedException if destination is not reached - */ - private void runHeuristicsInParallel() { - fwdHeuristics.withRequest(originalRequest); - revHeuristics.withRequest(originalRequest); - Future asyncResult = null; - try { - asyncResult = config.threadPool().submit(fwdHeuristics::run); - revHeuristics.run(); - asyncResult.get(); - LOG.debug( - "Route using RangeRaptor - " + "REVERSE and FORWARD heuristic search performed in parallel." - ); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - // propagate interruption to the running task. - asyncResult.cancel(true); - throw new OTPRequestTimeoutException(); - } catch (ExecutionException e) { - if (e.getCause() instanceof DestinationNotReachedException) { - throw new DestinationNotReachedException(); - } - LOG.error(e.getMessage() + ". Request: " + originalRequest, e); - throw new IllegalStateException( - "Failed to run FORWARD/REVERSE heuristic search in parallel. Details: " + e.getMessage() - ); - } - } - - /** - * @throws DestinationNotReachedException if destination is not reached - */ - private void runHeuristicsSequentially() { - List> tasks = listTasksInOrder(); - - if (tasks.isEmpty()) { - return; - } - - // Run the first heuristic search - Heuristics result = runHeuristicSearchTask(tasks.get(0)); - calculateDynamicSearchParametersFromHeuristics(result); - - if (tasks.size() == 1) { - return; - } - - // Run the second heuristic search - runHeuristicSearchTask(tasks.get(1)); - } - - private Heuristics runHeuristicSearchTask(HeuristicSearchTask task) { - RaptorRequest request = task.getDirection().isForward() - ? requestForForwardHeurSearchWithDynamicSearchParams() - : requestForReverseHeurSearchWithDynamicSearchParams(); - - task.withRequest(request).run(); - - return task.result(); - } - - /** - * If the earliest-departure-time(EDT) is set, the task order should be: - *

    - *
  1. {@code FORWARD}
    2. - *
  2. {@code REVERSE}
    3. - *
- * If no EDT is set, the latest-arrival-time is set, and the order should be the opposite, - * with {@code REVERSE} first - */ - private List> listTasksInOrder() { - boolean performForwardFirst = originalRequest.searchParams().isEarliestDepartureTimeSet(); - - List> list = performForwardFirst - ? List.of(fwdHeuristics, revHeuristics) - : List.of(revHeuristics, fwdHeuristics); - - return list.stream().filter(HeuristicSearchTask::isEnabled).collect(Collectors.toList()); - } - - private RaptorRequest requestForForwardHeurSearchWithDynamicSearchParams() { - if (originalRequest.searchParams().isEarliestDepartureTimeSet()) { - return originalRequest; - } - return originalRequest - .mutate() - .searchParams() - .earliestDepartureTime(transitData.getValidTransitDataStartTime()) - .build(); - } - - private RaptorRequest requestForReverseHeurSearchWithDynamicSearchParams() { - if (originalRequest.searchParams().isLatestArrivalTimeSet()) { - return originalRequest; - } - return originalRequest - .mutate() - .searchParams() - .latestArrivalTime( - transitData.getValidTransitDataEndTime() + - originalRequest.searchParams().accessEgressMaxDurationSeconds() - ) - .build(); - } - - private RaptorRequest requestWithDynamicSearchParams(RaptorRequest request) { - SearchParamsBuilder builder = request.mutate().searchParams(); - - if (!request.searchParams().isEarliestDepartureTimeSet()) { - builder.earliestDepartureTime(dynamicSearchWindowCalculator.getEarliestDepartureTime()); - } - if (!request.searchParams().isSearchWindowSet()) { - builder.searchWindowInSeconds(dynamicSearchWindowCalculator.getSearchWindowSeconds()); - } - // We do not set the latest-arrival-time, because we do not want to limit the forward - // multi-criteria search, it does not have much effect on the performance - we only risk - // losing optimal results. - return builder.build(); - } - - private void calculateDynamicSearchParametersFromHeuristics(@Nullable Heuristics heuristics) { - if (heuristics != null) { - dynamicSearchWindowCalculator - .withHeuristics( - heuristics.bestOverallJourneyTravelDuration(), - heuristics.minWaitTimeForJourneysReachingDestination() - ) - .calculate(); - } - } -} From a00003b225daf972a3875205bf2262ce59398ed6 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 21 Oct 2024 11:46:10 +0200 Subject: [PATCH 03/20] refactor: Small cleanup in Raptor --- .../raptor/configure/RaptorConfig.java | 17 ++++++++++------- .../raptor/service/HeuristicSearchTask.java | 3 ++- .../service/RangeRaptorDynamicSearch.java | 10 +++++----- .../raptor/RaptorArchitectureTest.java | 2 +- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java b/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java index e5c9d90a586..84ab67e4959 100644 --- a/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java +++ b/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java @@ -15,6 +15,7 @@ import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; import org.opentripplanner.raptor.rangeraptor.internalapi.PassThroughPointsService; import org.opentripplanner.raptor.rangeraptor.internalapi.RangeRaptorWorker; +import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouter; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouterResult; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorWorkerState; import org.opentripplanner.raptor.rangeraptor.internalapi.RoutingStrategy; @@ -27,14 +28,16 @@ /** * This class is responsible for creating a new search and holding application scoped Raptor state. *

- * This class should have APPLICATION scope. It manage a threadPool, and hold a reference to the - * application tuning parameters. + * This class should have APPLICATION scope. It keep a reference to the threadPool used by Raptor, + * and holds a reference to the application tuning parameters. * * @param The TripSchedule type defined by the user of the raptor API. */ public class RaptorConfig { + @Nullable private final ExecutorService threadPool; + private final RaptorTuningParameters tuningParameters; /** The service is not final, because it depends on the request. */ @@ -58,7 +61,7 @@ public SearchContext context(RaptorTransitDataProvider transit, RaptorRequ return SearchContext.of(request, tuningParameters, transit, acceptC2AtDestination).build(); } - public RangeRaptor createRangeRaptorWithStdWorker( + public RaptorRouter createRangeRaptorWithStdWorker( RaptorTransitDataProvider transitData, RaptorRequest request ) { @@ -70,7 +73,7 @@ public RangeRaptor createRangeRaptorWithStdWorker( ); } - public RangeRaptor createRangeRaptorWithMcWorker( + public RaptorRouter createRangeRaptorWithMcWorker( RaptorTransitDataProvider transitData, RaptorRequest request, Heuristics heuristics @@ -93,11 +96,10 @@ public RangeRaptor createRangeRaptorWithMcWorker( var c = new McRangeRaptorConfig<>(leg, passThroughPointsService).withHeuristics(heuristics); worker = createWorker(leg, c.state(), c.strategy()); } - return createRangeRaptor(context, worker); } - public RangeRaptor createRangeRaptorWithHeuristicSearch( + public RaptorRouter createRangeRaptorWithHeuristicSearch( RaptorTransitDataProvider transitData, RaptorRequest request ) { @@ -117,6 +119,7 @@ public boolean isMultiThreaded() { return threadPool != null; } + @Nullable public ExecutorService threadPool() { return threadPool; } @@ -156,7 +159,7 @@ private RangeRaptorWorker createWorker( ); } - private RangeRaptor createRangeRaptor(SearchContext ctx, RangeRaptorWorker worker) { + private RaptorRouter createRangeRaptor(SearchContext ctx, RangeRaptorWorker worker) { return new RangeRaptor<>( worker, ctx.transitData(), diff --git a/application/src/main/java/org/opentripplanner/raptor/service/HeuristicSearchTask.java b/application/src/main/java/org/opentripplanner/raptor/service/HeuristicSearchTask.java index 7c0158d2192..0de84283af8 100644 --- a/application/src/main/java/org/opentripplanner/raptor/service/HeuristicSearchTask.java +++ b/application/src/main/java/org/opentripplanner/raptor/service/HeuristicSearchTask.java @@ -10,6 +10,7 @@ import org.opentripplanner.raptor.configure.RaptorConfig; import org.opentripplanner.raptor.rangeraptor.RangeRaptor; import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; +import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouter; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouterResult; import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; import org.slf4j.Logger; @@ -33,7 +34,7 @@ public class HeuristicSearchTask { private final RaptorTransitDataProvider transitData; private boolean run = false; - private RangeRaptor search = null; + private RaptorRouter search = null; private RaptorRequest originalRequest; private RaptorRequest heuristicRequest; private RaptorRouterResult result = null; diff --git a/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java b/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java index 5353804f414..3af049a55ec 100644 --- a/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java +++ b/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java @@ -19,8 +19,8 @@ import org.opentripplanner.raptor.api.request.SearchParamsBuilder; import org.opentripplanner.raptor.api.response.RaptorResponse; import org.opentripplanner.raptor.configure.RaptorConfig; -import org.opentripplanner.raptor.rangeraptor.RangeRaptor; import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; +import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouter; import org.opentripplanner.raptor.rangeraptor.transit.RaptorSearchWindowCalculator; import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; import org.slf4j.Logger; @@ -129,18 +129,18 @@ private void runHeuristics() { private RaptorResponse createAndRunDynamicRRWorker(RaptorRequest request) { LOG.debug("Main request: {}", request); - RangeRaptor rangeRaptorRouter; + RaptorRouter raptorRouter; // Create worker if (request.profile().is(MULTI_CRITERIA)) { - rangeRaptorRouter = + raptorRouter = config.createRangeRaptorWithMcWorker(transitData, request, getDestinationHeuristics()); } else { - rangeRaptorRouter = config.createRangeRaptorWithStdWorker(transitData, request); + raptorRouter = config.createRangeRaptorWithStdWorker(transitData, request); } // Route - var result = rangeRaptorRouter.route(); + var result = raptorRouter.route(); // create and return response return new RaptorResponse<>( diff --git a/application/src/test/java/org/opentripplanner/raptor/RaptorArchitectureTest.java b/application/src/test/java/org/opentripplanner/raptor/RaptorArchitectureTest.java index 657e060c30b..13f341279f7 100644 --- a/application/src/test/java/org/opentripplanner/raptor/RaptorArchitectureTest.java +++ b/application/src/test/java/org/opentripplanner/raptor/RaptorArchitectureTest.java @@ -77,7 +77,7 @@ void enforcePackageDependenciesRaptorAPI() { @Test void enforcePackageDependenciesRaptorSPI() { - RAPTOR.subPackage("spi").dependsOn(FRAMEWORK_UTILS, API_MODEL, API_PATH).verify(); + RAPTOR_SPI.dependsOn(FRAMEWORK_UTILS, API_MODEL, API_PATH).verify(); } @Test From fc5e85b164ca0b3fa61ac7fbf042a6d9069108e2 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 21 Oct 2024 11:54:52 +0200 Subject: [PATCH 04/20] feature: Extend Raptor with the possibility to run an extra multi-criteria search. --- .../opentripplanner/raptor/RaptorService.java | 15 +++- .../raptor/configure/RaptorConfig.java | 32 +++++++-- .../raptor/rangeraptor/CompositeResult.java | 71 +++++++++++++++++++ .../ConcurrentCompositeRaptorRouter.java | 64 +++++++++++++++++ .../service/RangeRaptorDynamicSearch.java | 13 +++- .../raptor/spi/ExtraMcRouterSearch.java | 35 +++++++++ .../raptor/moduletests/I01_HeuristicTest.java | 4 +- 7 files changed, 225 insertions(+), 9 deletions(-) create mode 100644 application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java create mode 100644 application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java create mode 100644 application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java diff --git a/application/src/main/java/org/opentripplanner/raptor/RaptorService.java b/application/src/main/java/org/opentripplanner/raptor/RaptorService.java index 70156cbbfbe..599a4104414 100644 --- a/application/src/main/java/org/opentripplanner/raptor/RaptorService.java +++ b/application/src/main/java/org/opentripplanner/raptor/RaptorService.java @@ -1,6 +1,7 @@ package org.opentripplanner.raptor; import java.util.stream.Collectors; +import javax.annotation.Nullable; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.request.RaptorRequest; import org.opentripplanner.raptor.api.response.RaptorResponse; @@ -8,6 +9,7 @@ import org.opentripplanner.raptor.service.DefaultStopArrivals; import org.opentripplanner.raptor.service.HeuristicSearchTask; import org.opentripplanner.raptor.service.RangeRaptorDynamicSearch; +import org.opentripplanner.raptor.spi.ExtraMcRouterSearch; import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -23,8 +25,16 @@ public class RaptorService { private final RaptorConfig config; - public RaptorService(RaptorConfig config) { + @Nullable + private final ExtraMcRouterSearch extraMcSearch; + + public RaptorService(RaptorConfig config, @Nullable ExtraMcRouterSearch extraMcSearch) { this.config = config; + this.extraMcSearch = extraMcSearch; + } + + public RaptorService(RaptorConfig config) { + this(config, null); } public RaptorResponse route( @@ -35,7 +45,8 @@ public RaptorResponse route( RaptorResponse response; if (request.isDynamicSearch()) { - response = new RangeRaptorDynamicSearch<>(config, transitData, request).route(); + response = + new RangeRaptorDynamicSearch<>(config, transitData, extraMcSearch, request).route(); } else { response = routeUsingStdWorker(transitData, request); } diff --git a/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java b/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java index 84ab67e4959..6c86cfca4b0 100644 --- a/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java +++ b/application/src/main/java/org/opentripplanner/raptor/configure/RaptorConfig.java @@ -7,6 +7,7 @@ import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.request.RaptorRequest; import org.opentripplanner.raptor.api.request.RaptorTuningParameters; +import org.opentripplanner.raptor.rangeraptor.ConcurrentCompositeRaptorRouter; import org.opentripplanner.raptor.rangeraptor.DefaultRangeRaptorWorker; import org.opentripplanner.raptor.rangeraptor.RangeRaptor; import org.opentripplanner.raptor.rangeraptor.RangeRaptorWorkerComposite; @@ -23,6 +24,7 @@ import org.opentripplanner.raptor.rangeraptor.multicriteria.configure.McRangeRaptorConfig; import org.opentripplanner.raptor.rangeraptor.standard.configure.StdRangeRaptorConfig; import org.opentripplanner.raptor.rangeraptor.transit.RaptorSearchWindowCalculator; +import org.opentripplanner.raptor.spi.ExtraMcRouterSearch; import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; /** @@ -67,13 +69,35 @@ public RaptorRouter createRangeRaptorWithStdWorker( ) { var context = context(transitData, request); var stdConfig = new StdRangeRaptorConfig<>(context); - return createRangeRaptor( - context, - createWorker(context.legs().getFirst(), stdConfig.state(), stdConfig.strategy()) - ); + var worker = createWorker(context.legs().getFirst(), stdConfig.state(), stdConfig.strategy()); + return createRangeRaptor(context, worker); } public RaptorRouter createRangeRaptorWithMcWorker( + RaptorTransitDataProvider transitData, + RaptorRequest request, + Heuristics heuristics, + @Nullable ExtraMcRouterSearch extraMcSearch + ) { + var mainSearch = createRangeRaptorWithMcWorker(transitData, request, heuristics); + + if (extraMcSearch == null) { + return mainSearch; + } + var alternativeSearch = createRangeRaptorWithMcWorker( + extraMcSearch.createTransitDataAlternativeSearch(transitData), + request, + heuristics + ); + return new ConcurrentCompositeRaptorRouter<>( + mainSearch, + alternativeSearch, + extraMcSearch.merger(), + threadPool() + ); + } + + private RaptorRouter createRangeRaptorWithMcWorker( RaptorTransitDataProvider transitData, RaptorRequest request, Heuristics heuristics diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java new file mode 100644 index 00000000000..d92f0df79bb --- /dev/null +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java @@ -0,0 +1,71 @@ +package org.opentripplanner.raptor.rangeraptor; + +import java.util.Collection; +import java.util.function.BiFunction; +import org.opentripplanner.raptor.api.model.RaptorTripSchedule; +import org.opentripplanner.raptor.api.path.RaptorPath; +import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouterResult; +import org.opentripplanner.raptor.rangeraptor.internalapi.SingleCriteriaStopArrivals; + +/** + * Join two results together. + *

    + *
  • Everything from the first result is added
    • + *
  • The result is merged with the injected merge strategy.
    • + *
  • Some of the methods ONLY return the result of the main search!
    • + *
+ */ +class CompositeResult implements RaptorRouterResult { + + private final RaptorRouterResult mainResult; + private final Collection> result; + + CompositeResult( + RaptorRouterResult mainResult, + RaptorRouterResult alternativeResult, + BiFunction>, Collection>, Collection>> merger + ) { + this.mainResult = mainResult; + this.result = merger.apply(mainResult.extractPaths(), alternativeResult.extractPaths()); + } + + /** + * Return the merged result. + */ + @Override + public Collection> extractPaths() { + return result; + } + + /** + * Return the main result only. + */ + @Override + public SingleCriteriaStopArrivals extractBestOverallArrivals() { + return mainResult.extractBestOverallArrivals(); + } + + /** + * Return the main result only. + */ + @Override + public SingleCriteriaStopArrivals extractBestTransitArrivals() { + return mainResult.extractBestTransitArrivals(); + } + + /** + * Return the main result only. + */ + @Override + public SingleCriteriaStopArrivals extractBestNumberOfTransfers() { + return mainResult.extractBestNumberOfTransfers(); + } + + /** + * Return true if either the main or the alternative search has reached the destination. + */ + @Override + public boolean isDestinationReached() { + return !result.isEmpty(); + } +} diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java new file mode 100644 index 00000000000..dbf0c2d89c7 --- /dev/null +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java @@ -0,0 +1,64 @@ +package org.opentripplanner.raptor.rangeraptor; + +import java.util.Collection; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.function.BiFunction; +import org.opentripplanner.framework.application.OTPRequestTimeoutException; +import org.opentripplanner.raptor.api.model.RaptorTripSchedule; +import org.opentripplanner.raptor.api.path.RaptorPath; +import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouter; +import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouterResult; + +/** + * Run two Raptor routers and join the result. The two searches are run concurrent if an + * {@link ExecutorService} is provided. + * @see CompositeResult for joining results. + */ +public class ConcurrentCompositeRaptorRouter + implements RaptorRouter { + + private final RaptorRouter mainWorker; + private final RaptorRouter alternativeWorker; + private final BiFunction>, Collection>, Collection>> merger; + private final ExecutorService executorService; + + public ConcurrentCompositeRaptorRouter( + RaptorRouter mainWorker, + RaptorRouter alternativeWorker, + BiFunction>, Collection>, Collection>> merger, + ExecutorService executorService + ) { + this.mainWorker = mainWorker; + this.alternativeWorker = alternativeWorker; + this.merger = merger; + this.executorService = executorService; + } + + @Override + public RaptorRouterResult route() { + if (executorService == null) { + var mainResult = mainWorker.route(); + var alternativeResult = alternativeWorker.route(); + return new CompositeResult<>(mainResult, alternativeResult, merger); + } + + var mainResultFuture = executorService.submit(mainWorker::route); + var alternativeResultFuture = executorService.submit(alternativeWorker::route); + + try { + var mainResult = mainResultFuture.get(); + var alternativeResult = alternativeResultFuture.get(); + return new CompositeResult<>(mainResult, alternativeResult, merger); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + // propagate interruption to the running task. + + mainResultFuture.cancel(true); + alternativeResultFuture.cancel(true); + throw new OTPRequestTimeoutException(); + } catch (ExecutionException e) { + throw (e.getCause() instanceof RuntimeException re) ? re : new RuntimeException(e); + } + } +} diff --git a/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java b/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java index 3af049a55ec..cba79f3a115 100644 --- a/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java +++ b/application/src/main/java/org/opentripplanner/raptor/service/RangeRaptorDynamicSearch.java @@ -22,6 +22,7 @@ import org.opentripplanner.raptor.rangeraptor.internalapi.Heuristics; import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouter; import org.opentripplanner.raptor.rangeraptor.transit.RaptorSearchWindowCalculator; +import org.opentripplanner.raptor.spi.ExtraMcRouterSearch; import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,12 +46,16 @@ public class RangeRaptorDynamicSearch { private final RaptorRequest originalRequest; private final RaptorSearchWindowCalculator dynamicSearchWindowCalculator; + @Nullable + private final ExtraMcRouterSearch extraMcSearch; + private final HeuristicSearchTask fwdHeuristics; private final HeuristicSearchTask revHeuristics; public RangeRaptorDynamicSearch( RaptorConfig config, RaptorTransitDataProvider transitData, + @Nullable ExtraMcRouterSearch extraMcSearch, RaptorRequest originalRequest ) { this.config = config; @@ -58,6 +63,7 @@ public RangeRaptorDynamicSearch( this.originalRequest = originalRequest; this.dynamicSearchWindowCalculator = config.searchWindowCalculator().withSearchParams(originalRequest.searchParams()); + this.extraMcSearch = extraMcSearch; this.fwdHeuristics = new HeuristicSearchTask<>(FORWARD, "Forward", config, transitData); this.revHeuristics = new HeuristicSearchTask<>(REVERSE, "Reverse", config, transitData); @@ -134,7 +140,12 @@ private RaptorResponse createAndRunDynamicRRWorker(RaptorRequest request) // Create worker if (request.profile().is(MULTI_CRITERIA)) { raptorRouter = - config.createRangeRaptorWithMcWorker(transitData, request, getDestinationHeuristics()); + config.createRangeRaptorWithMcWorker( + transitData, + request, + getDestinationHeuristics(), + extraMcSearch + ); } else { raptorRouter = config.createRangeRaptorWithStdWorker(transitData, request); } diff --git a/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java b/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java new file mode 100644 index 00000000000..afebc107832 --- /dev/null +++ b/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java @@ -0,0 +1,35 @@ +package org.opentripplanner.raptor.spi; + +import java.util.Collection; +import java.util.function.BiFunction; +import org.opentripplanner.raptor.api.model.RaptorTripSchedule; +import org.opentripplanner.raptor.api.path.RaptorPath; + +/** + * This interface is used to run two mulit-criteria searches and merging the result. Raptor will + * run the heuristics as normal. Then create two multi-criteria searches, the main search and the + * alternative search. The caller must provide a {@code merger} and + * {@link RaptorTransitDataProvider}. The transit data is used for the alternative search. This + * allows the caller to filter the transit data or change the cost-calculator. + *

+ * When changing the transit data, you may also invalidate the heuristics created by Raptor. If this + * is the case, you need to turn off the {@link org.opentripplanner.raptor.api.request.Optimization#PARETO_CHECK_AGAINST_DESTINATION}. + * For the heuristics to work, you may add extra cost or filter away data. + *

+ * @param The TripSchedule type defined by the user of the raptor API. + */ +public interface ExtraMcRouterSearch { + /** + * The returned transit-data is used in the ALTERNATIVE search. The given transit data is used in + * the main search. It is the same data passed into Raptor. + */ + RaptorTransitDataProvider createTransitDataAlternativeSearch( + RaptorTransitDataProvider transitDataMainSearch + ); + + /** + * You must provide a merge strategy to merge the main result (first argument) with the + * alternative result(second argument). Make sure the end result does not have any duplicates. + */ + BiFunction>, Collection>, Collection>> merger(); +} diff --git a/application/src/test/java/org/opentripplanner/raptor/moduletests/I01_HeuristicTest.java b/application/src/test/java/org/opentripplanner/raptor/moduletests/I01_HeuristicTest.java index 7fe9cfa2c15..1e2f79cce44 100644 --- a/application/src/test/java/org/opentripplanner/raptor/moduletests/I01_HeuristicTest.java +++ b/application/src/test/java/org/opentripplanner/raptor/moduletests/I01_HeuristicTest.java @@ -90,7 +90,7 @@ public void setup() { public void regular() { var request = requestBuilder.build(); - var search = new RangeRaptorDynamicSearch<>(config, data, request); + var search = new RangeRaptorDynamicSearch<>(config, data, null, request); search.route(); @@ -105,7 +105,7 @@ public void withConstrainedTransfers() { var request = requestBuilder.build(); - var search = new RangeRaptorDynamicSearch<>(config, data, request); + var search = new RangeRaptorDynamicSearch<>(config, data, null, request); search.route(); From a20988a2c3f9648717d1bd6306af7a1366f5949b Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 21 Oct 2024 12:09:02 +0200 Subject: [PATCH 05/20] feature: Add method to calculate distance to WgsCoordinate. --- .../framework/geometry/WgsCoordinate.java | 14 ++++++++++++++ .../framework/geometry/WgsCoordinateTest.java | 10 ++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java b/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java index 6c3dbf0f9d3..136bb4264c0 100644 --- a/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java +++ b/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java @@ -163,6 +163,20 @@ public WgsCoordinate roundToApproximate100m() { return new WgsCoordinate(lat, lng); } + /** + * Compute a farly accurate distance between two coordinates. Use the fast version in + * {@link SphericalDistanceLibrary} if many computations are needed. Return the distance in + * meters between the two coordinates. + */ + public double distanceTo(WgsCoordinate other) { + return SphericalDistanceLibrary.distance( + this.latitude, + this.longitude, + other.latitude, + other.longitude + ); + } + /** * Return a new coordinate that is moved an approximate number of meters east. */ diff --git a/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java b/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java index 5cd7ac66123..4bb27fa6d36 100644 --- a/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java +++ b/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.opentripplanner.framework.geometry.WgsCoordinate.GREENWICH; import java.util.List; import org.junit.jupiter.api.Test; @@ -101,8 +102,8 @@ void add() { @Test void testGreenwich() { - assertEquals(51.48d, WgsCoordinate.GREENWICH.latitude()); - assertEquals(0d, WgsCoordinate.GREENWICH.longitude()); + assertEquals(51.48d, GREENWICH.latitude()); + assertEquals(0d, GREENWICH.longitude()); } @Test @@ -120,4 +121,9 @@ void roundingTo100m() { assertEquals(10, rounded.longitude()); assertEquals(53.557, rounded.latitude()); } + + @Test + void testDistanceTo() { + assertEquals(131_394, (int) GREENWICH.distanceTo(GREENWICH.add(-1.0, 1.0))); + } } From e416a175d897be05961697b2985359db6996fa88 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Mon, 21 Oct 2024 12:34:50 +0200 Subject: [PATCH 06/20] feature: Add sandbox feature Sorlandsbanen. --- .../sorlandsbanen/CoachCostCalculator.java | 79 ++++++++++++++ .../EnturSorlandsbanenService.java | 103 ++++++++++++++++++ .../ext/sorlandsbanen/MergePaths.java | 52 +++++++++ .../ext/sorlandsbanen/PathKey.java | 58 ++++++++++ .../configure/EnturSorlandsbanenModule.java | 17 +++ .../framework/application/OTPFeature.java | 5 + .../raptoradapter/router/TransitRouter.java | 24 +++- .../RaptorRoutingRequestTransitData.java | 17 +++ .../api/OtpServerRequestContext.java | 18 ++- .../ConstructApplicationFactory.java | 6 + .../configure/ConstructApplicationModule.java | 3 + .../server/DefaultServerRequestContext.java | 20 +++- .../opentripplanner/TestServerContext.java | 1 + .../mapping/TripRequestMapperTest.java | 1 + .../transit/speed_test/SpeedTest.java | 1 + doc/user/Configuration.md | 1 + doc/user/sandbox/Sorlandsbanen.md | 44 ++++++++ mkdocs.yml | 1 + 18 files changed, 441 insertions(+), 10 deletions(-) create mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java create mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java create mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java create mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java create mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java create mode 100644 doc/user/sandbox/Sorlandsbanen.md diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java new file mode 100644 index 00000000000..b7ce446d430 --- /dev/null +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java @@ -0,0 +1,79 @@ +package org.opentripplanner.ext.sorlandsbanen; + +import org.opentripplanner.raptor.api.model.RaptorAccessEgress; +import org.opentripplanner.raptor.api.model.RaptorTransferConstraint; +import org.opentripplanner.raptor.spi.RaptorCostCalculator; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.cost.RaptorCostConverter; +import org.opentripplanner.transit.model.basic.TransitMode; + + +/** + * This cost calculator increases the cost on mode coach by adding an extra reluctance. The + * reluctance is hardcoded in this class and cannot be configured. + */ +class CoachCostCalculator implements RaptorCostCalculator { + + private static final int EXTRA_RELUCTANCE_ON_COACH = RaptorCostConverter.toRaptorCost(0.6); + + private final RaptorCostCalculator delegate; + + CoachCostCalculator(RaptorCostCalculator delegate) { + this.delegate = delegate; + } + + @Override + public int boardingCost( + boolean firstBoarding, + int prevArrivalTime, + int boardStop, + int boardTime, + T trip, + RaptorTransferConstraint transferConstraints + ) { + return delegate.boardingCost( + firstBoarding, + prevArrivalTime, + boardStop, + boardTime, + trip, + transferConstraints + ); + } + + @Override + public int onTripRelativeRidingCost(int boardTime, T tripScheduledBoarded) { + return delegate.onTripRelativeRidingCost(boardTime, tripScheduledBoarded); + } + + @Override + public int transitArrivalCost( + int boardCost, + int alightSlack, + int transitTime, + T trip, + int toStop + ) { + int cost = delegate.transitArrivalCost(boardCost, alightSlack, transitTime, trip, toStop); + if(trip.transitReluctanceFactorIndex() == TransitMode.COACH.ordinal()) { + cost += transitTime * EXTRA_RELUCTANCE_ON_COACH; + } + return cost; + } + + @Override + public int waitCost(int waitTimeInSeconds) { + return delegate.waitCost(waitTimeInSeconds); + } + + @Override + public int calculateRemainingMinCost(int minTravelTime, int minNumTransfers, int fromStop) { + return delegate.calculateRemainingMinCost(minTravelTime, minNumTransfers, fromStop); + } + + @Override + public int costEgress(RaptorAccessEgress egress) { + return delegate.costEgress(egress); + } + +} diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java new file mode 100644 index 00000000000..a973617f257 --- /dev/null +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java @@ -0,0 +1,103 @@ +package org.opentripplanner.ext.sorlandsbanen; + +import java.util.Collection; +import java.util.function.BiFunction; +import org.opentripplanner.framework.geometry.WgsCoordinate; +import org.opentripplanner.model.GenericLocation; +import org.opentripplanner.raptor.api.path.RaptorPath; +import org.opentripplanner.raptor.spi.ExtraMcRouterSearch; +import org.opentripplanner.raptor.spi.RaptorTransitDataProvider; +import org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgresses; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.RoutingAccessEgress; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.TransitLayer; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.request.RaptorRoutingRequestTransitData; +import org.opentripplanner.routing.api.request.RouteRequest; +import org.opentripplanner.transit.model.framework.FeedScopedId; +import org.opentripplanner.transit.model.site.StopLocation; + +/** + * This is basically a big hack to produce results containing "Sørlandsbanen" in Norway. This + * railroad line is slow and goes inland fare from where people live. Despite this, people and the + * operator want to show it in the results for log travel along the southern part of Norway where + * ii is an option. Tuning the search has proven to be challenging. It is solved here by doing + * two searches. One normal search and one where the rail is given a big cost advantage over coach. + * If train results are found in the second search, then it is added to the results of the first + * search. Everything found in the first search is always returned. + */ +public class EnturSorlandsbanenService { + + private static final double SOUTH_BOARDER_LIMIT = 59.1; + private static final int MIN_DISTANCE_LIMIT = 120_000; + + + public ExtraMcRouterSearch createMcRouterFactory(RouteRequest request, AccessEgresses accessEgresses, TransitLayer transitLayer) { + WgsCoordinate from = findStopCoordinate( + request.from(), + accessEgresses.getAccesses(), + transitLayer + ); + WgsCoordinate to = findStopCoordinate(request.to(), accessEgresses.getEgresses(), transitLayer); + + if (from.latitude() > SOUTH_BOARDER_LIMIT && to.latitude() > SOUTH_BOARDER_LIMIT) { + return null; + } + + double distance = from.distanceTo(to); + if (distance < MIN_DISTANCE_LIMIT) { + return null; + } + + return new ExtraMcRouterSearch<>() { + @Override + public RaptorTransitDataProvider createTransitDataAlternativeSearch(RaptorTransitDataProvider transitDataMainSearch) { + return new RaptorRoutingRequestTransitData( + (RaptorRoutingRequestTransitData)transitDataMainSearch, + new CoachCostCalculator<>(transitDataMainSearch.multiCriteriaCostCalculator()) + ); + } + + @Override + public BiFunction>, Collection>, Collection>> merger() { + return new MergePaths<>(); + } + }; + } + + /** + * Find a coordinate matching the given location, in order: + * - First return the coordinate of the location if it exists. + * - Then loop through the access/egress stops and try to find the + * stop or station given by the location id, return the stop/station coordinate. + * - Return the fist stop in the access/egress list coordinate. + */ + @SuppressWarnings("ConstantConditions") + private static WgsCoordinate findStopCoordinate( + GenericLocation location, + Collection accessEgress, + TransitLayer transitLayer + ) { + if (location.lat != null) { + return new WgsCoordinate(location.lat, location.lng); + } + + StopLocation firstStop = null; + for (RoutingAccessEgress it : accessEgress) { + StopLocation stop = transitLayer.getStopByIndex(it.stop()); + if (stop.getId().equals(location.stopId)) { + return stop.getCoordinate(); + } + if (idIsParentStation(stop, location.stopId)) { + return stop.getParentStation().getCoordinate(); + } + if (firstStop == null) { + firstStop = stop; + } + } + return firstStop.getCoordinate(); + } + + private static boolean idIsParentStation(StopLocation stop, FeedScopedId pId) { + return stop.getParentStation() != null && stop.getParentStation().getId().equals(pId); + } +} diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java new file mode 100644 index 00000000000..b5f94019b72 --- /dev/null +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java @@ -0,0 +1,52 @@ +package org.opentripplanner.ext.sorlandsbanen; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; +import java.util.function.BiFunction; +import org.opentripplanner.raptor.api.model.RaptorTripSchedule; +import org.opentripplanner.raptor.api.path.PathLeg; +import org.opentripplanner.raptor.api.path.RaptorPath; +import org.opentripplanner.routing.algorithm.raptoradapter.transit.request.TripScheduleWithOffset; +import org.opentripplanner.transit.model.basic.TransitMode; + +/** + * Strategy for merging the main results and the extra rail results from Sorlandsbanen. + * Everything from the main result is kept, and any additional rail results from the alternative + * search are added. + */ +class MergePaths implements BiFunction>, Collection>, Collection>> { + + @Override + public Collection> apply(Collection> main, Collection> railAlternatives) { + Map> result = new HashMap<>(); + addAllToMap(result, main); + addRailToMap(result, railAlternatives); + return result.values(); + } + + private void addAllToMap(Map> map, Collection> paths) { + for (var it : paths) { + map.put(new PathKey(it), it); + } + } + + private void addRailToMap(Map> map, Collection> paths) { + for (var it : paths) { + if (hasRail(it)) { + map.put(new PathKey(it), it); + } + } + } + + private static boolean hasRail(RaptorPath path) { + return path + .legStream() + .filter(PathLeg::isTransitLeg) + .anyMatch(leg -> { + var trip = (TripScheduleWithOffset) leg.asTransitLeg().trip(); + var mode = trip.getOriginalTripPattern().getMode(); + return mode == TransitMode.RAIL; + }); + } +} diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java new file mode 100644 index 00000000000..28c1b1eac25 --- /dev/null +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java @@ -0,0 +1,58 @@ +package org.opentripplanner.ext.sorlandsbanen; + +import org.opentripplanner.raptor.api.path.PathLeg; +import org.opentripplanner.raptor.api.path.RaptorPath; + + +/** + * Uses a hash to create a key for access, egress and transit legs in a path. Transfers + * are not included. The key is used to exclude duplicates. This approach may drop valid results + * when there is a hash collision, but this whole sandbox feature is a hack - so we can tolerate + * this here. + */ +final class PathKey { + + private final int hash; + + PathKey(RaptorPath path) { + this.hash = hash(path); + } + + private static int hash(RaptorPath path) { + if (path == null) { + return 0; + } + int result = 1; + + PathLeg leg = path.accessLeg(); + + while (!leg.isEgressLeg()) { + result = 31 * result + leg.toStop(); + result = 31 * result + leg.toTime(); + + if (leg.isTransitLeg()) { + result = 31 * result + leg.asTransitLeg().trip().pattern().debugInfo().hashCode(); + } + leg = leg.nextLeg(); + } + result = 31 * result + leg.toTime(); + + return result; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o.getClass() != PathKey.class) { + return false; + } + return hash == ((PathKey) o).hash; + } + + @Override + public int hashCode() { + return hash; + } +} diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java new file mode 100644 index 00000000000..b1208f2c68b --- /dev/null +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java @@ -0,0 +1,17 @@ +package org.opentripplanner.ext.sorlandsbanen.configure; + +import dagger.Module; +import dagger.Provides; +import javax.annotation.Nullable; +import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; +import org.opentripplanner.framework.application.OTPFeature; + +@Module +public class EnturSorlandsbanenModule { + + @Provides + @Nullable + EnturSorlandsbanenService providesEnturSorlandsbanenService() { + return OTPFeature.Sorlandsbanen.isOn() ? new EnturSorlandsbanenService() : null; + } +} diff --git a/application/src/main/java/org/opentripplanner/framework/application/OTPFeature.java b/application/src/main/java/org/opentripplanner/framework/application/OTPFeature.java index 324f5397673..c4cb86ebfb9 100644 --- a/application/src/main/java/org/opentripplanner/framework/application/OTPFeature.java +++ b/application/src/main/java/org/opentripplanner/framework/application/OTPFeature.java @@ -118,6 +118,11 @@ public enum OTPFeature { SandboxAPIGeocoder(false, true, "Enable the Geocoder API."), SandboxAPIMapboxVectorTilesApi(false, true, "Enable Mapbox vector tiles API."), SandboxAPIParkAndRideApi(false, true, "Enable park-and-ride endpoint."), + Sorlandsbanen( + false, + true, + "Include train Sørlandsbanen in results when searchig in south of Norway. Only relevant in Norway." + ), TransferAnalyzer(false, true, "Analyze transfers during graph build."); private static final Object TEST_LOCK = new Object(); diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java index e72d8ee1427..d6c03c4d773 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java @@ -12,12 +12,14 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.stream.IntStream; +import javax.annotation.Nullable; import org.opentripplanner.ext.ridehailing.RideHailingAccessShifter; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.model.plan.Itinerary; import org.opentripplanner.raptor.RaptorService; import org.opentripplanner.raptor.api.path.RaptorPath; import org.opentripplanner.raptor.api.response.RaptorResponse; +import org.opentripplanner.raptor.spi.ExtraMcRouterSearch; import org.opentripplanner.routing.algorithm.mapping.RaptorPathToItineraryMapper; import org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgressPenaltyDecorator; import org.opentripplanner.routing.algorithm.raptoradapter.router.street.AccessEgressRouter; @@ -142,7 +144,10 @@ private TransitRouterResult route() { ); // Route transit - var raptorService = new RaptorService<>(serverContext.raptorConfig()); + var raptorService = new RaptorService<>( + serverContext.raptorConfig(), + createMcRouterFactory(accessEgresses, transitLayer) + ); var transitResponse = raptorService.route(raptorRequest, requestTransitDataProvider); checkIfTransitConnectionExists(transitResponse); @@ -387,4 +392,21 @@ private IntStream listStopIndexes(FeedScopedId stopLocationId) { } return stops.stream().mapToInt(StopLocation::getIndex); } + + /** + * An optional factory for creating a decorator around the multi-criteria RangeRaptor instance. + */ + @Nullable + private ExtraMcRouterSearch createMcRouterFactory( + AccessEgresses accessEgresses, + TransitLayer transitLayer + ) { + if (OTPFeature.Sorlandsbanen.isOff()) { + return null; + } + var service = serverContext.enturSorlandsbanenService(); + return service == null + ? null + : service.createMcRouterFactory(request, accessEgresses, transitLayer); + } } diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java index 0182598ca35..f1b462e031a 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/request/RaptorRoutingRequestTransitData.java @@ -128,6 +128,23 @@ public RaptorRoutingRequestTransitData( ); } + public RaptorRoutingRequestTransitData( + RaptorRoutingRequestTransitData original, + RaptorCostCalculator newCostCalculator + ) { + this.transitLayer = original.transitLayer; + this.transitSearchTimeZero = original.transitSearchTimeZero; + this.activeTripPatternsPerStop = original.activeTripPatternsPerStop; + this.patternIndex = original.patternIndex; + this.transferIndex = original.transferIndex; + this.transferService = original.transferService; + this.constrainedTransfers = original.constrainedTransfers; + this.validTransitDataStartTime = original.validTransitDataStartTime; + this.validTransitDataEndTime = original.validTransitDataEndTime; + this.generalizedCostCalculator = newCostCalculator; + this.slackProvider = original.slackProvider(); + } + @Override public Iterator getTransfersFromStop(int stopIndex) { return transferIndex.getForwardTransfers(stopIndex).iterator(); diff --git a/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java b/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java index 7ec71e589c7..765262840ee 100644 --- a/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java +++ b/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java @@ -10,6 +10,7 @@ import org.opentripplanner.ext.flex.FlexParameters; import org.opentripplanner.ext.geocoder.LuceneIndex; import org.opentripplanner.ext.ridehailing.RideHailingService; +import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.inspector.raster.TileRendererManager; @@ -101,16 +102,10 @@ public interface OtpServerRequestContext { List rideHailingServices(); - @Nullable - StopConsolidationService stopConsolidationService(); - StreetLimitationParametersService streetLimitationParametersService(); MeterRegistry meterRegistry(); - @Nullable - EmissionsService emissionsService(); - /** Inspector/debug services */ TileRendererManager tileRendererManager(); @@ -129,6 +124,8 @@ default GraphFinder graphFinder() { VectorTileConfig vectorTileConfig(); + /* Sandbox modules */ + @Nullable default DataOverlayContext dataOverlayContext(RouteRequest request) { return OTPFeature.DataOverlay.isOnElseNull(() -> @@ -139,6 +136,15 @@ default DataOverlayContext dataOverlayContext(RouteRequest request) { ); } + @Nullable + EmissionsService emissionsService(); + @Nullable LuceneIndex lucenceIndex(); + + @Nullable + StopConsolidationService stopConsolidationService(); + + @Nullable + EnturSorlandsbanenService enturSorlandsbanenService(); } diff --git a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java index b307776ef52..c76595a3eed 100644 --- a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java +++ b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java @@ -10,6 +10,8 @@ import org.opentripplanner.ext.geocoder.configure.GeocoderModule; import org.opentripplanner.ext.interactivelauncher.configuration.InteractiveLauncherModule; import org.opentripplanner.ext.ridehailing.configure.RideHailingServicesModule; +import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; +import org.opentripplanner.ext.sorlandsbanen.configure.EnturSorlandsbanenModule; import org.opentripplanner.ext.stopconsolidation.StopConsolidationRepository; import org.opentripplanner.ext.stopconsolidation.configure.StopConsolidationServiceModule; import org.opentripplanner.graph_builder.issue.api.DataImportIssueSummary; @@ -55,6 +57,7 @@ ConstructApplicationModule.class, RideHailingServicesModule.class, EmissionsServiceModule.class, + EnturSorlandsbanenModule.class, StopConsolidationServiceModule.class, InteractiveLauncherModule.class, StreetLimitationParametersServiceModule.class, @@ -90,6 +93,9 @@ public interface ConstructApplicationFactory { StreetLimitationParameters streetLimitationParameters(); + @Nullable + EnturSorlandsbanenService enturSorlandsbanenService(); + @Nullable LuceneIndex luceneIndex(); diff --git a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java index 6c830054c49..75e2d0940a3 100644 --- a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java +++ b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java @@ -10,6 +10,7 @@ import org.opentripplanner.ext.geocoder.LuceneIndex; import org.opentripplanner.ext.interactivelauncher.api.LauncherRequestDecorator; import org.opentripplanner.ext.ridehailing.RideHailingService; +import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.raptor.configure.RaptorConfig; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule; @@ -41,6 +42,7 @@ OtpServerRequestContext providesServerContext( StreetLimitationParametersService streetLimitationParametersService, @Nullable TraverseVisitor traverseVisitor, EmissionsService emissionsService, + @Nullable EnturSorlandsbanenService enturSorlandsbanenService, LauncherRequestDecorator launcherRequestDecorator, @Nullable LuceneIndex luceneIndex ) { @@ -58,6 +60,7 @@ OtpServerRequestContext providesServerContext( realtimeVehicleService, vehicleRentalService, emissionsService, + enturSorlandsbanenService, routerConfig.flexParameters(), rideHailingServices, stopConsolidationService, diff --git a/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java b/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java index 0e81193d787..2c35dfe6263 100644 --- a/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java +++ b/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java @@ -9,6 +9,7 @@ import org.opentripplanner.ext.flex.FlexParameters; import org.opentripplanner.ext.geocoder.LuceneIndex; import org.opentripplanner.ext.ridehailing.RideHailingService; +import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.inspector.raster.TileRendererManager; import org.opentripplanner.raptor.api.request.RaptorTuningParameters; @@ -48,6 +49,10 @@ public class DefaultServerRequestContext implements OtpServerRequestContext { private final RealtimeVehicleService realtimeVehicleService; private final VehicleRentalService vehicleRentalService; private final EmissionsService emissionsService; + + @Nullable + private final EnturSorlandsbanenService enturSorlandsbanenService; + private final StopConsolidationService stopConsolidationService; private final StreetLimitationParametersService streetLimitationParametersService; private final LuceneIndex luceneIndex; @@ -67,12 +72,13 @@ private DefaultServerRequestContext( WorldEnvelopeService worldEnvelopeService, RealtimeVehicleService realtimeVehicleService, VehicleRentalService vehicleRentalService, - EmissionsService emissionsService, + @Nullable EmissionsService emissionsService, + @Nullable EnturSorlandsbanenService enturSorlandsbanenService, List rideHailingServices, - StopConsolidationService stopConsolidationService, + @Nullable StopConsolidationService stopConsolidationService, StreetLimitationParametersService streetLimitationParametersService, FlexParameters flexParameters, - TraverseVisitor traverseVisitor, + @Nullable TraverseVisitor traverseVisitor, @Nullable LuceneIndex luceneIndex ) { this.graph = graph; @@ -90,6 +96,7 @@ private DefaultServerRequestContext( this.realtimeVehicleService = realtimeVehicleService; this.rideHailingServices = rideHailingServices; this.emissionsService = emissionsService; + this.enturSorlandsbanenService = enturSorlandsbanenService; this.stopConsolidationService = stopConsolidationService; this.streetLimitationParametersService = streetLimitationParametersService; this.luceneIndex = luceneIndex; @@ -110,6 +117,7 @@ public static DefaultServerRequestContext create( RealtimeVehicleService realtimeVehicleService, VehicleRentalService vehicleRentalService, @Nullable EmissionsService emissionsService, + @Nullable EnturSorlandsbanenService enturSorlandsbanenService, FlexParameters flexParameters, List rideHailingServices, @Nullable StopConsolidationService stopConsolidationService, @@ -130,6 +138,7 @@ public static DefaultServerRequestContext create( realtimeVehicleService, vehicleRentalService, emissionsService, + enturSorlandsbanenService, rideHailingServices, stopConsolidationService, streetLimitationParametersService, @@ -251,4 +260,9 @@ public LuceneIndex lucenceIndex() { public EmissionsService emissionsService() { return emissionsService; } + + @Nullable + public EnturSorlandsbanenService enturSorlandsbanenService() { + return enturSorlandsbanenService; + } } diff --git a/application/src/test/java/org/opentripplanner/TestServerContext.java b/application/src/test/java/org/opentripplanner/TestServerContext.java index 90dca6ff840..b403e9432fe 100644 --- a/application/src/test/java/org/opentripplanner/TestServerContext.java +++ b/application/src/test/java/org/opentripplanner/TestServerContext.java @@ -51,6 +51,7 @@ public static OtpServerRequestContext createServerContext( createRealtimeVehicleService(transitService), createVehicleRentalService(), createEmissionsService(), + null, routerConfig.flexParameters(), List.of(), null, diff --git a/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java b/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java index 38a411ac1cf..d71a11aad68 100644 --- a/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java +++ b/application/src/test/java/org/opentripplanner/apis/transmodel/mapping/TripRequestMapperTest.java @@ -141,6 +141,7 @@ void setup() { new DefaultRealtimeVehicleService(transitService), new DefaultVehicleRentalService(), new DefaultEmissionsService(new EmissionsDataModel()), + null, RouterConfig.DEFAULT.flexParameters(), List.of(), null, diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index 85a33281f81..087843644b1 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -117,6 +117,7 @@ public SpeedTest( TestServerContext.createRealtimeVehicleService(transitService), TestServerContext.createVehicleRentalService(), TestServerContext.createEmissionsService(), + null, config.flexConfig, List.of(), null, diff --git a/doc/user/Configuration.md b/doc/user/Configuration.md index bca974f8617..a45e4253a06 100644 --- a/doc/user/Configuration.md +++ b/doc/user/Configuration.md @@ -250,6 +250,7 @@ Here is a list of all features which can be toggled on/off and their default val | `SandboxAPIGeocoder` | Enable the Geocoder API. | | ✓️ | | `SandboxAPIMapboxVectorTilesApi` | Enable Mapbox vector tiles API. | | ✓️ | | `SandboxAPIParkAndRideApi` | Enable park-and-ride endpoint. | | ✓️ | +| `Sorlandsbanen` | Include train Sørlandsbanen in results when searchig in south of Norway. Only relevant in Norway. | | ✓️ | | `TransferAnalyzer` | Analyze transfers during graph build. | | ✓️ | diff --git a/doc/user/sandbox/Sorlandsbanen.md b/doc/user/sandbox/Sorlandsbanen.md new file mode 100644 index 00000000000..4898a29af37 --- /dev/null +++ b/doc/user/sandbox/Sorlandsbanen.md @@ -0,0 +1,44 @@ +# Sørlandsbanen - The southern railroad in Norway + +**This sandbox module is only working in Norway**, in particular only in the south of Norway. The +feature flag to turn it *on* should only be enabled if you are routing using the norwegian data set. + +The railroad in southern Norway is very slow and does not go by the cost where most people live. It +is easily beaten by coaches in the area. Despite this, we need to include it in results where it is +relevant. + +When the feature flag is enabled, two Raptor searches are performed. The first is the regular +search - unmodified, as requested by the user. The second search is modified to include train +results with Sørlandsbanen. This is achieved by setting a high COACH reluctance. We then take any +rail results(if they exist) from the second search and add it two to the results from the first +search. The new set of results will contain everything we found in the first search, plus the train +results in the second results. + +Note! This is a hack and the logic to enable this look at the origin and destination coordinates +in addition to the feature flag. + + +## Contact Info + +- Entur, Norway + +## Changelog + +- 2024-10-14: We have used this feature for som time, but now want it in the Sandbox so we do not + need to merge it everytime we create a new entur release. + + +### Configuration + +This is turned _off_ by default. To turn it on enable the `Sorlandsbanen` feature. + +```json +// otp-config.json +{ + "otpFeatures": { + "Sorlandsbanen": true + } +} +``` + + diff --git a/mkdocs.yml b/mkdocs.yml index 1364be7be1f..7d1c25bd45d 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -122,3 +122,4 @@ nav: - Ride Hailing: 'sandbox/RideHailing.md' - Emissions: 'sandbox/Emissions.md' - Stop Consolidation: 'sandbox/StopConsolidation.md' + - Sørlandsbanen: 'sandbox/Sorlandsbanen.md' From c895a9ef32c06c2ebc406eca985aad3e4c46cc65 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 24 Oct 2024 14:12:48 +0200 Subject: [PATCH 07/20] review/refactor: Reanme EnturSorlandsbanen to SorlandsbanenNorway --- ...ice.java => SorlandsbanenNorwayService.java} | 2 +- .../configure/EnturSorlandsbanenModule.java | 17 ----------------- .../configure/SorlandsbanenNorwayModule.java | 17 +++++++++++++++++ .../raptoradapter/router/TransitRouter.java | 2 +- .../standalone/api/OtpServerRequestContext.java | 4 ++-- .../configure/ConstructApplicationFactory.java | 8 ++++---- .../configure/ConstructApplicationModule.java | 6 +++--- .../server/DefaultServerRequestContext.java | 16 ++++++++-------- 8 files changed, 36 insertions(+), 36 deletions(-) rename application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/{EnturSorlandsbanenService.java => SorlandsbanenNorwayService.java} (99%) delete mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java create mode 100644 application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/SorlandsbanenNorwayModule.java diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java similarity index 99% rename from application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java rename to application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java index a973617f257..f08c0fc4f6b 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/EnturSorlandsbanenService.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java @@ -25,7 +25,7 @@ * If train results are found in the second search, then it is added to the results of the first * search. Everything found in the first search is always returned. */ -public class EnturSorlandsbanenService { +public class SorlandsbanenNorwayService { private static final double SOUTH_BOARDER_LIMIT = 59.1; private static final int MIN_DISTANCE_LIMIT = 120_000; diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java deleted file mode 100644 index b1208f2c68b..00000000000 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/EnturSorlandsbanenModule.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.opentripplanner.ext.sorlandsbanen.configure; - -import dagger.Module; -import dagger.Provides; -import javax.annotation.Nullable; -import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; -import org.opentripplanner.framework.application.OTPFeature; - -@Module -public class EnturSorlandsbanenModule { - - @Provides - @Nullable - EnturSorlandsbanenService providesEnturSorlandsbanenService() { - return OTPFeature.Sorlandsbanen.isOn() ? new EnturSorlandsbanenService() : null; - } -} diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/SorlandsbanenNorwayModule.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/SorlandsbanenNorwayModule.java new file mode 100644 index 00000000000..d0b177ac767 --- /dev/null +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/configure/SorlandsbanenNorwayModule.java @@ -0,0 +1,17 @@ +package org.opentripplanner.ext.sorlandsbanen.configure; + +import dagger.Module; +import dagger.Provides; +import javax.annotation.Nullable; +import org.opentripplanner.ext.sorlandsbanen.SorlandsbanenNorwayService; +import org.opentripplanner.framework.application.OTPFeature; + +@Module +public class SorlandsbanenNorwayModule { + + @Provides + @Nullable + SorlandsbanenNorwayService providesSorlandsbanenNorwayService() { + return OTPFeature.Sorlandsbanen.isOn() ? new SorlandsbanenNorwayService() : null; + } +} diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java index cb52d5bd6a4..f77711136e8 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java @@ -403,7 +403,7 @@ private ExtraMcRouterSearch createMcRouterFactory( if (OTPFeature.Sorlandsbanen.isOff()) { return null; } - var service = serverContext.enturSorlandsbanenService(); + var service = serverContext.sorlandsbanenService(); return service == null ? null : service.createMcRouterFactory(request, accessEgresses, transitLayer); diff --git a/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java b/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java index 765262840ee..49b43856011 100644 --- a/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java +++ b/application/src/main/java/org/opentripplanner/standalone/api/OtpServerRequestContext.java @@ -10,7 +10,7 @@ import org.opentripplanner.ext.flex.FlexParameters; import org.opentripplanner.ext.geocoder.LuceneIndex; import org.opentripplanner.ext.ridehailing.RideHailingService; -import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; +import org.opentripplanner.ext.sorlandsbanen.SorlandsbanenNorwayService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.framework.application.OTPFeature; import org.opentripplanner.inspector.raster.TileRendererManager; @@ -146,5 +146,5 @@ default DataOverlayContext dataOverlayContext(RouteRequest request) { StopConsolidationService stopConsolidationService(); @Nullable - EnturSorlandsbanenService enturSorlandsbanenService(); + SorlandsbanenNorwayService sorlandsbanenService(); } diff --git a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java index c93cca4887d..7191b82e814 100644 --- a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java +++ b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationFactory.java @@ -10,8 +10,8 @@ import org.opentripplanner.ext.geocoder.configure.GeocoderModule; import org.opentripplanner.ext.interactivelauncher.configuration.InteractiveLauncherModule; import org.opentripplanner.ext.ridehailing.configure.RideHailingServicesModule; -import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; -import org.opentripplanner.ext.sorlandsbanen.configure.EnturSorlandsbanenModule; +import org.opentripplanner.ext.sorlandsbanen.SorlandsbanenNorwayService; +import org.opentripplanner.ext.sorlandsbanen.configure.SorlandsbanenNorwayModule; import org.opentripplanner.ext.stopconsolidation.StopConsolidationRepository; import org.opentripplanner.ext.stopconsolidation.configure.StopConsolidationServiceModule; import org.opentripplanner.graph_builder.issue.api.DataImportIssueSummary; @@ -57,7 +57,7 @@ ConstructApplicationModule.class, RideHailingServicesModule.class, EmissionsServiceModule.class, - EnturSorlandsbanenModule.class, + SorlandsbanenNorwayModule.class, StopConsolidationServiceModule.class, InteractiveLauncherModule.class, StreetLimitationParametersServiceModule.class, @@ -94,7 +94,7 @@ public interface ConstructApplicationFactory { StreetLimitationParameters streetLimitationParameters(); @Nullable - EnturSorlandsbanenService enturSorlandsbanenService(); + SorlandsbanenNorwayService enturSorlandsbanenService(); @Nullable LuceneIndex luceneIndex(); diff --git a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java index 75e2d0940a3..ab0f242f834 100644 --- a/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java +++ b/application/src/main/java/org/opentripplanner/standalone/configure/ConstructApplicationModule.java @@ -10,7 +10,7 @@ import org.opentripplanner.ext.geocoder.LuceneIndex; import org.opentripplanner.ext.interactivelauncher.api.LauncherRequestDecorator; import org.opentripplanner.ext.ridehailing.RideHailingService; -import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; +import org.opentripplanner.ext.sorlandsbanen.SorlandsbanenNorwayService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.raptor.configure.RaptorConfig; import org.opentripplanner.routing.algorithm.raptoradapter.transit.TripSchedule; @@ -42,7 +42,7 @@ OtpServerRequestContext providesServerContext( StreetLimitationParametersService streetLimitationParametersService, @Nullable TraverseVisitor traverseVisitor, EmissionsService emissionsService, - @Nullable EnturSorlandsbanenService enturSorlandsbanenService, + @Nullable SorlandsbanenNorwayService sorlandsbanenService, LauncherRequestDecorator launcherRequestDecorator, @Nullable LuceneIndex luceneIndex ) { @@ -60,7 +60,7 @@ OtpServerRequestContext providesServerContext( realtimeVehicleService, vehicleRentalService, emissionsService, - enturSorlandsbanenService, + sorlandsbanenService, routerConfig.flexParameters(), rideHailingServices, stopConsolidationService, diff --git a/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java b/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java index 2c35dfe6263..c6d388ffd0e 100644 --- a/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java +++ b/application/src/main/java/org/opentripplanner/standalone/server/DefaultServerRequestContext.java @@ -9,7 +9,7 @@ import org.opentripplanner.ext.flex.FlexParameters; import org.opentripplanner.ext.geocoder.LuceneIndex; import org.opentripplanner.ext.ridehailing.RideHailingService; -import org.opentripplanner.ext.sorlandsbanen.EnturSorlandsbanenService; +import org.opentripplanner.ext.sorlandsbanen.SorlandsbanenNorwayService; import org.opentripplanner.ext.stopconsolidation.StopConsolidationService; import org.opentripplanner.inspector.raster.TileRendererManager; import org.opentripplanner.raptor.api.request.RaptorTuningParameters; @@ -51,7 +51,7 @@ public class DefaultServerRequestContext implements OtpServerRequestContext { private final EmissionsService emissionsService; @Nullable - private final EnturSorlandsbanenService enturSorlandsbanenService; + private final SorlandsbanenNorwayService sorlandsbanenService; private final StopConsolidationService stopConsolidationService; private final StreetLimitationParametersService streetLimitationParametersService; @@ -73,7 +73,7 @@ private DefaultServerRequestContext( RealtimeVehicleService realtimeVehicleService, VehicleRentalService vehicleRentalService, @Nullable EmissionsService emissionsService, - @Nullable EnturSorlandsbanenService enturSorlandsbanenService, + @Nullable SorlandsbanenNorwayService sorlandsbanenService, List rideHailingServices, @Nullable StopConsolidationService stopConsolidationService, StreetLimitationParametersService streetLimitationParametersService, @@ -96,7 +96,7 @@ private DefaultServerRequestContext( this.realtimeVehicleService = realtimeVehicleService; this.rideHailingServices = rideHailingServices; this.emissionsService = emissionsService; - this.enturSorlandsbanenService = enturSorlandsbanenService; + this.sorlandsbanenService = sorlandsbanenService; this.stopConsolidationService = stopConsolidationService; this.streetLimitationParametersService = streetLimitationParametersService; this.luceneIndex = luceneIndex; @@ -117,7 +117,7 @@ public static DefaultServerRequestContext create( RealtimeVehicleService realtimeVehicleService, VehicleRentalService vehicleRentalService, @Nullable EmissionsService emissionsService, - @Nullable EnturSorlandsbanenService enturSorlandsbanenService, + @Nullable SorlandsbanenNorwayService sorlandsbanenService, FlexParameters flexParameters, List rideHailingServices, @Nullable StopConsolidationService stopConsolidationService, @@ -138,7 +138,7 @@ public static DefaultServerRequestContext create( realtimeVehicleService, vehicleRentalService, emissionsService, - enturSorlandsbanenService, + sorlandsbanenService, rideHailingServices, stopConsolidationService, streetLimitationParametersService, @@ -262,7 +262,7 @@ public EmissionsService emissionsService() { } @Nullable - public EnturSorlandsbanenService enturSorlandsbanenService() { - return enturSorlandsbanenService; + public SorlandsbanenNorwayService sorlandsbanenService() { + return sorlandsbanenService; } } From b2f6fafed8bb9923443932cc459754785f736ff0 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 24 Oct 2024 14:29:12 +0200 Subject: [PATCH 08/20] doc: Update JavaDoc in sandbox sorlandsbanen --- .../ext/sorlandsbanen/CoachCostCalculator.java | 3 +++ .../sorlandsbanen/SorlandsbanenNorwayService.java | 15 ++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java index b7ce446d430..204878072d5 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java @@ -55,6 +55,9 @@ public int transitArrivalCost( int toStop ) { int cost = delegate.transitArrivalCost(boardCost, alightSlack, transitTime, trip, toStop); + + // This is a bit ugly, since it relays on the fact that the 'transitReluctanceFactorIndex' + // returns the 'route.getMode().ordinal()' if(trip.transitReluctanceFactorIndex() == TransitMode.COACH.ordinal()) { cost += transitTime * EXTRA_RELUCTANCE_ON_COACH; } diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java index f08c0fc4f6b..5acffe65788 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java @@ -17,13 +17,14 @@ import org.opentripplanner.transit.model.site.StopLocation; /** - * This is basically a big hack to produce results containing "Sørlandsbanen" in Norway. This - * railroad line is slow and goes inland fare from where people live. Despite this, people and the - * operator want to show it in the results for log travel along the southern part of Norway where - * ii is an option. Tuning the search has proven to be challenging. It is solved here by doing - * two searches. One normal search and one where the rail is given a big cost advantage over coach. - * If train results are found in the second search, then it is added to the results of the first - * search. Everything found in the first search is always returned. + * This service is responsible for producing results with rail for the south of Norway. The rail + * line is called "Sørlandsbanen". This rail line is slow and goes inland fare from where people + * live. Despite this, people and the operator want to show it in the results for log travel along + * the southern part of Norway where ii is an option. Tuning the search has proven to be + * challenging. It is solved here by doing two searches. One normal search and one where the rail + * is given a big cost advantage over coach. If train results are found in the second search, then + * it is added to the results of the first search. Everything found in the first search is always + * returned. */ public class SorlandsbanenNorwayService { From 9f2f507cf9d5eb32bf2f4574bc1657e051dc258b Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 24 Oct 2024 14:07:44 +0200 Subject: [PATCH 09/20] feature: Add 'isNorthOf(lat)' to WgsCoordinate --- .../opentripplanner/framework/geometry/WgsCoordinate.java | 4 ++++ .../framework/geometry/WgsCoordinateTest.java | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java b/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java index 136bb4264c0..3a5d4b23e73 100644 --- a/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java +++ b/application/src/main/java/org/opentripplanner/framework/geometry/WgsCoordinate.java @@ -177,6 +177,10 @@ public double distanceTo(WgsCoordinate other) { ); } + public boolean isNorthOf(double latitudeBorder) { + return latitude > latitudeBorder; + } + /** * Return a new coordinate that is moved an approximate number of meters east. */ diff --git a/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java b/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java index 4bb27fa6d36..d9da08044a5 100644 --- a/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java +++ b/application/src/test/java/org/opentripplanner/framework/geometry/WgsCoordinateTest.java @@ -100,6 +100,13 @@ void add() { assertEquals(new WgsCoordinate(12d, 5d), new WgsCoordinate(9d, 1d).add(3d, 4d)); } + @Test + void testIsNorthOf() { + var c = new WgsCoordinate(10.0, 30.0); + assertTrue(c.isNorthOf(9.9)); + assertFalse(c.isNorthOf(10.1)); + } + @Test void testGreenwich() { assertEquals(51.48d, GREENWICH.latitude()); From 1d52e7bccdc9c48e0e5bea79e023d7428c07b1fa Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 24 Oct 2024 14:31:37 +0200 Subject: [PATCH 10/20] refactor: MAke code more readable --- .../org/opentripplanner/ext/sorlandsbanen/MergePaths.java | 4 ++-- .../ext/sorlandsbanen/SorlandsbanenNorwayService.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java index b5f94019b72..830ec4574ea 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java @@ -18,10 +18,10 @@ class MergePaths implements BiFunction>, Collection>, Collection>> { @Override - public Collection> apply(Collection> main, Collection> railAlternatives) { + public Collection> apply(Collection> main, Collection> alternatives) { Map> result = new HashMap<>(); addAllToMap(result, main); - addRailToMap(result, railAlternatives); + addRailToMap(result, alternatives); return result.values(); } diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java index 5acffe65788..783f6dc1200 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java @@ -40,7 +40,7 @@ public ExtraMcRouterSearch createMcRouterFactory(RouteRequest requ ); WgsCoordinate to = findStopCoordinate(request.to(), accessEgresses.getEgresses(), transitLayer); - if (from.latitude() > SOUTH_BOARDER_LIMIT && to.latitude() > SOUTH_BOARDER_LIMIT) { + if (from.isNorthOf(SOUTH_BOARDER_LIMIT) && to.isNorthOf(SOUTH_BOARDER_LIMIT)) { return null; } From 8a0dfc70b66662aee097da8e4caaf2e259e58feb Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 24 Oct 2024 14:36:03 +0200 Subject: [PATCH 11/20] feature: Avoid replacing existing paths in the result for Sorlandsbanen --- .../org/opentripplanner/ext/sorlandsbanen/MergePaths.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java index 830ec4574ea..8abf35ffa4c 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java @@ -34,7 +34,9 @@ private void addAllToMap(Map> map, Collection> map, Collection> paths) { for (var it : paths) { if (hasRail(it)) { - map.put(new PathKey(it), it); + // Avoid replacing an existing value if it exists, there might be minor differences in the + // path, in witch case we want to keep the main result. + map.computeIfAbsent(new PathKey(it), k -> it); } } } From 2e2be55036512d933f16b516bf3de1ed08b671d2 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Oct 2024 10:52:26 +0200 Subject: [PATCH 12/20] review: Cleanup CompositeResult --- .../raptor/rangeraptor/CompositeResult.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java index d92f0df79bb..b547493a7df 100644 --- a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/CompositeResult.java @@ -17,7 +17,8 @@ */ class CompositeResult implements RaptorRouterResult { - private final RaptorRouterResult mainResult; + public static final String UNSOPORTED_OPERATION = + "Merging all stop arrivals will be a complicated and memory intensive process, unless we need this this should not be done."; private final Collection> result; CompositeResult( @@ -25,7 +26,6 @@ class CompositeResult implements RaptorRouterResul RaptorRouterResult alternativeResult, BiFunction>, Collection>, Collection>> merger ) { - this.mainResult = mainResult; this.result = merger.apply(mainResult.extractPaths(), alternativeResult.extractPaths()); } @@ -37,28 +37,19 @@ public Collection> extractPaths() { return result; } - /** - * Return the main result only. - */ @Override public SingleCriteriaStopArrivals extractBestOverallArrivals() { - return mainResult.extractBestOverallArrivals(); + throw new UnsupportedOperationException(UNSOPORTED_OPERATION); } - /** - * Return the main result only. - */ @Override public SingleCriteriaStopArrivals extractBestTransitArrivals() { - return mainResult.extractBestTransitArrivals(); + throw new UnsupportedOperationException(UNSOPORTED_OPERATION); } - /** - * Return the main result only. - */ @Override public SingleCriteriaStopArrivals extractBestNumberOfTransfers() { - return mainResult.extractBestNumberOfTransfers(); + throw new UnsupportedOperationException(UNSOPORTED_OPERATION); } /** From d67d4d064581a21313d961fd7bd5fa13dde9b437 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Oct 2024 10:52:58 +0200 Subject: [PATCH 13/20] review: Cleanup PathKey --- .../java/org/opentripplanner/ext/sorlandsbanen/PathKey.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java index 28c1b1eac25..9d0d1a14936 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java @@ -19,9 +19,6 @@ final class PathKey { } private static int hash(RaptorPath path) { - if (path == null) { - return 0; - } int result = 1; PathLeg leg = path.accessLeg(); From 1cb96d66f0df91672ded08b10bcd9f9647be62c3 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Oct 2024 10:53:49 +0200 Subject: [PATCH 14/20] review: Add more doc to ExtraMcRouterSearch --- .../org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java b/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java index afebc107832..deb22107036 100644 --- a/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java +++ b/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java @@ -14,7 +14,11 @@ *

* When changing the transit data, you may also invalidate the heuristics created by Raptor. If this * is the case, you need to turn off the {@link org.opentripplanner.raptor.api.request.Optimization#PARETO_CHECK_AGAINST_DESTINATION}. - * For the heuristics to work, you may add extra cost or filter away data. + * For the heuristics to work, you may add extra cost or filter away data. But you cannot decrease + * the cost, add transfer or add new trips. + *

+ * This will alter the multi-criteria search, if only a sadard search is requested any extra + * multi-criteria search is ignored. *

* @param The TripSchedule type defined by the user of the raptor API. */ From af4121d8d93b85881a9994116af6039098e185c1 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Oct 2024 10:54:51 +0200 Subject: [PATCH 15/20] review: Rename 'createMcRouterFactory' to 'createExtraMcRouterSearch' --- .../ext/sorlandsbanen/SorlandsbanenNorwayService.java | 8 +++++--- .../algorithm/raptoradapter/router/TransitRouter.java | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java index 783f6dc1200..9314400c649 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java @@ -2,6 +2,7 @@ import java.util.Collection; import java.util.function.BiFunction; +import javax.annotation.Nullable; import org.opentripplanner.framework.geometry.WgsCoordinate; import org.opentripplanner.model.GenericLocation; import org.opentripplanner.raptor.api.path.RaptorPath; @@ -28,11 +29,12 @@ */ public class SorlandsbanenNorwayService { - private static final double SOUTH_BOARDER_LIMIT = 59.1; + private static final double SOUTH_BORDER_LIMIT = 59.1; private static final int MIN_DISTANCE_LIMIT = 120_000; - public ExtraMcRouterSearch createMcRouterFactory(RouteRequest request, AccessEgresses accessEgresses, TransitLayer transitLayer) { + @Nullable + public ExtraMcRouterSearch createExtraMcRouterSearch(RouteRequest request, AccessEgresses accessEgresses, TransitLayer transitLayer) { WgsCoordinate from = findStopCoordinate( request.from(), accessEgresses.getAccesses(), @@ -40,7 +42,7 @@ public ExtraMcRouterSearch createMcRouterFactory(RouteRequest requ ); WgsCoordinate to = findStopCoordinate(request.to(), accessEgresses.getEgresses(), transitLayer); - if (from.isNorthOf(SOUTH_BOARDER_LIMIT) && to.isNorthOf(SOUTH_BOARDER_LIMIT)) { + if (from.isNorthOf(SOUTH_BORDER_LIMIT) && to.isNorthOf(SOUTH_BORDER_LIMIT)) { return null; } diff --git a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java index f77711136e8..2b9c0136e07 100644 --- a/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java +++ b/application/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/router/TransitRouter.java @@ -146,7 +146,7 @@ private TransitRouterResult route() { // Route transit var raptorService = new RaptorService<>( serverContext.raptorConfig(), - createMcRouterFactory(accessEgresses, transitLayer) + createExtraMcRouterSearch(accessEgresses, transitLayer) ); var transitResponse = raptorService.route(raptorRequest, requestTransitDataProvider); @@ -396,7 +396,7 @@ private IntStream listStopIndexes(FeedScopedId stopLocationId) { * An optional factory for creating a decorator around the multi-criteria RangeRaptor instance. */ @Nullable - private ExtraMcRouterSearch createMcRouterFactory( + private ExtraMcRouterSearch createExtraMcRouterSearch( AccessEgresses accessEgresses, TransitLayer transitLayer ) { @@ -406,6 +406,6 @@ private ExtraMcRouterSearch createMcRouterFactory( var service = serverContext.sorlandsbanenService(); return service == null ? null - : service.createMcRouterFactory(request, accessEgresses, transitLayer); + : service.createExtraMcRouterSearch(request, accessEgresses, transitLayer); } } From 736daffe10406150ea3724bca4dbfc1e76030315 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Fri, 25 Oct 2024 11:05:06 +0200 Subject: [PATCH 16/20] review: Add @Nullable to 'executorService' in ConcurrentCompositeRaptorRouter --- .../raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java index dbf0c2d89c7..5cc0b7f2189 100644 --- a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java @@ -4,6 +4,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.function.BiFunction; +import javax.annotation.Nullable; import org.opentripplanner.framework.application.OTPRequestTimeoutException; import org.opentripplanner.raptor.api.model.RaptorTripSchedule; import org.opentripplanner.raptor.api.path.RaptorPath; @@ -21,13 +22,15 @@ public class ConcurrentCompositeRaptorRouter private final RaptorRouter mainWorker; private final RaptorRouter alternativeWorker; private final BiFunction>, Collection>, Collection>> merger; + + @Nullable private final ExecutorService executorService; public ConcurrentCompositeRaptorRouter( RaptorRouter mainWorker, RaptorRouter alternativeWorker, BiFunction>, Collection>, Collection>> merger, - ExecutorService executorService + @Nullable ExecutorService executorService ) { this.mainWorker = mainWorker; this.alternativeWorker = alternativeWorker; From aa0c69cde738a9fa75bcb6be630f6c3252b0efd9 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 29 Oct 2024 21:03:46 +0100 Subject: [PATCH 17/20] Apply suggestions from code review Co-authored-by: Vincent Paturet <46598384+vpaturet@users.noreply.github.com> Co-authored-by: Henrik Abrahamsson <127481124+habrahamsson-skanetrafiken@users.noreply.github.com> --- .../ext/sorlandsbanen/CoachCostCalculator.java | 2 +- .../org/opentripplanner/ext/sorlandsbanen/MergePaths.java | 2 +- .../ext/sorlandsbanen/SorlandsbanenNorwayService.java | 6 +++--- .../opentripplanner/framework/application/OTPFeature.java | 2 +- .../opentripplanner/framework/geometry/WgsCoordinate.java | 2 +- .../raptor/rangeraptor/CompositeResult.java | 8 ++++---- .../rangeraptor/ConcurrentCompositeRaptorRouter.java | 2 +- .../opentripplanner/raptor/spi/ExtraMcRouterSearch.java | 4 ++-- doc/user/sandbox/Sorlandsbanen.md | 6 +++--- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java index 204878072d5..89a3071f975 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/CoachCostCalculator.java @@ -56,7 +56,7 @@ public int transitArrivalCost( ) { int cost = delegate.transitArrivalCost(boardCost, alightSlack, transitTime, trip, toStop); - // This is a bit ugly, since it relays on the fact that the 'transitReluctanceFactorIndex' + // This is a bit ugly, since it relies on the fact that the 'transitReluctanceFactorIndex' // returns the 'route.getMode().ordinal()' if(trip.transitReluctanceFactorIndex() == TransitMode.COACH.ordinal()) { cost += transitTime * EXTRA_RELUCTANCE_ON_COACH; diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java index 8abf35ffa4c..2f7b38a7a08 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/MergePaths.java @@ -35,7 +35,7 @@ private void addRailToMap(Map> map, Collection it); } } diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java index 9314400c649..ecccb5d2370 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/SorlandsbanenNorwayService.java @@ -19,9 +19,9 @@ /** * This service is responsible for producing results with rail for the south of Norway. The rail - * line is called "Sørlandsbanen". This rail line is slow and goes inland fare from where people + * line is called "Sørlandsbanen". This rail line is slow and goes inland far from where people * live. Despite this, people and the operator want to show it in the results for log travel along - * the southern part of Norway where ii is an option. Tuning the search has proven to be + * the southern part of Norway where it is an option. Tuning the search has proven to be * challenging. It is solved here by doing two searches. One normal search and one where the rail * is given a big cost advantage over coach. If train results are found in the second search, then * it is added to the results of the first search. Everything found in the first search is always @@ -72,7 +72,7 @@ public BiFunction>, Collection implements RaptorRouterResult { - public static final String UNSOPORTED_OPERATION = + private static final String UNSUPPORTED_OPERATION = "Merging all stop arrivals will be a complicated and memory intensive process, unless we need this this should not be done."; private final Collection> result; @@ -39,17 +39,17 @@ public Collection> extractPaths() { @Override public SingleCriteriaStopArrivals extractBestOverallArrivals() { - throw new UnsupportedOperationException(UNSOPORTED_OPERATION); + throw new UnsupportedOperationException(UNSUPPORTED_OPERATION); } @Override public SingleCriteriaStopArrivals extractBestTransitArrivals() { - throw new UnsupportedOperationException(UNSOPORTED_OPERATION); + throw new UnsupportedOperationException(UNSUPPORTED_OPERATION); } @Override public SingleCriteriaStopArrivals extractBestNumberOfTransfers() { - throw new UnsupportedOperationException(UNSOPORTED_OPERATION); + throw new UnsupportedOperationException(UNSUPPORTED_OPERATION); } /** diff --git a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java index 5cc0b7f2189..a4ddf36347f 100644 --- a/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java +++ b/application/src/main/java/org/opentripplanner/raptor/rangeraptor/ConcurrentCompositeRaptorRouter.java @@ -12,7 +12,7 @@ import org.opentripplanner.raptor.rangeraptor.internalapi.RaptorRouterResult; /** - * Run two Raptor routers and join the result. The two searches are run concurrent if an + * Run two Raptor routers and join the result. The two searches are run concurrently if an * {@link ExecutorService} is provided. * @see CompositeResult for joining results. */ diff --git a/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java b/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java index deb22107036..e6fdc4fd0d0 100644 --- a/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java +++ b/application/src/main/java/org/opentripplanner/raptor/spi/ExtraMcRouterSearch.java @@ -6,7 +6,7 @@ import org.opentripplanner.raptor.api.path.RaptorPath; /** - * This interface is used to run two mulit-criteria searches and merging the result. Raptor will + * This interface is used to run two multi-criteria searches and merging the result. Raptor will * run the heuristics as normal. Then create two multi-criteria searches, the main search and the * alternative search. The caller must provide a {@code merger} and * {@link RaptorTransitDataProvider}. The transit data is used for the alternative search. This @@ -17,7 +17,7 @@ * For the heuristics to work, you may add extra cost or filter away data. But you cannot decrease * the cost, add transfer or add new trips. *

- * This will alter the multi-criteria search, if only a sadard search is requested any extra + * This will alter the multi-criteria search, if only a standard search is requested any extra * multi-criteria search is ignored. *

* @param The TripSchedule type defined by the user of the raptor API. diff --git a/doc/user/sandbox/Sorlandsbanen.md b/doc/user/sandbox/Sorlandsbanen.md index 4898a29af37..b9846da0f37 100644 --- a/doc/user/sandbox/Sorlandsbanen.md +++ b/doc/user/sandbox/Sorlandsbanen.md @@ -3,7 +3,7 @@ **This sandbox module is only working in Norway**, in particular only in the south of Norway. The feature flag to turn it *on* should only be enabled if you are routing using the norwegian data set. -The railroad in southern Norway is very slow and does not go by the cost where most people live. It +The railroad in southern Norway is very slow and does not go by the coast where most people live. It is easily beaten by coaches in the area. Despite this, we need to include it in results where it is relevant. @@ -24,8 +24,8 @@ in addition to the feature flag. ## Changelog -- 2024-10-14: We have used this feature for som time, but now want it in the Sandbox so we do not - need to merge it everytime we create a new entur release. +- 2024-10-14: We have used this feature for some time, but now want it in the Sandbox so we do not + need to merge it everytime we create a new Entur release. ### Configuration From d5527761c13a68e12593ea5d0bf9a0fee26bbd70 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 29 Oct 2024 21:24:16 +0100 Subject: [PATCH 18/20] review: Update JavaDoc for PathKey --- .../opentripplanner/ext/sorlandsbanen/PathKey.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java index 9d0d1a14936..e4504b3ed14 100644 --- a/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java +++ b/application/src/ext/java/org/opentripplanner/ext/sorlandsbanen/PathKey.java @@ -5,10 +5,15 @@ /** - * Uses a hash to create a key for access, egress and transit legs in a path. Transfers - * are not included. The key is used to exclude duplicates. This approach may drop valid results - * when there is a hash collision, but this whole sandbox feature is a hack - so we can tolerate - * this here. + * The purpose of this class is to create a key to be able to compare paths so duplicate results + * can be ignored. + *

+ * Creating a good key for a path is not easy. For example, should a small variation in the street + * routing for an access/egress leg count as a significant difference? The solution here is + * straightforward. It creates a hash of the access-, egress- and transit-legs in the path, + * ignoring transfer legs. This approach may drop valid results if there are hash collisions, + * but since this is a Sandbox module and the investment in this code is minimal, we will accept + * the risk. */ final class PathKey { From a3548650878377c8b0a60ca018196c2090b95486 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 29 Oct 2024 21:24:49 +0100 Subject: [PATCH 19/20] review: Generate updated doc --- doc/user/Configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/user/Configuration.md b/doc/user/Configuration.md index a45e4253a06..8d94e332e40 100644 --- a/doc/user/Configuration.md +++ b/doc/user/Configuration.md @@ -250,7 +250,7 @@ Here is a list of all features which can be toggled on/off and their default val | `SandboxAPIGeocoder` | Enable the Geocoder API. | | ✓️ | | `SandboxAPIMapboxVectorTilesApi` | Enable Mapbox vector tiles API. | | ✓️ | | `SandboxAPIParkAndRideApi` | Enable park-and-ride endpoint. | | ✓️ | -| `Sorlandsbanen` | Include train Sørlandsbanen in results when searchig in south of Norway. Only relevant in Norway. | | ✓️ | +| `Sorlandsbanen` | Include train Sørlandsbanen in results when searching in south of Norway. Only relevant in Norway. | | ✓️ | | `TransferAnalyzer` | Analyze transfers during graph build. | | ✓️ | From eb5588750fffa03d3414a3983c9e2f0f938e6ba8 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Tue, 29 Oct 2024 21:42:24 +0100 Subject: [PATCH 20/20] review: Improve Sandbox documentation --- doc/user/sandbox/Sorlandsbanen.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/user/sandbox/Sorlandsbanen.md b/doc/user/sandbox/Sorlandsbanen.md index b9846da0f37..13bc9163456 100644 --- a/doc/user/sandbox/Sorlandsbanen.md +++ b/doc/user/sandbox/Sorlandsbanen.md @@ -14,8 +14,11 @@ rail results(if they exist) from the second search and add it two to the results search. The new set of results will contain everything we found in the first search, plus the train results in the second results. -Note! This is a hack and the logic to enable this look at the origin and destination coordinates -in addition to the feature flag. +Note! This looks at origin and destination coordinates in addition to the feature flag to enable +the second search. It is automatically enabled if: + - the `OTPFeature.Sorlandsbanen` is on. + - the origin and/or destination is in the south of Norway. + - the search is a long-distance search, origin and destination are fare apart from each other. ## Contact Info