Merge pull request #2928 from entur/otp2_unexpected_wait_cost_between…

…_access_and_transit

Unexpected wait cost between access and transit

docs/Changelog.md

@@ -19,6 +19,7 @@
 - New Transit search algorithm, Raptor, replaces the AStar for all transit searches. 
 - Ability to switch off the fare service(#2912).
 - Limit the transit service period(#2925).
+- Removed unwanted cost added for wait time between access and transit with RangeRaptor (#2927)
 
 ## Ported over from the 1.x
 - Add application/x-protobuf to accepted protobuf content-types (#2839)

src/main/java/org/opentripplanner/transit/raptor/api/request/ArrivalAndDeparturePreference.java

@@ -0,0 +1,22 @@
+package org.opentripplanner.transit.raptor.api.request;
+
+/**
+ * TODO OTP2 - Write doc if implemented
+ */
+public enum ArrivalAndDeparturePreference {
+    /**
+     * Find the best results within the time window. The traveler is flexible within the
+     * search window. This is the default.
+     */
+    TIME_TABLE,
+
+    /**
+     * The traveler prefer to arrive as early as possible given the earliest departure time.
+     */
+    ARRIVE_EARLY,
+
+    /**
+     * The traveler prefer to depart as late as possible given the latest arrival time.
+     */
+    DEPART_LATE
+}

src/main/java/org/opentripplanner/transit/raptor/api/request/SearchParams.java

@@ -3,7 +3,6 @@
 import org.opentripplanner.transit.raptor.api.transit.TransferLeg;
 import org.opentripplanner.transit.raptor.util.TimeUtils;
 
-import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
@@ -21,7 +20,6 @@ public class SearchParams {
     private final int earliestDepartureTime;
     private final int latestArrivalTime;
     private final int searchWindowInSeconds;
-    private final boolean arrivedBy;
     private final int boardSlackInSeconds;
     private final int numberOfAdditionalTransfers;
     private final double relaxCostAtDestination;
@@ -38,7 +36,6 @@ private SearchParams() {
         earliestDepartureTime = NOT_SET;
         latestArrivalTime = NOT_SET;
         searchWindowInSeconds = DEFAULT_SEARCH_WINDOW_IN_SECONDS;
-        arrivedBy = false;
         boardSlackInSeconds = 60;
         numberOfAdditionalTransfers = 5;
         relaxCostAtDestination = NOT_SET;
@@ -53,16 +50,14 @@ private SearchParams() {
         this.earliestDepartureTime = builder.earliestDepartureTime();
         this.latestArrivalTime = builder.latestArrivalTime();
         this.searchWindowInSeconds = builder.searchWindowInSeconds();
-        this.arrivedBy = builder.arrivedBy();
-        // TODO JAVA_9 - Cleanup next 3 lines: Set.of(...) and List.of(...)
         this.boardSlackInSeconds = builder.boardSlackInSeconds();
         this.numberOfAdditionalTransfers = builder.numberOfAdditionalTransfers();
         this.relaxCostAtDestination = builder.relaxCostAtDestination();
         this.timetableEnabled = builder.timetableEnabled();
         this.waitAtBeginningEnabled = builder.waitAtBeginningEnabled();
         this.stopFilter = builder.stopFilter();
-        this.accessLegs = Collections.unmodifiableList(new ArrayList<>(builder.accessLegs()));
-        this.egressLegs = Collections.unmodifiableList(new ArrayList<>(builder.egressLegs()));
+        this.accessLegs = java.util.List.copyOf(builder.accessLegs());
+        this.egressLegs = java.util.List.copyOf(builder.egressLegs());
     }
 
     static SearchParams defaults() {
@@ -116,14 +111,12 @@ public int searchWindowInSeconds() {
 
 
     /**
-     * This parameter decide if a search is a 'depart after' or 'arrive by' search.
+     * TODO OTP2 - Describe this
      * <p/>
-     * Optional. Default value is 'false'.
-     *
-     * @return true is the search is a 'arrive by' search.
+     * Optional. Default value is 'TIME_TABLE'.
      */
-    public boolean arrivedBy() {
-        return arrivedBy;
+    public ArrivalAndDeparturePreference arrivalAndDeparturePreference() {
+        return ArrivalAndDeparturePreference.TIME_TABLE;
     }
 
     /**
@@ -136,7 +129,6 @@ public int boardSlackInSeconds() {
         return boardSlackInSeconds;
     }
 
-
     /**
      * RangeRaptor is designed to search until the destination is reached and then
      * {@code numberOfAdditionalTransfers} more rounds.
@@ -147,8 +139,6 @@ public int numberOfAdditionalTransfers() {
         return numberOfAdditionalTransfers;
     }
 
-
-
     /**
      * This accept none optimal trips if they are close enough - if and only if they represent an optimal path
      * for their given iteration. I other words this slack only relax the pareto comparison at the destination.
@@ -243,7 +233,6 @@ public String toString() {
                 "earliestDepartureTime=" + TimeUtils.timeToStrCompact(earliestDepartureTime, NOT_SET) +
                 ", latestArrivalTime=" + TimeUtils.timeToStrCompact(latestArrivalTime, NOT_SET) +
                 ", searchWindowInSeconds=" + TimeUtils.timeToStrCompact(searchWindowInSeconds) +
-                ", arrivedBy=" + arrivedBy +
                 ", accessLegs=" + accessLegs +
                 ", egressLegs=" + egressLegs +
                 ", boardSlackInSeconds=" + boardSlackInSeconds +
@@ -259,7 +248,6 @@ public boolean equals(Object o) {
         return earliestDepartureTime == that.earliestDepartureTime &&
                 latestArrivalTime == that.latestArrivalTime &&
                 searchWindowInSeconds == that.searchWindowInSeconds &&
-                arrivedBy == that.arrivedBy &&
                 boardSlackInSeconds == that.boardSlackInSeconds &&
                 numberOfAdditionalTransfers == that.numberOfAdditionalTransfers &&
                 accessLegs.equals(that.accessLegs) &&
@@ -269,7 +257,7 @@ public boolean equals(Object o) {
     @Override
     public int hashCode() {
         return Objects.hash(
-                earliestDepartureTime, latestArrivalTime, searchWindowInSeconds, arrivedBy, accessLegs,
+                earliestDepartureTime, latestArrivalTime, searchWindowInSeconds, accessLegs,
                 egressLegs, boardSlackInSeconds, numberOfAdditionalTransfers
         );
     }

src/main/java/org/opentripplanner/transit/raptor/api/request/SearchParamsBuilder.java

@@ -14,7 +14,6 @@ public class SearchParamsBuilder {
     private int earliestDepartureTime;
     private int latestArrivalTime;
     private int searchWindowInSeconds;
-    private boolean arrivedBy;
     private int boardSlackInSeconds;
     private int numberOfAdditionalTransfers;
     private double relaxCostAtDestination;
@@ -34,7 +33,6 @@ public SearchParamsBuilder(SearchParams defaults) {
         this.earliestDepartureTime = defaults.earliestDepartureTime();
         this.latestArrivalTime = defaults.latestArrivalTime();
         this.searchWindowInSeconds = defaults.searchWindowInSeconds();
-        this.arrivedBy = defaults.arrivedBy();
         this.boardSlackInSeconds = defaults.boardSlackInSeconds();
         this.numberOfAdditionalTransfers = defaults.numberOfAdditionalTransfers();
         this.relaxCostAtDestination = defaults.relaxCostAtDestination();
@@ -77,15 +75,6 @@ public SearchParamsBuilder searchWindowInSeconds(int searchWindowInSeconds) {
         return this;
     }
 
-    public boolean arrivedBy() {
-        return arrivedBy;
-    }
-
-    public SearchParamsBuilder arrivedBy(boolean arrivedBy) {
-        this.arrivedBy = arrivedBy;
-        return this;
-    }
-
     public int boardSlackInSeconds() {
         return boardSlackInSeconds;
     }

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/RangeRaptorWorker.java

@@ -149,7 +149,7 @@ private void runRaptorForMinute(int iterationDepartureTime) {
         doTransfersForAccessLegs(iterationDepartureTime);
 
         while (hasMoreRounds()) {
-            lifeCycle.prepareForNextRound();
+            lifeCycle.prepareForNextRound(roundTracker.round());
 
             // NB since we have transfer limiting not bothering to cut off search when there are no more transfers
             // as that will be rare and complicates the code

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/WorkerLifeCycle.java

@@ -36,13 +36,12 @@ public interface WorkerLifeCycle {
     /**
      * Subscribe to 'prepare for next round' events by register listener.
      * Every time a new round start the listener(the input parameter) is
-     * notified/invoked.
+     * notified/invoked with the current round as a argument.
      *
      * @param prepareForNextRound if {@code null} nothing is added to the publisher.
+     *                            The round number(0..n) is passed to the subscriber.
      */
-    void onPrepareForNextRound(Runnable prepareForNextRound);
-
-
+    void onPrepareForNextRound(IntConsumer prepareForNextRound);
 
     /**
      * Subscribe to 'transits for round complete' events by register listener.
@@ -53,7 +52,6 @@ public interface WorkerLifeCycle {
      */
     void onTransitsForRoundComplete(Runnable transitsForRoundComplete);
 
-
     /**
      * Subscribe to 'transfers for round complete' events by register listener.
      * This event occur when the all transfers are calculated in each round.

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/path/DestinationArrivalPaths.java

@@ -44,7 +44,7 @@ public DestinationArrivalPaths(
         this.debugHandler = debugHandlerFactory.debugStopArrival();
         this.calculator = calculator;
         this.pathMapper = calculator.createPathMapper();
-        lifeCycle.onPrepareForNextRound(this::clearReachedCurrentRoundFlag);
+        lifeCycle.onPrepareForNextRound(round -> clearReachedCurrentRoundFlag());
     }
 
     public void add(ArrivalView<T> egressStopArrival, TransferLeg egressLeg, int additionalCost) {

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/standard/besttimes/BestTimes.java

@@ -55,7 +55,7 @@ public BestTimes(int nStops, TransitCalculator calculator, WorkerLifeCycle lifeC
 
         // Attach to Worker life cycle
         lifeCycle.onSetupIteration((ignore) -> setupIteration());
-        lifeCycle.onPrepareForNextRound(this::prepareForNextRound);
+        lifeCycle.onPrepareForNextRound(round -> prepareForNextRound());
     }
 
     public int time(int stop) {

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/transit/CostCalculator.java

@@ -1,6 +1,8 @@
 package org.opentripplanner.transit.raptor.rangeraptor.transit;
 
 
+import org.opentripplanner.transit.raptor.rangeraptor.WorkerLifeCycle;
+
 /**
  * The responsibility for the cost calculator is to calculate the
  * multi-criteria cost.
@@ -15,25 +17,33 @@ public class CostCalculator {
     private final int waitFactor;
     private final int transitFactor;
 
+    /**
+     * We only apply the wait factor between transits, not between access and transit;
+     * Hence we start with 0 (zero) and after the first round we set this to the
+     * provided {@link #waitFactor}.
+     */
+    private int waitFactorApplied = 0;
+
 
     CostCalculator(
             int boardCost,
             int boardSlackInSeconds,
             double walkReluctanceFactor,
-            double waitReluctanceFactor
+            double waitReluctanceFactor,
+            WorkerLifeCycle lifeCycle
     ) {
         this.boardCost = PRECISION * boardCost;
         this.walkFactor = (int) (PRECISION * walkReluctanceFactor);
         this.waitFactor = (int) (PRECISION * waitReluctanceFactor);
         this.transitFactor = PRECISION;
         this.minTransferCost = this.boardCost +  this.waitFactor * boardSlackInSeconds;
+        lifeCycle.onPrepareForNextRound(this::initWaitFactor);
     }
 
-
     public int transitArrivalCost(int prevStopArrivalTime, int boardTime, int alightTime) {
         int waitTime = boardTime - prevStopArrivalTime;
         int transitTime = alightTime - boardTime;
-        return waitFactor * waitTime + transitFactor * transitTime + boardCost;
+        return waitFactorApplied * waitTime + transitFactor * transitTime + boardCost;
     }
 
     public int walkCost(int walkTimeInSeconds) {
@@ -43,4 +53,10 @@ public int walkCost(int walkTimeInSeconds) {
     public int calculateMinCost(int minTravelTime, int minNumTransfers) {
         return transitFactor * minTravelTime + minNumTransfers * minTransferCost;
     }
+
+    private void initWaitFactor(int round) {
+        // For access(round 0) and the first transit round(1) skip adding a cost for waiting,
+        // we assume we can time-shift the access leg.
+        this.waitFactorApplied = round < 2 ? 0 : waitFactor;
+    }
 }

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/transit/SearchContext.java

@@ -14,7 +14,7 @@
 import org.opentripplanner.transit.raptor.rangeraptor.WorkerLifeCycle;
 import org.opentripplanner.transit.raptor.rangeraptor.debug.DebugHandlerFactory;
 import org.opentripplanner.transit.raptor.rangeraptor.debug.WorkerPerformanceTimers;
-import org.opentripplanner.transit.raptor.rangeraptor.workerlifecycle.LifeCycleBuilder;
+import org.opentripplanner.transit.raptor.rangeraptor.workerlifecycle.LifeCycleSubscriptions;
 import org.opentripplanner.transit.raptor.rangeraptor.workerlifecycle.LifeCycleEventPublisher;
 
 import java.util.Collection;
@@ -41,11 +41,10 @@ public class SearchContext<T extends TripScheduleInfo> {
     private final TuningParameters tuningParameters;
     private final RoundTracker roundTracker;
     private final WorkerPerformanceTimers timers;
-    private final DebugRequest<T> debugRequest;
     private final DebugHandlerFactory<T> debugFactory;
     private final StopFilter stopFilter;
 
-    private LifeCycleBuilder lifeCycleBuilder = new LifeCycleBuilder();
+    private LifeCycleSubscriptions lifeCycleSubscriptions = new LifeCycleSubscriptions();
 
     public SearchContext(
             RangeRaptorRequest<T> request,
@@ -60,8 +59,7 @@ public SearchContext(
         this.calculator = createCalculator(this.request, tuningParameters);
         this.roundTracker = new RoundTracker(nRounds(), request.searchParams().numberOfAdditionalTransfers(), lifeCycle());
         this.timers = timers;
-        this.debugRequest = debugRequest(request);
-        this.debugFactory = new DebugHandlerFactory<>(this.debugRequest, lifeCycle());
+        this.debugFactory = new DebugHandlerFactory<>(debugRequest(request), lifeCycle());
         this.stopFilter = request.searchParams().stopFilter() != null
                 ? new StopFilterBitSet(request.searchParams().stopFilter())
                 : (s -> true);
@@ -105,7 +103,8 @@ public CostCalculator costCalculator() {
                 f.boardCost(),
                 request.searchParams().boardSlackInSeconds(),
                 f.walkReluctanceFactor(),
-                f.waitReluctanceFactor()
+                f.waitReluctanceFactor(),
+                lifeCycle()
         );
     }
 
@@ -153,14 +152,14 @@ public RoundProvider roundProvider() {
     }
 
     public WorkerLifeCycle lifeCycle() {
-        return lifeCycleBuilder;
+        return lifeCycleSubscriptions;
     }
 
     public LifeCycleEventPublisher createLifeCyclePublisher() {
-        LifeCycleEventPublisher publisher = new LifeCycleEventPublisher(lifeCycleBuilder);
+        LifeCycleEventPublisher publisher = new LifeCycleEventPublisher(lifeCycleSubscriptions);
         // We want the code to fail (NPE) if someone try to attach to the worker workerlifecycle
         // after it is iniziated; Hence set the builder to null:
-        lifeCycleBuilder = null;
+        lifeCycleSubscriptions = null;
         return publisher;
     }
 }

src/main/java/org/opentripplanner/transit/raptor/rangeraptor/workerlifecycle/LifeCycleEventPublisher.java

@@ -9,20 +9,20 @@
  */
 public class LifeCycleEventPublisher {
     private final IntConsumer[] setupIterationListeners;
-    private final Runnable[] prepareForNextRoundListeners;
+    private final IntConsumer[] prepareForNextRoundListeners;
     private final Runnable[] transitsForRoundCompleteListeners;
     private final Runnable[] transfersForRoundCompleteListeners;
     private final Consumer<Boolean>[] roundCompleteListeners;
     private final Runnable[] iterationCompleteListeners;
 
     @SuppressWarnings("unchecked")
-    public LifeCycleEventPublisher(LifeCycleBuilder builder) {
-        this.setupIterationListeners = builder.setupIterationListeners.toArray(new IntConsumer[0]);
-        this.prepareForNextRoundListeners = builder.prepareForNextRoundListeners.toArray(new Runnable[0]);
-        this.transitsForRoundCompleteListeners = builder.transitsForRoundCompleteListeners.toArray(new Runnable[0]);
-        this.transfersForRoundCompleteListeners = builder.transfersForRoundCompleteListeners.toArray(new Runnable[0]);
-        this.roundCompleteListeners = builder.roundCompleteListeners.toArray(new Consumer[0]);
-        this.iterationCompleteListeners = builder.iterationCompleteListeners.toArray(new Runnable[0]);
+    public LifeCycleEventPublisher(LifeCycleSubscriptions subscriptions) {
+        this.setupIterationListeners = subscriptions.setupIterationListeners.toArray(new IntConsumer[0]);
+        this.prepareForNextRoundListeners = subscriptions.prepareForNextRoundListeners.toArray(new IntConsumer[0]);
+        this.transitsForRoundCompleteListeners = subscriptions.transitsForRoundCompleteListeners.toArray(new Runnable[0]);
+        this.transfersForRoundCompleteListeners = subscriptions.transfersForRoundCompleteListeners.toArray(new Runnable[0]);
+        this.roundCompleteListeners = subscriptions.roundCompleteListeners.toArray(new Consumer[0]);
+        this.iterationCompleteListeners = subscriptions.iterationCompleteListeners.toArray(new Runnable[0]);
     }
 
     /* Lifecycle methods invoked by the Range Raptor Worker */
@@ -33,9 +33,9 @@ public final void setupIteration(int iterationDepartureTime) {
         }
     }
 
-    public final void prepareForNextRound() {
-        for (Runnable it : prepareForNextRoundListeners) {
-            it.run();
+    public final void prepareForNextRound(final int round) {
+        for (IntConsumer it : prepareForNextRoundListeners) {
+            it.accept(round);
         }
     }