Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Enable mode-specific transfers by storing mode information in transfers #6293

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Map;
import org.opentripplanner.ext.flex.trip.FlexTrip;
import org.opentripplanner.model.PathTransfer;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.transit.model.framework.FeedScopedId;
import org.opentripplanner.transit.model.network.Route;
import org.opentripplanner.transit.model.site.GroupStop;
Expand All @@ -18,15 +19,19 @@ public class FlexIndex {

private final Multimap<StopLocation, PathTransfer> transfersToStop = ArrayListMultimap.create();

private final Multimap<StopLocation, PathTransfer> transfersFromStop = ArrayListMultimap.create();

private final Multimap<StopLocation, FlexTrip<?, ?>> flexTripsByStop = HashMultimap.create();

private final Map<FeedScopedId, Route> routeById = new HashMap<>();

private final Map<FeedScopedId, FlexTrip<?, ?>> tripById = new HashMap<>();

public FlexIndex(TimetableRepository timetableRepository) {
for (PathTransfer transfer : timetableRepository.getAllPathTransfers()) {
// Flex transfers should only use WALK mode transfers.
for (PathTransfer transfer : timetableRepository.findTransfers(StreetMode.WALK)) {
transfersToStop.put(transfer.to, transfer);
transfersFromStop.put(transfer.from, transfer);
leonardehrenfried marked this conversation as resolved.
Show resolved Hide resolved
}
for (FlexTrip<?, ?> flexTrip : timetableRepository.getAllFlexTrips()) {
routeById.put(flexTrip.getTrip().getRoute().getId(), flexTrip.getTrip().getRoute());
Expand All @@ -47,6 +52,10 @@ public Collection<PathTransfer> getTransfersToStop(StopLocation stopLocation) {
return transfersToStop.get(stopLocation);
}

public Collection<PathTransfer> getTransfersFromStop(StopLocation stopLocation) {
return transfersFromStop.get(stopLocation);
}

public Collection<FlexTrip<?, ?>> getFlexTripsByStop(StopLocation stopLocation) {
return flexTripsByStop.get(stopLocation);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ public TransitStopVertex getStopVertexForStopId(FeedScopedId stopId) {

@Override
public Collection<PathTransfer> getTransfersFromStop(StopLocation stop) {
return transitService.findPathTransfers(stop);
return transitService.getFlexIndex().getTransfersFromStop(stop);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,10 @@ public RequestModes getRequestModes() {
mBuilder.withEgressMode(StreetMode.CAR_HAILING);
mBuilder.withDirectMode(StreetMode.WALK);
} else {
mBuilder.withAccessMode(StreetMode.WALK);
mBuilder.withTransferMode(StreetMode.WALK);
mBuilder.withEgressMode(StreetMode.WALK);
// This is necessary for transfer calculations.
mBuilder.withAccessMode(StreetMode.CAR);
mBuilder.withTransferMode(StreetMode.CAR);
mBuilder.withEgressMode(StreetMode.CAR);
mBuilder.withDirectMode(StreetMode.CAR);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimaps;
import java.time.Duration;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import org.opentripplanner.framework.application.OTPFeature;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
import org.opentripplanner.graph_builder.issues.StopNotLinkedForTransfers;
Expand All @@ -17,6 +20,7 @@
import org.opentripplanner.graph_builder.module.nearbystops.StreetNearbyStopFinder;
import org.opentripplanner.model.PathTransfer;
import org.opentripplanner.routing.api.request.RouteRequest;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.routing.graphfinder.NearbyStop;
import org.opentripplanner.street.model.edge.Edge;
Expand Down Expand Up @@ -86,6 +90,17 @@ public void buildGraph() {
HashMultimap.create()
);

List<RouteRequest> flexTransferRequests = new ArrayList<>();
// Flex transfer requests only use the WALK mode.
if (OTPFeature.FlexRouting.isOn()) {
flexTransferRequests.addAll(
transferRequests
.stream()
.filter(transferProfile -> transferProfile.journey().transfer().mode() == StreetMode.WALK)
.toList()
);
}

stops
.stream()
.parallel()
Expand All @@ -101,7 +116,9 @@ public void buildGraph() {

LOG.debug("Linking stop '{}' {}", stop, ts0);

// Calculate default transfers.
for (RouteRequest transferProfile : transferRequests) {
StreetMode mode = transferProfile.journey().transfer().mode();
for (NearbyStop sd : nearbyStopFinder.findNearbyStops(
ts0,
transferProfile,
Expand All @@ -115,31 +132,51 @@ public void buildGraph() {
if (sd.stop.transfersNotAllowed()) {
continue;
}
distinctTransfers.put(
new TransferKey(stop, sd.stop, sd.edges),
new PathTransfer(stop, sd.stop, sd.distance, sd.edges)
);
TransferKey transferKey = new TransferKey(stop, sd.stop, sd.edges);
PathTransfer pathTransfer = distinctTransfers.get(transferKey);
if (pathTransfer == null) {
// If the PathTransfer can't be found, it is created.
distinctTransfers.put(
transferKey,
new PathTransfer(stop, sd.stop, sd.distance, sd.edges, EnumSet.of(mode))
);
} else {
// If the PathTransfer is found, a new PathTransfer with the added mode is created.
distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode));
}
}
if (OTPFeature.FlexRouting.isOn()) {
// This code is for finding transfers from AreaStops to Stops, transfers
// from Stops to AreaStops and between Stops are already covered above.
for (NearbyStop sd : nearbyStopFinder.findNearbyStops(
ts0,
transferProfile,
transferProfile.journey().transfer(),
true
)) {
// Skip the origin stop, loop transfers are not needed.
if (sd.stop == stop) {
continue;
}
if (sd.stop instanceof RegularStop) {
continue;
}
}
// Calculate flex transfers if flex routing is enabled.
for (RouteRequest transferProfile : flexTransferRequests) {
// Flex transfer requests only use the WALK mode.
StreetMode mode = StreetMode.WALK;
// This code is for finding transfers from AreaStops to Stops, transfers
// from Stops to AreaStops and between Stops are already covered above.
for (NearbyStop sd : nearbyStopFinder.findNearbyStops(
ts0,
transferProfile,
transferProfile.journey().transfer(),
true
)) {
// Skip the origin stop, loop transfers are not needed.
if (sd.stop == stop) {
continue;
}
if (sd.stop instanceof RegularStop) {
continue;
}
// The TransferKey and PathTransfer are created differently for flex routing.
TransferKey transferKey = new TransferKey(sd.stop, stop, sd.edges);
PathTransfer pathTransfer = distinctTransfers.get(transferKey);
if (pathTransfer == null) {
// If the PathTransfer can't be found, it is created.
distinctTransfers.put(
new TransferKey(sd.stop, stop, sd.edges),
new PathTransfer(sd.stop, stop, sd.distance, sd.edges)
transferKey,
new PathTransfer(sd.stop, stop, sd.distance, sd.edges, EnumSet.of(mode))
);
} else {
// If the PathTransfer is found, a new PathTransfer with the added mode is created.
distinctTransfers.put(transferKey, pathTransfer.withAddedMode(mode));
}
}
}
Expand Down Expand Up @@ -172,6 +209,17 @@ public void buildGraph() {
nTransfersTotal,
nLinkedStops
);
transferRequests
.stream()
.map(transferProfile -> transferProfile.journey().transfer().mode())
.distinct()
.forEach(mode ->
LOG.info(
"Created {} transfers for mode {}.",
timetableRepository.findTransfers(mode).size(),
mode
)
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
package org.opentripplanner.model;

import java.io.Serializable;
import java.util.EnumSet;
import java.util.List;
import org.opentripplanner.model.transfer.ConstrainedTransfer;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.transit.model.site.StopLocation;
import org.opentripplanner.utils.tostring.ToStringBuilder;

/**
* Represents a transfer between stops with the street network path attatched to it.
* Represents a transfer for a set of modes between stops with the street network path attached to it.
* <p>
* Do not confuse this with {@link ConstrainedTransfer}.
*
* <p>
* TODO these should really have a set of valid modes in case bike vs. walk transfers are different
* TODO Should we just store the NearbyStop as a field here, or even switch to using it instead
* where this class is used
*/
Expand All @@ -27,11 +28,20 @@ public class PathTransfer implements Serializable {

private final List<Edge> edges;

public PathTransfer(StopLocation from, StopLocation to, double distanceMeters, List<Edge> edges) {
private final EnumSet<StreetMode> modes;

public PathTransfer(
StopLocation from,
StopLocation to,
double distanceMeters,
List<Edge> edges,
EnumSet<StreetMode> modes
) {
this.from = from;
this.to = to;
this.distanceMeters = distanceMeters;
this.edges = edges;
this.modes = modes;
}

public String getName() {
Expand All @@ -43,7 +53,18 @@ public double getDistanceMeters() {
}

public List<Edge> getEdges() {
return this.edges;
return edges;
}

public EnumSet<StreetMode> getModes() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to expose the mutable EnumSet. Can you replace it with a method like allows(CAR)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the getModes method to clone the enum set

return EnumSet.copyOf(modes);
}

/** Create a new PathTransfer based on the current one with the mode added to the valid modes. */
public PathTransfer withAddedMode(StreetMode mode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc please.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

EnumSet<StreetMode> newModes = EnumSet.copyOf(modes);
newModes.add(mode);
return new PathTransfer(from, to, distanceMeters, edges, newModes);
}

@Override
Expand All @@ -54,6 +75,7 @@ public String toString() {
.addObj("to", to)
.addNum("distance", distanceMeters)
.addColSize("edges", edges)
.addColSize("modes", modes)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ else if (pathLeg.isTransferLeg()) {
legs.addAll(
mapTransferLeg(
pathLeg.asTransferLeg(),
transferMode == StreetMode.BIKE ? TraverseMode.BICYCLE : TraverseMode.WALK
StreetModeToTransferTraverseModeMapper.map(transferMode)
)
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.opentripplanner.routing.algorithm.mapping;

import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.street.search.TraverseMode;

/**
* Maps street mode to transfer traverse mode.
*/
public class StreetModeToTransferTraverseModeMapper {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that you extraced the mapper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks


public static TraverseMode map(StreetMode mode) {
return switch (mode) {
case WALK -> TraverseMode.WALK;
case BIKE -> TraverseMode.BICYCLE;
case CAR -> TraverseMode.CAR;
default -> throw new IllegalArgumentException(
String.format("StreetMode %s can not be mapped to a TraverseMode for transfers.", mode)
);
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.List;
import java.util.function.Function;
import org.opentripplanner.raptor.api.model.RaptorTransfer;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.street.search.request.StreetSearchRequest;

public class RaptorTransferIndex {
Expand All @@ -29,9 +30,9 @@ public static RaptorTransferIndex create(
) {
var forwardTransfers = new ArrayList<List<RaptorTransfer>>(transfersByStopIndex.size());
var reversedTransfers = new ArrayList<List<RaptorTransfer>>(transfersByStopIndex.size());
StreetMode mode = request.mode();

for (int i = 0; i < transfersByStopIndex.size(); i++) {
forwardTransfers.add(new ArrayList<>());
reversedTransfers.add(new ArrayList<>());
}

Expand All @@ -41,13 +42,14 @@ public static RaptorTransferIndex create(
var transfers = transfersByStopIndex
.get(fromStop)
.stream()
.filter(transfer -> transfer.allowsMode(mode))
.flatMap(s -> s.asRaptorTransfer(request).stream())
.collect(
toMap(RaptorTransfer::stop, Function.identity(), (a, b) -> a.c1() < b.c1() ? a : b)
)
.values();

forwardTransfers.get(fromStop).addAll(transfers);
forwardTransfers.add(new ArrayList<>(transfers));

for (RaptorTransfer forwardTransfer : transfers) {
reversedTransfers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import org.locationtech.jts.geom.Coordinate;
import org.opentripplanner.raptor.api.model.RaptorCostConverter;
import org.opentripplanner.raptor.api.model.RaptorTransfer;
import org.opentripplanner.routing.api.request.StreetMode;
import org.opentripplanner.routing.api.request.preference.WalkPreferences;
import org.opentripplanner.street.model.edge.Edge;
import org.opentripplanner.street.search.request.StreetSearchRequest;
Expand All @@ -31,16 +34,20 @@ public class Transfer {

private final List<Edge> edges;

public Transfer(int toStop, List<Edge> edges) {
private final Set<StreetMode> modes;

public Transfer(int toStop, List<Edge> edges, EnumSet<StreetMode> modes) {
this.toStop = toStop;
this.edges = edges;
this.distanceMeters = (int) edges.stream().mapToDouble(Edge::getDistanceMeters).sum();
this.modes = Collections.unmodifiableSet(modes);
}

public Transfer(int toStopIndex, int distanceMeters) {
public Transfer(int toStopIndex, int distanceMeters, EnumSet<StreetMode> modes) {
this.toStop = toStopIndex;
this.distanceMeters = distanceMeters;
this.edges = null;
this.modes = Collections.unmodifiableSet(modes);
}

public List<Coordinate> getCoordinates() {
Expand Down Expand Up @@ -68,6 +75,11 @@ public List<Edge> getEdges() {
return edges;
}

/** Check if the given mode is a valid mode for the transfer. */
public boolean allowsMode(StreetMode mode) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Javadoc here and then we are good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix this next week. Btw does this need the bump serialization id label?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. You are storing a new field into the graph.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now has a comment

return modes.contains(mode);
}

public Optional<RaptorTransfer> asRaptorTransfer(StreetSearchRequest request) {
WalkPreferences walkPreferences = request.preferences().walk();
if (edges == null || edges.isEmpty()) {
Expand Down
Loading
Loading