diff --git a/docs/BuildConfiguration.md b/docs/BuildConfiguration.md index 7d2d90ea41b..cefd8d2cf2f 100644 --- a/docs/BuildConfiguration.md +++ b/docs/BuildConfiguration.md @@ -731,7 +731,7 @@ but we want to calculate the transfers always from OSM data. **Since version:** `2.3` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"allowed"` **Path:** /gtfsDefaults -**Enum values:** `discouraged` | `allowed` | `recommended` | `preferred` +**Enum values:** `preferred` | `recommended` | `allowed` | `discouraged` Should there be some preference or aversion for transfers at stops that are part of a station. @@ -980,7 +980,7 @@ but we want to calculate the transfers always from OSM data. **Since version:** `2.3` ∙ **Type:** `enum` ∙ **Cardinality:** `Optional` ∙ **Default value:** `"allowed"` **Path:** /transitFeeds/[0] -**Enum values:** `discouraged` | `allowed` | `recommended` | `preferred` +**Enum values:** `preferred` | `recommended` | `allowed` | `discouraged` Should there be some preference or aversion for transfers at stops that are part of a station. Overrides the value specified in `gtfsDefaults`. diff --git a/docs/RouterConfiguration.md b/docs/RouterConfiguration.md index 19a70c44d0f..5f588cf99c9 100644 --- a/docs/RouterConfiguration.md +++ b/docs/RouterConfiguration.md @@ -363,7 +363,7 @@ for more info." **Since version:** `2.0` ∙ **Type:** `enum map of integer` ∙ **Cardinality:** `Optional` **Path:** /transit -**Enum keys:** `discouraged` | `allowed` | `recommended` | `preferred` +**Enum keys:** `preferred` | `recommended` | `allowed` | `discouraged` Costs for boarding and alighting during transfers at stops with a given transfer priority. diff --git a/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParameters.java b/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParameters.java index fa23e02bac5..68b8874d5e6 100644 --- a/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParameters.java +++ b/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParameters.java @@ -24,9 +24,6 @@ public record GtfsFeedParameters( implements DataSourceConfig { public static final boolean DEFAULT_REMOVE_REPEATED_STOPS = true; - public static final StopTransferPriority DEFAULT_STATION_TRANSFER_PREFERENCE = - StopTransferPriority.ALLOWED; - public static final boolean DEFAULT_DISCARD_MIN_TRANSFER_TIMES = false; public static final boolean DEFAULT_BLOCK_BASED_INTERLINING = true; diff --git a/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParametersBuilder.java b/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParametersBuilder.java index 9dd3ad3a25e..1e799dbecbb 100644 --- a/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParametersBuilder.java +++ b/src/main/java/org/opentripplanner/gtfs/graphbuilder/GtfsFeedParametersBuilder.java @@ -18,7 +18,7 @@ public class GtfsFeedParametersBuilder { public GtfsFeedParametersBuilder() { this.removeRepeatedStops = GtfsFeedParameters.DEFAULT_REMOVE_REPEATED_STOPS; - this.stationTransferPreference = GtfsFeedParameters.DEFAULT_STATION_TRANSFER_PREFERENCE; + this.stationTransferPreference = StopTransferPriority.defaultValue(); this.discardMinTransferTimes = GtfsFeedParameters.DEFAULT_DISCARD_MIN_TRANSFER_TIMES; this.blockBasedInterlining = GtfsFeedParameters.DEFAULT_BLOCK_BASED_INTERLINING; this.maxInterlineDistance = GtfsFeedParameters.DEFAULT_MAX_INTERLINE_DISTANCE; diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java index 3cb259d3a67..74a5d7a6352 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransfersMapper.java @@ -19,6 +19,11 @@ static List> mapTransfers(StopModel stopModel, TransitModel trans for (int i = 0; i < stopModel.stopIndexSize(); ++i) { var stop = stopModel.stopByIndex(i); + + if (stop == null) { + continue; + } + ArrayList list = new ArrayList<>(); for (PathTransfer pathTransfer : transitModel.getTransfersByStop(stop)) { diff --git a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java index 210c8b42066..33a076bb8d6 100644 --- a/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java +++ b/src/main/java/org/opentripplanner/routing/algorithm/raptoradapter/transit/mappers/TransitLayerMapper.java @@ -190,12 +190,21 @@ static int[] createStopBoardAlightTransferCosts( if (!tuningParams.enableStopTransferPriority()) { return null; } + int defaultCost = RaptorCostConverter.toRaptorCost( + tuningParams.stopBoardAlightDuringTransferCost(StopTransferPriority.defaultValue()) + ); int[] stopBoardAlightTransferCosts = new int[stops.stopIndexSize()]; for (int i = 0; i < stops.stopIndexSize(); ++i) { - StopTransferPriority priority = stops.stopByIndex(i).getPriority(); - int domainCost = tuningParams.stopBoardAlightDuringTransferCost(priority); - stopBoardAlightTransferCosts[i] = RaptorCostConverter.toRaptorCost(domainCost); + // There can be holes in the stop index, so we need to account for 'null' here. + var stop = stops.stopByIndex(i); + if (stop == null) { + stopBoardAlightTransferCosts[i] = defaultCost; + } else { + var priority = stop.getPriority(); + int domainCost = tuningParams.stopBoardAlightDuringTransferCost(priority); + stopBoardAlightTransferCosts[i] = RaptorCostConverter.toRaptorCost(domainCost); + } } return stopBoardAlightTransferCosts; } diff --git a/src/main/java/org/opentripplanner/transit/model/site/RegularStop.java b/src/main/java/org/opentripplanner/transit/model/site/RegularStop.java index 4c1638e7bae..9f3327beff9 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/RegularStop.java +++ b/src/main/java/org/opentripplanner/transit/model/site/RegularStop.java @@ -127,7 +127,9 @@ public Point getGeometry() { @Override @Nonnull public StopTransferPriority getPriority() { - return isPartOfStation() ? getParentStation().getPriority() : StopTransferPriority.ALLOWED; + return isPartOfStation() + ? getParentStation().getPriority() + : StopTransferPriority.defaultValue(); } @Override diff --git a/src/main/java/org/opentripplanner/transit/model/site/Station.java b/src/main/java/org/opentripplanner/transit/model/site/Station.java index 3bfbcb7594e..68ce2d6d5a2 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/Station.java +++ b/src/main/java/org/opentripplanner/transit/model/site/Station.java @@ -30,8 +30,6 @@ public class Station extends AbstractTransitEntity implements StopLocationsGroup, LogInfo { - public static final StopTransferPriority DEFAULT_PRIORITY = StopTransferPriority.ALLOWED; - private final I18NString name; private final String code; private final I18NString description; @@ -52,7 +50,8 @@ public class Station // Required fields this.name = Objects.requireNonNull(builder.getName()); this.coordinate = Objects.requireNonNull(builder.getCoordinate()); - this.priority = Objects.requireNonNullElse(builder.getPriority(), DEFAULT_PRIORITY); + this.priority = + Objects.requireNonNullElse(builder.getPriority(), StopTransferPriority.defaultValue()); this.transfersNotAllowed = builder.isTransfersNotAllowed(); // Optional fields diff --git a/src/main/java/org/opentripplanner/transit/model/site/StopLocation.java b/src/main/java/org/opentripplanner/transit/model/site/StopLocation.java index a3cbd1a8014..9cb4411437c 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/StopLocation.java +++ b/src/main/java/org/opentripplanner/transit/model/site/StopLocation.java @@ -142,7 +142,7 @@ default ZoneId getTimeZone() { @Nonnull default StopTransferPriority getPriority() { - return StopTransferPriority.ALLOWED; + return StopTransferPriority.defaultValue(); } boolean isPartOfSameStationAs(StopLocation alternativeStop); diff --git a/src/main/java/org/opentripplanner/transit/model/site/StopTransferPriority.java b/src/main/java/org/opentripplanner/transit/model/site/StopTransferPriority.java index 1bc214fd47e..0e072c19ed6 100644 --- a/src/main/java/org/opentripplanner/transit/model/site/StopTransferPriority.java +++ b/src/main/java/org/opentripplanner/transit/model/site/StopTransferPriority.java @@ -9,31 +9,35 @@ */ public enum StopTransferPriority { /** - * Block transfers from/to this stop. In OTP this is not a definitive block, just a huge penalty - * is added to the cost function. + * Preferred place to transfer, strongly recommended. *

- * NeTEx equivalent is NO_INTERCHANGE. + * NeTEx equivalent is PREFERRED_INTERCHANGE. */ - DISCOURAGED, - + PREFERRED, + /** + * Recommended stop place. + *

+ * NeTEx equivalent is RECOMMENDED_INTERCHANGE. + */ + RECOMMENDED, /** * Allow transfers from/to this stop. This is the default. *

* NeTEx equivalent is INTERCHANGE_ALLOWED. */ ALLOWED, - /** - * Recommended stop place. + * Block transfers from/to this stop. In OTP this is not a definitive block, just a huge penalty + * is added to the cost function. *

- * NeTEx equivalent is RECOMMENDED_INTERCHANGE. + * NeTEx equivalent is NO_INTERCHANGE. */ - RECOMMENDED, + DISCOURAGED; /** - * Preferred place to transfer, strongly recommended. - *

- * NeTEx equivalent is PREFERRED_INTERCHANGE. + * The {@link #ALLOWED} is used as default value in cases where the value is not set. */ - PREFERRED, + public static StopTransferPriority defaultValue() { + return ALLOWED; + } } diff --git a/src/main/java/org/opentripplanner/transit/service/StopModel.java b/src/main/java/org/opentripplanner/transit/service/StopModel.java index 763aa504c40..4f55ca25b5b 100644 --- a/src/main/java/org/opentripplanner/transit/service/StopModel.java +++ b/src/main/java/org/opentripplanner/transit/service/StopModel.java @@ -162,6 +162,7 @@ public Collection listGroupStops() { return groupStopById.values(); } + @Nullable public StopLocation stopByIndex(int stopIndex) { return index.stopByIndex(stopIndex); } diff --git a/src/main/java/org/opentripplanner/transit/service/StopModelIndex.java b/src/main/java/org/opentripplanner/transit/service/StopModelIndex.java index 16337d980a1..c12c6f715f7 100644 --- a/src/main/java/org/opentripplanner/transit/service/StopModelIndex.java +++ b/src/main/java/org/opentripplanner/transit/service/StopModelIndex.java @@ -1,8 +1,11 @@ package org.opentripplanner.transit.service; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Objects; +import javax.annotation.Nullable; import org.locationtech.jts.geom.Envelope; import org.opentripplanner.framework.collection.CollectionsView; import org.opentripplanner.framework.geometry.HashGridSpatialIndex; @@ -12,6 +15,9 @@ import org.opentripplanner.transit.model.site.RegularStop; import org.opentripplanner.transit.model.site.Station; import org.opentripplanner.transit.model.site.StopLocation; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.event.Level; /** * Indexed access to Stop entities. @@ -20,6 +26,8 @@ */ class StopModelIndex { + private static final Logger LOG = LoggerFactory.getLogger(StopModelIndex.class); + private final HashGridSpatialIndex regularStopSpatialIndex = new HashGridSpatialIndex<>(); private final Map multiModalStationForStations = new HashMap<>(); private final HashGridSpatialIndex locationIndex = new HashGridSpatialIndex<>(); @@ -58,6 +66,8 @@ class StopModelIndex { // Trim the sizes of the indices regularStopSpatialIndex.compact(); locationIndex.compact(); + + logHolesInIndex(); } /** @@ -71,6 +81,7 @@ MultiModalStation getMultiModalStationForStation(Station station) { return multiModalStationForStations.get(station); } + @Nullable StopLocation stopByIndex(int index) { return stopsByIndex[index]; } @@ -82,4 +93,19 @@ int stopIndexSize() { Collection findAreaStops(Envelope envelope) { return locationIndex.query(envelope); } + + /** + * A small number of holes in the stop-index is ok, but if there are many, it will affect + * the Raptor performance. + */ + private void logHolesInIndex() { + int c = (int) Arrays.stream(stopsByIndex).filter(Objects::isNull).count(); + if (c > 0) { + double p = (100.0 * c) / stopsByIndex.length; + // Log this as warning if more than 5% of the space is null + LOG + .atLevel(p >= 5.0 ? Level.WARN : Level.INFO) + .log("The stop index contains holes in it. {} of {} is null.", c, stopsByIndex.length); + } + } }