From c0ad73ed51f35bc7b8cefd41324673a9c71ac2ef Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Fri, 13 Nov 2020 11:14:39 -0500 Subject: [PATCH 01/46] refactor(point-to-point): remove unnecessary CORS headers, all use of point-to-point router server is internal --- .../PointToPointRouterServer.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java b/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java index 6dc06c770..4393999e3 100644 --- a/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java +++ b/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java @@ -154,26 +154,6 @@ private static void run(TransportNetwork transportNetwork) { PointToPointQuery pointToPointQuery = new PointToPointQuery(transportNetwork); ParetoServer paretoServer = new ParetoServer(transportNetwork); - // add cors header - before((req, res) -> res.header("Access-Control-Allow-Origin", "*")); - - options("/*", (request, response) -> { - - String accessControlRequestHeaders = request - .headers("Access-Control-Request-Headers"); - if (accessControlRequestHeaders != null) { - response.header("Access-Control-Allow-Headers", accessControlRequestHeaders); - } - - String accessControlRequestMethod = request - .headers("Access-Control-Request-Method"); - if (accessControlRequestMethod != null) { - response.header("Access-Control-Allow-Methods", accessControlRequestMethod); - } - - return "OK"; - }); - get("/metadata", (request, response) -> { response.header("Content-Type", "application/json"); RouterInfo routerInfo = new RouterInfo(); From f4e2823e65fa63c54cfb8fcef0f244d32530b732 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sat, 11 Jul 2020 18:35:28 -0400 Subject: [PATCH 02/46] feat(gtfs): read some components of gtfs-fares v2 (#124) --- src/main/java/com/conveyal/gtfs/GTFSFeed.java | 55 +++++--- .../com/conveyal/gtfs/model/FareArea.java | 67 ++++++++++ .../com/conveyal/gtfs/model/FareLegRule.java | 121 ++++++++++++++++++ .../com/conveyal/gtfs/model/FareNetwork.java | 49 +++++++ .../conveyal/gtfs/model/FareTransferRule.java | 81 ++++++++++++ 5 files changed, 355 insertions(+), 18 deletions(-) create mode 100644 src/main/java/com/conveyal/gtfs/model/FareArea.java create mode 100644 src/main/java/com/conveyal/gtfs/model/FareLegRule.java create mode 100644 src/main/java/com/conveyal/gtfs/model/FareNetwork.java create mode 100644 src/main/java/com/conveyal/gtfs/model/FareTransferRule.java diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index 588638c24..3c75afa44 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -1,24 +1,7 @@ package com.conveyal.gtfs; import com.conveyal.gtfs.error.GTFSError; -import com.conveyal.gtfs.model.Agency; -import com.conveyal.gtfs.model.Calendar; -import com.conveyal.gtfs.model.CalendarDate; -import com.conveyal.gtfs.model.Entity; -import com.conveyal.gtfs.model.Fare; -import com.conveyal.gtfs.model.FareAttribute; -import com.conveyal.gtfs.model.FareRule; -import com.conveyal.gtfs.model.FeedInfo; -import com.conveyal.gtfs.model.Frequency; -import com.conveyal.gtfs.model.Pattern; -import com.conveyal.gtfs.model.Route; -import com.conveyal.gtfs.model.Service; -import com.conveyal.gtfs.model.Shape; -import com.conveyal.gtfs.model.ShapePoint; -import com.conveyal.gtfs.model.Stop; -import com.conveyal.gtfs.model.StopTime; -import com.conveyal.gtfs.model.Transfer; -import com.conveyal.gtfs.model.Trip; +import com.conveyal.gtfs.model.*; import com.conveyal.gtfs.validator.service.GeoUtils; import com.google.common.collect.HashMultimap; import com.google.common.collect.Iterables; @@ -144,6 +127,18 @@ public class GTFSFeed implements Cloneable, Closeable { /** A fare is a fare_attribute and all fare_rules that reference that fare_attribute. TODO what is the path? */ public final Map fares; + /** GTFS-Fares V2: One entry per fare area, containing all the rows for that fare area */ + public final Map fare_areas; + + /** GTFS-Fares V2: One entry per fare network, containing all members of that network */ + public final Map fare_networks; + + /** GTFS Fares V2: Fare leg rules */ + public final NavigableSet fare_leg_rules; + + /** GTFS Fares V2: Fare transfer rules */ + public final NavigableSet fare_transfer_rules; + /** A service is a calendar entry and all calendar_dates that modify that calendar entry. TODO what is the path? */ public final BTreeMap services; @@ -164,6 +159,8 @@ public class GTFSFeed implements Cloneable, Closeable { /** Map from each trip_id to ID of trip pattern containing that trip. */ public final Map patternForTrip; + /** Map from + /** Once a GTFSFeed has one feed loaded into it, we set this to true to block loading any additional feeds. */ private boolean loaded = false; @@ -228,6 +225,24 @@ else if (feedId == null || feedId.isEmpty()) { // Joined Fares have been persisted to MapDB. Release in-memory HashMap for garbage collection. fares = null; + // Read GTFS-Fares V2 + + // FareAreas are joined into a single object for each FareArea. Use an in-memory map since + // there will be a lot of changing of values that are immutable once placed in MapDB. + Map fare_areas = new HashMap<>(); + new FareArea.Loader(this, fare_areas).loadTable(zip); + this.fare_areas.putAll(fare_areas); + fare_areas = null; // allow gc + + // FareNetworks are likewise joined into single objects + Map fare_networks = new HashMap<>(); + new FareNetwork.Loader(this, fare_networks).loadTable(zip); + this.fare_networks.putAll(fare_networks); + fare_networks = null; // allow gc + + new FareLegRule.Loader(this).loadTable(zip); + new FareTransferRule.Loader(this).loadTable(zip); + // Comment out the StopTime and/or ShapePoint loaders for quick testing on large feeds. new Route.Loader(this).loadTable(zip); new ShapePoint.Loader(this).loadTable(zip); @@ -812,6 +827,10 @@ private GTFSFeed (DB db) { fares = db.getTreeMap("fares"); services = db.getTreeMap("services"); shape_points = db.getTreeMap("shape_points"); + fare_areas = db.getTreeMap("fare_areas"); + fare_networks = db.getTreeMap("fare_networks"); + fare_leg_rules = db.getTreeSet("fare_leg_rules"); + fare_transfer_rules = db.getTreeSet("fare_transfer_rules"); // Note that the feedId and checksum fields are manually read in and out of entries in the MapDB, rather than // the class fields themselves being of type Atomic.String and Atomic.Long. This avoids any locking and diff --git a/src/main/java/com/conveyal/gtfs/model/FareArea.java b/src/main/java/com/conveyal/gtfs/model/FareArea.java new file mode 100644 index 000000000..d7b3f5791 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/model/FareArea.java @@ -0,0 +1,67 @@ +package com.conveyal.gtfs.model; + +import com.conveyal.gtfs.GTFSFeed; + +import java.io.IOException; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Map; + +/** A FareArea represents a group of stops in the GTFS Fares V2 specification */ +public class FareArea extends Entity { + private static final long serialVersionUID = 1L; + + public String fare_area_id; + public String fare_area_name; + public String ticketing_fare_area_id; + public Collection members = new ArrayList<>(); + + public static class Loader extends Entity.Loader { + private Map fareAreas; + + public Loader (GTFSFeed feed, Map fareAreas) { + super(feed, "fare_areas"); + this.fareAreas = fareAreas; + } + + @Override + protected boolean isRequired() { + return false; + } + + @Override + protected void loadOneRow() throws IOException { + // Fare areas are composed of members that refer to specific stops or trip/stop combos + FareAreaMember member = new FareAreaMember(); + member.stop_id = getStringField("stop_id", false); + member.trip_id = getStringField("trip_id", false); + member.stop_sequence = getIntField("stop_sequence", false, 0, Integer.MAX_VALUE, INT_MISSING); + member.sourceFileLine = row + 1; + + String fareAreaId = getStringField("fare_area_id", true); + + FareArea fareArea; + if (fareAreas.containsKey(fareAreaId)) { + fareArea = fareAreas.get(fareAreaId); + // TODO make sure that fare_area_name, etc all match + } else { + fareArea = new FareArea(); + fareArea.fare_area_id = fareAreaId; + fareArea.fare_area_name = getStringField("fare_area_name", false); + fareArea.ticketing_fare_area_id = getStringField("ticketing_fare_area_id", false); + fareAreas.put(fareAreaId, fareArea); + } + fareArea.members.add(member); + } + } + + /** What are the members of this FareArea? */ + public static class FareAreaMember implements Serializable { + private static final long serialVersionUID = 1L; + public String stop_id; + public String trip_id; + public int stop_sequence; + public int sourceFileLine; + } +} diff --git a/src/main/java/com/conveyal/gtfs/model/FareLegRule.java b/src/main/java/com/conveyal/gtfs/model/FareLegRule.java new file mode 100644 index 000000000..6c1188a13 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/model/FareLegRule.java @@ -0,0 +1,121 @@ +package com.conveyal.gtfs.model; + +import com.conveyal.gtfs.GTFSFeed; +import com.google.common.collect.ComparisonChain; +import com.google.common.collect.Ordering; +import org.apache.commons.lang3.builder.CompareToBuilder; + +import java.io.IOException; +import java.util.Objects; + +/** + * A GTFS-Fares V2 FareLegRule + */ +public class FareLegRule extends Entity implements Comparable { + public static final long serialVersionUID = 1L; + + public int order; + public String fare_network_id; + public String from_area_id; + public String contains_area_id; + public String to_area_id; + public int is_symmetrical; + public String from_timeframe_id; + public String to_timeframe_id; + public double min_fare_distance; + public double max_fare_distance; + public String service_id; + public double amount; + public double min_amount; + public double max_amount; + public String currency; + public String leg_group_id; + + @Override + public int compareTo(Object other) { + FareLegRule o = (FareLegRule) other; + return ComparisonChain.start() + .compare(order, o.order) + .compare(fare_network_id, o.fare_network_id, Ordering.natural().nullsFirst()) + .compare(from_area_id, o.from_area_id, Ordering.natural().nullsFirst()) + .compare(contains_area_id, o.contains_area_id, Ordering.natural().nullsFirst()) + .compare(to_area_id, o.to_area_id, Ordering.natural().nullsFirst()) + .compare(is_symmetrical, o.is_symmetrical) + .compare(from_timeframe_id, o.from_timeframe_id, Ordering.natural().nullsFirst()) + .compare(to_timeframe_id, o.to_timeframe_id, Ordering.natural().nullsFirst()) + .compare(min_fare_distance, o.min_fare_distance) + .compare(max_fare_distance, o.max_fare_distance) + .compare(service_id, o.service_id, Ordering.natural().nullsFirst()) + .compare(amount, o.amount) + .compare(min_amount, o.min_amount) + .compare(max_amount, o.max_amount) + .compare(currency, o.currency, Ordering.natural().nullsFirst()) + .compare(leg_group_id, o.leg_group_id, Ordering.natural().nullsFirst()) + .result(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FareLegRule that = (FareLegRule) o; + return order == that.order && + is_symmetrical == that.is_symmetrical && + Double.compare(that.min_fare_distance, min_fare_distance) == 0 && + Double.compare(that.max_fare_distance, max_fare_distance) == 0 && + Double.compare(that.amount, amount) == 0 && + Double.compare(that.min_amount, min_amount) == 0 && + Double.compare(that.max_amount, max_amount) == 0 && + Objects.equals(fare_network_id, that.fare_network_id) && + Objects.equals(from_area_id, that.from_area_id) && + Objects.equals(contains_area_id, that.contains_area_id) && + Objects.equals(to_area_id, that.to_area_id) && + Objects.equals(from_timeframe_id, that.from_timeframe_id) && + Objects.equals(to_timeframe_id, that.to_timeframe_id) && + Objects.equals(service_id, that.service_id) && + Objects.equals(currency, that.currency) && + Objects.equals(leg_group_id, that.leg_group_id); + } + + @Override + public int hashCode() { + return Objects.hash(order, fare_network_id, from_area_id, contains_area_id, to_area_id, is_symmetrical, + from_timeframe_id, to_timeframe_id, min_fare_distance, max_fare_distance, service_id, amount, + min_amount, max_amount, currency, leg_group_id); + } + + public static class Loader extends Entity.Loader { + public Loader (GTFSFeed feed) { + super(feed, "fare_leg_rules"); + } + + @Override + protected boolean isRequired() { + return false; + } + + @Override + protected void loadOneRow() throws IOException { + FareLegRule rule = new FareLegRule(); + rule.sourceFileLine = row + 1; + rule.order = getIntField("order", true, 0, Integer.MAX_VALUE); + rule.fare_network_id = getStringField("fare_network_id", false); + rule.from_area_id = getStringField("from_area_id", false); + rule.to_area_id = getStringField("to_area_id", false); + rule.contains_area_id = getStringField("contains_area_id", false); + rule.is_symmetrical = getIntField("is_symmetrical", false, 0, 1, 0); + rule.from_timeframe_id = getStringField("from_timeframe_id", false); + rule.to_timeframe_id = getStringField("to_timeframe_id", false); + rule.min_fare_distance = getDoubleField("min_fare_distance", false, 0, Double.MAX_VALUE); + rule.max_fare_distance = getDoubleField("max_fare_distance", false, 0, Double.MAX_VALUE); + rule.service_id = getStringField("service_id", false); + rule.amount = getDoubleField("amount", false, 0, Double.MAX_VALUE); + rule.min_amount = getDoubleField("min_amount", false, 0, Double.MAX_VALUE); + rule.max_amount = getDoubleField("max_amount", false, 0, Double.MAX_VALUE); + rule.currency = getStringField("currency", true); + rule.leg_group_id = getStringField("leg_group_id", true); + + feed.fare_leg_rules.add(rule); + } + } +} diff --git a/src/main/java/com/conveyal/gtfs/model/FareNetwork.java b/src/main/java/com/conveyal/gtfs/model/FareNetwork.java new file mode 100644 index 000000000..179625cf7 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/model/FareNetwork.java @@ -0,0 +1,49 @@ +package com.conveyal.gtfs.model; + +import com.conveyal.gtfs.GTFSFeed; + +import java.io.IOException; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** GTFS-Fares V2 FareNetwork. Not represented exactly in GTFS, but a single entry for each FareNetwork */ +public class FareNetwork extends Entity { + public static final long serialVersionUID = 1L; + + public String fare_network_id; + public int as_route; + public Set route_ids = new HashSet<>(); + + public static class Loader extends Entity.Loader { + private Map fareNetworks; + + public Loader (GTFSFeed feed, Map fareNetworks) { + super(feed, "fare_networks"); + this.fareNetworks = fareNetworks; + } + + @Override + protected boolean isRequired() { + return false; + } + + @Override + protected void loadOneRow() throws IOException { + String fareNetworkId = getStringField("fare_network_id", true); + + FareNetwork fareNetwork; + if (fareNetworks.containsKey(fareNetworkId)) { + fareNetwork = fareNetworks.get(fareNetworkId); + // TODO confirm as_route is consistent + } else { + fareNetwork = new FareNetwork(); + fareNetwork.fare_network_id = fareNetworkId; + fareNetwork.as_route = getIntField("as_route", false, 0, 1, 0); + fareNetworks.put(fareNetworkId, fareNetwork); + } + + fareNetwork.route_ids.add(getStringField("route_id", true)); + } + } +} diff --git a/src/main/java/com/conveyal/gtfs/model/FareTransferRule.java b/src/main/java/com/conveyal/gtfs/model/FareTransferRule.java new file mode 100644 index 000000000..e5f47ebe0 --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/model/FareTransferRule.java @@ -0,0 +1,81 @@ +package com.conveyal.gtfs.model; + +import com.conveyal.gtfs.GTFSFeed; +import com.google.common.collect.ComparisonChain; +import com.google.common.collect.Ordering; + +import java.io.IOException; + +public class FareTransferRule extends Entity implements Comparable { + public static final long serialVersionUID = 1L; + + public int order; + public String from_leg_group_id; + public String to_leg_group_id; + public int is_symmetrical; // is_symetrical in the spec + public int spanning_limit; + public int duration_limit_type; + public int duration_limit; + public int fare_transfer_type; + public double amount; + public double min_amount; + public double max_amount; + public String currency; + + @Override + public int compareTo(Object other) { + FareTransferRule o = (FareTransferRule) other; + return ComparisonChain.start() + .compare(order, o.order) + .compare(from_leg_group_id, o.from_leg_group_id, Ordering.natural().nullsFirst()) + .compare(to_leg_group_id, o.to_leg_group_id, Ordering.natural().nullsFirst()) + .compare(is_symmetrical, o.is_symmetrical) + .compare(spanning_limit, o.spanning_limit) + .compare(duration_limit_type, o.duration_limit_type) + .compare(duration_limit, o.duration_limit) + .compare(fare_transfer_type, o.fare_transfer_type) + .compare(amount, o.amount) + .compare(min_amount, o.min_amount) + .compare(max_amount, o.max_amount) + .compare(currency, o.currency, Ordering.natural().nullsFirst()) + .result(); + } + + public static class Loader extends Entity.Loader { + public Loader (GTFSFeed feed) { + super(feed, "fare_transfer_rules"); + } + + @Override + protected boolean isRequired() { + return false; + } + + @Override + protected void loadOneRow() throws IOException { + FareTransferRule rule = new FareTransferRule(); + rule.sourceFileLine = row + 1; + rule.order = getIntField("order", true, 0, Integer.MAX_VALUE); + rule.from_leg_group_id = getStringField("from_leg_group_id", false); + rule.to_leg_group_id = getStringField("to_leg_group_id", false); + + // allow is_symmetrical to be misspelled is_symetrical due to typo in original spec + rule.is_symmetrical = getIntField("is_symmetrical", false, 0, 1, INT_MISSING); + if (rule.is_symmetrical == INT_MISSING) { + rule.is_symmetrical = getIntField("is_symetrical", false, 0, 1, 0); + } + + rule.spanning_limit = getIntField("spanning_limit", false, 0, 1, 0); + rule.duration_limit = getIntField("duration_limit", false, 0, Integer.MAX_VALUE); + rule.duration_limit_type = getIntField("duration_limit_type", false, 0, 2, 0); + rule.fare_transfer_type = getIntField("fare_transfer_type", false, 0, 2, INT_MISSING); + // can be less than zero to represent a discount (in fact, often will be) + rule.amount = getDoubleField("amount", false, -Double.MAX_VALUE, Double.MAX_VALUE); + rule.min_amount = getDoubleField("min_amount", false, -Double.MAX_VALUE, Double.MAX_VALUE); + rule.max_amount = getDoubleField("max_amount", false, -Double.MAX_VALUE, Double.MAX_VALUE); + rule.currency = getStringField("currency", false); + + feed.fare_transfer_rules.add(rule); + } + } +} From b5af23e0b371fd9feb0cdfbf98ce5cd7cdca2932 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sat, 11 Jul 2020 18:45:49 -0400 Subject: [PATCH 03/46] refactor(gtfs): remove * import --- src/main/java/com/conveyal/gtfs/GTFSFeed.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index 3c75afa44..63cf2eaa5 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -1,7 +1,28 @@ package com.conveyal.gtfs; import com.conveyal.gtfs.error.GTFSError; -import com.conveyal.gtfs.model.*; +import com.conveyal.gtfs.model.Agency; +import com.conveyal.gtfs.model.Calendar; +import com.conveyal.gtfs.model.CalendarDate; +import com.conveyal.gtfs.model.Entity; +import com.conveyal.gtfs.model.Fare; +import com.conveyal.gtfs.model.FareArea; +import com.conveyal.gtfs.model.FareAttribute; +import com.conveyal.gtfs.model.FareLegRule; +import com.conveyal.gtfs.model.FareNetwork; +import com.conveyal.gtfs.model.FareRule; +import com.conveyal.gtfs.model.FareTransferRule; +import com.conveyal.gtfs.model.FeedInfo; +import com.conveyal.gtfs.model.Frequency; +import com.conveyal.gtfs.model.Pattern; +import com.conveyal.gtfs.model.Route; +import com.conveyal.gtfs.model.Service; +import com.conveyal.gtfs.model.Shape; +import com.conveyal.gtfs.model.ShapePoint; +import com.conveyal.gtfs.model.Stop; +import com.conveyal.gtfs.model.StopTime; +import com.conveyal.gtfs.model.Transfer; +import com.conveyal.gtfs.model.Trip; import com.conveyal.gtfs.validator.service.GeoUtils; import com.google.common.collect.HashMultimap; import com.google.common.collect.Iterables; From 20ae03d6ab6308f14ee2b7137110e7f4fb156739 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 12 Jul 2020 16:01:26 -0400 Subject: [PATCH 04/46] feat(fares-v2): load subset of GTFS fares-v2 into transport network (#124) --- .../com/conveyal/r5/transit/TransitLayer.java | 275 +++++++++++++++++- .../conveyal/r5/transit/faresv2/Currency.java | 13 + .../r5/transit/faresv2/FareLegRuleInfo.java | 37 +++ .../transit/faresv2/FareTransferRuleInfo.java | 71 +++++ .../r5/transit/faresv2/package-info.java | 4 + 5 files changed, 391 insertions(+), 9 deletions(-) create mode 100644 src/main/java/com/conveyal/r5/transit/faresv2/Currency.java create mode 100644 src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java create mode 100644 src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java create mode 100644 src/main/java/com/conveyal/r5/transit/faresv2/package-info.java diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 3195e7f46..f9447b256 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -1,30 +1,27 @@ package com.conveyal.r5.transit; import com.conveyal.gtfs.GTFSFeed; -import com.conveyal.gtfs.model.Agency; -import com.conveyal.gtfs.model.Fare; -import com.conveyal.gtfs.model.Frequency; -import com.conveyal.gtfs.model.Route; -import com.conveyal.gtfs.model.Service; -import com.conveyal.gtfs.model.Shape; -import com.conveyal.gtfs.model.Stop; -import com.conveyal.gtfs.model.StopTime; -import com.conveyal.gtfs.model.Trip; +import com.conveyal.gtfs.model.*; import com.conveyal.r5.api.util.TransitModes; import com.conveyal.r5.common.GeometryUtils; import com.conveyal.r5.streets.EdgeStore; import com.conveyal.r5.streets.StreetRouter; import com.conveyal.r5.streets.VertexStore; +import com.conveyal.r5.transit.faresv2.FareLegRuleInfo; +import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo; import com.conveyal.r5.util.LambdaCounter; import com.conveyal.r5.util.LocationIndexedLineInLocalCoordinateSystem; import com.google.common.base.Strings; import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; +import gnu.trove.list.TDoubleList; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; import gnu.trove.map.TIntIntMap; +import gnu.trove.map.TIntObjectMap; import gnu.trove.map.TObjectIntMap; import gnu.trove.map.hash.TIntIntHashMap; +import gnu.trove.map.hash.TIntObjectHashMap; import gnu.trove.map.hash.TObjectIntHashMap; import gnu.trove.set.TIntSet; import gnu.trove.set.hash.TIntHashSet; @@ -81,6 +78,24 @@ public class TransitLayer implements Serializable, Cloneable { private static final Logger LOG = LoggerFactory.getLogger(TransitLayer.class); + /** + * The offset for fare areas. Since each stop is also a fare area, explicit fare areas are numbered starting with + * this to keep them from ever colliding with stop IDs, as long as there are less than 1 billion stops in the network. + * + * TODO if this were smaller would it speed serialization due to variable-width integer encoding? 1 billion might be + * excessive. + */ + public static final int EXPLICIT_FARE_AREA_OFFSET = 1_000_000_000; + + /** + * The offset for fare networks. Since each route is also a fare network, explicit fare networks are numbered starting with + * this to keep them from ever colliding with stop IDs, as long as there are less than 1 billion stops in the network. + */ + public static final int EXPLICIT_FARE_NETWORK_OFFSET = 1_000_000_000; + + /** Special int value used to indicate a field in GTFS-fares was left blank in the fare indexes */ + public static final int FARE_ID_BLANK = -1; + /** * The time zone in which this TransportNetwork falls. It is read from a GTFS agency. * It defaults to GMT if no valid time zone can be found. @@ -163,6 +178,57 @@ public class TransitLayer implements Serializable, Cloneable { public Map fares; + /** + * The fare areas for each stop, in GTFS Fares v2. + * + * This is a list of fare area IDs. Each stop is implicitly a fare area, so the list of fare areas for a stop always + * contains the stop ID. Explicit fare area IDs are made unique from stop int IDs by adding a large constant (10 million). + */ + public TIntObjectMap fareAreasForStop = new TIntObjectHashMap<>(); + + // TODO Fare Areas for trip, stop_sequence pair + + /** + * Is fare network EXPLICIT_FARE_NETWORK_OFFSET + i an as_route fare network (i.e. several legs within the network + * should be matched as a single journey?) + * + * Note that fare networks are offset to keep them distinct from integer route IDs since routes are also implicit + * fare networks. It is not recommended to query this directly but instead use getFareNetworkAsRoute(fareNetworkId) + * and setFareNetworkAsRoute(fareNetworkId) which handles the integer conversions. + */ + public BitSet fareNetworkAsRoute = new BitSet(); + + /** + * The fare leg rules for this transport network. + * If memory becomes a problem, FareLegRule could be replaced with a proxy class that only contains amount + */ + public List fareLegRules = new ArrayList<>(); + + // indices for fare leg rules. Note that each index also contains an entry for key FARE_ID_BLANK which indicates that + //the field was left blank + + /** Fare leg rule for fare network ID (either explicit or implicit) */ + public TIntObjectMap fareLegRulesForFareNetworkId = new TIntObjectHashMap<>(); + + /** Fare leg rule for from area id */ + public TIntObjectMap fareLegRulesForFromAreaId = new TIntObjectHashMap<>(); + + /** Fare leg rule for to area id */ + public TIntObjectMap fareLegRulesForToAreaId = new TIntObjectHashMap<>(); + + // TODO contains_area_id, leg_group_id, timeframes + + /** + * The fare transfer rules for this transport network. + */ + public List fareTransferRules = new ArrayList<>(); + + /** Fare transfer rule index for from_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty from_leg_group_id */ + public TIntObjectMap fareTransferRulesForFromLegGroupId = new TIntObjectHashMap<>(); + + /** Fare transfer rule index for to_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty to_leg_group_id */ + public TIntObjectMap fareTransferRulesForToLegGroupId = new TIntObjectHashMap<>(); + /** Map from feed ID to feed CRC32 to ensure that we can't apply scenarios to the wrong feeds */ public Map feedChecksums = new HashMap<>(); @@ -176,6 +242,19 @@ public class TransitLayer implements Serializable, Cloneable { */ public String scenarioId; + /** Does fare_network (which may be a route index or an explicit fare network index) have as_route set? */ + public boolean getFareNetworkAsRoute (int fareNetwork) { + if (fareNetwork < EXPLICIT_FARE_NETWORK_OFFSET) return false; + else return fareNetworkAsRoute.get(fareNetwork - EXPLICIT_FARE_NETWORK_OFFSET); + } + + /** Set fare_network as_route */ + public void setFareNetworkAsRoute (int fareNetwork, boolean as_route) { + if (fareNetwork < EXPLICIT_FARE_NETWORK_OFFSET) + throw new IllegalArgumentException("attempt to set as_route on implicit fare network."); + else fareNetworkAsRoute.set(fareNetwork - EXPLICIT_FARE_NETWORK_OFFSET, as_route); + } + /** Load a GTFS feed with full load level */ public void loadFromGtfs (GTFSFeed gtfs) throws DuplicateFeedException { loadFromGtfs(gtfs, LoadLevel.FULL); @@ -466,6 +545,184 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx // RouteTopology topology = new RouteTopology(routeAndDirection.first, routeAndDirection.second, patternsForRouteDirection.get(routeAndDirection)); // } + try { + // we only support a subset of Fares V2, and many exceptions may be thrown if the feed uses more than that + // subset. Still allow graph to build without fare information if that is the case. + loadFaresV2(gtfs, indexForUnscopedStopId); + } catch (Exception e) { + LOG.warn("Exception loading GTFS Fares V2, fare routing will not be available", e); + clearFaresV2(); + } + + if (feedChecksums.size() > 1) { + LOG.warn("Multiple feeds specified, GTFS-Fares V2 will be unavailable"); + clearFaresV2(); + } + } + + /** Clear GTFS-Fares V2 information after a load error */ + private void clearFaresV2 () { + fareLegRules.clear(); + fareTransferRules.clear(); + fareLegRulesForFromAreaId.clear(); + fareLegRulesForFareNetworkId.clear(); + fareLegRulesForToAreaId.clear(); + fareTransferRulesForFromLegGroupId.clear(); + fareTransferRulesForToLegGroupId.clear(); + fareAreasForStop.clear(); + fareNetworkAsRoute.clear(); + } + + /** Load GTFS-Fares V2 information from a feed */ + private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedStopId) { + LOG.info("Loading GTFS-Fares V2"); + + TObjectIntMap fareLegRuleForLegGroupId = new TObjectIntHashMap<>(); + TObjectIntMap fareNetworkForId = new TObjectIntHashMap<>(); + TObjectIntMap fareAreaForId = new TObjectIntHashMap<>(); + + LOG.info("Loading fare areas"); + for (int i = 0; i < stopIdForIndex.size(); i++) { + fareAreasForStop.put(i, new TIntArrayList()); + fareAreasForStop.get(i).add(i); // every stop is a fare area + fareAreaForId.put(stopIdForIndex.get(i), i); + } + + // TODO this will not work if there are multiple feeds + int fareAreaIdx = EXPLICIT_FARE_AREA_OFFSET; + for (FareArea fareArea : feed.fare_areas.values()) { + for (FareArea.FareAreaMember member : fareArea.members) { + if (member.trip_id != null) + throw new IllegalArgumentException("Trip-based fare area membership not supported!"); + + int stopIndex = indexForUnscopedStopId.get(member.stop_id); + fareAreasForStop.get(stopIndex).add(fareAreaIdx); + } + fareAreaForId.put(fareArea.fare_area_id, fareAreaIdx); + fareAreaIdx++; + } + + LOG.info("Loaded {} fare areas", fareAreaForId.size()); + + LOG.info("Loading fare networks"); + for (int i = 0; i < routes.size(); i++) { + fareNetworkForId.put(routes.get(i).route_id, i); + } + + // TODO this will not work if there are multiple feeds + int fareNetworkIdx = EXPLICIT_FARE_NETWORK_OFFSET; + for (FareNetwork fareNetwork : feed.fare_networks.values()) { + fareNetworkForId.put(fareNetwork.fare_network_id, fareNetworkIdx); + setFareNetworkAsRoute(fareNetworkIdx, fareNetwork.as_route == 1); + fareNetworkIdx++; + } + + LOG.info("Loaded {} fare networks", fareNetworkForId.size()); + + LOG.info("Loading fare leg rules"); + int fareLegRuleIdx = 0; + for (FareLegRule rule : feed.fare_leg_rules) { + fareLegRules.add(new FareLegRuleInfo(rule)); + if (rule.leg_group_id != null) fareLegRuleForLegGroupId.put(rule.leg_group_id, fareLegRuleIdx); + + // build indices + int fareNetworkId; + if (rule.fare_network_id != null) { + if (!fareNetworkForId.containsKey(rule.fare_network_id)) { + throw new IllegalArgumentException("Fare network ID referenced in fare_leg_rules not present: " + + rule.fare_network_id); + } + + fareNetworkId = fareNetworkForId.get(rule.fare_network_id); + } else fareNetworkId = FARE_ID_BLANK; + + if (!fareLegRulesForFareNetworkId.containsKey(fareNetworkId)) + fareLegRulesForFareNetworkId.put(fareNetworkId, new TIntHashSet()); + fareLegRulesForFareNetworkId.get(fareNetworkId).add(fareLegRuleIdx); + + int fromAreaIdx; + if (rule.from_area_id != null) { + if (!fareAreaForId.containsKey(rule.from_area_id)) { + throw new IllegalArgumentException("Fare area ID referenced in fare_leg_rules not present: " + + rule.from_area_id); + } + fromAreaIdx = fareAreaForId.get(rule.from_area_id); + } else fromAreaIdx = FARE_ID_BLANK; + + if (!fareLegRulesForFromAreaId.containsKey(fromAreaIdx)) { + fareLegRulesForFromAreaId.put(fromAreaIdx, new TIntHashSet()); + } + fareLegRulesForFromAreaId.get(fromAreaIdx).add(fareLegRuleIdx); + + int toAreaIdx; + if (rule.to_area_id != null) { + if (!fareAreaForId.containsKey(rule.to_area_id)) { + throw new IllegalArgumentException("Fare area ID referenced in fare_leg_rules not present: " + + rule.to_area_id); + } + toAreaIdx = fareAreaForId.get(rule.to_area_id); + } else toAreaIdx = FARE_ID_BLANK; + + if (!fareLegRulesForToAreaId.containsKey(toAreaIdx)) { + fareLegRulesForToAreaId.put(toAreaIdx, new TIntHashSet()); + } + fareLegRulesForToAreaId.get(toAreaIdx).add(fareLegRuleIdx); + + if (rule.is_symmetrical == 1) { + // add the same rule backwards + fareLegRulesForFromAreaId.get(toAreaIdx).add(fareLegRuleIdx); + fareLegRulesForToAreaId.get(fromAreaIdx).add(fareLegRuleIdx); + } + + if (rule.service_id != null) throw new IllegalArgumentException("Service IDs not supported in fare_leg_rules"); + if (rule.contains_area_id != null) throw new IllegalArgumentException("contains_area_id not supported in fare_leg_rules"); + if (rule.to_timeframe_id != null || rule.from_timeframe_id != null) + throw new IllegalArgumentException("timeframes not supported in fare_leg_rules"); + if (!Double.isNaN(rule.min_fare_distance) || !Double.isNaN(rule.max_fare_distance)) + throw new IllegalArgumentException("Fare distances not supported in fare_leg_rules"); + + fareLegRuleIdx++; + } + + LOG.info("Loaded {} fare leg rules", fareLegRuleIdx); + + LOG.info("Loading fare transfer rules"); + int fareTransferRuleIdx = 0; + for (FareTransferRule rule : feed.fare_transfer_rules) { + fareTransferRules.add(new FareTransferRuleInfo(rule)); + + int fromLegIdx; + if (rule.from_leg_group_id != null) { + if (!fareLegRuleForLegGroupId.containsKey(rule.from_leg_group_id)) { + throw new IllegalArgumentException("Fare leg group ID referenced in fare_transfer_rules not present: " + + rule.from_leg_group_id); + } + fromLegIdx = fareLegRuleForLegGroupId.get(rule.from_leg_group_id); + } else fromLegIdx = FARE_ID_BLANK; + + if (!fareTransferRulesForFromLegGroupId.containsKey(fromLegIdx)) { + fareTransferRulesForFromLegGroupId.put(fromLegIdx, new TIntHashSet()); + } + fareTransferRulesForFromLegGroupId.get(fromLegIdx).add(fareTransferRuleIdx); + + int toLegIdx; + if (rule.to_leg_group_id != null) { + if (!fareLegRuleForLegGroupId.containsKey(rule.to_leg_group_id)) { + throw new IllegalArgumentException("Fare leg group ID referenced in fare_transfer_rules not present: " + + rule.to_leg_group_id); + } + toLegIdx = fareLegRuleForLegGroupId.get(rule.to_leg_group_id); + } else toLegIdx = FARE_ID_BLANK; + + if (!fareTransferRulesForToLegGroupId.containsKey(toLegIdx)) { + fareTransferRulesForToLegGroupId.put(toLegIdx, new TIntHashSet()); + } + fareTransferRulesForToLegGroupId.get(toLegIdx).add(fareTransferRuleIdx); + + fareTransferRuleIdx++; + } + + LOG.info("Loaded {} fare transfer rules", fareTransferRuleIdx); } // The median of all stopTimes would be best but that involves sorting a huge list of numbers. diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/Currency.java b/src/main/java/com/conveyal/r5/transit/faresv2/Currency.java new file mode 100644 index 000000000..da488a513 --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/faresv2/Currency.java @@ -0,0 +1,13 @@ +package com.conveyal.r5.transit.faresv2; + +import gnu.trove.map.TObjectIntMap; +import gnu.trove.map.hash.TObjectIntHashMap; + +public class Currency { + /** Map from currency code to what to multiply by to convert to fixed-point values */ + public static final TObjectIntMap scalarForCurrency = new TObjectIntHashMap<>(); + static { + scalarForCurrency.put("USD", 100); + scalarForCurrency.put("CAD", 100); + } +} diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java b/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java new file mode 100644 index 000000000..f7e3c98dd --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java @@ -0,0 +1,37 @@ +package com.conveyal.r5.transit.faresv2; + +import com.conveyal.gtfs.model.FareLegRule; +import com.google.common.collect.ComparisonChain; + +import java.io.Serializable; + +/** contains the order and amount for a FareLegRule */ +public class FareLegRuleInfo implements Serializable, Comparable { + public static final long serialVersionUID = 1L; + + /** the cost of this fare leg rule in fixed-point currency */ + public int amount; + + /** the order of this fare leg rule */ + public int order; + + public FareLegRuleInfo(FareLegRule rule) { + if (!Currency.scalarForCurrency.containsKey(rule.currency)) + throw new IllegalStateException("No scalar value specified in scalarForCurrency for currency " + rule.currency); + int currencyScalar = Currency.scalarForCurrency.get(rule.currency); + if (Double.isNaN(rule.amount)) + throw new IllegalArgumentException("Amount missing from fare_leg_rule (min_amount/max_amount not supported!"); + amount = (int) (rule.amount * currencyScalar); + order = rule.order; + } + + @Override + public int compareTo(Object other) { + FareLegRuleInfo o = (FareLegRuleInfo) other; + return ComparisonChain.start() + // lowest order first then lowest amount + .compare(order, o.order) + .compare(amount, o.amount) + .result(); + } +} diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java b/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java new file mode 100644 index 000000000..4f8bcf0f3 --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java @@ -0,0 +1,71 @@ +package com.conveyal.r5.transit.faresv2; + +import com.conveyal.gtfs.model.FareTransferRule; + +import java.io.Serializable; + +/** + * Information about a fare transfer rule. + */ +public class FareTransferRuleInfo implements Serializable { + public static final long serialVersionUID = 1L; + + public int order; + public int spanning_limit; + public int duration_limit; + public DurationLimitType duration_limit_type; + public FareTransferType fare_transfer_type; + public int amount; + + public FareTransferRuleInfo (FareTransferRule rule) { + if (!Currency.scalarForCurrency.containsKey(rule.currency)) + throw new IllegalStateException("No scalar value specified in scalarForCurrency for currency " + rule.currency); + int currencyScalar = Currency.scalarForCurrency.get(rule.currency); + if (Double.isNaN(rule.amount)) + throw new IllegalArgumentException("Amount missing from fare_leg_rule (min_amount/max_amount not supported!"); + amount = (int) (rule.amount * currencyScalar); + order = rule.order; + spanning_limit = rule.spanning_limit; + duration_limit = rule.duration_limit; + duration_limit_type = DurationLimitType.forGtfs(rule.duration_limit_type); + fare_transfer_type = FareTransferType.forGtfs(rule.fare_transfer_type); + } + + public static enum DurationLimitType { + FIRST_DEPARTURE_LAST_ARRIVAL, + FIRST_DEPARTURE_LAST_DEPARTURE, + FIRST_ARRIVAL_LAST_DEPARTURE; + + public static DurationLimitType forGtfs (int i) { + switch (i) { + case 0: + return FIRST_DEPARTURE_LAST_ARRIVAL; + case 1: + return FIRST_DEPARTURE_LAST_DEPARTURE; + case 2: + return FIRST_ARRIVAL_LAST_DEPARTURE; + default: + throw new IllegalArgumentException("invalid GTFS duration_limit_type"); + } + } + } + + public static enum FareTransferType { + FIRST_LEG_PLUS_AMOUNT, + TOTAL_COST_PLUS_AMOUNT, + MOST_EXPENSIVE_PLUS_AMOUNT; + + public static FareTransferType forGtfs (int i) { + switch (i) { + case 0: + return FIRST_LEG_PLUS_AMOUNT; + case 1: + return TOTAL_COST_PLUS_AMOUNT; + case 2: + return MOST_EXPENSIVE_PLUS_AMOUNT; + default: + throw new IllegalArgumentException("invalid GTFS fare_transfer_type"); + } + } + } +} diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/package-info.java b/src/main/java/com/conveyal/r5/transit/faresv2/package-info.java new file mode 100644 index 000000000..dc7c397f5 --- /dev/null +++ b/src/main/java/com/conveyal/r5/transit/faresv2/package-info.java @@ -0,0 +1,4 @@ +/** + * This package contains classes to represent + */ +package com.conveyal.r5.transit.faresv2; \ No newline at end of file From 0db2b87d284dd1b24b0fd9cd5eb31ce4ec2de79b Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 12 Jul 2020 18:26:50 -0400 Subject: [PATCH 05/46] feat(fares-v2): first implementation of fares v2 router; too slow (#124) --- .../analyst/fare/InRoutingFareCalculator.java | 4 +- .../FaresV2InRoutingFareCalculator.java | 236 ++++++++++++++++++ .../faresv2/FaresV2TransferAllowance.java | 75 ++++++ .../r5/analyst/fare/faresv2/package-info.java | 2 + .../com/conveyal/r5/transit/TransitLayer.java | 19 +- .../transit/faresv2/FareTransferRuleInfo.java | 6 + 6 files changed, 339 insertions(+), 3 deletions(-) create mode 100644 src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java create mode 100644 src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java create mode 100644 src/main/java/com/conveyal/r5/analyst/fare/faresv2/package-info.java diff --git a/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java index e9f444524..32e96199a 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java @@ -1,6 +1,7 @@ package com.conveyal.r5.analyst.fare; import com.conveyal.gtfs.model.Fare; +import com.conveyal.r5.analyst.fare.faresv2.FaresV2InRoutingFareCalculator; import com.conveyal.r5.analyst.fare.nyc.NYCInRoutingFareCalculator; import com.conveyal.r5.profile.FastRaptorWorker; import com.conveyal.r5.profile.McRaptorSuboptimalPathProfileRouter.McRaptorState; @@ -28,7 +29,8 @@ @JsonSubTypes.Type(name = "chicago", value = ChicagoInRoutingFareCalculator.class), @JsonSubTypes.Type(name = "simple", value = SimpleInRoutingFareCalculator.class), @JsonSubTypes.Type(name = "bogota-mixed", value = BogotaMixedInRoutingFareCalculator.class), - @JsonSubTypes.Type(name = "nyc", value = NYCInRoutingFareCalculator.class) + @JsonSubTypes.Type(name = "nyc", value = NYCInRoutingFareCalculator.class), + @JsonSubTypes.Type(name = "fares-v2", value = FaresV2InRoutingFareCalculator.class) }) public abstract class InRoutingFareCalculator implements Serializable { public static final long serialVersionUID = 0L; diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java new file mode 100644 index 000000000..ef7065562 --- /dev/null +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -0,0 +1,236 @@ +package com.conveyal.r5.analyst.fare.faresv2; + +import com.conveyal.r5.analyst.fare.FareBounds; +import com.conveyal.r5.analyst.fare.InRoutingFareCalculator; +import com.conveyal.r5.profile.McRaptorSuboptimalPathProfileRouter; +import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.faresv2.FareLegRuleInfo; +import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo; +import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo.FareTransferType; +import gnu.trove.TIntCollection; +import gnu.trove.iterator.TIntIterator; +import gnu.trove.list.TIntList; +import gnu.trove.list.array.TIntArrayList; +import gnu.trove.map.TIntObjectMap; +import gnu.trove.map.TObjectIntMap; +import gnu.trove.set.TIntSet; +import gnu.trove.set.hash.TIntHashSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A fare calculator for feeds compliant with the GTFS Fares V2 standard (https://bit.ly/gtfs-fares) + * + * @author mattwigway + */ +public class FaresV2InRoutingFareCalculator extends InRoutingFareCalculator { + private static final Logger LOG = LoggerFactory.getLogger(FaresV2InRoutingFareCalculator.class); + + @Override + public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorState state, int maxClockTime) { + TIntList patterns = new TIntArrayList(); + TIntList boardStops = new TIntArrayList(); + TIntList alightStops = new TIntArrayList(); + TIntList boardTimes = new TIntArrayList(); + TIntList alightTimes = new TIntArrayList(); + + McRaptorSuboptimalPathProfileRouter.McRaptorState stateForTraversal = state; + while (stateForTraversal != null) { + if (stateForTraversal.pattern == -1) { + stateForTraversal = stateForTraversal.back; + continue; // on the street, not on transit + } + patterns.add(stateForTraversal.pattern); + alightStops.add(stateForTraversal.stop); + boardStops.add(transitLayer.tripPatterns.get(stateForTraversal.pattern).stops[stateForTraversal.boardStopPosition]); + boardTimes.add(stateForTraversal.boardTime); + alightTimes.add(stateForTraversal.time); + + stateForTraversal = stateForTraversal.back; + } + + int prevFareLegRuleIdx = -1; + int cumulativeFare = 0; + + for (int i = 0; i < patterns.size(); i++) { + int pattern = patterns.get(i); + int boardStop = boardStops.get(i); + int alightStop = alightStops.get(i); + int boardTime = boardTimes.get(i); + int alightTime = alightTimes.get(i); + + // CHECK FOR AS_ROUTE FARE NETWORK + // NB this is applied greedily, if it is cheaper to buy separate tickets that will not be found + TIntSet fareNetworks = getFareNetworksForPattern(pattern); + TIntSet asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); + if (asRouteFareNetworks.size() > 0) { + for (int j = i + 1; j < patterns.size(); j++) { + TIntSet nextAsRouteFareNetworks = getAsRouteFareNetworksForPattern(patterns.get(j)); + asRouteFareNetworks.retainAll(nextAsRouteFareNetworks); + + if (asRouteFareNetworks.size() > 0) { + // extend ride + alightStop = alightStops.get(j); + alightTime = alightTimes.get(j); + // these are the fare networks actually in use, other fare leg rules should not match + fareNetworks = asRouteFareNetworks; + i++; // don't process this ride again + } + } + } + + // FIND THE FARE LEG RULE + int fareLegRuleIdx = getFareLegRuleForLeg(boardStop, alightStop, fareNetworks); + FareLegRuleInfo fareLegRule = transitLayer.fareLegRules.get(fareLegRuleIdx); + + // CHECK IF THERE ARE ANY TRANSFER DISCOUNTS + if (prevFareLegRuleIdx != -1) { + int transferRuleIdx = getFareTransferRule(prevFareLegRuleIdx, fareLegRuleIdx); + if (transferRuleIdx == -1) { + // pay full fare, no transfer found + cumulativeFare += fareLegRule.amount; + } else { + FareTransferRuleInfo transferRule = transitLayer.fareTransferRules.get(transferRuleIdx); + if (FareTransferType.TOTAL_COST_PLUS_AMOUNT.equals(transferRule.fare_transfer_type)) { + if (transferRule.amount > 0) { + LOG.warn("Negatively discounted transfer"); + } + cumulativeFare += fareLegRule.amount + transferRule.amount; + } else if (FareTransferType.FIRST_LEG_PLUS_AMOUNT.equals(transferRule.fare_transfer_type)) { + cumulativeFare += transferRule.amount; + } else { + throw new UnsupportedOperationException("Only total cost plus amount and first leg plus amount transfer rules are supported."); + } + } + } else { + // pay full fare + cumulativeFare += fareLegRule.amount; + } + + prevFareLegRuleIdx = fareLegRuleIdx; + } + + return new FareBounds(cumulativeFare, new FaresV2TransferAllowance(prevFareLegRuleIdx, transitLayer)); + } + + /** Get the as_route fare networks for a pattern (used to merge with later rides) */ + private TIntSet getAsRouteFareNetworksForPattern (int patIdx) { + TIntSet fareNetworks = getFareNetworksForPattern(patIdx); + TIntSet asRouteFareNetworks = new TIntHashSet(); + for (TIntIterator it = fareNetworks.iterator(); it.hasNext();) { + int fareNetwork = it.next(); + if (transitLayer.getFareNetworkAsRoute(fareNetwork)) asRouteFareNetworks.add(fareNetwork); + } + return asRouteFareNetworks; + } + + private TIntSet getFareNetworksForPattern (int patIdx) { + int routeIdx = transitLayer.tripPatterns.get(patIdx).routeIndex; + return transitLayer.fareNetworksForRoute.get(routeIdx); + } + + /** + * Get the fare leg rule for a leg. If there is more than one, which one is returned is undefined and a warning is logged. + * TODO handle multiple fare leg rules + */ + private int getFareLegRuleForLeg (int boardStop, int alightStop, TIntSet fareNetworks) { + // find leg rules that match the board stop + TIntList boardAreas = transitLayer.fareAreasForStop.get(boardStop); + TIntSet boardAreaMatch = getMatching(transitLayer.fareLegRulesForFromAreaId, boardAreas); + + // find leg rules that match the alight stop + TIntList alightAreas = transitLayer.fareAreasForStop.get(alightStop); + TIntSet alightAreaMatch = getMatching(transitLayer.fareLegRulesForToAreaId, alightAreas); + + // NB is_symmetrical is handled by network build process which materializes the reverse rule + + // find leg rules that match the fare network + TIntSet fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); + + // AND board area match with alight area match and fare network match + boardAreaMatch.retainAll(alightAreaMatch); + boardAreaMatch.retainAll(fareNetworkMatch); + // boardAreaMatch now contains only rules that match _all_ criteria + + if (boardAreaMatch.size() == 0) { + throw new IllegalStateException("no fare leg rule found for leg!"); + } else if (boardAreaMatch.size() == 1) { + return boardAreaMatch.iterator().next(); + } else { + // figure out what matches, first finding the lowest order + int lowestOrder = Integer.MAX_VALUE; + TIntList rulesWithLowestOrder = new TIntArrayList(); + for (TIntIterator it = boardAreaMatch.iterator(); it.hasNext();) { + int ruleIdx = it.next(); + int order = transitLayer.fareLegRules.get(ruleIdx).order; + if (order < lowestOrder) { + lowestOrder = order; + rulesWithLowestOrder.clear(); + rulesWithLowestOrder.add(ruleIdx); + } else if (order == lowestOrder) { + rulesWithLowestOrder.add(ruleIdx); + } + } + + if (rulesWithLowestOrder.size() > 1) + LOG.warn("Found multiple matching fare_leg_rules, results may be unstable or not find the lowest fare path!"); + + return rulesWithLowestOrder.get(0); + } + } + + /** get a fare transfer rule, if one exists, between fromLegRule and toLegRule */ + public int getFareTransferRule (int fromLegRule, int toLegRule) { + TIntSet fromLegMatch = getMatching(transitLayer.fareTransferRulesForFromLegGroupId, fromLegRule); + TIntSet toLegMatch = getMatching(transitLayer.fareTransferRulesForToLegGroupId, toLegRule); + + fromLegMatch.retainAll(toLegMatch); + + if (fromLegMatch.size() == 0) return -1; // no discounted transfer + else if (fromLegMatch.size() == 1) return fromLegMatch.iterator().next(); + else { + int lowestOrder = Integer.MAX_VALUE; + TIntList rulesWithLowestOrder = new TIntArrayList(); + for (TIntIterator it = fromLegMatch.iterator(); it.hasNext();) { + int ruleIdx = it.next(); + int order = transitLayer.fareTransferRules.get(ruleIdx).order; + if (order < lowestOrder) { + lowestOrder = order; + rulesWithLowestOrder.clear(); + rulesWithLowestOrder.add(ruleIdx); + } else if (order == lowestOrder) { + rulesWithLowestOrder.add(ruleIdx); + } + } + + if (rulesWithLowestOrder.size() > 1) + LOG.warn("Found multiple matching fare_leg_rules, results may be unstable or not find the lowest fare path!"); + + return rulesWithLowestOrder.get(0); + } + } + + /** Get all rules that match indices, either directly or because that field was left blank */ + public static TIntSet getMatching (TIntObjectMap rules, TIntCollection indices) { + TIntSet ret = new TIntHashSet(); + for (TIntIterator it = indices.iterator(); it.hasNext();) { + int index = it.next(); + if (rules.containsKey(index)) ret.addAll(rules.get(index)); + } + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.addAll(rules.get(TransitLayer.FARE_ID_BLANK)); + return ret; + } + + /** Get all rules that match index, either directly or because that field was left blank */ + public static TIntSet getMatching (TIntObjectMap rules, int index) { + TIntSet ret = new TIntHashSet(); + if (rules.containsKey(index)) ret.addAll(rules.get(index)); + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.addAll(rules.get(TransitLayer.FARE_ID_BLANK)); + return ret; + } + + @Override + public String getType() { + return "fares-v2"; + } +} diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java new file mode 100644 index 000000000..b95a9c223 --- /dev/null +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -0,0 +1,75 @@ +package com.conveyal.r5.analyst.fare.faresv2; + +import com.conveyal.r5.analyst.fare.TransferAllowance; +import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo; +import gnu.trove.iterator.TIntIterator; +import gnu.trove.set.TIntSet; +import gnu.trove.set.hash.TIntHashSet; + +import java.util.ArrayList; +import java.util.BitSet; +import java.util.Comparator; +import java.util.List; + +/** + * Transfer allowance for Fares V2. + */ +public class FaresV2TransferAllowance extends TransferAllowance { + /** The transfer rules that this allowance could be the start of */ + private TIntSet potentialTransferRules; + + /** need to hold on to a ref to this so that getTransferRuleSummary works - but make sure it's not accidentally serialized */ + private transient TransitLayer transitLayer; + + public FaresV2TransferAllowance (int prevFareLegRuleIdx, TransitLayer transitLayer) { + // the value is really high to effectively disable Theorem 3.1 for now, so we don't have to actually calculate + // the max value, at the cost of some performance. + super(10_000_000_00, 0, 0); + potentialTransferRules = new TIntHashSet(); + + if (prevFareLegRuleIdx != -1) { + // not at start of trip, so we may have transfers available + // TODO this will all have to change once we properly handle chains of multiple transfers + potentialTransferRules = FaresV2InRoutingFareCalculator.getMatching( + transitLayer.fareTransferRulesForFromLegGroupId, prevFareLegRuleIdx); + } + + this.transitLayer = transitLayer; + } + + @Override + public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { + if (other instanceof FaresV2TransferAllowance) { + // at least as good if it provides a superset of the transfers the other does + return potentialTransferRules.containsAll(((FaresV2TransferAllowance) other).potentialTransferRules); + } else { + throw new IllegalArgumentException("mixing of transfer allowance types!"); + } + } + + @Override + public TransferAllowance tightenExpiration(int maxClockTime) { + return this; // expiration time not implemented + } + + /** + * Displaying a bunch of ints in the debug interface is going to be impossible to debug. Instead, generate an + * on the fly string representation. This is not called in routing so performance isn't really an issue. + */ + public String getTransferRuleSummary () { + if (transitLayer == null) return potentialTransferRules.toString(); + + List transfers = new ArrayList<>(); + + for (TIntIterator it = potentialTransferRules.iterator(); it.hasNext();) { + int transferRuleIdx = it.next(); + FareTransferRuleInfo info = transitLayer.fareTransferRules.get(transferRuleIdx); + transfers.add(info.from_leg_group_id + " " + info.to_leg_group_id); + } + + transfers.sort(Comparator.naturalOrder()); + + return String.join("\n", transfers); + } +} diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/package-info.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/package-info.java new file mode 100644 index 000000000..f17f63ef8 --- /dev/null +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/package-info.java @@ -0,0 +1,2 @@ +/** Contains classes used to implement fare routing with GTFS Fares V2 compliant feeds */ +package com.conveyal.r5.analyst.fare.faresv2; \ No newline at end of file diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index f9447b256..294fa63f0 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -14,6 +14,7 @@ import com.google.common.base.Strings; import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; +import gnu.trove.iterator.TIntIterator; import gnu.trove.list.TDoubleList; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; @@ -188,6 +189,9 @@ public class TransitLayer implements Serializable, Cloneable { // TODO Fare Areas for trip, stop_sequence pair + /** Fare networks for each route */ + public TIntObjectMap fareNetworksForRoute = new TIntObjectHashMap<>(); + /** * Is fare network EXPLICIT_FARE_NETWORK_OFFSET + i an as_route fare network (i.e. several legs within the network * should be matched as a single journey?) @@ -548,7 +552,7 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx try { // we only support a subset of Fares V2, and many exceptions may be thrown if the feed uses more than that // subset. Still allow graph to build without fare information if that is the case. - loadFaresV2(gtfs, indexForUnscopedStopId); + loadFaresV2(gtfs, indexForUnscopedStopId, routeIndexForRoute); } catch (Exception e) { LOG.warn("Exception loading GTFS Fares V2, fare routing will not be available", e); clearFaresV2(); @@ -571,10 +575,12 @@ private void clearFaresV2 () { fareTransferRulesForToLegGroupId.clear(); fareAreasForStop.clear(); fareNetworkAsRoute.clear(); + fareNetworksForRoute.clear(); } /** Load GTFS-Fares V2 information from a feed */ - private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedStopId) { + private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedStopId, + TObjectIntMap indexForUnscopedRouteId) { LOG.info("Loading GTFS-Fares V2"); TObjectIntMap fareLegRuleForLegGroupId = new TObjectIntHashMap<>(); @@ -605,8 +611,11 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS LOG.info("Loaded {} fare areas", fareAreaForId.size()); LOG.info("Loading fare networks"); + // TODO will not work if there are multiple feeds for (int i = 0; i < routes.size(); i++) { fareNetworkForId.put(routes.get(i).route_id, i); + fareNetworksForRoute.put(i, new TIntHashSet()); + fareNetworksForRoute.get(i).add(i); // every route is an implicit fare network } // TODO this will not work if there are multiple feeds @@ -614,6 +623,12 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS for (FareNetwork fareNetwork : feed.fare_networks.values()) { fareNetworkForId.put(fareNetwork.fare_network_id, fareNetworkIdx); setFareNetworkAsRoute(fareNetworkIdx, fareNetwork.as_route == 1); + + for (String routeId : fareNetwork.route_ids) { + int routeIdx = indexForUnscopedRouteId.get(routeId); + fareNetworksForRoute.get(routeIdx).add(fareNetworkIdx); + } + fareNetworkIdx++; } diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java b/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java index 4f8bcf0f3..61f795483 100644 --- a/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java +++ b/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java @@ -17,6 +17,10 @@ public class FareTransferRuleInfo implements Serializable { public FareTransferType fare_transfer_type; public int amount; + // saved to be presented in Fareto debug interface + public String from_leg_group_id; + public String to_leg_group_id; + public FareTransferRuleInfo (FareTransferRule rule) { if (!Currency.scalarForCurrency.containsKey(rule.currency)) throw new IllegalStateException("No scalar value specified in scalarForCurrency for currency " + rule.currency); @@ -29,6 +33,8 @@ public FareTransferRuleInfo (FareTransferRule rule) { duration_limit = rule.duration_limit; duration_limit_type = DurationLimitType.forGtfs(rule.duration_limit_type); fare_transfer_type = FareTransferType.forGtfs(rule.fare_transfer_type); + from_leg_group_id = rule.from_leg_group_id; + to_leg_group_id = rule.to_leg_group_id; } public static enum DurationLimitType { From 87e09e030b2041780b99e1b1f45475231238b3f4 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 12 Jul 2020 22:08:12 -0400 Subject: [PATCH 06/46] feat(fares-v2): use RoaringBitSets for tractability (#124). --- .../FaresV2InRoutingFareCalculator.java | 107 +++++++++++------- .../faresv2/FaresV2TransferAllowance.java | 11 +- .../com/conveyal/r5/transit/TransitLayer.java | 44 +++---- 3 files changed, 89 insertions(+), 73 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index ef7065562..05bf6ccfc 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -15,6 +15,8 @@ import gnu.trove.map.TObjectIntMap; import gnu.trove.set.TIntSet; import gnu.trove.set.hash.TIntHashSet; +import org.roaringbitmap.PeekableIntIterator; +import org.roaringbitmap.RoaringBitmap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,6 +51,12 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat stateForTraversal = stateForTraversal.back; } + patterns.reverse(); + boardStops.reverse(); + alightStops.reverse(); + boardTimes.reverse(); + alightTimes.reverse(); + int prevFareLegRuleIdx = -1; int cumulativeFare = 0; @@ -61,20 +69,23 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat // CHECK FOR AS_ROUTE FARE NETWORK // NB this is applied greedily, if it is cheaper to buy separate tickets that will not be found - TIntSet fareNetworks = getFareNetworksForPattern(pattern); - TIntSet asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); - if (asRouteFareNetworks.size() > 0) { + RoaringBitmap fareNetworks = getFareNetworksForPattern(pattern); + RoaringBitmap asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); + if (asRouteFareNetworks.getCardinality() > 0) { for (int j = i + 1; j < patterns.size(); j++) { - TIntSet nextAsRouteFareNetworks = getAsRouteFareNetworksForPattern(patterns.get(j)); - asRouteFareNetworks.retainAll(nextAsRouteFareNetworks); + RoaringBitmap nextAsRouteFareNetworks = getAsRouteFareNetworksForPattern(patterns.get(j)); + // can't modify asRouteFareNetworks in-place as it may have already been set as fareNetworks below + asRouteFareNetworks = RoaringBitmap.and(asRouteFareNetworks, nextAsRouteFareNetworks); - if (asRouteFareNetworks.size() > 0) { + if (asRouteFareNetworks.getCardinality() > 0) { // extend ride alightStop = alightStops.get(j); alightTime = alightTimes.get(j); // these are the fare networks actually in use, other fare leg rules should not match fareNetworks = asRouteFareNetworks; - i++; // don't process this ride again + i = j; // don't process this ride again + } else { + break; } } } @@ -95,7 +106,10 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat if (transferRule.amount > 0) { LOG.warn("Negatively discounted transfer"); } - cumulativeFare += fareLegRule.amount + transferRule.amount; + int fareIncrement = fareLegRule.amount + transferRule.amount; + if (fareIncrement < 0) + LOG.warn("Fare increment is negative!"); + cumulativeFare += fareIncrement; } else if (FareTransferType.FIRST_LEG_PLUS_AMOUNT.equals(transferRule.fare_transfer_type)) { cumulativeFare += transferRule.amount; } else { @@ -114,17 +128,15 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat } /** Get the as_route fare networks for a pattern (used to merge with later rides) */ - private TIntSet getAsRouteFareNetworksForPattern (int patIdx) { - TIntSet fareNetworks = getFareNetworksForPattern(patIdx); - TIntSet asRouteFareNetworks = new TIntHashSet(); - for (TIntIterator it = fareNetworks.iterator(); it.hasNext();) { - int fareNetwork = it.next(); - if (transitLayer.getFareNetworkAsRoute(fareNetwork)) asRouteFareNetworks.add(fareNetwork); - } - return asRouteFareNetworks; + private RoaringBitmap getAsRouteFareNetworksForPattern (int patIdx) { + RoaringBitmap fareNetworks = new RoaringBitmap(); + // protective copy + fareNetworks.or(getFareNetworksForPattern(patIdx)); + fareNetworks.and(transitLayer.fareNetworkAsRoute); + return fareNetworks; } - private TIntSet getFareNetworksForPattern (int patIdx) { + private RoaringBitmap getFareNetworksForPattern (int patIdx) { int routeIdx = transitLayer.tripPatterns.get(patIdx).routeIndex; return transitLayer.fareNetworksForRoute.get(routeIdx); } @@ -133,34 +145,36 @@ private TIntSet getFareNetworksForPattern (int patIdx) { * Get the fare leg rule for a leg. If there is more than one, which one is returned is undefined and a warning is logged. * TODO handle multiple fare leg rules */ - private int getFareLegRuleForLeg (int boardStop, int alightStop, TIntSet fareNetworks) { + private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap fareNetworks) { // find leg rules that match the board stop TIntList boardAreas = transitLayer.fareAreasForStop.get(boardStop); - TIntSet boardAreaMatch = getMatching(transitLayer.fareLegRulesForFromAreaId, boardAreas); + RoaringBitmap boardAreaMatch = getMatching(transitLayer.fareLegRulesForFromAreaId, boardAreas); // find leg rules that match the alight stop TIntList alightAreas = transitLayer.fareAreasForStop.get(alightStop); - TIntSet alightAreaMatch = getMatching(transitLayer.fareLegRulesForToAreaId, alightAreas); + RoaringBitmap alightAreaMatch = getMatching(transitLayer.fareLegRulesForToAreaId, alightAreas); // NB is_symmetrical is handled by network build process which materializes the reverse rule // find leg rules that match the fare network - TIntSet fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); + RoaringBitmap fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); // AND board area match with alight area match and fare network match - boardAreaMatch.retainAll(alightAreaMatch); - boardAreaMatch.retainAll(fareNetworkMatch); + boardAreaMatch.and(alightAreaMatch); + boardAreaMatch.and(fareNetworkMatch); // boardAreaMatch now contains only rules that match _all_ criteria - if (boardAreaMatch.size() == 0) { - throw new IllegalStateException("no fare leg rule found for leg!"); - } else if (boardAreaMatch.size() == 1) { + if (boardAreaMatch.getCardinality() == 0) { + String fromStopId = transitLayer.stopIdForIndex.get(boardStop); + String toStopId = transitLayer.stopIdForIndex.get(alightStop); + throw new IllegalStateException("no fare leg rule found for leg from " + fromStopId + " to " + toStopId + "!"); + } else if (boardAreaMatch.getCardinality() == 1) { return boardAreaMatch.iterator().next(); } else { // figure out what matches, first finding the lowest order int lowestOrder = Integer.MAX_VALUE; TIntList rulesWithLowestOrder = new TIntArrayList(); - for (TIntIterator it = boardAreaMatch.iterator(); it.hasNext();) { + for (PeekableIntIterator it = boardAreaMatch.getIntIterator(); it.hasNext();) { int ruleIdx = it.next(); int order = transitLayer.fareLegRules.get(ruleIdx).order; if (order < lowestOrder) { @@ -181,17 +195,17 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, TIntSet fareNet /** get a fare transfer rule, if one exists, between fromLegRule and toLegRule */ public int getFareTransferRule (int fromLegRule, int toLegRule) { - TIntSet fromLegMatch = getMatching(transitLayer.fareTransferRulesForFromLegGroupId, fromLegRule); - TIntSet toLegMatch = getMatching(transitLayer.fareTransferRulesForToLegGroupId, toLegRule); + RoaringBitmap fromLegMatch = getMatching(transitLayer.fareTransferRulesForFromLegGroupId, fromLegRule); + RoaringBitmap toLegMatch = getMatching(transitLayer.fareTransferRulesForToLegGroupId, toLegRule); - fromLegMatch.retainAll(toLegMatch); + fromLegMatch.and(toLegMatch); - if (fromLegMatch.size() == 0) return -1; // no discounted transfer - else if (fromLegMatch.size() == 1) return fromLegMatch.iterator().next(); + if (fromLegMatch.getCardinality() == 0) return -1; // no discounted transfer + else if (fromLegMatch.getCardinality() == 1) return fromLegMatch.iterator().next(); else { int lowestOrder = Integer.MAX_VALUE; TIntList rulesWithLowestOrder = new TIntArrayList(); - for (TIntIterator it = fromLegMatch.iterator(); it.hasNext();) { + for (PeekableIntIterator it = fromLegMatch.getIntIterator(); it.hasNext();) { int ruleIdx = it.next(); int order = transitLayer.fareTransferRules.get(ruleIdx).order; if (order < lowestOrder) { @@ -211,21 +225,32 @@ public int getFareTransferRule (int fromLegRule, int toLegRule) { } /** Get all rules that match indices, either directly or because that field was left blank */ - public static TIntSet getMatching (TIntObjectMap rules, TIntCollection indices) { - TIntSet ret = new TIntHashSet(); + public static RoaringBitmap getMatching (TIntObjectMap rules, TIntCollection indices) { + RoaringBitmap ret = new RoaringBitmap(); for (TIntIterator it = indices.iterator(); it.hasNext();) { int index = it.next(); - if (rules.containsKey(index)) ret.addAll(rules.get(index)); + if (rules.containsKey(index)) ret.or(rules.get(index)); + } + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); + return ret; + } + + /** Get all rules that match indices, either directly or because that field was left blank */ + public static RoaringBitmap getMatching (TIntObjectMap rules, RoaringBitmap indices) { + RoaringBitmap ret = new RoaringBitmap(); + for (PeekableIntIterator it = indices.getIntIterator(); it.hasNext();) { + int index = it.next(); + if (rules.containsKey(index)) ret.or(rules.get(index)); } - if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.addAll(rules.get(TransitLayer.FARE_ID_BLANK)); + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); return ret; } /** Get all rules that match index, either directly or because that field was left blank */ - public static TIntSet getMatching (TIntObjectMap rules, int index) { - TIntSet ret = new TIntHashSet(); - if (rules.containsKey(index)) ret.addAll(rules.get(index)); - if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.addAll(rules.get(TransitLayer.FARE_ID_BLANK)); + public static RoaringBitmap getMatching (TIntObjectMap rules, int index) { + RoaringBitmap ret = new RoaringBitmap(); + if (rules.containsKey(index)) ret.or(rules.get(index)); + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); return ret; } diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index b95a9c223..6ce5a9a94 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -6,6 +6,8 @@ import gnu.trove.iterator.TIntIterator; import gnu.trove.set.TIntSet; import gnu.trove.set.hash.TIntHashSet; +import org.roaringbitmap.PeekableIntIterator; +import org.roaringbitmap.RoaringBitmap; import java.util.ArrayList; import java.util.BitSet; @@ -17,7 +19,7 @@ */ public class FaresV2TransferAllowance extends TransferAllowance { /** The transfer rules that this allowance could be the start of */ - private TIntSet potentialTransferRules; + private RoaringBitmap potentialTransferRules; /** need to hold on to a ref to this so that getTransferRuleSummary works - but make sure it's not accidentally serialized */ private transient TransitLayer transitLayer; @@ -26,13 +28,14 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, TransitLayer transitLay // the value is really high to effectively disable Theorem 3.1 for now, so we don't have to actually calculate // the max value, at the cost of some performance. super(10_000_000_00, 0, 0); - potentialTransferRules = new TIntHashSet(); if (prevFareLegRuleIdx != -1) { // not at start of trip, so we may have transfers available // TODO this will all have to change once we properly handle chains of multiple transfers potentialTransferRules = FaresV2InRoutingFareCalculator.getMatching( transitLayer.fareTransferRulesForFromLegGroupId, prevFareLegRuleIdx); + } else { + potentialTransferRules = new RoaringBitmap(); } this.transitLayer = transitLayer; @@ -42,7 +45,7 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, TransitLayer transitLay public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { if (other instanceof FaresV2TransferAllowance) { // at least as good if it provides a superset of the transfers the other does - return potentialTransferRules.containsAll(((FaresV2TransferAllowance) other).potentialTransferRules); + return potentialTransferRules.contains(((FaresV2TransferAllowance) other).potentialTransferRules); } else { throw new IllegalArgumentException("mixing of transfer allowance types!"); } @@ -62,7 +65,7 @@ public String getTransferRuleSummary () { List transfers = new ArrayList<>(); - for (TIntIterator it = potentialTransferRules.iterator(); it.hasNext();) { + for (PeekableIntIterator it = potentialTransferRules.getIntIterator(); it.hasNext();) { int transferRuleIdx = it.next(); FareTransferRuleInfo info = transitLayer.fareTransferRules.get(transferRuleIdx); transfers.add(info.from_leg_group_id + " " + info.to_leg_group_id); diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 294fa63f0..68d0bfe4e 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -31,6 +31,7 @@ import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.Point; import org.locationtech.jts.linearref.LinearLocation; +import org.roaringbitmap.RoaringBitmap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -190,7 +191,7 @@ public class TransitLayer implements Serializable, Cloneable { // TODO Fare Areas for trip, stop_sequence pair /** Fare networks for each route */ - public TIntObjectMap fareNetworksForRoute = new TIntObjectHashMap<>(); + public TIntObjectMap fareNetworksForRoute = new TIntObjectHashMap<>(); /** * Is fare network EXPLICIT_FARE_NETWORK_OFFSET + i an as_route fare network (i.e. several legs within the network @@ -200,7 +201,7 @@ public class TransitLayer implements Serializable, Cloneable { * fare networks. It is not recommended to query this directly but instead use getFareNetworkAsRoute(fareNetworkId) * and setFareNetworkAsRoute(fareNetworkId) which handles the integer conversions. */ - public BitSet fareNetworkAsRoute = new BitSet(); + public RoaringBitmap fareNetworkAsRoute = new RoaringBitmap(); /** * The fare leg rules for this transport network. @@ -212,13 +213,13 @@ public class TransitLayer implements Serializable, Cloneable { //the field was left blank /** Fare leg rule for fare network ID (either explicit or implicit) */ - public TIntObjectMap fareLegRulesForFareNetworkId = new TIntObjectHashMap<>(); + public TIntObjectMap fareLegRulesForFareNetworkId = new TIntObjectHashMap<>(); /** Fare leg rule for from area id */ - public TIntObjectMap fareLegRulesForFromAreaId = new TIntObjectHashMap<>(); + public TIntObjectMap fareLegRulesForFromAreaId = new TIntObjectHashMap<>(); /** Fare leg rule for to area id */ - public TIntObjectMap fareLegRulesForToAreaId = new TIntObjectHashMap<>(); + public TIntObjectMap fareLegRulesForToAreaId = new TIntObjectHashMap<>(); // TODO contains_area_id, leg_group_id, timeframes @@ -228,10 +229,10 @@ public class TransitLayer implements Serializable, Cloneable { public List fareTransferRules = new ArrayList<>(); /** Fare transfer rule index for from_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty from_leg_group_id */ - public TIntObjectMap fareTransferRulesForFromLegGroupId = new TIntObjectHashMap<>(); + public TIntObjectMap fareTransferRulesForFromLegGroupId = new TIntObjectHashMap<>(); /** Fare transfer rule index for to_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty to_leg_group_id */ - public TIntObjectMap fareTransferRulesForToLegGroupId = new TIntObjectHashMap<>(); + public TIntObjectMap fareTransferRulesForToLegGroupId = new TIntObjectHashMap<>(); /** Map from feed ID to feed CRC32 to ensure that we can't apply scenarios to the wrong feeds */ public Map feedChecksums = new HashMap<>(); @@ -246,19 +247,6 @@ public class TransitLayer implements Serializable, Cloneable { */ public String scenarioId; - /** Does fare_network (which may be a route index or an explicit fare network index) have as_route set? */ - public boolean getFareNetworkAsRoute (int fareNetwork) { - if (fareNetwork < EXPLICIT_FARE_NETWORK_OFFSET) return false; - else return fareNetworkAsRoute.get(fareNetwork - EXPLICIT_FARE_NETWORK_OFFSET); - } - - /** Set fare_network as_route */ - public void setFareNetworkAsRoute (int fareNetwork, boolean as_route) { - if (fareNetwork < EXPLICIT_FARE_NETWORK_OFFSET) - throw new IllegalArgumentException("attempt to set as_route on implicit fare network."); - else fareNetworkAsRoute.set(fareNetwork - EXPLICIT_FARE_NETWORK_OFFSET, as_route); - } - /** Load a GTFS feed with full load level */ public void loadFromGtfs (GTFSFeed gtfs) throws DuplicateFeedException { loadFromGtfs(gtfs, LoadLevel.FULL); @@ -591,7 +579,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS for (int i = 0; i < stopIdForIndex.size(); i++) { fareAreasForStop.put(i, new TIntArrayList()); fareAreasForStop.get(i).add(i); // every stop is a fare area - fareAreaForId.put(stopIdForIndex.get(i), i); + fareAreaForId.put(stopForIndex.get(i).stop_id, i); // need to get unscoped id } // TODO this will not work if there are multiple feeds @@ -614,7 +602,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS // TODO will not work if there are multiple feeds for (int i = 0; i < routes.size(); i++) { fareNetworkForId.put(routes.get(i).route_id, i); - fareNetworksForRoute.put(i, new TIntHashSet()); + fareNetworksForRoute.put(i, new RoaringBitmap()); fareNetworksForRoute.get(i).add(i); // every route is an implicit fare network } @@ -622,7 +610,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS int fareNetworkIdx = EXPLICIT_FARE_NETWORK_OFFSET; for (FareNetwork fareNetwork : feed.fare_networks.values()) { fareNetworkForId.put(fareNetwork.fare_network_id, fareNetworkIdx); - setFareNetworkAsRoute(fareNetworkIdx, fareNetwork.as_route == 1); + if (fareNetwork.as_route == 1) fareNetworkAsRoute.add(fareNetworkIdx); for (String routeId : fareNetwork.route_ids) { int routeIdx = indexForUnscopedRouteId.get(routeId); @@ -652,7 +640,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } else fareNetworkId = FARE_ID_BLANK; if (!fareLegRulesForFareNetworkId.containsKey(fareNetworkId)) - fareLegRulesForFareNetworkId.put(fareNetworkId, new TIntHashSet()); + fareLegRulesForFareNetworkId.put(fareNetworkId, new RoaringBitmap()); fareLegRulesForFareNetworkId.get(fareNetworkId).add(fareLegRuleIdx); int fromAreaIdx; @@ -665,7 +653,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } else fromAreaIdx = FARE_ID_BLANK; if (!fareLegRulesForFromAreaId.containsKey(fromAreaIdx)) { - fareLegRulesForFromAreaId.put(fromAreaIdx, new TIntHashSet()); + fareLegRulesForFromAreaId.put(fromAreaIdx, new RoaringBitmap()); } fareLegRulesForFromAreaId.get(fromAreaIdx).add(fareLegRuleIdx); @@ -679,7 +667,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } else toAreaIdx = FARE_ID_BLANK; if (!fareLegRulesForToAreaId.containsKey(toAreaIdx)) { - fareLegRulesForToAreaId.put(toAreaIdx, new TIntHashSet()); + fareLegRulesForToAreaId.put(toAreaIdx, new RoaringBitmap()); } fareLegRulesForToAreaId.get(toAreaIdx).add(fareLegRuleIdx); @@ -716,7 +704,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } else fromLegIdx = FARE_ID_BLANK; if (!fareTransferRulesForFromLegGroupId.containsKey(fromLegIdx)) { - fareTransferRulesForFromLegGroupId.put(fromLegIdx, new TIntHashSet()); + fareTransferRulesForFromLegGroupId.put(fromLegIdx, new RoaringBitmap()); } fareTransferRulesForFromLegGroupId.get(fromLegIdx).add(fareTransferRuleIdx); @@ -730,7 +718,7 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } else toLegIdx = FARE_ID_BLANK; if (!fareTransferRulesForToLegGroupId.containsKey(toLegIdx)) { - fareTransferRulesForToLegGroupId.put(toLegIdx, new TIntHashSet()); + fareTransferRulesForToLegGroupId.put(toLegIdx, new RoaringBitmap()); } fareTransferRulesForToLegGroupId.get(toLegIdx).add(fareTransferRuleIdx); From 48185789552500365c07397b8422b556775ce271 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 21 Jul 2020 10:59:58 -0400 Subject: [PATCH 07/46] fix(fares-v2): better log statements --- .../analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index 05bf6ccfc..afaad1cea 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -187,7 +187,7 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap f } if (rulesWithLowestOrder.size() > 1) - LOG.warn("Found multiple matching fare_leg_rules, results may be unstable or not find the lowest fare path!"); + LOG.warn("Found multiple matching fare_leg_rules with same order, results may be unstable or not find the lowest fare path!"); return rulesWithLowestOrder.get(0); } @@ -218,7 +218,7 @@ public int getFareTransferRule (int fromLegRule, int toLegRule) { } if (rulesWithLowestOrder.size() > 1) - LOG.warn("Found multiple matching fare_leg_rules, results may be unstable or not find the lowest fare path!"); + LOG.warn("Found multiple matching fare_leg_rules with same order, results may be unstable or not find the lowest fare path!"); return rulesWithLowestOrder.get(0); } From 1eab3b5f807b1de759d32380b82fdc0d613c2eeb Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 21 Jul 2020 11:00:36 -0400 Subject: [PATCH 08/46] fix(fares-v2): allow is_symmetrical fare leg rules correctly (#124) --- .../com/conveyal/r5/transit/TransitLayer.java | 86 +++++++++++++------ 1 file changed, 60 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 68d0bfe4e..12dee507c 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -571,7 +571,8 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS TObjectIntMap indexForUnscopedRouteId) { LOG.info("Loading GTFS-Fares V2"); - TObjectIntMap fareLegRuleForLegGroupId = new TObjectIntHashMap<>(); + // due to symmetrical fare legs, one fare leg rule ID can match multiple fare leg rules + Map fareLegRuleForLegGroupId = new HashMap<>(); TObjectIntMap fareNetworkForId = new TObjectIntHashMap<>(); TObjectIntMap fareAreaForId = new TObjectIntHashMap<>(); @@ -623,10 +624,16 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS LOG.info("Loaded {} fare networks", fareNetworkForId.size()); LOG.info("Loading fare leg rules"); - int fareLegRuleIdx = 0; for (FareLegRule rule : feed.fare_leg_rules) { fareLegRules.add(new FareLegRuleInfo(rule)); - if (rule.leg_group_id != null) fareLegRuleForLegGroupId.put(rule.leg_group_id, fareLegRuleIdx); + int fareLegRuleIdx = fareLegRules.size() - 1; + if (rule.leg_group_id != null) { + if (fareLegRuleForLegGroupId.containsKey(rule.leg_group_id)) { + throw new IllegalArgumentException("Fare leg group ID " + rule.leg_group_id + " is duplicated"); + } + fareLegRuleForLegGroupId.put(rule.leg_group_id, new TIntArrayList()); + fareLegRuleForLegGroupId.get(rule.leg_group_id).add(fareLegRuleIdx); + } // build indices int fareNetworkId; @@ -671,12 +678,6 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } fareLegRulesForToAreaId.get(toAreaIdx).add(fareLegRuleIdx); - if (rule.is_symmetrical == 1) { - // add the same rule backwards - fareLegRulesForFromAreaId.get(toAreaIdx).add(fareLegRuleIdx); - fareLegRulesForToAreaId.get(fromAreaIdx).add(fareLegRuleIdx); - } - if (rule.service_id != null) throw new IllegalArgumentException("Service IDs not supported in fare_leg_rules"); if (rule.contains_area_id != null) throw new IllegalArgumentException("contains_area_id not supported in fare_leg_rules"); if (rule.to_timeframe_id != null || rule.from_timeframe_id != null) @@ -684,48 +685,81 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS if (!Double.isNaN(rule.min_fare_distance) || !Double.isNaN(rule.max_fare_distance)) throw new IllegalArgumentException("Fare distances not supported in fare_leg_rules"); - fareLegRuleIdx++; + if (rule.is_symmetrical == 1) { + // Can't just add the same rule backwards, because of how the matching works. Consider a rule for travel + // from Zone A to Zone B that is symmetrical. If we add the same rule index to both directions, the rule + // will also match trips from A to A or B to B. + fareLegRules.add(new FareLegRuleInfo(rule)); + fareLegRuleIdx = fareLegRules.size() -1; + if (rule.leg_group_id != null) + // no contains check needed, forward rule already added above + fareLegRuleForLegGroupId.get(rule.leg_group_id).add(fareLegRuleIdx); + + // no contains check needed for same reason + fareLegRulesForFareNetworkId.get(fareNetworkId).add(fareLegRuleIdx); + + if (!fareLegRulesForFromAreaId.containsKey(toAreaIdx)) { + fareLegRulesForFromAreaId.put(toAreaIdx, new RoaringBitmap()); + } + fareLegRulesForFromAreaId.get(toAreaIdx).add(fareLegRuleIdx); + + if (!fareLegRulesForToAreaId.containsKey(fromAreaIdx)) { + fareLegRulesForToAreaId.put(fromAreaIdx, new RoaringBitmap()); + } + fareLegRulesForToAreaId.get(fromAreaIdx).add(fareLegRuleIdx); + } } - LOG.info("Loaded {} fare leg rules", fareLegRuleIdx); + LOG.info("Loaded {} fare leg rules", fareLegRules.size()); LOG.info("Loading fare transfer rules"); - int fareTransferRuleIdx = 0; + TIntList blankFareList = new TIntArrayList(); + blankFareList.add(FARE_ID_BLANK); for (FareTransferRule rule : feed.fare_transfer_rules) { fareTransferRules.add(new FareTransferRuleInfo(rule)); + int fareTransferRuleIdx = fareTransferRules.size() - 1; - int fromLegIdx; + TIntList fromLegIdxs; if (rule.from_leg_group_id != null) { if (!fareLegRuleForLegGroupId.containsKey(rule.from_leg_group_id)) { throw new IllegalArgumentException("Fare leg group ID referenced in fare_transfer_rules not present: " + rule.from_leg_group_id); } - fromLegIdx = fareLegRuleForLegGroupId.get(rule.from_leg_group_id); - } else fromLegIdx = FARE_ID_BLANK; + fromLegIdxs = fareLegRuleForLegGroupId.get(rule.from_leg_group_id); + } else fromLegIdxs = blankFareList; + - if (!fareTransferRulesForFromLegGroupId.containsKey(fromLegIdx)) { - fareTransferRulesForFromLegGroupId.put(fromLegIdx, new RoaringBitmap()); + for (TIntIterator it = fromLegIdxs.iterator(); it.hasNext(); ) { + int fromLegIdx = it.next(); + if (!fareTransferRulesForFromLegGroupId.containsKey(fromLegIdx)) { + fareTransferRulesForFromLegGroupId.put(fromLegIdx, new RoaringBitmap()); + } + fareTransferRulesForFromLegGroupId.get(fromLegIdx).add(fareTransferRuleIdx); } - fareTransferRulesForFromLegGroupId.get(fromLegIdx).add(fareTransferRuleIdx); - int toLegIdx; + TIntList toLegIdxs; if (rule.to_leg_group_id != null) { if (!fareLegRuleForLegGroupId.containsKey(rule.to_leg_group_id)) { throw new IllegalArgumentException("Fare leg group ID referenced in fare_transfer_rules not present: " + rule.to_leg_group_id); } - toLegIdx = fareLegRuleForLegGroupId.get(rule.to_leg_group_id); - } else toLegIdx = FARE_ID_BLANK; + toLegIdxs = fareLegRuleForLegGroupId.get(rule.to_leg_group_id); + } else toLegIdxs = blankFareList; - if (!fareTransferRulesForToLegGroupId.containsKey(toLegIdx)) { - fareTransferRulesForToLegGroupId.put(toLegIdx, new RoaringBitmap()); + for (TIntIterator it = toLegIdxs.iterator(); it.hasNext(); ) { + int toLegIdx = it.next(); + if (!fareTransferRulesForToLegGroupId.containsKey(toLegIdx)) { + fareTransferRulesForToLegGroupId.put(toLegIdx, new RoaringBitmap()); + } + fareTransferRulesForToLegGroupId.get(toLegIdx).add(fareTransferRuleIdx); } - fareTransferRulesForToLegGroupId.get(toLegIdx).add(fareTransferRuleIdx); - fareTransferRuleIdx++; + if (rule.is_symmetrical == 1) { + throw new UnsupportedOperationException("is_symmetrical not yet supported for fare_transfer_rules"); + } } - LOG.info("Loaded {} fare transfer rules", fareTransferRuleIdx); + LOG.info("Loaded {} fare transfer rules", fareTransferRules.size()); } // The median of all stopTimes would be best but that involves sorting a huge list of numbers. From 87e5e80c9f70bd90cc2f4d7426fff878ca26ff78 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 21 Jul 2020 12:22:33 -0400 Subject: [PATCH 09/46] feat(fares-v2): improve performance with caching (#124) --- .../FaresV2InRoutingFareCalculator.java | 55 +++---------------- .../faresv2/FaresV2TransferAllowance.java | 2 +- .../r5/analyst/fare/faresv2/IndexUtils.java | 46 ++++++++++++++++ .../com/conveyal/r5/transit/TransitLayer.java | 37 +++++++++++++ 4 files changed, 92 insertions(+), 48 deletions(-) create mode 100644 src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index afaad1cea..a796e14e0 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -19,6 +19,7 @@ import org.roaringbitmap.RoaringBitmap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static com.conveyal.r5.analyst.fare.faresv2.IndexUtils.getMatching; /** * A fare calculator for feeds compliant with the GTFS Fares V2 standard (https://bit.ly/gtfs-fares) @@ -146,35 +147,25 @@ private RoaringBitmap getFareNetworksForPattern (int patIdx) { * TODO handle multiple fare leg rules */ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap fareNetworks) { - // find leg rules that match the board stop - TIntList boardAreas = transitLayer.fareAreasForStop.get(boardStop); - RoaringBitmap boardAreaMatch = getMatching(transitLayer.fareLegRulesForFromAreaId, boardAreas); - - // find leg rules that match the alight stop - TIntList alightAreas = transitLayer.fareAreasForStop.get(alightStop); - RoaringBitmap alightAreaMatch = getMatching(transitLayer.fareLegRulesForToAreaId, alightAreas); - - // NB is_symmetrical is handled by network build process which materializes the reverse rule - // find leg rules that match the fare network + // getMatching returns a new RoaringBitmap so okay to modify RoaringBitmap fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); + fareNetworkMatch.and(transitLayer.fareLegRulesForFromStopId.get(boardStop)); + fareNetworkMatch.and(transitLayer.fareLegRulesForToStopId.get(alightStop)); - // AND board area match with alight area match and fare network match - boardAreaMatch.and(alightAreaMatch); - boardAreaMatch.and(fareNetworkMatch); // boardAreaMatch now contains only rules that match _all_ criteria - if (boardAreaMatch.getCardinality() == 0) { + if (fareNetworkMatch.getCardinality() == 0) { String fromStopId = transitLayer.stopIdForIndex.get(boardStop); String toStopId = transitLayer.stopIdForIndex.get(alightStop); throw new IllegalStateException("no fare leg rule found for leg from " + fromStopId + " to " + toStopId + "!"); - } else if (boardAreaMatch.getCardinality() == 1) { - return boardAreaMatch.iterator().next(); + } else if (fareNetworkMatch.getCardinality() == 1) { + return fareNetworkMatch.iterator().next(); } else { // figure out what matches, first finding the lowest order int lowestOrder = Integer.MAX_VALUE; TIntList rulesWithLowestOrder = new TIntArrayList(); - for (PeekableIntIterator it = boardAreaMatch.getIntIterator(); it.hasNext();) { + for (PeekableIntIterator it = fareNetworkMatch.getIntIterator(); it.hasNext();) { int ruleIdx = it.next(); int order = transitLayer.fareLegRules.get(ruleIdx).order; if (order < lowestOrder) { @@ -224,36 +215,6 @@ public int getFareTransferRule (int fromLegRule, int toLegRule) { } } - /** Get all rules that match indices, either directly or because that field was left blank */ - public static RoaringBitmap getMatching (TIntObjectMap rules, TIntCollection indices) { - RoaringBitmap ret = new RoaringBitmap(); - for (TIntIterator it = indices.iterator(); it.hasNext();) { - int index = it.next(); - if (rules.containsKey(index)) ret.or(rules.get(index)); - } - if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); - return ret; - } - - /** Get all rules that match indices, either directly or because that field was left blank */ - public static RoaringBitmap getMatching (TIntObjectMap rules, RoaringBitmap indices) { - RoaringBitmap ret = new RoaringBitmap(); - for (PeekableIntIterator it = indices.getIntIterator(); it.hasNext();) { - int index = it.next(); - if (rules.containsKey(index)) ret.or(rules.get(index)); - } - if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); - return ret; - } - - /** Get all rules that match index, either directly or because that field was left blank */ - public static RoaringBitmap getMatching (TIntObjectMap rules, int index) { - RoaringBitmap ret = new RoaringBitmap(); - if (rules.containsKey(index)) ret.or(rules.get(index)); - if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); - return ret; - } - @Override public String getType() { return "fares-v2"; diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index 6ce5a9a94..1246ff769 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -32,7 +32,7 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, TransitLayer transitLay if (prevFareLegRuleIdx != -1) { // not at start of trip, so we may have transfers available // TODO this will all have to change once we properly handle chains of multiple transfers - potentialTransferRules = FaresV2InRoutingFareCalculator.getMatching( + potentialTransferRules = IndexUtils.getMatching( transitLayer.fareTransferRulesForFromLegGroupId, prevFareLegRuleIdx); } else { potentialTransferRules = new RoaringBitmap(); diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java new file mode 100644 index 000000000..b6a7a49e1 --- /dev/null +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java @@ -0,0 +1,46 @@ +package com.conveyal.r5.analyst.fare.faresv2; + +import com.conveyal.r5.transit.TransitLayer; +import gnu.trove.TIntCollection; +import gnu.trove.iterator.TIntIterator; +import gnu.trove.map.TIntObjectMap; +import org.roaringbitmap.PeekableIntIterator; +import org.roaringbitmap.RoaringBitmap; + +/** Utility funtions for indexing using RoaringBitmaps */ +public class IndexUtils { + /** + * Get all rules that match indices, either directly or because that field was left blank. + * Combine those rules into a single RoaringBitmap. Used for instance with fareLegRulesForFareAreaId in TransitLayer, + * passing in a collection fare area indices, and returning a RoaringBitmap of all FareLegRules that match any of those + * indices. + */ + public static RoaringBitmap getMatching (TIntObjectMap rules, TIntCollection indices) { + RoaringBitmap ret = new RoaringBitmap(); + for (TIntIterator it = indices.iterator(); it.hasNext();) { + int index = it.next(); + if (rules.containsKey(index)) ret.or(rules.get(index)); + } + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); + return ret; + } + + /** Get all rules that match indices, either directly or because that field was left blank */ + public static RoaringBitmap getMatching (TIntObjectMap rules, RoaringBitmap indices) { + RoaringBitmap ret = new RoaringBitmap(); + for (PeekableIntIterator it = indices.getIntIterator(); it.hasNext();) { + int index = it.next(); + if (rules.containsKey(index)) ret.or(rules.get(index)); + } + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); + return ret; + } + + /** Get all rules that match index, either directly or because that field was left blank */ + public static RoaringBitmap getMatching (TIntObjectMap rules, int index) { + RoaringBitmap ret = new RoaringBitmap(); + if (rules.containsKey(index)) ret.or(rules.get(index)); + if (rules.containsKey(TransitLayer.FARE_ID_BLANK)) ret.or(rules.get(TransitLayer.FARE_ID_BLANK)); + return ret; + } +} diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 12dee507c..84cc4f57b 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -2,6 +2,7 @@ import com.conveyal.gtfs.GTFSFeed; import com.conveyal.gtfs.model.*; +import com.conveyal.r5.analyst.fare.faresv2.IndexUtils; import com.conveyal.r5.api.util.TransitModes; import com.conveyal.r5.common.GeometryUtils; import com.conveyal.r5.streets.EdgeStore; @@ -218,9 +219,21 @@ public class TransitLayer implements Serializable, Cloneable { /** Fare leg rule for from area id */ public TIntObjectMap fareLegRulesForFromAreaId = new TIntObjectHashMap<>(); + /** + * Fare leg rules for from stop ID. This is computed based on fareAreasForStopId and fareLegRulesForFromAreaId, and + * cached for higher performance. + */ + public transient TIntObjectMap fareLegRulesForFromStopId; + /** Fare leg rule for to area id */ public TIntObjectMap fareLegRulesForToAreaId = new TIntObjectHashMap<>(); + /** + * Fare leg rules for to stop ID. This is computed based on fareAreasForStopId and fareLegRulesForToAreaId, and + * cached for higher performance. + */ + public transient TIntObjectMap fareLegRulesForToStopId; + // TODO contains_area_id, leg_group_id, timeframes /** @@ -825,9 +838,33 @@ public void rebuildTransientIndexes () { } } + if (!fareLegRules.isEmpty()) rebuildFaresV2TransientIndices(); + LOG.info("Done rebuilding transient indices."); } + /** + * Rebuild transient indices used in Fares V2 routing + */ + private void rebuildFaresV2TransientIndices () { + fareLegRulesForFromStopId = indexFareLegRulesForStops(fareLegRulesForFromAreaId); + fareLegRulesForToStopId = indexFareLegRulesForStops(fareLegRulesForToAreaId); + } + + /** Build an index for which fare leg rules are applicable for trips at each stop. Used for both from and to stops + * by passing in fareLegRulesForFromAreaId or fareLegRulesForToAreaId, respectively. + */ + private TIntObjectMap indexFareLegRulesForStops(TIntObjectMap fareLegRulesForFareAreaId) { + TIntObjectMap forStops = new TIntObjectHashMap<>(); + Map bitmaps = new HashMap<>(); + for (int stop = 0; stop < stopIdForIndex.size(); stop++) { + TIntList fareAreas = fareAreasForStop.get(stop); + // TODO could intern these RoaringBitmaps to save some memory if it becomes a problem + forStops.put(stop, IndexUtils.getMatching(fareLegRulesForFareAreaId, fareAreas)); + } + return forStops; + } + /** * Run a distance-constrained street search from every transit stop in the graph. * Store the distance to every reachable street vertex for each of these origin stops. From 65eab0a9013fa799ed81a80ba5921b6f5435c3a0 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 21 Jul 2020 14:56:55 -0400 Subject: [PATCH 10/46] feat(fares): properly handle as_route fares in transfer allowance --- .../FaresV2InRoutingFareCalculator.java | 24 ++++++++++- .../faresv2/FaresV2TransferAllowance.java | 42 ++++++++++++++++--- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index a796e14e0..0148ac2e8 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -61,6 +61,8 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat int prevFareLegRuleIdx = -1; int cumulativeFare = 0; + RoaringBitmap asRouteFareNetworks = null; + int asRouteBoardStop = -1; for (int i = 0; i < patterns.size(); i++) { int pattern = patterns.get(i); int boardStop = boardStops.get(i); @@ -71,8 +73,9 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat // CHECK FOR AS_ROUTE FARE NETWORK // NB this is applied greedily, if it is cheaper to buy separate tickets that will not be found RoaringBitmap fareNetworks = getFareNetworksForPattern(pattern); - RoaringBitmap asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); + asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); if (asRouteFareNetworks.getCardinality() > 0) { + asRouteBoardStop = boardStop; for (int j = i + 1; j < patterns.size(); j++) { RoaringBitmap nextAsRouteFareNetworks = getAsRouteFareNetworksForPattern(patterns.get(j)); // can't modify asRouteFareNetworks in-place as it may have already been set as fareNetworks below @@ -89,6 +92,9 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat break; } } + } else { + // reset as-route board stop if this leg is not a part of any as-route fare networks + asRouteBoardStop = -1; } // FIND THE FARE LEG RULE @@ -125,7 +131,21 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat prevFareLegRuleIdx = fareLegRuleIdx; } - return new FareBounds(cumulativeFare, new FaresV2TransferAllowance(prevFareLegRuleIdx, transitLayer)); + FaresV2TransferAllowance allowance; + // asRouteFareNetworks contains the as route fare networks that the last leg was a part of. If multiple rides + // have been spliced together, these will be the as-route fare networks that can be used to splice those rides, + // even if there are additional as_route fare networks that apply to later legs of the splice; we apply as_route + // fare networks greedily. + if (asRouteFareNetworks != null && asRouteFareNetworks.getCardinality() > 0) { + if (asRouteBoardStop == -1) + throw new IllegalStateException("as route board stop not set even though there are as route fare networks."); + // NB it is important the second argument here be sorted. This is guaranteed by RoaringBitmap.toArray() + allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, asRouteFareNetworks.toArray(), asRouteBoardStop, transitLayer); + } else { + allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, null, -1, transitLayer); + } + + return new FareBounds(cumulativeFare, allowance); } /** Get the as_route fare networks for a pattern (used to merge with later rides) */ diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index 1246ff769..0d8066236 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -9,10 +9,7 @@ import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; -import java.util.ArrayList; -import java.util.BitSet; -import java.util.Comparator; -import java.util.List; +import java.util.*; /** * Transfer allowance for Fares V2. @@ -24,11 +21,27 @@ public class FaresV2TransferAllowance extends TransferAllowance { /** need to hold on to a ref to this so that getTransferRuleSummary works - but make sure it's not accidentally serialized */ private transient TransitLayer transitLayer; - public FaresV2TransferAllowance (int prevFareLegRuleIdx, TransitLayer transitLayer) { + /** as_route fare networks we are currently in, that could have routes extended */ + public int[] asRouteFareNetworks; + + /** Where we boarded the as_route fare networks */ + public int asRouteFareNetworksBoardStop = -1; + + /** + * + * @param prevFareLegRuleIdx + * @param asRouteFareNetworks The as route fare networks this leg is in. Must be sorted. + * @param asRouteFareNetworksBoardStop + * @param transitLayer + */ + public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetworks, int asRouteFareNetworksBoardStop, TransitLayer transitLayer) { // the value is really high to effectively disable Theorem 3.1 for now, so we don't have to actually calculate // the max value, at the cost of some performance. super(10_000_000_00, 0, 0); + this.asRouteFareNetworks = asRouteFareNetworks; + this.asRouteFareNetworksBoardStop = asRouteFareNetworksBoardStop; + if (prevFareLegRuleIdx != -1) { // not at start of trip, so we may have transfers available // TODO this will all have to change once we properly handle chains of multiple transfers @@ -44,8 +57,25 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, TransitLayer transitLay @Override public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { if (other instanceof FaresV2TransferAllowance) { + FaresV2TransferAllowance o = (FaresV2TransferAllowance) other; + boolean exactlyOneHasAsRoute = asRouteFareNetworks == null && o.asRouteFareNetworks != null || + asRouteFareNetworks != null && o.asRouteFareNetworks == null; + // either could be better, one is in as_route network. Could assume that being in an as_route network is always + // better than not, conditional on same potentialTransferRules, but this is not always the case. + // see bullet 4 at https://indicatrix.org/post/regular-2-for-you-3-when-is-a-discount-not-a-discount/ + if (exactlyOneHasAsRoute) return false; + + // if both have as route, only comparable if they have the same as route networks and same board stop. + boolean bothHaveAsRoute = asRouteFareNetworks != null && o.asRouteFareNetworks != null; + if (bothHaveAsRoute) { + // asRouteFareNetworks is always sorted since it comes from RoaringBitset.toArray, so simple equality + // comparison is fine. + if (asRouteFareNetworksBoardStop != o.asRouteFareNetworksBoardStop || + !Arrays.equals(asRouteFareNetworks, o.asRouteFareNetworks)) return false; + } + // at least as good if it provides a superset of the transfers the other does - return potentialTransferRules.contains(((FaresV2TransferAllowance) other).potentialTransferRules); + return potentialTransferRules.contains(o.potentialTransferRules); } else { throw new IllegalArgumentException("mixing of transfer allowance types!"); } From e4d570fe0b5cdbd5aae89aed9ba4f2e72129a2e2 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sat, 25 Jul 2020 18:25:31 -0400 Subject: [PATCH 11/46] feat(fares-v2): improve performance by precalculating fare transfer rules for all from leg groups --- .../FaresV2InRoutingFareCalculator.java | 30 +++++++++++++++---- .../com/conveyal/r5/transit/TransitLayer.java | 29 ++++++++++++++++-- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index 0148ac2e8..709811612 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -206,17 +206,35 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap f /** get a fare transfer rule, if one exists, between fromLegRule and toLegRule */ public int getFareTransferRule (int fromLegRule, int toLegRule) { - RoaringBitmap fromLegMatch = getMatching(transitLayer.fareTransferRulesForFromLegGroupId, fromLegRule); - RoaringBitmap toLegMatch = getMatching(transitLayer.fareTransferRulesForToLegGroupId, toLegRule); + RoaringBitmap fromLegMatch; + if (transitLayer.fareTransferRulesForFromLegGroupId.containsKey(fromLegRule)) + // this is OR'ed with rules for fare_id_blank at build time + fromLegMatch = transitLayer.fareTransferRulesForFromLegGroupId.get(fromLegRule); + else if (transitLayer.fareTransferRulesForFromLegGroupId.containsKey(TransitLayer.FARE_ID_BLANK)) + // no explicit match, use implicit matches + fromLegMatch = transitLayer.fareTransferRulesForFromLegGroupId.get(TransitLayer.FARE_ID_BLANK); + else + return -1; - fromLegMatch.and(toLegMatch); + RoaringBitmap toLegMatch; + if (transitLayer.fareTransferRulesForToLegGroupId.containsKey(toLegRule)) + // this is OR'ed with rules for fare_id_blank at build time + toLegMatch = transitLayer.fareTransferRulesForToLegGroupId.get(toLegRule); + else if (transitLayer.fareTransferRulesForToLegGroupId.containsKey(TransitLayer.FARE_ID_BLANK)) + // no explicit match, use implicit matches + toLegMatch = transitLayer.fareTransferRulesForToLegGroupId.get(TransitLayer.FARE_ID_BLANK); + else + return -1; - if (fromLegMatch.getCardinality() == 0) return -1; // no discounted transfer - else if (fromLegMatch.getCardinality() == 1) return fromLegMatch.iterator().next(); + // use static and to create a new RoaringBitmap, don't destruct transitlayer values. + RoaringBitmap bothMatch = RoaringBitmap.and(fromLegMatch, toLegMatch); + + if (bothMatch.getCardinality() == 0) return -1; // no discounted transfer + else if (bothMatch.getCardinality() == 1) return bothMatch.iterator().next(); else { int lowestOrder = Integer.MAX_VALUE; TIntList rulesWithLowestOrder = new TIntArrayList(); - for (PeekableIntIterator it = fromLegMatch.getIntIterator(); it.hasNext();) { + for (PeekableIntIterator it = bothMatch.getIntIterator(); it.hasNext();) { int ruleIdx = it.next(); int order = transitLayer.fareTransferRules.get(ruleIdx).order; if (order < lowestOrder) { diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 84cc4f57b..579165c8f 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -16,6 +16,7 @@ import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; import gnu.trove.iterator.TIntIterator; +import gnu.trove.iterator.TIntObjectIterator; import gnu.trove.list.TDoubleList; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; @@ -241,10 +242,16 @@ public class TransitLayer implements Serializable, Cloneable { */ public List fareTransferRules = new ArrayList<>(); - /** Fare transfer rule index for from_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty from_leg_group_id */ + /** + * Fare transfer rule index for from_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty from_leg_group_id + * All rules from FARE_ID_BLANK are also included in rules for specific fare IDs. + */ public TIntObjectMap fareTransferRulesForFromLegGroupId = new TIntObjectHashMap<>(); - /** Fare transfer rule index for to_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty to_leg_group_id */ + /** + * Fare transfer rule index for to_leg_group_id, with key for FARE_ID_BLANK containing fare transfer rules with empty to_leg_group_id + * All rules from FARE_ID_BLANK are also included in rules for specific fare IDs. + */ public TIntObjectMap fareTransferRulesForToLegGroupId = new TIntObjectHashMap<>(); /** Map from feed ID to feed CRC32 to ensure that we can't apply scenarios to the wrong feeds */ @@ -772,6 +779,24 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS } } + // OR all FARE_ID_BLANK rules into fareTransferRulesForToLegGroupId, so it does not have to be done at + // runtime. FARE_ID_BLANK matches all fare transfer rules + if (fareTransferRulesForFromLegGroupId.containsKey(FARE_ID_BLANK)) { + RoaringBitmap wildcardRules = fareTransferRulesForFromLegGroupId.get(FARE_ID_BLANK); + for (TIntObjectIterator it = fareTransferRulesForFromLegGroupId.iterator(); it.hasNext();) { + it.advance(); + it.value().or(wildcardRules); + } + } + + if (fareTransferRulesForToLegGroupId.containsKey(FARE_ID_BLANK)) { + RoaringBitmap wildcardRules = fareTransferRulesForToLegGroupId.get(FARE_ID_BLANK); + for (TIntObjectIterator it = fareTransferRulesForToLegGroupId.iterator(); it.hasNext();) { + it.advance(); + it.value().or(wildcardRules); + } + } + LOG.info("Loaded {} fare transfer rules", fareTransferRules.size()); } From 33b75aae0e85bd4169864f5786ee9c34abe64624 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 26 Jul 2020 13:15:29 -0400 Subject: [PATCH 12/46] fix(fares-v2): use new fareTransferRuleForLegGroupId in transfer allowance --- .../r5/analyst/fare/faresv2/FaresV2TransferAllowance.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index 0d8066236..511e84ffa 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -42,11 +42,9 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetwor this.asRouteFareNetworks = asRouteFareNetworks; this.asRouteFareNetworksBoardStop = asRouteFareNetworksBoardStop; - if (prevFareLegRuleIdx != -1) { + if (prevFareLegRuleIdx != -1 && transitLayer.fareTransferRulesForFromLegGroupId.containsKey(prevFareLegRuleIdx)) { // not at start of trip, so we may have transfers available - // TODO this will all have to change once we properly handle chains of multiple transfers - potentialTransferRules = IndexUtils.getMatching( - transitLayer.fareTransferRulesForFromLegGroupId, prevFareLegRuleIdx); + potentialTransferRules = transitLayer.fareTransferRulesForFromLegGroupId.get(prevFareLegRuleIdx); } else { potentialTransferRules = new RoaringBitmap(); } @@ -68,7 +66,7 @@ public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { // if both have as route, only comparable if they have the same as route networks and same board stop. boolean bothHaveAsRoute = asRouteFareNetworks != null && o.asRouteFareNetworks != null; if (bothHaveAsRoute) { - // asRouteFareNetworks is always sorted since it comes from RoaringBitset.toArray, so simple equality + // asRouteFareNetworks is always sorted since it comes from RoaringBitset.toArray, so simple Arrays.equal // comparison is fine. if (asRouteFareNetworksBoardStop != o.asRouteFareNetworksBoardStop || !Arrays.equals(asRouteFareNetworks, o.asRouteFareNetworks)) return false; From a5e47665d9b0ef5549483656cbd0fa1e9228ad29 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 26 Jul 2020 13:31:01 -0400 Subject: [PATCH 13/46] feat(fares-v2): add LRU cache for better performance finding transfer leg rules --- .../FaresV2InRoutingFareCalculator.java | 59 ++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index 709811612..96f65b8e4 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -7,6 +7,9 @@ import com.conveyal.r5.transit.faresv2.FareLegRuleInfo; import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo; import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo.FareTransferType; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; import gnu.trove.TIntCollection; import gnu.trove.iterator.TIntIterator; import gnu.trove.list.TIntList; @@ -19,6 +22,10 @@ import org.roaringbitmap.RoaringBitmap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + +import java.util.Objects; +import java.util.concurrent.ExecutionException; + import static com.conveyal.r5.analyst.fare.faresv2.IndexUtils.getMatching; /** @@ -29,6 +36,15 @@ public class FaresV2InRoutingFareCalculator extends InRoutingFareCalculator { private static final Logger LOG = LoggerFactory.getLogger(FaresV2InRoutingFareCalculator.class); + private transient LoadingCache fareTransferRuleCache = CacheBuilder.newBuilder() + .maximumSize(1000) + .build(new CacheLoader<>() { + @Override + public Integer load(FareTransferRuleKey fareTransferRuleKey) { + return searchFareTransferRule(fareTransferRuleKey); + } + }); + @Override public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorState state, int maxClockTime) { TIntList patterns = new TIntArrayList(); @@ -204,8 +220,24 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap f } } - /** get a fare transfer rule, if one exists, between fromLegRule and toLegRule */ + /** + * get a fare transfer rule, if one exists, between fromLegRule and toLegRule + * + * This uses an LRU cache, because often we will be searching for the same fromLegRule and toLegRule repeatedly + * (e.g. transfers from a Toronto bus to many other possible Toronto buses you could transfer to.) + */ public int getFareTransferRule (int fromLegRule, int toLegRule) { + try { + return fareTransferRuleCache.get(new FareTransferRuleKey(fromLegRule, toLegRule)); + } catch (ExecutionException e) { + // should not happen. if it does, catch and re-throw. + throw new RuntimeException(e); + } + } + + private int searchFareTransferRule (FareTransferRuleKey key) { + int fromLegRule = key.fromLegGroupId; + int toLegRule = key.toLegGroupId; RoaringBitmap fromLegMatch; if (transitLayer.fareTransferRulesForFromLegGroupId.containsKey(fromLegRule)) // this is OR'ed with rules for fare_id_blank at build time @@ -253,6 +285,31 @@ else if (transitLayer.fareTransferRulesForToLegGroupId.containsKey(TransitLayer. } } + /** Used as a key into the LRU cache for fare transfer rules */ + private static class FareTransferRuleKey { + int fromLegGroupId; + int toLegGroupId; + + public FareTransferRuleKey (int fromLegGroupId, int toLegGroupId) { + this.fromLegGroupId = fromLegGroupId; + this.toLegGroupId = toLegGroupId; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + FareTransferRuleKey that = (FareTransferRuleKey) o; + return fromLegGroupId == that.fromLegGroupId && + toLegGroupId == that.toLegGroupId; + } + + @Override + public int hashCode() { + return Objects.hash(fromLegGroupId, toLegGroupId); + } + } + @Override public String getType() { return "fares-v2"; From 78db523cf9781913e7ec8b5d9d2bf1cefb111161 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 26 Jul 2020 14:31:23 -0400 Subject: [PATCH 14/46] feat(fares-v2): simplify fare network finding --- .../fare/faresv2/FaresV2InRoutingFareCalculator.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index 96f65b8e4..c1f141e67 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -166,11 +166,8 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat /** Get the as_route fare networks for a pattern (used to merge with later rides) */ private RoaringBitmap getAsRouteFareNetworksForPattern (int patIdx) { - RoaringBitmap fareNetworks = new RoaringBitmap(); - // protective copy - fareNetworks.or(getFareNetworksForPattern(patIdx)); - fareNetworks.and(transitLayer.fareNetworkAsRoute); - return fareNetworks; + // static so we do not modify underlying bitmaps + return RoaringBitmap.and(getFareNetworksForPattern(patIdx), transitLayer.fareNetworkAsRoute); } private RoaringBitmap getFareNetworksForPattern (int patIdx) { From 629a3e13e31c8099868716629d26bbac8fc39291 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sun, 26 Jul 2020 18:03:52 -0400 Subject: [PATCH 15/46] feat(fares-v2): add hack to compute as_route fare based on all stations traversed, not from and to station --- .../FaresV2InRoutingFareCalculator.java | 122 +++++++++++++++--- .../faresv2/FaresV2TransferAllowance.java | 27 +++- 2 files changed, 133 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index c1f141e67..925353c18 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -10,12 +10,9 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; -import gnu.trove.TIntCollection; import gnu.trove.iterator.TIntIterator; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; -import gnu.trove.map.TIntObjectMap; -import gnu.trove.map.TObjectIntMap; import gnu.trove.set.TIntSet; import gnu.trove.set.hash.TIntHashSet; import org.roaringbitmap.PeekableIntIterator; @@ -45,6 +42,31 @@ public Integer load(FareTransferRuleKey fareTransferRuleKey) { } }); + /** + * This is hack to address a situation where GTFS-Fares V2 is not (as of this writing) able to correctly represent + * the GO fare system. The GO fare chart _appears_ to be a simple from-station-A-to-station-B chart, a la WMATA etc., + * but it's more nuanced - because of one little word in the fare bylaws + * (https://www.gotransit.com/static_files/gotransit/assets/pdf/Policies/By-Law_No2A.pdf): "Tariff of Fares attached + * hereto, setting out the amount to be paid for single one-way travel on the transit system _within the + * enumerated zones_" - within, not from or to. So if you start in Zone B, backtrack to Zone A, and then ride on to + * Zone C, you actually owe the A - C fare, not the B - C fare, because you traveled to Zone A. There is currently + * active discussion in the GTFS-Fares V2 document for how to address this conundrum, but with deadlines looming I + * have implemented this hack. When useAllStopsWhenCalculatingAsRouteFareNetwork is set to true, when evaluating an + * as_route fare network, the router will consider rules matching from_area_ids of _any_ stop within the joined + * as_route trips except the final alight stop, and to_area_ids of _any_ stop except the first board stop. It is not + * only board stops considered for from and alight stops considered for to, because you might do a trip + * C - A walk to B - D, and this should cost the A-D fare even though you didn't ever board at A. + * By setting the order of rules in the feed to have the most + * expensive first, the proper fare will be found (assuming that extending the trip into a new zone always causes a + * nonnegative change in the fare). + * + * This is not a hypothetical concern in Toronto. Consider this trip: + * https://projects.indicatrix.org/fareto-examples/?load=broken-yyz-downtown-to-york + * The second option here is $6.80 but should be $7.80, because it requires a change at Unionville, and Toronto to + * Unionville is 7.80 even though Toronto to Yonge/407 is only $6.80. + */ + public boolean useAllStopsWhenCalculatingAsRouteFareNetwork = false; + @Override public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorState state, int maxClockTime) { TIntList patterns = new TIntArrayList(); @@ -77,6 +99,12 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat int prevFareLegRuleIdx = -1; int cumulativeFare = 0; + // these are used to implement the functionality described in the comment above + // useAllStopsWhenCalculatingAsRouteFareNetwork + // They keep track of _all_ the boarding and alighting stops in a multi-vehicle journey on an as_route fare network. + TIntSet allAsRouteFromStops = new TIntHashSet(); + TIntSet allAsRouteToStops = new TIntHashSet(); + RoaringBitmap asRouteFareNetworks = null; int asRouteBoardStop = -1; for (int i = 0; i < patterns.size(); i++) { @@ -88,9 +116,19 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat // CHECK FOR AS_ROUTE FARE NETWORK // NB this is applied greedily, if it is cheaper to buy separate tickets that will not be found + + // reset anything left over from previous rides + // note that if the rides are a part of the same as_route fare network, the ride is extended in the + // nested loop below. + asRouteBoardStop = -1; + allAsRouteFromStops.clear(); + allAsRouteToStops.clear(); + RoaringBitmap fareNetworks = getFareNetworksForPattern(pattern); asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); if (asRouteFareNetworks.getCardinality() > 0) { + allAsRouteFromStops.add(boardStop); + allAsRouteToStops.add(alightStop); asRouteBoardStop = boardStop; for (int j = i + 1; j < patterns.size(); j++) { RoaringBitmap nextAsRouteFareNetworks = getAsRouteFareNetworksForPattern(patterns.get(j)); @@ -98,23 +136,61 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat asRouteFareNetworks = RoaringBitmap.and(asRouteFareNetworks, nextAsRouteFareNetworks); if (asRouteFareNetworks.getCardinality() > 0) { + // alight stop from previous ride is now a from stop and a to stop, b/c it is in the middle of the ride + // This is true _even if_ there is a transfer rather than another boarding at the alight stop, see + // the example above in the javadoc for useAllStopsWhenCalculatingAsRouteFareNetwork + allAsRouteFromStops.add(alightStop); + + // board stop for new ride is now both a from and a to stop b/c is middle of ride + allAsRouteFromStops.add(boardStops.get(j)); + allAsRouteToStops.add(boardStops.get(j)); + // extend ride alightStop = alightStops.get(j); alightTime = alightTimes.get(j); + + allAsRouteToStops.add(alightStop); // these are the fare networks actually in use, other fare leg rules should not match fareNetworks = asRouteFareNetworks; i = j; // don't process this ride again } else { break; + // i is now the last ride in the as-route fare network. Process the entire thing as a single ride. } } - } else { - // reset as-route board stop if this leg is not a part of any as-route fare networks - asRouteBoardStop = -1; } // FIND THE FARE LEG RULE - int fareLegRuleIdx = getFareLegRuleForLeg(boardStop, alightStop, fareNetworks); + int fareLegRuleIdx; + if (asRouteBoardStop != -1 && useAllStopsWhenCalculatingAsRouteFareNetwork) { + // when useAllStopsWhenCalculatingAsRouteFareNetwork, we find the first fare leg rule that matches + // any combination of from and to stops. + RoaringBitmap fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); + RoaringBitmap fromAreaMatch = new RoaringBitmap(); + for (TIntIterator it = allAsRouteFromStops.iterator(); it.hasNext();) { + // okay to use forFromStopId even though this might be a to stop, because + // we're treating all intermediate stops as "effective from" stops that should match from_area_id. + fromAreaMatch.or(transitLayer.fareLegRulesForFromStopId.get(it.next())); + } + + RoaringBitmap toAreaMatch = new RoaringBitmap(); + for (TIntIterator it = allAsRouteToStops.iterator(); it.hasNext();) { + // okay to use forFromStopId even though this might be a to stop, because + // we're treating all intermediate stops as "effective from" stops that should match from_area_id. + toAreaMatch.or(transitLayer.fareLegRulesForToStopId.get(it.next())); + } + + fareNetworkMatch.and(fromAreaMatch); + fareNetworkMatch.and(toAreaMatch); + + try { + fareLegRuleIdx = findDominantLegRuleMatch(fareNetworkMatch); + } catch (NoFareLegRuleMatch noFareLegRuleMatch) { + throw new IllegalStateException("no leg rule found for as_route network!"); + } + } else { + fareLegRuleIdx = getFareLegRuleForLeg(boardStop, alightStop, fareNetworks); + } FareLegRuleInfo fareLegRule = transitLayer.fareLegRules.get(fareLegRuleIdx); // CHECK IF THERE ARE ANY TRANSFER DISCOUNTS @@ -156,9 +232,14 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat if (asRouteBoardStop == -1) throw new IllegalStateException("as route board stop not set even though there are as route fare networks."); // NB it is important the second argument here be sorted. This is guaranteed by RoaringBitmap.toArray() - allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, asRouteFareNetworks.toArray(), asRouteBoardStop, transitLayer); + if (!useAllStopsWhenCalculatingAsRouteFareNetwork) { + allAsRouteFromStops = null; + allAsRouteToStops = null; + } + allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, asRouteFareNetworks.toArray(), asRouteBoardStop, + allAsRouteFromStops, allAsRouteToStops, transitLayer); } else { - allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, null, -1, transitLayer); + allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, null, -1, null, null, transitLayer); } return new FareBounds(cumulativeFare, allowance); @@ -186,19 +267,26 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap f fareNetworkMatch.and(transitLayer.fareLegRulesForFromStopId.get(boardStop)); fareNetworkMatch.and(transitLayer.fareLegRulesForToStopId.get(alightStop)); - // boardAreaMatch now contains only rules that match _all_ criteria - - if (fareNetworkMatch.getCardinality() == 0) { + try { + return findDominantLegRuleMatch(fareNetworkMatch); + } catch (NoFareLegRuleMatch noFareLegRuleMatch) { String fromStopId = transitLayer.stopIdForIndex.get(boardStop); String toStopId = transitLayer.stopIdForIndex.get(alightStop); throw new IllegalStateException("no fare leg rule found for leg from " + fromStopId + " to " + toStopId + "!"); - } else if (fareNetworkMatch.getCardinality() == 1) { - return fareNetworkMatch.iterator().next(); + } + } + + /** of all the leg rules in match, which one is dominant (lowest order)? */ + private int findDominantLegRuleMatch (RoaringBitmap candidateLegRules) throws NoFareLegRuleMatch { + if (candidateLegRules.getCardinality() == 0) { + throw new NoFareLegRuleMatch(); + } else if (candidateLegRules.getCardinality() == 1) { + return candidateLegRules.iterator().next(); } else { // figure out what matches, first finding the lowest order int lowestOrder = Integer.MAX_VALUE; TIntList rulesWithLowestOrder = new TIntArrayList(); - for (PeekableIntIterator it = fareNetworkMatch.getIntIterator(); it.hasNext();) { + for (PeekableIntIterator it = candidateLegRules.getIntIterator(); it.hasNext();) { int ruleIdx = it.next(); int order = transitLayer.fareLegRules.get(ruleIdx).order; if (order < lowestOrder) { @@ -311,4 +399,8 @@ public int hashCode() { public String getType() { return "fares-v2"; } + + private class NoFareLegRuleMatch extends Exception { + + } } diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index 511e84ffa..40f8235a4 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -27,6 +27,13 @@ public class FaresV2TransferAllowance extends TransferAllowance { /** Where we boarded the as_route fare networks */ public int asRouteFareNetworksBoardStop = -1; + /** When useAllStopsWhenCalculatingAsRouteFareNetwork = true in FaresV2InRoutingFareCalculator, we need to + * differentiate trips inside as_route fare networks by all the stops they use, not just the board stop. + */ + private TIntSet allAsRouteFromStops; + + private TIntSet allAsRouteToStops; + /** * * @param prevFareLegRuleIdx @@ -34,13 +41,16 @@ public class FaresV2TransferAllowance extends TransferAllowance { * @param asRouteFareNetworksBoardStop * @param transitLayer */ - public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetworks, int asRouteFareNetworksBoardStop, TransitLayer transitLayer) { + public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetworks, int asRouteFareNetworksBoardStop, + TIntSet allAsRouteFromStops, TIntSet allAsRouteToStops, TransitLayer transitLayer) { // the value is really high to effectively disable Theorem 3.1 for now, so we don't have to actually calculate // the max value, at the cost of some performance. super(10_000_000_00, 0, 0); this.asRouteFareNetworks = asRouteFareNetworks; this.asRouteFareNetworksBoardStop = asRouteFareNetworksBoardStop; + this.allAsRouteFromStops = allAsRouteFromStops; + this.allAsRouteToStops = allAsRouteToStops; if (prevFareLegRuleIdx != -1 && transitLayer.fareTransferRulesForFromLegGroupId.containsKey(prevFareLegRuleIdx)) { // not at start of trip, so we may have transfers available @@ -69,6 +79,10 @@ public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { // asRouteFareNetworks is always sorted since it comes from RoaringBitset.toArray, so simple Arrays.equal // comparison is fine. if (asRouteFareNetworksBoardStop != o.asRouteFareNetworksBoardStop || + // both will be null if useAllStopsWhenCalculatingAsRouteFareNetwork is false, so Objects.equals + // will return true and these conditions will be a no-op. + !Objects.equals(allAsRouteFromStops, o.allAsRouteFromStops) || + !Objects.equals(allAsRouteToStops, o.allAsRouteToStops) || !Arrays.equals(asRouteFareNetworks, o.asRouteFareNetworks)) return false; } @@ -103,4 +117,15 @@ public String getTransferRuleSummary () { return String.join("\n", transfers); } + + /** better JSON serialization for the TIntSets */ + public int[] getAllAsRouteFromStops () { + if (allAsRouteFromStops == null) return null; + else return allAsRouteFromStops.toArray(); + } + + public int[] getAllAsRouteToStops () { + if (allAsRouteToStops == null) return null; + else return allAsRouteToStops.toArray(); + } } From 0931547e2a91cc7d63ef1a72154eab92eae041cf Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 27 Jul 2020 20:48:51 -0400 Subject: [PATCH 16/46] feat(fares-v2): improve performance for useAllStopsWhenCalculatingAsRouteFareNetwork --- .../FaresV2InRoutingFareCalculator.java | 67 +++++++------ .../faresv2/FaresV2TransferAllowance.java | 95 ++++++++++++++----- .../r5/analyst/fare/faresv2/IndexUtils.java | 8 +- .../r5/transit/faresv2/FareLegRuleInfo.java | 4 + 4 files changed, 118 insertions(+), 56 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index 925353c18..a4ef5f8b7 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -20,6 +20,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Arrays; import java.util.Objects; import java.util.concurrent.ExecutionException; @@ -99,14 +100,13 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat int prevFareLegRuleIdx = -1; int cumulativeFare = 0; - // these are used to implement the functionality described in the comment above - // useAllStopsWhenCalculatingAsRouteFareNetwork - // They keep track of _all_ the boarding and alighting stops in a multi-vehicle journey on an as_route fare network. - TIntSet allAsRouteFromStops = new TIntHashSet(); - TIntSet allAsRouteToStops = new TIntHashSet(); - RoaringBitmap asRouteFareNetworks = null; int asRouteBoardStop = -1; + + // What fare leg rules are potentially applicable to a trip in an as_route fare network + // used in transfer allowance when useAllStopsWhenCalculatingAsRouteFareNetwork = true. + // see comment on same-named variable in transfer allowance for detailed explanation. + int[] potentialAsRouteFareLegRules = null; for (int i = 0; i < patterns.size(); i++) { int pattern = patterns.get(i); int boardStop = boardStops.get(i); @@ -121,8 +121,11 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat // note that if the rides are a part of the same as_route fare network, the ride is extended in the // nested loop below. asRouteBoardStop = -1; - allAsRouteFromStops.clear(); - allAsRouteToStops.clear(); + // these are used to implement the functionality described in the comment above + // useAllStopsWhenCalculatingAsRouteFareNetwork + // They keep track of _all_ the boarding and alighting stops in a multi-vehicle journey on an as_route fare network. + TIntSet allAsRouteFromStops = new TIntHashSet(); + TIntSet allAsRouteToStops = new TIntHashSet(); RoaringBitmap fareNetworks = getFareNetworksForPattern(pattern); asRouteFareNetworks = getAsRouteFareNetworksForPattern(pattern); @@ -161,11 +164,12 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat } // FIND THE FARE LEG RULE - int fareLegRuleIdx; + int[] fareLegRules; if (asRouteBoardStop != -1 && useAllStopsWhenCalculatingAsRouteFareNetwork) { // when useAllStopsWhenCalculatingAsRouteFareNetwork, we find the first fare leg rule that matches // any combination of from and to stops. - RoaringBitmap fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); + // getMatching returns a new RoaringBitmap, okay for us to mutate + RoaringBitmap candidateLegs = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); RoaringBitmap fromAreaMatch = new RoaringBitmap(); for (TIntIterator it = allAsRouteFromStops.iterator(); it.hasNext();) { // okay to use forFromStopId even though this might be a to stop, because @@ -180,17 +184,29 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat toAreaMatch.or(transitLayer.fareLegRulesForToStopId.get(it.next())); } - fareNetworkMatch.and(fromAreaMatch); - fareNetworkMatch.and(toAreaMatch); + candidateLegs.and(fromAreaMatch); + candidateLegs.and(toAreaMatch); try { - fareLegRuleIdx = findDominantLegRuleMatch(fareNetworkMatch); + fareLegRules = findDominantLegRuleMatches(candidateLegs); + Arrays.sort(fareLegRules); // I think they should already be sorted, this may not be necessary. + potentialAsRouteFareLegRules = fareLegRules; } catch (NoFareLegRuleMatch noFareLegRuleMatch) { throw new IllegalStateException("no leg rule found for as_route network!"); } + + // it is not unexpected to find multiple matching fare leg rules here, as there may be multiple + // fare leg rules that have the same price for different portions of the trip. As long as they provide + // the same transfer privileges, this is okay. } else { - fareLegRuleIdx = getFareLegRuleForLeg(boardStop, alightStop, fareNetworks); + fareLegRules = getFareLegRulesForLeg(boardStop, alightStop, fareNetworks); + + if (fareLegRules.length > 1) { + LOG.warn("Found multiple matching fare leg rules - routes and fares may be unstable!"); + } } + + int fareLegRuleIdx = fareLegRules[0]; FareLegRuleInfo fareLegRule = transitLayer.fareLegRules.get(fareLegRuleIdx); // CHECK IF THERE ARE ANY TRANSFER DISCOUNTS @@ -232,14 +248,12 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat if (asRouteBoardStop == -1) throw new IllegalStateException("as route board stop not set even though there are as route fare networks."); // NB it is important the second argument here be sorted. This is guaranteed by RoaringBitmap.toArray() - if (!useAllStopsWhenCalculatingAsRouteFareNetwork) { - allAsRouteFromStops = null; - allAsRouteToStops = null; - } + allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, asRouteFareNetworks.toArray(), asRouteBoardStop, - allAsRouteFromStops, allAsRouteToStops, transitLayer); + potentialAsRouteFareLegRules, transitLayer); } else { - allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, null, -1, null, null, transitLayer); + allowance = new FaresV2TransferAllowance(prevFareLegRuleIdx, null, -1, + null, transitLayer); } return new FareBounds(cumulativeFare, allowance); @@ -260,7 +274,7 @@ private RoaringBitmap getFareNetworksForPattern (int patIdx) { * Get the fare leg rule for a leg. If there is more than one, which one is returned is undefined and a warning is logged. * TODO handle multiple fare leg rules */ - private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap fareNetworks) { + private int[] getFareLegRulesForLeg (int boardStop, int alightStop, RoaringBitmap fareNetworks) { // find leg rules that match the fare network // getMatching returns a new RoaringBitmap so okay to modify RoaringBitmap fareNetworkMatch = getMatching(transitLayer.fareLegRulesForFareNetworkId, fareNetworks); @@ -268,7 +282,7 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap f fareNetworkMatch.and(transitLayer.fareLegRulesForToStopId.get(alightStop)); try { - return findDominantLegRuleMatch(fareNetworkMatch); + return findDominantLegRuleMatches(fareNetworkMatch); } catch (NoFareLegRuleMatch noFareLegRuleMatch) { String fromStopId = transitLayer.stopIdForIndex.get(boardStop); String toStopId = transitLayer.stopIdForIndex.get(alightStop); @@ -277,11 +291,11 @@ private int getFareLegRuleForLeg (int boardStop, int alightStop, RoaringBitmap f } /** of all the leg rules in match, which one is dominant (lowest order)? */ - private int findDominantLegRuleMatch (RoaringBitmap candidateLegRules) throws NoFareLegRuleMatch { + private int[] findDominantLegRuleMatches (RoaringBitmap candidateLegRules) throws NoFareLegRuleMatch { if (candidateLegRules.getCardinality() == 0) { throw new NoFareLegRuleMatch(); } else if (candidateLegRules.getCardinality() == 1) { - return candidateLegRules.iterator().next(); + return new int[] { candidateLegRules.iterator().next() }; } else { // figure out what matches, first finding the lowest order int lowestOrder = Integer.MAX_VALUE; @@ -298,10 +312,7 @@ private int findDominantLegRuleMatch (RoaringBitmap candidateLegRules) throws No } } - if (rulesWithLowestOrder.size() > 1) - LOG.warn("Found multiple matching fare_leg_rules with same order, results may be unstable or not find the lowest fare path!"); - - return rulesWithLowestOrder.get(0); + return rulesWithLowestOrder.toArray(); } } diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index 40f8235a4..97458a7c3 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -2,14 +2,18 @@ import com.conveyal.r5.analyst.fare.TransferAllowance; import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.faresv2.FareLegRuleInfo; import com.conveyal.r5.transit.faresv2.FareTransferRuleInfo; -import gnu.trove.iterator.TIntIterator; -import gnu.trove.set.TIntSet; -import gnu.trove.set.hash.TIntHashSet; import org.roaringbitmap.PeekableIntIterator; import org.roaringbitmap.RoaringBitmap; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; +import java.util.stream.IntStream; /** * Transfer allowance for Fares V2. @@ -27,30 +31,62 @@ public class FaresV2TransferAllowance extends TransferAllowance { /** Where we boarded the as_route fare networks */ public int asRouteFareNetworksBoardStop = -1; - /** When useAllStopsWhenCalculatingAsRouteFareNetwork = true in FaresV2InRoutingFareCalculator, we need to - * differentiate trips inside as_route fare networks by all the stops they use, not just the board stop. + /** + * When useAllStopsWhenCalculatingAsRouteFareNetwork = true in FaresV2InRoutingFareCalculator, we need to + * differentiate trips inside as_route fare networks by the fare zones they use, not just the board stop. + * + * potentialAsRouteFareLegRules should contain all the fare leg rules that could apply to the as_route trip. More + * specifically, two conditions must hold for potentialAsRouteFareLegs: 1) it must include the fare leg rule that + * represents the full extent of the trip (e.g. for a B - C - A trip, it must include C - A), and it must _not_ include + * any fare leg rule larger than the full extent of the trip. (Full extent imagines the fare zones as being linear, + * but in Toronto where this useAllStopsWhenCalculatingAsRouteFareNetwork we are taking "full extent" to mean "most + * expensive", and though space is two-dimensional, money is one-dimensional.) If the most expensive fare leg rules + * are the ones that are in this bitmap, then the same logic should apply - two routes that have the same most + * expensive fare leg rule(s) cover the same extents. + * + * To keep this tractable, the bitmap only retains the fare leg rules with the lowest order. Thus, the fare leg rule + * for the full extent must always have the lowest order in the feed. This is generally true anyways, since the fare + * leg rule with the lowest order will be the one returned, but if A-B and A-C are the same price, you might be sloppy + * and assign order randomly for these two fare pairs. But to get proper transfer allowance domination logic, A-C + * must have a lower order or the same order as A-B. If they have the same fare and transfer privileges, routing will + * not be affected if they have the same order - the A-B fare leg may be used when A-C should really be, but that + * will not affect the result as the two fare legs are equivalent. + * + * The second condition is that all of the fare leg rules in potentialAsRouteFareLegRules must be applicable to the + * full as_route journey or subjourneys of it. + * + * If both of these conditions hold, then the two fare leg rules cover the same territory and can be considered + * equivalent. If fare leg rules 1 and 2 are the most expensive ("full extent"), and + * B.potentialAsRouteFareLegRules == A.potentialAsRouteFareRules, then A and B cover the same territory. + * Proof: + * 1. By condition 1, if 1 and 2 are the most extensive/expensive fare leg rules for option A, then they must appear in + * A.potentialAsRouteFareRules. + * 2. By condition 2, no other more expensive/extensive fare leg rules can appear in A.potentialAsRouteFareRules. + * 3. If A.potentialAsRouteFareRules == B.potentialAsRouteFareRules, then 1 and 2 are the most expensive/extensive + * fare leg rules for B as well as A. + * 4. A and B are thus equally extensive/expensive. + * Q.E.D. */ - private TIntSet allAsRouteFromStops; - - private TIntSet allAsRouteToStops; + private int[] potentialAsRouteFareLegRules; /** * * @param prevFareLegRuleIdx * @param asRouteFareNetworks The as route fare networks this leg is in. Must be sorted. + * @param potentialAsRouteFareLegRules potential fare rules for an as_route network; see comment in javadoc on + * this.potentialAsRouteFareLegs. must be sorted. * @param asRouteFareNetworksBoardStop * @param transitLayer */ public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetworks, int asRouteFareNetworksBoardStop, - TIntSet allAsRouteFromStops, TIntSet allAsRouteToStops, TransitLayer transitLayer) { + int[] potentialAsRouteFareLegRules, TransitLayer transitLayer) { // the value is really high to effectively disable Theorem 3.1 for now, so we don't have to actually calculate // the max value, at the cost of some performance. super(10_000_000_00, 0, 0); this.asRouteFareNetworks = asRouteFareNetworks; this.asRouteFareNetworksBoardStop = asRouteFareNetworksBoardStop; - this.allAsRouteFromStops = allAsRouteFromStops; - this.allAsRouteToStops = allAsRouteToStops; + this.potentialAsRouteFareLegRules = potentialAsRouteFareLegRules; if (prevFareLegRuleIdx != -1 && transitLayer.fareTransferRulesForFromLegGroupId.containsKey(prevFareLegRuleIdx)) { // not at start of trip, so we may have transfers available @@ -77,12 +113,12 @@ public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { boolean bothHaveAsRoute = asRouteFareNetworks != null && o.asRouteFareNetworks != null; if (bothHaveAsRoute) { // asRouteFareNetworks is always sorted since it comes from RoaringBitset.toArray, so simple Arrays.equal - // comparison is fine. + // comparison is fine. potentialAsRouteFareLegs is also always sorted. if (asRouteFareNetworksBoardStop != o.asRouteFareNetworksBoardStop || // both will be null if useAllStopsWhenCalculatingAsRouteFareNetwork is false, so Objects.equals // will return true and these conditions will be a no-op. - !Objects.equals(allAsRouteFromStops, o.allAsRouteFromStops) || - !Objects.equals(allAsRouteToStops, o.allAsRouteToStops) || + // See proof in javadoc for potentialAsRouteFareLegRules for why this comparison is correct + !Arrays.equals(potentialAsRouteFareLegRules, o.potentialAsRouteFareLegRules) || !Arrays.equals(asRouteFareNetworks, o.asRouteFareNetworks)) return false; } @@ -102,8 +138,10 @@ public TransferAllowance tightenExpiration(int maxClockTime) { * Displaying a bunch of ints in the debug interface is going to be impossible to debug. Instead, generate an * on the fly string representation. This is not called in routing so performance isn't really an issue. */ - public String getTransferRuleSummary () { - if (transitLayer == null) return potentialTransferRules.toString(); + public List getTransferRuleSummary () { + if (transitLayer == null) return IntStream.of(potentialTransferRules.toArray()) + .mapToObj(Integer::toString) + .collect(Collectors.toList()); List transfers = new ArrayList<>(); @@ -115,17 +153,22 @@ public String getTransferRuleSummary () { transfers.sort(Comparator.naturalOrder()); - return String.join("\n", transfers); + return transfers; } - /** better JSON serialization for the TIntSets */ - public int[] getAllAsRouteFromStops () { - if (allAsRouteFromStops == null) return null; - else return allAsRouteFromStops.toArray(); - } + public List getPotentialAsRouteFareLegRules () { + if (potentialAsRouteFareLegRules == null) return null; + List result = IntStream.of(potentialAsRouteFareLegRules) + .mapToObj(legRule -> { + if (transitLayer == null) return Integer.toString(legRule); + FareLegRuleInfo info = transitLayer.fareLegRules.get(legRule); + if (info.leg_group_id != null) return info.leg_group_id; + else return Integer.toString(legRule); + }) + .collect(Collectors.toList()); + + result.sort(Comparator.naturalOrder()); - public int[] getAllAsRouteToStops () { - if (allAsRouteToStops == null) return null; - else return allAsRouteToStops.toArray(); + return result; } } diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java index b6a7a49e1..7e0f23000 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/IndexUtils.java @@ -14,6 +14,8 @@ public class IndexUtils { * Combine those rules into a single RoaringBitmap. Used for instance with fareLegRulesForFareAreaId in TransitLayer, * passing in a collection fare area indices, and returning a RoaringBitmap of all FareLegRules that match any of those * indices. + * + * Always returns a new bitmap, okay to mutate return value. */ public static RoaringBitmap getMatching (TIntObjectMap rules, TIntCollection indices) { RoaringBitmap ret = new RoaringBitmap(); @@ -25,7 +27,8 @@ public static RoaringBitmap getMatching (TIntObjectMap rules, TIn return ret; } - /** Get all rules that match indices, either directly or because that field was left blank */ + /** Get all rules that match indices, either directly or because that field was left blank. Always returns + * a new bitmap, okay to mutate return value. */ public static RoaringBitmap getMatching (TIntObjectMap rules, RoaringBitmap indices) { RoaringBitmap ret = new RoaringBitmap(); for (PeekableIntIterator it = indices.getIntIterator(); it.hasNext();) { @@ -36,7 +39,8 @@ public static RoaringBitmap getMatching (TIntObjectMap rules, Roa return ret; } - /** Get all rules that match index, either directly or because that field was left blank */ + /** Get all rules that match index, either directly or because that field was left blank. Always returns + * a new bitmap, okay to mutate return value. */ public static RoaringBitmap getMatching (TIntObjectMap rules, int index) { RoaringBitmap ret = new RoaringBitmap(); if (rules.containsKey(index)) ret.or(rules.get(index)); diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java b/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java index f7e3c98dd..949bd0573 100644 --- a/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java +++ b/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java @@ -15,6 +15,9 @@ public class FareLegRuleInfo implements Serializable, Comparable { /** the order of this fare leg rule */ public int order; + /** leg group ID of this fare leg rule */ + public String leg_group_id; + public FareLegRuleInfo(FareLegRule rule) { if (!Currency.scalarForCurrency.containsKey(rule.currency)) throw new IllegalStateException("No scalar value specified in scalarForCurrency for currency " + rule.currency); @@ -23,6 +26,7 @@ public FareLegRuleInfo(FareLegRule rule) { throw new IllegalArgumentException("Amount missing from fare_leg_rule (min_amount/max_amount not supported!"); amount = (int) (rule.amount * currencyScalar); order = rule.order; + leg_group_id = rule.leg_group_id; } @Override From f810c7716911a827e38cc4e0f46d9404ff8accb6 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 3 Aug 2020 15:33:04 -0400 Subject: [PATCH 17/46] docs(gtfs-fares-v2): update comment in faresv2 transfer allowance --- .../faresv2/FaresV2TransferAllowance.java | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index 97458a7c3..a23d77dfc 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -37,34 +37,35 @@ public class FaresV2TransferAllowance extends TransferAllowance { * * potentialAsRouteFareLegRules should contain all the fare leg rules that could apply to the as_route trip. More * specifically, two conditions must hold for potentialAsRouteFareLegs: 1) it must include the fare leg rule that - * represents the full extent of the trip (e.g. for a B - C - A trip, it must include C - A), and it must _not_ include + * represents the full extent of the trip (e.g. for a B - C - A trip, it must include C - A), and 2) it must _not_ include * any fare leg rule larger than the full extent of the trip. (Full extent imagines the fare zones as being linear, * but in Toronto where this useAllStopsWhenCalculatingAsRouteFareNetwork we are taking "full extent" to mean "most * expensive", and though space is two-dimensional, money is one-dimensional.) If the most expensive fare leg rules - * are the ones that are in this bitmap, then the same logic should apply - two routes that have the same most - * expensive fare leg rule(s) cover the same extents. + * are in this array, then the same logic should apply - two routes that have the same most expensive fare leg rule(s) + * cover the same extents. * - * To keep this tractable, the bitmap only retains the fare leg rules with the lowest order. Thus, the fare leg rule + * To keep this tractable, the array only retains the fare leg rules with the lowest order. Thus, the fare leg rule * for the full extent must always have the lowest order in the feed. This is generally true anyways, since the fare * leg rule with the lowest order will be the one returned, but if A-B and A-C are the same price, you might be sloppy * and assign order randomly for these two fare pairs. But to get proper transfer allowance domination logic, A-C * must have a lower order or the same order as A-B. If they have the same fare and transfer privileges, routing will * not be affected if they have the same order - the A-B fare leg may be used when A-C should really be, but that - * will not affect the result as the two fare legs are equivalent. + * will not affect the result as the two fare legs are equivalent, and A-C will still appear in this array and satisfy + * the two conditions above. * - * The second condition is that all of the fare leg rules in potentialAsRouteFareLegRules must be applicable to the - * full as_route journey or subjourneys of it. + * If both of these conditions hold, then the two journeys with the same potentialAsRouteFareRules cover the same + * territory and can be considered equivalent. * - * If both of these conditions hold, then the two fare leg rules cover the same territory and can be considered - * equivalent. If fare leg rules 1 and 2 are the most expensive ("full extent"), and - * B.potentialAsRouteFareLegRules == A.potentialAsRouteFareRules, then A and B cover the same territory. * Proof: - * 1. By condition 1, if 1 and 2 are the most extensive/expensive fare leg rules for option A, then they must appear in - * A.potentialAsRouteFareRules. - * 2. By condition 2, no other more expensive/extensive fare leg rules can appear in A.potentialAsRouteFareRules. - * 3. If A.potentialAsRouteFareRules == B.potentialAsRouteFareRules, then 1 and 2 are the most expensive/extensive - * fare leg rules for B as well as A. - * 4. A and B are thus equally extensive/expensive. + * Suppose without loss of generality that fare leg rules 1 and 2 are the most expensive ("full extent") for journey + * Q, and R.potentialAsRouteFareLegRules == Q.potentialAsRouteFareRules. + * + * 1. By condition 1, if 1 and 2 are the most extensive/expensive fare leg rules for journey Q, then they must appear in + * Q.potentialAsRouteFareRules. + * 2. By condition 2, no other more expensive/extensive fare leg rules can appear in Q.potentialAsRouteFareRules. + * 3. If Q.potentialAsRouteFareRules == R.potentialAsRouteFareRules, then 1 and 2 are the most expensive/extensive + * fare leg rules for R as well as Q. + * 4. Q and R are thus equally extensive/expensive. * Q.E.D. */ private int[] potentialAsRouteFareLegRules; From 25d4517ea2e27f06fba3987f93a89115eb34b6c3 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 3 Aug 2020 15:44:42 -0400 Subject: [PATCH 18/46] docs(fares-v2): additional comment updates. Co-authored-by: Anson Stewart --- .../r5/analyst/fare/faresv2/FaresV2TransferAllowance.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index a23d77dfc..ae6066035 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -44,9 +44,10 @@ public class FaresV2TransferAllowance extends TransferAllowance { * are in this array, then the same logic should apply - two routes that have the same most expensive fare leg rule(s) * cover the same extents. * - * To keep this tractable, the array only retains the fare leg rules with the lowest order. Thus, the fare leg rule - * for the full extent must always have the lowest order in the feed. This is generally true anyways, since the fare - * leg rule with the lowest order will be the one returned, but if A-B and A-C are the same price, you might be sloppy + * To keep this tractable, the array only retains the fare leg rules with the lowest order + * (as set in fare_leg_rules.txt). Thus, the fare leg rulefor the full extent must always have the + * lowest order in the feed. This is generally true anyways, since the fare leg rule with the lowest + * order will be the one returned, but if A-B and A-C are the same price, you might be sloppy * and assign order randomly for these two fare pairs. But to get proper transfer allowance domination logic, A-C * must have a lower order or the same order as A-B. If they have the same fare and transfer privileges, routing will * not be affected if they have the same order - the A-B fare leg may be used when A-C should really be, but that From ca2414c9c744a7b85cc6733592969075dd63b8e6 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 3 Aug 2020 19:37:59 -0400 Subject: [PATCH 19/46] docs(fares-v2): additional comments --- .../faresv2/FaresV2InRoutingFareCalculator.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java index a4ef5f8b7..776e5e218 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2InRoutingFareCalculator.java @@ -44,6 +44,18 @@ public Integer load(FareTransferRuleKey fareTransferRuleKey) { }); /** + * If true, consider all stops at which an as_route journey boards or alights as potentially triggering a higher + * fare (e.g. "via" fares used in London, Toronto), and calculate fares based on the most extensive fare leg rule + * for the trip. + * + * If false, simply use the first boarding and last alighting stop of a journey to calculate the fare. + * + * This requires setting the order field in fare_leg_rules so that more extensive + * fare leg rules have lower order. If the system does not have linear systems of zones, the order can be set based on the + * cost of the fare_leg_rule, as long as adding an additional zone to a trip cannot make the fare go down. Fare leg rules + * with the same fare must be given the same order for transfer allowances to work correctly; see comments on + * FaresV2TransferAllowance.potentialAsRouteFareRules. + * * This is hack to address a situation where GTFS-Fares V2 is not (as of this writing) able to correctly represent * the GO fare system. The GO fare chart _appears_ to be a simple from-station-A-to-station-B chart, a la WMATA etc., * but it's more nuanced - because of one little word in the fare bylaws @@ -61,8 +73,8 @@ public Integer load(FareTransferRuleKey fareTransferRuleKey) { * expensive first, the proper fare will be found (assuming that extending the trip into a new zone always causes a * nonnegative change in the fare). * - * This is not a hypothetical concern in Toronto. Consider this trip: - * https://projects.indicatrix.org/fareto-examples/?load=broken-yyz-downtown-to-york + * The need to calculate as_route fares based on the full journey extent is not a hypothetical concern in Toronto. + * Consider this trip: https://projects.indicatrix.org/fareto-examples/?load=broken-yyz-downtown-to-york * The second option here is $6.80 but should be $7.80, because it requires a change at Unionville, and Toronto to * Unionville is 7.80 even though Toronto to Yonge/407 is only $6.80. */ From 30aa94de09e8c9f64b0a0cd805e3fd689b371f8a Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 3 Aug 2020 19:38:29 -0400 Subject: [PATCH 20/46] fix(fares-v2): round currency instead of flooring, workaround for #131 --- .../r5/transit/faresv2/FareLegRuleInfo.java | 23 ++++++++++++++++++- .../transit/faresv2/FareTransferRuleInfo.java | 6 ++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java b/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java index 949bd0573..f8ada89aa 100644 --- a/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java +++ b/src/main/java/com/conveyal/r5/transit/faresv2/FareLegRuleInfo.java @@ -24,7 +24,28 @@ public FareLegRuleInfo(FareLegRule rule) { int currencyScalar = Currency.scalarForCurrency.get(rule.currency); if (Double.isNaN(rule.amount)) throw new IllegalArgumentException("Amount missing from fare_leg_rule (min_amount/max_amount not supported!"); - amount = (int) (rule.amount * currencyScalar); + // it is important to round here, rather than just cast to int, because though in theory + // rule.amount * currencyScalar should always exactly equal an integer, the subleties of floating point math + // mean that is not always the case. For instance, consider a fare of 8.20. In double-precision floating point + // math, 8.2 * 100 = 819.999999, and (int) (8.2 * 100) = 819, so we lose a cent. This creates havoc with, for + // example, this trip in Toronto: https://projects.indicatrix.org/fareto-examples/?load=broken-yyz-floating-point + // The fare for this trip should be 3.20 for TTC + 8.20 for GO + 0.80 discounted transfer to MiWay = 12.20, and + // in fact 12.20 is the answer we get (making this issue more confusing). However, when you look at the + // intermediate fares, we have 320 + 819 + 81, which took me a long time to figure out. First, note that the + // 0.80 discounted transfer is implemented as 3.10 fare minus a 2.30 discount. Then note that: + // > Math.floor(3.2 * 100) // -> 320, correct TTC fare + // > Math.floor(8.2 * 100) // -> 819, one cent less than it should be + // > Math.floor(3.1 * 100) - Math.floor(2.3 * 100) // -> 81, one cent _more_ than it should be + + // NB this was computed on a c. 2015 Macbook Pro with an Intel Core i7 (x86_64 architecture). It is possible + // that results would be different on different CPU architectures, e.g. ARM. + + // The roundoff errors cancel here, making this a very difficult problem to understand. + // Ideally we wouldn't be representing currency as doubles at all, but rounding should solve the problem for all + // reasonable fare levels, as the resolution of a float + + // "In theory, theory and practice are the same thing." -- Yogi Berra + amount = (int) Math.round(rule.amount * currencyScalar); order = rule.order; leg_group_id = rule.leg_group_id; } diff --git a/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java b/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java index 61f795483..844a13424 100644 --- a/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java +++ b/src/main/java/com/conveyal/r5/transit/faresv2/FareTransferRuleInfo.java @@ -27,7 +27,11 @@ public FareTransferRuleInfo (FareTransferRule rule) { int currencyScalar = Currency.scalarForCurrency.get(rule.currency); if (Double.isNaN(rule.amount)) throw new IllegalArgumentException("Amount missing from fare_leg_rule (min_amount/max_amount not supported!"); - amount = (int) (rule.amount * currencyScalar); + + // it is important to round here, rather than just cast to int, because though in theory + // rule.amount * currencyScalar should always exactly equal an integer, the subleties of floating point math + // mean that is not always the case. See extensive comment in FareLegRuleInfo. + amount = (int) Math.round(rule.amount * currencyScalar); order = rule.order; spanning_limit = rule.spanning_limit; duration_limit = rule.duration_limit; From fb236946164a88c01323f54b9e6f03437fc7b60b Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 4 Aug 2020 20:00:46 -0400 Subject: [PATCH 21/46] feat(gtfs-fares-v2): make the search process a lot faster by replacing bitset comparison with int comparison. Previously we compared whether a leg had a superset of the potentially-active fare transfer rules of another leg, but because most fare_transfer_rules specify a from_leg_group_id, the superset would only be active in a few cases: 1) when there are transfer rules with no from_leg_group_id 2) when two trips have the same price but one has no transfer allowance 3) when the two trips have the same previous leg group 99% of the time (not measured), it was the third condition that eliminated legs. We can implement the third condition with a simple integer equality, rather than a BitSet operation. The others are rare enough that ignoring them makes us retain maybe a few more trips, but not enough to outweight the performance penalty of a big BitSet comparison. --- .../faresv2/FaresV2TransferAllowance.java | 58 ++++++++++--------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index ae6066035..f1b84d753 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -19,8 +19,8 @@ * Transfer allowance for Fares V2. */ public class FaresV2TransferAllowance extends TransferAllowance { - /** The transfer rules that this allowance could be the start of */ - private RoaringBitmap potentialTransferRules; + /** The last fare leg rule used. Two routes are equivalent if they have the same last fare leg rule. */ + public int lastFareLegRule; /** need to hold on to a ref to this so that getTransferRuleSummary works - but make sure it's not accidentally serialized */ private transient TransitLayer transitLayer; @@ -90,12 +90,13 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetwor this.asRouteFareNetworksBoardStop = asRouteFareNetworksBoardStop; this.potentialAsRouteFareLegRules = potentialAsRouteFareLegRules; - if (prevFareLegRuleIdx != -1 && transitLayer.fareTransferRulesForFromLegGroupId.containsKey(prevFareLegRuleIdx)) { - // not at start of trip, so we may have transfers available - potentialTransferRules = transitLayer.fareTransferRulesForFromLegGroupId.get(prevFareLegRuleIdx); - } else { - potentialTransferRules = new RoaringBitmap(); - } + this.lastFareLegRule = prevFareLegRuleIdx; +// if (prevFareLegRuleIdx != -1 && transitLayer.fareTransferRulesForFromLegGroupId.containsKey(prevFareLegRuleIdx)) { +// // not at start of trip, so we may have transfers available +// potentialTransferRules = transitLayer.fareTransferRulesForFromLegGroupId.get(prevFareLegRuleIdx); +// } else { +// potentialTransferRules = new RoaringBitmap(); +// } this.transitLayer = transitLayer; } @@ -124,8 +125,8 @@ public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { !Arrays.equals(asRouteFareNetworks, o.asRouteFareNetworks)) return false; } - // at least as good if it provides a superset of the transfers the other does - return potentialTransferRules.contains(o.potentialTransferRules); + // at least as good if it is the same fare leg rule + return lastFareLegRule == o.lastFareLegRule; } else { throw new IllegalArgumentException("mixing of transfer allowance types!"); } @@ -140,22 +141,27 @@ public TransferAllowance tightenExpiration(int maxClockTime) { * Displaying a bunch of ints in the debug interface is going to be impossible to debug. Instead, generate an * on the fly string representation. This is not called in routing so performance isn't really an issue. */ - public List getTransferRuleSummary () { - if (transitLayer == null) return IntStream.of(potentialTransferRules.toArray()) - .mapToObj(Integer::toString) - .collect(Collectors.toList()); - - List transfers = new ArrayList<>(); - - for (PeekableIntIterator it = potentialTransferRules.getIntIterator(); it.hasNext();) { - int transferRuleIdx = it.next(); - FareTransferRuleInfo info = transitLayer.fareTransferRules.get(transferRuleIdx); - transfers.add(info.from_leg_group_id + " " + info.to_leg_group_id); - } - - transfers.sort(Comparator.naturalOrder()); - - return transfers; +// public List getTransferRuleSummary () { +// if (transitLayer == null) return IntStream.of(potentialTransferRules.toArray()) +// .mapToObj(Integer::toString) +// .collect(Collectors.toList()); +// +// List transfers = new ArrayList<>(); +// +// for (PeekableIntIterator it = potentialTransferRules.getIntIterator(); it.hasNext();) { +// int transferRuleIdx = it.next(); +// FareTransferRuleInfo info = transitLayer.fareTransferRules.get(transferRuleIdx); +// transfers.add(info.from_leg_group_id + " " + info.to_leg_group_id); +// } +// +// transfers.sort(Comparator.naturalOrder()); +// +// return transfers; +// } + + /** For debug interface */ + public String getLastFareLegGroupId () { + return transitLayer.fareLegRules.get(lastFareLegRule).leg_group_id; } public List getPotentialAsRouteFareLegRules () { From 98745fae7485884a0d256acbb54bb4331e5b1613 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 4 Aug 2020 22:45:17 -0400 Subject: [PATCH 22/46] docs(fares-v2): add docs for fares-v2 calculator --- docs/fares/gtfs-fares-v2.md | 99 +++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 docs/fares/gtfs-fares-v2.md diff --git a/docs/fares/gtfs-fares-v2.md b/docs/fares/gtfs-fares-v2.md new file mode 100644 index 000000000..1cfa46873 --- /dev/null +++ b/docs/fares/gtfs-fares-v2.md @@ -0,0 +1,99 @@ +# GTFS Fares V2 support + +[GTFS-Fares V2](https://bit.ly/gtfs-fares) is a proposed standard to incorporate more complex fare scenarios into GTFS than are supported by the current `fare_rules.txt` and `fare_attributes.txt` system, which cannot represent fares in many places. GTFS-Fares V2 still cannot represent all fare systems, though extensions to the specification are regular. Conveyal Analysis currently supports a subset of GTFS-Fares V2, which is described in this page. This subset is used to provide fare support in Toronto; as applications of fare routing continue, this fare calculator should be extended. GTFS Fares v2 routing can be requested by setting the InRoutingFareCalculator type to `fares-v2` in the profile request. + +## Fare structure + +Several files are used to specify the fare structure within GTFS Fares. + +### `fare_leg_rules` + +Fare leg rules specify the fare for a single leg of a journey (except in the case of as_route fare networks, described below). The fare leg rule for a leg is found based on the `fare_network_id`, `from_area_id` and `to_area_id` in the current implementation. The matching fare leg rule with the lowest `order` is used, and `is_symmetrical` is supported to allow trips in either direction. `amount` and `currency` are used to define the cost. Other fields are not supported, and will either cause an error or be ignored. If multiple rules match with the same order, one is selected; which one is undefined. `from_area_id` and `to_area_id` can be blank (wildcard match), an area ID (defined below), or a stop_id. `fare_network_id` can be either a fare network ID or a route ID. + +`fare_leg_id` should be used when a leg is referred to in `fare_transfer_rules`. `amount` must be specified; `min_amount` and `max_amount` are not supported. + +Multiple rules are allowed to have the same `order` in GTFS-Fares v2, in which case the user is allowed to choose a fare. This is not supported in Conveyal Analysis. If multiple rules that apply to the same leg have the same order, one of them will be used; which one is undefined. + +### `fare_transfer_rules` + +Fare transfer rules define discounted transfers between fare legs. Currently, Conveyal Analysis only considers the from and to leg when evaluating a transfer; that is, the second transfer in a rail -> bus -> bus trip is treated the same as the transfer in a bus -> bus trip. Fares v2 does contain a field `spanning_limit` but this is insufficient for representing complex transfer systems where [the number of transfers allowed depends on the services involved](../newyork.md#staten-island-railway). + +Currently, the fields `from_leg_group_id` and `to_leg_group_id` are used to match fare transfer rules to the `fare_leg_rules` that were matched for each leg. Other fields are not used in matching. Duration limits are not currently supported, although in most accessibility analyses these limits are non-binding. `fare_transfer_type` can be either 0 ("the cost of the sub-journey is the cost of the first leg PLUS the cost in the amount field"), in which case amount would be expected to be positive, or 1 ("the cost of the sub-journey is the sum of the cost PLUS the cost in the amount field"), in which case amount would be expected to be negative. 3 ("the cost of the sub-journey is the cost of the most expensive leg of the sub-journey PLUS the cost in the amount field.") is not currently supported, although adding support for it would not be difficult. + +### `fare_areas` + +Fare areas are ways to group stops for fare purposes. Currently, members of fare areas can only be specified as `stop_id`s; `trip_id` + `stop_sequence` is not currently supported. + +### `fare_networks` + +Fare networks are used to group routes together to make specifying fares more concise. They can be used to refer to groups of routes in `fare_leg_rules`. Moreover, a fare network can have `as_route` set to 1, in which case any set of journeys within the network is matched as a single journey (for instance, subway systems where fare is based on where you entered and left the system, not what you did in between). Currently there is discussion on how to code the situation where someone leaves the paid area and reboards—for example, by making an on-street transfer from the Blue Line at Bowdoin to the Red Line at Charles-MGH in Boston. + +More complex fare networks where the total fare depends not only on where you boarded and alighted, but also which zones you passed through (for instance, Long Island Rail Road, London Underground, or GO in Toronto) are not currently supported; there is discussion of extending `contains_area_id` to handle these cases. See below for a specific workaround for GO in Toronto. + +`as_route` fare networks are supported in Conveyal Analysis. When calculating the fare for a ride on a vehicle in an `as_route` network, we peek forward to see if the next ride is a member of any of the same `as_route` networks (a route can be a member of multiple overlapping networks). This process is repeated until we get to a ride that does not share any common networks with all of the previous `as_route` rides. + +When the last ride in a trip is in an `as_route` network, information about the ride is included in the transfer allowance to make sure it is not pruned in favor of a trip with different `as_route` transfer privileges; this is described below. + +## Handling multiple feeds + +GTFS-Fares v2 does not allow for interfeed transfer discounts. Conveyal Analysis does not support multiple GTFS-Fares v2 feeds in the same network, regardless of whether interfeed discounts exist or not; feeds should be merged if fare support is desired. + +## Algorithm implementation + +The algorithm consists of two parts: the fare calculation engine, which calculates fares for _bona fide_ trips where we know all the rides, and a transfer allowance that keeps track of potential discounts that could be realized in the future due to having taken a particular fare in the past. + +## Fare calculator + +The fare calculator for GTFS Fares V2 is a bit different from and shorter than other fare calculators because it does not have a lot of city-specific logic. It loops through all of the all of the rides in a journey and performs the following steps: + +1. Peek ahead to see if the next ride can be merged with this one through an `as_route` fare network. + First, identify the `as_route` fare networks this ride is a part of, then for each following rides + a. Identify the `as_route` fare networks the following ride is a part of + b. AND this with the fare networks already identified + c. If there are common `as_route` fare networks: + i. Move the alight stop and alight time for the combined leg to this stop and alight time + ii. Advance the outer loop counter + This leaves us with a pseudo-ride that stretches from the first board stop to the last alight stop in an `as_route` network. This greedy matching means that if there are three rides, the first on a route in network A, the second on a route in networks A and B, and the final on a route in network B only, the first two legs will be merged and the third will be treated as a new leg, even if merging the second two legs and treating the first separately would be advantageous. This situation of overlapping networks is believed to be rare. +2. A fare leg rule rule for the ride is found based on the fare networks the ride is in, and the board and alight stops. The rule with the lowest order that matches the networks, from stop, and to stop (either explicitly or via a blank/wildcard field) is returned. If there are multiple such rules, which one is returned is undefined. +3. If this is not the first ride, a transfer rule is searched for +4. If this is the first ride, or no transfer rule is found, the fare from the leg rule is added to the cumulative fare for the journey. + +## Transfer allowance definition + +A key component of the algorithm for finding low-cost paths in transit networks described by [Conway and Stewart 2019](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf) is the "transfer allowance," which represents all of the potential discounts that could be realized by having used a particular journey suffix. In the current GTFS Fares v2 implementation, the transfer allowance is based entirely on the fare leg rule you traversed most recently; since `spanning_limit` is not implemented and fare transfer rules can only refer to two legs, the transfer privileges are the same regardless of what you rode 2, 3, etc. rides ago. + +For rides that are not in `as_route` fare networks, the only thing in the transfer allowance is the index of the last `fare_leg_group`. In the Fareto interface, the `fare_leg_group` will display as a string, but internally they are represented as integers for fast equality checks. Transfer allowances that have the same most recent fare leg group are considered comparable, those that do not are not. + +This seems to perform just fine in Toronto, but it will not perform well in systems that are coded with many leg rules for fares with equivalent transfer privileges. In the future, more efficiency might be gained by actually having some representation of the theoretical concept of transfer allowance—that is, a vector of the discounts on all possible journey suffixes. Then a fare with better transfer privileges could kick out one with the same or worse, even if they didn't have the same most recent fare leg rule. This could even be precomputed at network build time to create a list of what fare leg rules have better transfer privileges than other fare leg rules. + +For `fare_leg_rules` that _are_ in `as_route` fare networks, the transfer allowance additional contains an array of _which_ `as_route` networks they are in, and where they boarded. These must be equal for domination to occur. This will overretain trips, but `as_route` networks tend to be small (e.g. commuter rail systems or subways) so this is immaterial. + +### Max transfer allowance value + +The maximum transfer allowance value is not computed, but rather is hard-wired at 10,000,000 CAD. This is presumably more expensive than the most expensive trip on any transit system, meaning that no trips will be eliminated by Theorem 3.1 in [the paper](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). Routing is still correct, because this will overretain trips. The maximum transfer allowance when it is defined is the maximum discount you could get off any future journey suffix, but there is no guarantee that that journey suffix will actually be taken. You can think of the very high maximum transfer allowance as being a transfer to a "ghost" train that is normally very expensive, but heavily discounted with the fare paid so far, but that does not connect to any destinations. + +## City-specific extensions + +At least as of this writing it is not possible to represent the complexities of all cities in GTFS-Fares v2, but it does come close for many cities. This section documents city-specific extensions that can be enabled through properties of the in-routing fare calculator. + +### Toronto + +#### Extension to properly model GO fares + +GO fares in Toronto as implemented as an `as_route` fare network, since the fare is based on the zones you travel through. However, in some cases, it may be optimal to travel beyond your origin or destination zones, change trains/buses, and double back. For instance, consider [the second option for trip from Union Station to the Thornhill neighborhood](https://projects.indicatrix.org/fareto-examples/?load=broken-yyz-downtown-to-york). The GO fare for the origin and destination stations for the full trip is $6.80, but you have to actually travel beyond the destination station, to Unionville, to transfer, so the correct fare is actually $7.80—and the [fare calculator on the GO website](https://www.gotransit.com/en/trip-planning/calculate-fare/your-fare) reflects this when you select a transfer station of Unionville. I think this is because [the fare bylaw, on page 1 of the appendix](https://www.gotransit.com/static_files/gotransit/assets/pdf/Policies/By-Law_No2A.pdf) says that "This Tariff of Fares sets out the base fares applicable for a single one-way ride on the transit system _within_ the enumerated zones, including all applicable taxes" (emphasis mine). So a trip from Union Station to Thornhill via Unionville is _within_ the zones from Union Station to Unionville. + +To support this in GTFS Fares v2, it would have to be possible to specify multiple `contains_area_ids` for each fare. Since that is not possible, a workaround is implemented in the fare calculator. When `useAllStopsWhenCalculatingAsRouteFareNetwork` is set to true, rather than only search for fare rules that apply to the origin and destination stops of the whole journey, we search for fare leg rules matching from_area_ids of _any_ stop within the joined as_route trips except the final alight stop, and to_area_ids of _any_ stop except the first board stop. It is not only board stops considered for from_area_ids and alight stops considered for to_area_ids, because you might do a trip C - A walk to B - D, and this should cost the A-D fare even though you didn't ever board at A. When this switch is enabled, [the router finds the correct fare for the example trip above](https://projects.indicatrix.org/fareto-examples/?load=fixed-yyz-downtown-to-york) (some options no longer appear because I disabled usage of subways in this example so that the now-more-expensive GO trip would not be above the Pareto curve). + +The way this is implemented in the router is that when the as_route legs are compressed to a single leg, the `fare_leg_rule`s for each from stop ID are OR'ed together, the `fare_leg_rule`s for each to stop ID are similarly OR'ed together, and the results of those operations are AND'ed together to get all possible fare rules. The one with the lowest order is then used. This requires that the orders in the GTFS be set such that the most extensive `fare_leg_rule` have the lowest order. + +This also requires some changes to the transfer allowance, because two journeys that start at the same stop but transfer at different stops might have different fares. So an array of all the lowest-order fare leg rules for the journey within the `as_route` network thus far is added to the transfer allowance, and transfer allowances are considered comparable iff they have the same lowest-order fare rules. As long as (1) the most extensive fare rule is among the lowest-order fare rules, and (2) there are no more extensive fare rules among the lowest order fare rules, two journeys with the same lowest-order fare rules have the same extents. + +_Proof_: Suppose without loss of generality that fare leg rule 1 is the most extensive ("full extent") for journey Q, and R.potentialAsRouteFareLegRules == Q.potentialAsRouteFareRules. +1. By condition 1, if 1 is the most extensive fare leg rules for journey Q, then it must appear in Q.potentialAsRouteFareRules. +2. By condition 2, no other more extensive fare leg rules can appear in Q.potentialAsRouteFareRules. +3. If Q.potentialAsRouteFareRules == R.potentialAsRouteFareRules, then 1 must appear in R.potentialAsRouteFareRules +4. If Q.potentialAsRouteFareRules == R.potentialAsRouteFareRules and 1 was the most extensive fare rule in R.potentialAsRouteFareRules, it must also be the most extensive fare leg rule for R because the potential fare leg rules are the same. +4. Q and R are thus equally extensive. +Q.E.D. + +In Toronto, the fare system is not a simple linear map (like it is on, say, Caltrain or the MBTA). However, I assume that the most extensive fare leg rule is also (one of) the most expensive fare leg rules for a particular set of from and to stops, and assign order the fare rules based on descending fare, with ties receiving the same order. This last point is critical. If `fare_leg_rule` `order`s are set based on cost, as they are in Toronto so that the most expensive trip is always the one returned, `fare_leg_rule`s within the `as_route` network with the same fare must also have the same order. If A-C and B-C are the same price, you might be sloppy and assign order randomly for these two fare pairs. But to get proper transfer allowance domination logic, A-C must have a lower order or the same order as B-C. Otherwise, an A-C trip could kick out a B-C trip in domination because B-C would appear in its set of potential fare rules while A-C did not, which could lead to an incorrect result if the final journey is A-D, which might be more expensive than B-D. If A-C and B-C both have the same order, they will both appear in the potential fare rules, and an A-C trip will not be able to kick out a B-C trip that would not have A-C in its potential fare rules. From e4b0e847ef56b9aa56dff93ddb20b10e11ebb99a Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 6 Aug 2020 18:57:09 -0400 Subject: [PATCH 23/46] feat(fares-v2): lower max transfer allowance to better support ttc fares --- .../r5/analyst/fare/faresv2/FaresV2TransferAllowance.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java index f1b84d753..a08e23bc9 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/faresv2/FaresV2TransferAllowance.java @@ -84,7 +84,7 @@ public FaresV2TransferAllowance (int prevFareLegRuleIdx, int[] asRouteFareNetwor int[] potentialAsRouteFareLegRules, TransitLayer transitLayer) { // the value is really high to effectively disable Theorem 3.1 for now, so we don't have to actually calculate // the max value, at the cost of some performance. - super(10_000_000_00, 0, 0); + super(1_000_00, 0, 0); this.asRouteFareNetworks = asRouteFareNetworks; this.asRouteFareNetworksBoardStop = asRouteFareNetworksBoardStop; From 65d0cbd27e225f7729005a28f33e9da1349aa587 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Fri, 7 Aug 2020 11:17:46 -0400 Subject: [PATCH 24/46] fix(fares-v2): bounds check, fixes #135 --- .../java/com/conveyal/r5/transit/TransitLayer.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 579165c8f..4bd0319f7 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -634,8 +634,16 @@ private void loadFaresV2 (GTFSFeed feed, TObjectIntMap indexForUnscopedS if (fareNetwork.as_route == 1) fareNetworkAsRoute.add(fareNetworkIdx); for (String routeId : fareNetwork.route_ids) { - int routeIdx = indexForUnscopedRouteId.get(routeId); - fareNetworksForRoute.get(routeIdx).add(fareNetworkIdx); + if (indexForUnscopedRouteId.containsKey(routeId)) { + int routeIdx = indexForUnscopedRouteId.get(routeId); + if (!routes.get(routeIdx).route_id.equals(routeId)) + throw new IllegalStateException("Route ID mismatch!"); + fareNetworksForRoute.get(routeIdx).add(fareNetworkIdx); + } else { + // don't error out here, it's not illegal to have a route with no trips, and this can happen when + // GTFS is trimmed + LOG.warn("Route ID {} referenced in fare_networks not in routes, or has no trips!", routeId); + } } fareNetworkIdx++; From 97dc47f3a03fe3118b8f55d6319b293b1c7a852b Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Fri, 7 Aug 2020 19:37:44 -0400 Subject: [PATCH 25/46] docs(fares): update fares index --- docs/fares/index.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/fares/index.md b/docs/fares/index.md index 75f9808f8..6b23f6c17 100644 --- a/docs/fares/index.md +++ b/docs/fares/index.md @@ -8,5 +8,6 @@ Finding cheapest paths is implemented in the McRAPTOR (multi-criteria RAPTOR) ro Unfortunately, while there is a common data format for transit timetables (GTFS), no such format exists for fares. GTFS does include two different fare specifications (GTFS-fares and GTFS-fares v2), but they are not able to represent complex fare systems. As such, unless and until such a specification becomes available, Conveyal Analysis includes location-specific fare calculators for a number of locations around the world. They have their own documentation: -- [New York](newyork.html) +- [New York](newyork.md) - Boston (documentation coming soon) +- [GTFS-Fares v2](gtfs-fares-v2.md) From 413e356febd0f18d2b11d10db4c20190bfc861b6 Mon Sep 17 00:00:00 2001 From: ansons Date: Wed, 5 Aug 2020 00:29:25 -0400 Subject: [PATCH 26/46] refactor: rm unused imports and stray comment --- src/main/java/com/conveyal/gtfs/GTFSFeed.java | 2 -- .../r5/analyst/fare/InRoutingFareCalculator.java | 1 - .../com/conveyal/r5/transit/TransitLayer.java | 15 +++++++++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index 63cf2eaa5..c8747ef4c 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -180,8 +180,6 @@ public class GTFSFeed implements Cloneable, Closeable { /** Map from each trip_id to ID of trip pattern containing that trip. */ public final Map patternForTrip; - /** Map from - /** Once a GTFSFeed has one feed loaded into it, we set this to true to block loading any additional feeds. */ private boolean loaded = false; diff --git a/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java index 32e96199a..db9c03443 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/InRoutingFareCalculator.java @@ -13,7 +13,6 @@ import java.io.Serializable; import java.util.Collection; import java.util.Map; -import java.util.function.ToIntFunction; /** * A fare calculator used in Analyst searches. The currency is not important as long as it is integer and constant diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 4bd0319f7..997ce1967 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -1,7 +1,19 @@ package com.conveyal.r5.transit; import com.conveyal.gtfs.GTFSFeed; -import com.conveyal.gtfs.model.*; +import com.conveyal.gtfs.model.Agency; +import com.conveyal.gtfs.model.Fare; +import com.conveyal.gtfs.model.FareArea; +import com.conveyal.gtfs.model.FareLegRule; +import com.conveyal.gtfs.model.FareNetwork; +import com.conveyal.gtfs.model.FareTransferRule; +import com.conveyal.gtfs.model.Frequency; +import com.conveyal.gtfs.model.Route; +import com.conveyal.gtfs.model.Service; +import com.conveyal.gtfs.model.Shape; +import com.conveyal.gtfs.model.Stop; +import com.conveyal.gtfs.model.StopTime; +import com.conveyal.gtfs.model.Trip; import com.conveyal.r5.analyst.fare.faresv2.IndexUtils; import com.conveyal.r5.api.util.TransitModes; import com.conveyal.r5.common.GeometryUtils; @@ -17,7 +29,6 @@ import com.google.common.collect.Multimap; import gnu.trove.iterator.TIntIterator; import gnu.trove.iterator.TIntObjectIterator; -import gnu.trove.list.TDoubleList; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; import gnu.trove.map.TIntIntMap; From 94d1c7739f09940eb9ac1a0f1062e146ea506212 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Fri, 28 Aug 2020 09:54:01 -0400 Subject: [PATCH 27/46] refactor(fares-v2): rm unused variable --- src/main/java/com/conveyal/r5/transit/TransitLayer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 997ce1967..a993a4055 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -900,7 +900,6 @@ private void rebuildFaresV2TransientIndices () { */ private TIntObjectMap indexFareLegRulesForStops(TIntObjectMap fareLegRulesForFareAreaId) { TIntObjectMap forStops = new TIntObjectHashMap<>(); - Map bitmaps = new HashMap<>(); for (int stop = 0; stop < stopIdForIndex.size(); stop++) { TIntList fareAreas = fareAreasForStop.get(stop); // TODO could intern these RoaringBitmaps to save some memory if it becomes a problem From 7538dcd7c83a3d915bec9ba3b76dd3ae056b33a9 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Sat, 11 Apr 2020 23:42:46 -0400 Subject: [PATCH 28/46] fix(boston-fares): allow behind-the-gates transfers even after a transfer from a bus, fixes #588. --- .../fare/BostonInRoutingFareCalculator.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 25f71aec7..df6c5265f 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -318,14 +318,34 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat TransferRuleGroup issuing = transferAllowance.transferRuleGroup; TransferRuleGroup receiving = fareGroups.get(fare.fare_id); - // servicesConnectedBehindFareGates contains an implicit bounds check that ride >= 1 - if (servicesConnectedBehindFareGates(issuing, receiving)) { - int fromStopIndex = alightStops.get(ride - 1); - String fromStation = transitLayer.parentStationIdForStop.get(fromStopIndex); - // if the previous alighting stop and this boarding stop are connected behind fare - // gates (and without riding a vehicle!), continue to the next ride. There is no CharlieCard tap - // and thus for fare purposes these are a single ride. - if (platformsConnected(fromStopIndex, fromStation, boardStopIndex, boardStation)) continue; + // Check if there was actually a fare interaction + if (ride > 0) { + int prevPattern = patterns.get(ride - 1); + RouteInfo prevRoute = transitLayer.routes.get(transitLayer.tripPatterns.get(prevPattern).routeIndex); + + // board stop for this ride + int prevBoardStopIndex = boardStops.get(ride - 1); + String prevBoardStation = transitLayer.parentStationIdForStop.get(prevBoardStopIndex); + String prevBoardStopZoneId = transitLayer.fareZoneForStop.get(prevBoardStopIndex); + + // alight stop for this ride + int prevAlightStopIndex = alightStops.get(ride - 1); + String prevAlightStopZoneId = transitLayer.fareZoneForStop.get(prevAlightStopIndex); + + Fare prevFare = fares.getFareOrDefault(getRouteId(prevRoute), prevBoardStopZoneId, prevAlightStopZoneId); + TransferRuleGroup previous = fareGroups.get(prevFare.fare_id); + + // servicesConnectedBehindFareGates contains an implicit bounds check that ride >= 1 + // this is actually not right, as issuing service could be bus with a free transfer to subway + // Previous is not always the same as issuing, e.g. when there was a previous bus -> subway transfer + if (servicesConnectedBehindFareGates(previous, receiving)) { + int fromStopIndex = alightStops.get(ride - 1); + String fromStation = transitLayer.parentStationIdForStop.get(fromStopIndex); + // if the previous alighting stop and this boarding stop are connected behind fare + // gates (and without riding a vehicle!), continue to the next ride. There is no CharlieCard tap + // and thus for fare purposes these are a single ride. + if (platformsConnected(fromStopIndex, fromStation, boardStopIndex, boardStation)) continue; + } } // Check for transferValue expiration From 07d1ac805ecd7e45b38a7c7575176f7e5ebe5f76 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 13 Apr 2020 14:07:51 -0400 Subject: [PATCH 29/46] fix(boston-fares): don't allow out-of-system subway transfers (fixes #592) --- .../analyst/fare/BostonInRoutingFareCalculator.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index df6c5265f..2c5dd59b3 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -42,6 +42,8 @@ private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_AIRPORT, LOC private static final Map fareGroups = new HashMap() { {put(LOCAL_BUS_FARE_ID, TransferRuleGroup.LOCAL_BUS); } {put(SUBWAY_FARE_ID, TransferRuleGroup.SUBWAY); } + // okay for inner and outer express buses to use same rule group, as they have the same + // transfer privileges {put("innerExpressBus", TransferRuleGroup.EXPRESS_BUS); } {put("outerExpressBus", TransferRuleGroup.EXPRESS_BUS); } {put("slairport", TransferRuleGroup.SL_AIRPORT); } @@ -50,15 +52,21 @@ private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_AIRPORT, LOC private static final Set> transferEligibleSequencePairs = new HashSet<>( Arrays.asList( Arrays.asList(TransferRuleGroup.LOCAL_BUS, TransferRuleGroup.LOCAL_BUS), - Arrays.asList(TransferRuleGroup.SUBWAY, TransferRuleGroup.SUBWAY), + // Subway -> Subway only eligible if within fare gates, which is handled separately + // since it does not require a fare interaction + //Arrays.asList(TransferRuleGroup.SUBWAY, TransferRuleGroup.SUBWAY), Arrays.asList(TransferRuleGroup.LOCAL_BUS, TransferRuleGroup.SUBWAY), Arrays.asList(TransferRuleGroup.SUBWAY, TransferRuleGroup.LOCAL_BUS), Arrays.asList(TransferRuleGroup.EXPRESS_BUS, TransferRuleGroup.SUBWAY), Arrays.asList(TransferRuleGroup.SUBWAY, TransferRuleGroup.EXPRESS_BUS), Arrays.asList(TransferRuleGroup.EXPRESS_BUS, TransferRuleGroup.LOCAL_BUS), Arrays.asList(TransferRuleGroup.LOCAL_BUS, TransferRuleGroup.EXPRESS_BUS), - Arrays.asList(TransferRuleGroup.LOCAL_BUS_TO_SUBWAY, TransferRuleGroup.SUBWAY), + // see comment about SUBWAY, SUBWAY + //Arrays.asList(TransferRuleGroup.LOCAL_BUS_TO_SUBWAY, TransferRuleGroup.SUBWAY), Arrays.asList(TransferRuleGroup.LOCAL_BUS_TO_SUBWAY, TransferRuleGroup.LOCAL_BUS) + + // No need to include the OUT_OF_SUBWAY group here, as it is only used for the last state in the + // chain and is never built upon. ) ); From 16e5a0367c2d3e50612ff4b5e91feb7b8a0a3e23 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 13 Apr 2020 16:36:30 -0400 Subject: [PATCH 30/46] fix(boston-fares): correctly compare out-of-subway fares (fixes #593) and use boston-specific logic in dominance (addresses #595) --- .../fare/BostonInRoutingFareCalculator.java | 109 +++++++++++------- 1 file changed, 68 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 2c5dd59b3..211114ca4 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -33,7 +33,7 @@ public class BostonInRoutingFareCalculator extends InRoutingFareCalculator { // Some fares may confer different transfer allowance values, but have the same issuing and acceptance rules. // For example, in Boston, the transfer allowances from inner and outer express bus fares have different values, // but they are issued and accepted under the same circumstances. - private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_AIRPORT, LOCAL_BUS_TO_SUBWAY, OUT_OF_SUBWAY, + private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_FREE, LOCAL_BUS_TO_SUBWAY, OTHER, NONE} // Map fare_id values from GTFS fare_attributes.txt to these transfer rule groups @@ -46,7 +46,7 @@ private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_AIRPORT, LOC // transfer privileges {put("innerExpressBus", TransferRuleGroup.EXPRESS_BUS); } {put("outerExpressBus", TransferRuleGroup.EXPRESS_BUS); } - {put("slairport", TransferRuleGroup.SL_AIRPORT); } + {put("slairport", TransferRuleGroup.SL_FREE); } }; private static final Set> transferEligibleSequencePairs = new HashSet<>( @@ -65,11 +65,12 @@ private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_AIRPORT, LOC //Arrays.asList(TransferRuleGroup.LOCAL_BUS_TO_SUBWAY, TransferRuleGroup.SUBWAY), Arrays.asList(TransferRuleGroup.LOCAL_BUS_TO_SUBWAY, TransferRuleGroup.LOCAL_BUS) - // No need to include the OUT_OF_SUBWAY group here, as it is only used for the last state in the - // chain and is never built upon. + // No free transfers from SL_FREE, except behind gates in the subway handled elsewhere ) ); + private static final Set modesWithBehindGateTransfers = new HashSet<>(Arrays.asList(TransferRuleGroup.SUBWAY, TransferRuleGroup.SL_FREE)); + private static final String DEFAULT_FARE_ID = LOCAL_BUS_FARE_ID; private static final Set stationsWithoutBehindGateTransfers = new HashSet<>(Arrays.asList( "place-coecl", "place-aport")); @@ -114,12 +115,21 @@ public class BostonTransferAllowance extends TransferAllowance { */ private final TransferRuleGroup transferRuleGroup; + /** + * Once the subway is ridden, if you leave the subway, you can't get back on for free. + * + * This does not matter during the fare calculation loop, only when partial fares are compared, so we only + * bother to set it then. + */ + private final boolean behindGates; + /** * No transfer allowance */ private BostonTransferAllowance () { super(); this.transferRuleGroup = TransferRuleGroup.NONE; + this.behindGates = false; } /** @@ -135,6 +145,7 @@ private BostonTransferAllowance (TransferRuleGroup transferRuleGroup, Fare fare, fare.fare_attribute.transfers, startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = transferRuleGroup; + this.behindGates = false; } /** @@ -148,6 +159,15 @@ private BostonTransferAllowance(Fare fare, int startTime){ priceToInt(Math.min(fares.byId.get(SUBWAY_FARE_ID).fare_attribute.price, fare.fare_attribute.price)), startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = fareGroups.get(fare.fare_id); + this.behindGates = false; + } + + /** Used to set whether the rider is still behind the fare gates, and to tighten expiration times */ + private BostonTransferAllowance(int value, int number, int expirationTime, TransferRuleGroup transferRuleGroup, + boolean behindGates) { + super(value, number, expirationTime); + this.transferRuleGroup = transferRuleGroup; + this.behindGates = behindGates; } /** @@ -160,22 +180,7 @@ private BostonTransferAllowance updateTransferAllowance(Fare fare, int clockTime // journeyStages return new BostonTransferAllowance(fare, clockTime); } else { - // We have boarded a service that does not provide a transfer allowance, preserve the previous transfer - // allowance UNLESS we are coming from the subway, in which case any other service will require the user to - // leave the paid area. - if (this.transferRuleGroup == TransferRuleGroup.SUBWAY) { - // if we've gone from subway to a fare that does not allow transfers (e.g. Commuter Rail, Ferry), we - // could still transfer to a bus, but boarding the subway again would require full fare payment. - // This example arises in Boston for travel between Back Bay and South Station. If you make this - // trip using Orange Line -> Red Line, you have full subway transfer privileges at South Station - // (e.g. to Silver Line 1 behind fare gates or Silver Line 4 on the surface). But if you make it - // using Commuter Rail, you would need to pay full subway fare again to pass through the fare - // gates to access the SL1, though you'd still have a free transfer to the SL4. - return new BostonTransferAllowance(TransferRuleGroup.OUT_OF_SUBWAY, - fares.byId.get(SUBWAY_FARE_ID), - expirationTime); - } - //otherwise return the previous transfer privilege. + //otherwise return the previous transfer privilege, which the user can hold on to to use later. return this; } } @@ -187,28 +192,25 @@ private BostonTransferAllowance localBusToSubwayTransferAllowance(){ return new BostonTransferAllowance(TransferRuleGroup.LOCAL_BUS_TO_SUBWAY, fare, expirationTime); } - private BostonTransferAllowance checkForSubwayExit(int fromStopIndex, McRaptorSuboptimalPathProfileRouter - .McRaptorState state, TransitLayer transitLayer){ - String fromStation = transitLayer.parentStationIdForStop.get(fromStopIndex); - int toStopIndex = state.stop; - String toStation = transitLayer.parentStationIdForStop.get(toStopIndex); - if (platformsConnected(fromStopIndex, fromStation, toStopIndex, toStation)) { - // Have not exited subway through fare gates; maintain transfer privilege - return this; - } else { - // exited subway through fare gates; value can still be used for transfers to bus, but a subsequent - // subway boarding requires payment of full subway fare. - Fare fare = fares.byId.get(SUBWAY_FARE_ID); - // Expiration time should be from original transfer allowance, not updated - int expirationTime = this.expirationTime; - return new BostonTransferAllowance(TransferRuleGroup.OUT_OF_SUBWAY, fare, expirationTime); - } + /** called at the end of the fare calc loop to record whether the last state is behind gates or not. */ + private BostonTransferAllowance setBehindGates (boolean behindGates) { + if (behindGates == this.behindGates) return this; + else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, behindGates); } @Override public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { return super.atLeastAsGoodForAllFutureRedemptions(other) && - this.transferRuleGroup == ((BostonTransferAllowance) other).transferRuleGroup; + this.transferRuleGroup == ((BostonTransferAllowance) other).transferRuleGroup && + // if this is behind gates, or other is not behind gates, they are comparable + // if other is behind gates and this is not, it could possibly be better. + (this.behindGates || !((BostonTransferAllowance) other).behindGates); + } + + public BostonTransferAllowance tightenExpiration (int maxClockTime) { + // copied from TransferAllowance but need to override so that everything stays a BostonTransferAllowance + int expirationTime = Math.min(this.expirationTime, maxClockTime); + return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates); } } @@ -227,8 +229,8 @@ public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { * where both services share a station and can be transferred between without leaving the paid area). */ private static boolean servicesConnectedBehindFareGates(TransferRuleGroup issuing, TransferRuleGroup receiving){ - return ((issuing == TransferRuleGroup.SUBWAY || issuing == TransferRuleGroup.SL_AIRPORT) && - (receiving == TransferRuleGroup.SUBWAY || receiving == TransferRuleGroup.SL_AIRPORT)); + return ((issuing == TransferRuleGroup.SUBWAY || issuing == TransferRuleGroup.SL_FREE) && + (receiving == TransferRuleGroup.SUBWAY || receiving == TransferRuleGroup.SL_FREE)); } private static boolean platformsConnected(int fromStopIndex, String fromStation, int toStopIndex, String toStation){ @@ -413,10 +415,35 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ // platforms are connected) to another subway stop, we do not know the next ride, but know that it cannot be a // free boarding to the subway. MBTA doesn't have designated free transfer stops, although it would be a good // idea e.g. between the platforms of Copley, Charles/MGH and Bowdoin, or Cleveland Circle and Reservoir. - if (transferAllowance.transferRuleGroup == TransferRuleGroup.SUBWAY){ - transferAllowance = transferAllowance.checkForSubwayExit(alightStopIndex, state, transitLayer); + if (patterns.size() > 0) { + int prevPattern = patterns.get(patterns.size() - 1); + RouteInfo prevRoute = transitLayer.routes.get(transitLayer.tripPatterns.get(prevPattern).routeIndex); + + // board stop for this ride + int prevBoardStopIndex = boardStops.get(boardStops.size() - 1); + String prevBoardStopZoneId = transitLayer.fareZoneForStop.get(prevBoardStopIndex); + + // alight stop for this ride + int prevAlightStopIndex = alightStops.get(alightStops.size() - 1); + String prevAlightStation = transitLayer.parentStationIdForStop.get(prevAlightStopIndex); + String prevAlightStopZoneId = transitLayer.fareZoneForStop.get(prevAlightStopIndex); + + Fare prevFare = fares.getFareOrDefault(getRouteId(prevRoute), prevBoardStopZoneId, prevAlightStopZoneId); + TransferRuleGroup previous = fareGroups.get(prevFare.fare_id); + + if (modesWithBehindGateTransfers.contains(previous)) { + // it is possible that we are inside fare gates, because the last vehicle we rode would have left us there + String currentStation = transitLayer.parentStationIdForStop.get(state.stop); + boolean behindGates = platformsConnected(prevAlightStopIndex, prevAlightStation, state.stop, currentStation); + transferAllowance = transferAllowance.setBehindGates(behindGates); + } else { + transferAllowance = transferAllowance.setBehindGates(false); + } + } else { + transferAllowance = transferAllowance.setBehindGates(false); } + return new FareBounds(cumulativeFarePaid, transferAllowance.tightenExpiration(maxClockTime)); } From b437360c9d02f03a98d68144559603f61421c942 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 11 May 2020 11:22:48 -0400 Subject: [PATCH 31/46] fix(fares): throw UnsupportedOperationException when tightenExpiration called in superclass, adresses #595. --- .../com/conveyal/r5/analyst/fare/TransferAllowance.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java index 4e0ecd716..050ed9835 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java @@ -66,13 +66,18 @@ public int payDifference(int grossFare){ public TransferAllowance tightenExpiration(int maxClockTime){ // cap expiration time of transfer at max clock time of search, so that transfer slips that technically have more time // remaining, but that time cannot be used within the constraints of this search, can be pruned. - return new TransferAllowance(this.value, this.number, Math.min(this.expirationTime, maxClockTime)); + + // THIS METHOD SHOULD NOT BE USED BECAUSE IT INADVERTNELTY CONVERTS SUBCLASSES INTO REGULAR TRANSFERALLOWANCES + // CAUSING PATHS THAT SHOULD NOT BE DISCARDED TO BE DISCARDED! + throw new UnsupportedOperationException("tightenExpiration called unsafely. Override in subclasses."); + + //return new TransferAllowance(this.value, this.number, Math.min(this.expirationTime, maxClockTime)); } /** * Is this transfer allowance as good as or better than another transfer allowance? This does not consider the fare - * paid so fare, and can be thought of as follows. If you are standing at a stop, and a perfectly trustworthy person + * paid so far, and can be thought of as follows. If you are standing at a stop, and a perfectly trustworthy person * comes up to you and offers you two tickets, one with this transfer allowance, and one with the other transfer * allowance, is this one as good as or better than the other one for any trip that you might make? (Assume you have * no moral scruples about obtaining a transfer slip from someone else who is probably not supposed to be giving From 85eb9647973fb3f899480086837d308b7f4fcb10 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 18 Jun 2020 15:22:53 -0400 Subject: [PATCH 32/46] feat(boston-fares): mark fields in BostonTransferAllowance public so they show in Fareto --- .../r5/analyst/fare/BostonInRoutingFareCalculator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 211114ca4..a49032d7e 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -113,7 +113,7 @@ public class BostonTransferAllowance extends TransferAllowance { * * actually, due to implementation, the transfer allowance from the train is 2.25, vs. 1.70 for the bus, because the algorithm doesn't * know there is no behind the gates transfer to any other train at Cleveland Circle. */ - private final TransferRuleGroup transferRuleGroup; + public final TransferRuleGroup transferRuleGroup; /** * Once the subway is ridden, if you leave the subway, you can't get back on for free. @@ -121,7 +121,7 @@ public class BostonTransferAllowance extends TransferAllowance { * This does not matter during the fare calculation loop, only when partial fares are compared, so we only * bother to set it then. */ - private final boolean behindGates; + public final boolean behindGates; /** * No transfer allowance @@ -247,7 +247,6 @@ private static boolean platformsConnected(int fromStopIndex, String fromStation, @Override public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorState state, int maxClockTime) { - // First, load fare data from GTFS if (fares == null){ synchronized (this) { @@ -415,7 +414,8 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ // platforms are connected) to another subway stop, we do not know the next ride, but know that it cannot be a // free boarding to the subway. MBTA doesn't have designated free transfer stops, although it would be a good // idea e.g. between the platforms of Copley, Charles/MGH and Bowdoin, or Cleveland Circle and Reservoir. - if (patterns.size() > 0) { + // After a transfer to the destination (state.stop == -1) you are by defintion outside the subway. + if (patterns.size() > 0 && state.stop != -1) { int prevPattern = patterns.get(patterns.size() - 1); RouteInfo prevRoute = transitLayer.routes.get(transitLayer.tripPatterns.get(prevPattern).routeIndex); From 3d0b4716910d7e9fc1960f4d36020f3decc9cc48 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 18 Jun 2020 15:23:05 -0400 Subject: [PATCH 33/46] fix(fares): throw UnsupportedOperationException when superclass tightenExpiration called unsafely. --- .../com/conveyal/r5/analyst/fare/TransferAllowance.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java b/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java index 050ed9835..23cacb8f4 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/TransferAllowance.java @@ -23,6 +23,11 @@ public class TransferAllowance { public final int number; public final int expirationTime; + /** Return the class of transfer allowance, for Fareto display */ + public String getType () { + return this.getClass().getSimpleName(); + } + /** * Constructor used for no transfer allowance */ @@ -67,7 +72,7 @@ public TransferAllowance tightenExpiration(int maxClockTime){ // cap expiration time of transfer at max clock time of search, so that transfer slips that technically have more time // remaining, but that time cannot be used within the constraints of this search, can be pruned. - // THIS METHOD SHOULD NOT BE USED BECAUSE IT INADVERTNELTY CONVERTS SUBCLASSES INTO REGULAR TRANSFERALLOWANCES + // THIS METHOD SHOULD NOT BE USED BECAUSE IT INADVERTENTLY CONVERTS SUBCLASSES INTO REGULAR TRANSFERALLOWANCES // CAUSING PATHS THAT SHOULD NOT BE DISCARDED TO BE DISCARDED! throw new UnsupportedOperationException("tightenExpiration called unsafely. Override in subclasses."); From fc7b0c81325c63b7eea4e1242ba17878af656e53 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 18 Jun 2020 19:45:06 -0400 Subject: [PATCH 34/46] fix(boston-fares): allow same-direction transfers on commuter rail. --- .../fare/BostonInRoutingFareCalculator.java | 116 ++++++++++++++++-- 1 file changed, 103 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index a49032d7e..769fe550b 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -4,6 +4,7 @@ import com.conveyal.r5.profile.McRaptorSuboptimalPathProfileRouter; import com.conveyal.r5.transit.RouteInfo; import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.TripPattern; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; import org.apache.commons.math3.random.MersenneTwister; @@ -27,6 +28,13 @@ public class BostonInRoutingFareCalculator extends InRoutingFareCalculator { /** If true, log a random 1e-6 sample of fares for spot checking */ public static final boolean LOG_FARES = false; + /** + * Rather than computing the max transfer allowance on commuter rail (b/c you can change to another train in the same + * direction) we just assume that the transfer allowance cannot be larger than the max fare. This will over-retain trips, + * but the CR network is small enough that's okay. + */ + public static final int MAX_CR_FARE = 1250; + private static final WeakHashMap fareSystemCache = new WeakHashMap<>(); private RouteBasedFareRules fares; @@ -115,6 +123,12 @@ public class BostonTransferAllowance extends TransferAllowance { */ public final TransferRuleGroup transferRuleGroup; + public final int commuterRailBoardStop; + + public final int commuterRailRoute; + + public final int commuterRailDirection; + /** * Once the subway is ridden, if you leave the subway, you can't get back on for free. * @@ -130,6 +144,7 @@ private BostonTransferAllowance () { super(); this.transferRuleGroup = TransferRuleGroup.NONE; this.behindGates = false; + commuterRailBoardStop = commuterRailRoute = commuterRailDirection = -1; } /** @@ -146,6 +161,7 @@ private BostonTransferAllowance (TransferRuleGroup transferRuleGroup, Fare fare, startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = transferRuleGroup; this.behindGates = false; + commuterRailBoardStop = commuterRailRoute = commuterRailDirection = -1; } /** @@ -160,14 +176,18 @@ private BostonTransferAllowance(Fare fare, int startTime){ startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = fareGroups.get(fare.fare_id); this.behindGates = false; + commuterRailBoardStop = commuterRailRoute = commuterRailDirection = -1; } /** Used to set whether the rider is still behind the fare gates, and to tighten expiration times */ private BostonTransferAllowance(int value, int number, int expirationTime, TransferRuleGroup transferRuleGroup, - boolean behindGates) { + boolean behindGates, int commuterRailBoardStop, int commuterRailRoute, int commuterRailDirection) { super(value, number, expirationTime); this.transferRuleGroup = transferRuleGroup; this.behindGates = behindGates; + this.commuterRailBoardStop = commuterRailBoardStop; + this.commuterRailRoute = commuterRailRoute; + this.commuterRailDirection = commuterRailDirection; } /** @@ -195,22 +215,50 @@ private BostonTransferAllowance localBusToSubwayTransferAllowance(){ /** called at the end of the fare calc loop to record whether the last state is behind gates or not. */ private BostonTransferAllowance setBehindGates (boolean behindGates) { if (behindGates == this.behindGates) return this; - else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, behindGates); + else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, + behindGates, this.commuterRailBoardStop, this.commuterRailRoute, this.commuterRailDirection); + } + + /** + * While it's not well-documented, you can actually get a discounted transfer on the commuter rail. Consider a + * trip from Worcester to West Newton, which costs $5.50. In the morning, this trip requires you to change to a + * local train, because the train from Worcester will express past West Newton. This does not require you to buy two + * tickets. So there is logic in the fare calculator to allow extending a commuter rail trip as if it were a single trip, + * as long as the route and direction match. To get transfer allowances right, we need to include this information + * in the transfer allowance. + */ + private BostonTransferAllowance setCommuterRailAllowance (int commuterRailBoardStop, int commuterRailRoute, + int commuterRailDirection, int commuterRailMaxAllowance) { + return new BostonTransferAllowance(this.value + commuterRailMaxAllowance, this.number, this.expirationTime, this.transferRuleGroup, + this.behindGates, commuterRailBoardStop, commuterRailRoute, commuterRailDirection); } @Override public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { - return super.atLeastAsGoodForAllFutureRedemptions(other) && - this.transferRuleGroup == ((BostonTransferAllowance) other).transferRuleGroup && - // if this is behind gates, or other is not behind gates, they are comparable - // if other is behind gates and this is not, it could possibly be better. - (this.behindGates || !((BostonTransferAllowance) other).behindGates); + if (other instanceof BostonTransferAllowance) { + BostonTransferAllowance o = (BostonTransferAllowance) other; + + return super.atLeastAsGoodForAllFutureRedemptions(o) && + this.transferRuleGroup == o.transferRuleGroup && + // if this is behind gates, or other is not behind gates, they are comparable + // if other is behind gates and this is not, it could possibly be better. + (this.behindGates || !o.behindGates) && + // there's not check here that commuterRailDirection etc. not be -1 (unset) + // because it's conceivable that you could be better off _not_ having a commuter rail transfer + // allowance, because it might be cheaper to board further down the line. + (this.commuterRailDirection == o.commuterRailDirection) && + (this.commuterRailRoute == o.commuterRailRoute) && + (this.commuterRailBoardStop == o.commuterRailBoardStop); + } else { + throw new IllegalArgumentException("Incomparable transfer allowances!"); + } } public BostonTransferAllowance tightenExpiration (int maxClockTime) { // copied from TransferAllowance but need to override so that everything stays a BostonTransferAllowance int expirationTime = Math.min(this.expirationTime, maxClockTime); - return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates); + return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates, + this.commuterRailBoardStop, this.commuterRailRoute, this.commuterRailDirection); } } @@ -280,11 +328,44 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat stateForTraversal = stateForTraversal.back; continue; // on the street, not on transit } - patterns.add(stateForTraversal.pattern); - alightStops.add(stateForTraversal.stop); - boardStops.add(transitLayer.tripPatterns.get(stateForTraversal.pattern).stops[stateForTraversal.boardStopPosition]); - boardTimes.add(stateForTraversal.boardTime); - stateForTraversal = stateForTraversal.back; + + // special handling for commuter rail - merge subsequent trips that use same route and direction together + // and treat them as a single ride. + TripPattern pat = transitLayer.tripPatterns.get(stateForTraversal.pattern); + RouteInfo ri = transitLayer.routes.get(pat.routeIndex); + if (ri.route_type == 2) { + // not completely correct as the merged ride used multiple patterns, but pattern is only used to look up + // route, so it's okay. + patterns.add(stateForTraversal.pattern); + int commuterRailAlightStop = stateForTraversal.stop; + int commuterRailDirection = pat.directionId; + int commuterRailRoute = pat.routeIndex; + int commuterRailBoardTime = stateForTraversal.boardTime; + + while (stateForTraversal != null) { + if (stateForTraversal.pattern == -1) break; // no on-street transfers with CR + + pat = transitLayer.tripPatterns.get(stateForTraversal.pattern); + if (pat.directionId != commuterRailDirection) break; // can't change direction on CR for free + if (pat.routeIndex != commuterRailRoute) break; // can't change routes on CR TODO Is this true? + // note that this last condition also enforces route_type == 2 implicitly + + commuterRailBoardTime = stateForTraversal.boardTime; + stateForTraversal = stateForTraversal.back; + } + + int commuterRailBoardStop = stateForTraversal.stop; + alightStops.add(commuterRailAlightStop); + boardStops.add(commuterRailBoardStop); + boardTimes.add(commuterRailBoardTime); + // don't increment backwards here - already done in loop above + } else { + patterns.add(stateForTraversal.pattern); + alightStops.add(stateForTraversal.stop); + boardStops.add(transitLayer.tripPatterns.get(stateForTraversal.pattern).stops[stateForTraversal.boardStopPosition]); + boardTimes.add(stateForTraversal.boardTime); + stateForTraversal = stateForTraversal.back; + } } // reverse data about the rides so we can step forward through them @@ -443,6 +524,15 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ transferAllowance = transferAllowance.setBehindGates(false); } + // if we ended on commuter rail, and did not have an on-street transfer, set the commuter rail transfer allowance + if (patterns.size() > 0 && state.pattern != -1) { + TripPattern lastPattern = transitLayer.tripPatterns.get(patterns.get(patterns.size() - 1)); + RouteInfo lastRoute = transitLayer.routes.get(lastPattern.routeIndex); + if (lastRoute.route_type == 2) { + transferAllowance = transferAllowance.setCommuterRailAllowance(boardStops.get(boardStops.size() - 1), lastPattern.routeIndex, + lastPattern.directionId, MAX_CR_FARE); + } + } return new FareBounds(cumulativeFarePaid, transferAllowance.tightenExpiration(maxClockTime)); } From 7e3dd6261f74c7a993c7ec5c5158d96e68e3e11d Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 2 Jul 2020 16:12:38 -0400 Subject: [PATCH 35/46] Revert "fix(boston-fares): allow same-direction transfers on commuter rail." This reverts commit db439cf0ebe9725d3d01dc72cdedc369d489f85d. Allowing free transfers on commuter rail violates nonnegativity of transfer allowances. --- .../fare/BostonInRoutingFareCalculator.java | 116 ++---------------- 1 file changed, 13 insertions(+), 103 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 769fe550b..a49032d7e 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -4,7 +4,6 @@ import com.conveyal.r5.profile.McRaptorSuboptimalPathProfileRouter; import com.conveyal.r5.transit.RouteInfo; import com.conveyal.r5.transit.TransitLayer; -import com.conveyal.r5.transit.TripPattern; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; import org.apache.commons.math3.random.MersenneTwister; @@ -28,13 +27,6 @@ public class BostonInRoutingFareCalculator extends InRoutingFareCalculator { /** If true, log a random 1e-6 sample of fares for spot checking */ public static final boolean LOG_FARES = false; - /** - * Rather than computing the max transfer allowance on commuter rail (b/c you can change to another train in the same - * direction) we just assume that the transfer allowance cannot be larger than the max fare. This will over-retain trips, - * but the CR network is small enough that's okay. - */ - public static final int MAX_CR_FARE = 1250; - private static final WeakHashMap fareSystemCache = new WeakHashMap<>(); private RouteBasedFareRules fares; @@ -123,12 +115,6 @@ public class BostonTransferAllowance extends TransferAllowance { */ public final TransferRuleGroup transferRuleGroup; - public final int commuterRailBoardStop; - - public final int commuterRailRoute; - - public final int commuterRailDirection; - /** * Once the subway is ridden, if you leave the subway, you can't get back on for free. * @@ -144,7 +130,6 @@ private BostonTransferAllowance () { super(); this.transferRuleGroup = TransferRuleGroup.NONE; this.behindGates = false; - commuterRailBoardStop = commuterRailRoute = commuterRailDirection = -1; } /** @@ -161,7 +146,6 @@ private BostonTransferAllowance (TransferRuleGroup transferRuleGroup, Fare fare, startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = transferRuleGroup; this.behindGates = false; - commuterRailBoardStop = commuterRailRoute = commuterRailDirection = -1; } /** @@ -176,18 +160,14 @@ private BostonTransferAllowance(Fare fare, int startTime){ startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = fareGroups.get(fare.fare_id); this.behindGates = false; - commuterRailBoardStop = commuterRailRoute = commuterRailDirection = -1; } /** Used to set whether the rider is still behind the fare gates, and to tighten expiration times */ private BostonTransferAllowance(int value, int number, int expirationTime, TransferRuleGroup transferRuleGroup, - boolean behindGates, int commuterRailBoardStop, int commuterRailRoute, int commuterRailDirection) { + boolean behindGates) { super(value, number, expirationTime); this.transferRuleGroup = transferRuleGroup; this.behindGates = behindGates; - this.commuterRailBoardStop = commuterRailBoardStop; - this.commuterRailRoute = commuterRailRoute; - this.commuterRailDirection = commuterRailDirection; } /** @@ -215,50 +195,22 @@ private BostonTransferAllowance localBusToSubwayTransferAllowance(){ /** called at the end of the fare calc loop to record whether the last state is behind gates or not. */ private BostonTransferAllowance setBehindGates (boolean behindGates) { if (behindGates == this.behindGates) return this; - else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, - behindGates, this.commuterRailBoardStop, this.commuterRailRoute, this.commuterRailDirection); - } - - /** - * While it's not well-documented, you can actually get a discounted transfer on the commuter rail. Consider a - * trip from Worcester to West Newton, which costs $5.50. In the morning, this trip requires you to change to a - * local train, because the train from Worcester will express past West Newton. This does not require you to buy two - * tickets. So there is logic in the fare calculator to allow extending a commuter rail trip as if it were a single trip, - * as long as the route and direction match. To get transfer allowances right, we need to include this information - * in the transfer allowance. - */ - private BostonTransferAllowance setCommuterRailAllowance (int commuterRailBoardStop, int commuterRailRoute, - int commuterRailDirection, int commuterRailMaxAllowance) { - return new BostonTransferAllowance(this.value + commuterRailMaxAllowance, this.number, this.expirationTime, this.transferRuleGroup, - this.behindGates, commuterRailBoardStop, commuterRailRoute, commuterRailDirection); + else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, behindGates); } @Override public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { - if (other instanceof BostonTransferAllowance) { - BostonTransferAllowance o = (BostonTransferAllowance) other; - - return super.atLeastAsGoodForAllFutureRedemptions(o) && - this.transferRuleGroup == o.transferRuleGroup && - // if this is behind gates, or other is not behind gates, they are comparable - // if other is behind gates and this is not, it could possibly be better. - (this.behindGates || !o.behindGates) && - // there's not check here that commuterRailDirection etc. not be -1 (unset) - // because it's conceivable that you could be better off _not_ having a commuter rail transfer - // allowance, because it might be cheaper to board further down the line. - (this.commuterRailDirection == o.commuterRailDirection) && - (this.commuterRailRoute == o.commuterRailRoute) && - (this.commuterRailBoardStop == o.commuterRailBoardStop); - } else { - throw new IllegalArgumentException("Incomparable transfer allowances!"); - } + return super.atLeastAsGoodForAllFutureRedemptions(other) && + this.transferRuleGroup == ((BostonTransferAllowance) other).transferRuleGroup && + // if this is behind gates, or other is not behind gates, they are comparable + // if other is behind gates and this is not, it could possibly be better. + (this.behindGates || !((BostonTransferAllowance) other).behindGates); } public BostonTransferAllowance tightenExpiration (int maxClockTime) { // copied from TransferAllowance but need to override so that everything stays a BostonTransferAllowance int expirationTime = Math.min(this.expirationTime, maxClockTime); - return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates, - this.commuterRailBoardStop, this.commuterRailRoute, this.commuterRailDirection); + return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates); } } @@ -328,44 +280,11 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat stateForTraversal = stateForTraversal.back; continue; // on the street, not on transit } - - // special handling for commuter rail - merge subsequent trips that use same route and direction together - // and treat them as a single ride. - TripPattern pat = transitLayer.tripPatterns.get(stateForTraversal.pattern); - RouteInfo ri = transitLayer.routes.get(pat.routeIndex); - if (ri.route_type == 2) { - // not completely correct as the merged ride used multiple patterns, but pattern is only used to look up - // route, so it's okay. - patterns.add(stateForTraversal.pattern); - int commuterRailAlightStop = stateForTraversal.stop; - int commuterRailDirection = pat.directionId; - int commuterRailRoute = pat.routeIndex; - int commuterRailBoardTime = stateForTraversal.boardTime; - - while (stateForTraversal != null) { - if (stateForTraversal.pattern == -1) break; // no on-street transfers with CR - - pat = transitLayer.tripPatterns.get(stateForTraversal.pattern); - if (pat.directionId != commuterRailDirection) break; // can't change direction on CR for free - if (pat.routeIndex != commuterRailRoute) break; // can't change routes on CR TODO Is this true? - // note that this last condition also enforces route_type == 2 implicitly - - commuterRailBoardTime = stateForTraversal.boardTime; - stateForTraversal = stateForTraversal.back; - } - - int commuterRailBoardStop = stateForTraversal.stop; - alightStops.add(commuterRailAlightStop); - boardStops.add(commuterRailBoardStop); - boardTimes.add(commuterRailBoardTime); - // don't increment backwards here - already done in loop above - } else { - patterns.add(stateForTraversal.pattern); - alightStops.add(stateForTraversal.stop); - boardStops.add(transitLayer.tripPatterns.get(stateForTraversal.pattern).stops[stateForTraversal.boardStopPosition]); - boardTimes.add(stateForTraversal.boardTime); - stateForTraversal = stateForTraversal.back; - } + patterns.add(stateForTraversal.pattern); + alightStops.add(stateForTraversal.stop); + boardStops.add(transitLayer.tripPatterns.get(stateForTraversal.pattern).stops[stateForTraversal.boardStopPosition]); + boardTimes.add(stateForTraversal.boardTime); + stateForTraversal = stateForTraversal.back; } // reverse data about the rides so we can step forward through them @@ -524,15 +443,6 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ transferAllowance = transferAllowance.setBehindGates(false); } - // if we ended on commuter rail, and did not have an on-street transfer, set the commuter rail transfer allowance - if (patterns.size() > 0 && state.pattern != -1) { - TripPattern lastPattern = transitLayer.tripPatterns.get(patterns.get(patterns.size() - 1)); - RouteInfo lastRoute = transitLayer.routes.get(lastPattern.routeIndex); - if (lastRoute.route_type == 2) { - transferAllowance = transferAllowance.setCommuterRailAllowance(boardStops.get(boardStops.size() - 1), lastPattern.routeIndex, - lastPattern.directionId, MAX_CR_FARE); - } - } return new FareBounds(cumulativeFarePaid, transferAllowance.tightenExpiration(maxClockTime)); } From 59c73a898d0c480dc67b6933431e483407279e83 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 4 Aug 2020 14:52:42 -0400 Subject: [PATCH 36/46] fix(boston-fares): avoid floating point roundoff --- .../conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index a49032d7e..05b065bf1 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -217,7 +217,7 @@ public BostonTransferAllowance tightenExpiration (int maxClockTime) { private final BostonTransferAllowance noTransferAllowance = new BostonTransferAllowance(); - private static int priceToInt(double price) {return (int) (price * 100);} // usd to cents + private static int priceToInt(double price) {return (int) Math.round(price * 100);} // usd to cents private static int payFullFare(Fare fare) {return priceToInt(fare.fare_attribute.price);} From a6e34afe3b33517df6ee39e7aa69cb00c41f500b Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Fri, 28 Aug 2020 11:10:45 -0400 Subject: [PATCH 37/46] fix(bos-fares): make sure that max transfer allowance is set correctly after riding the silver line for free --- .../r5/analyst/fare/BostonInRoutingFareCalculator.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 05b065bf1..7dc33248e 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -443,6 +443,16 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ transferAllowance = transferAllowance.setBehindGates(false); } + // if we ended up behind gates in the subway, we can get a free transfer to the subway. This is not neeed in + // fare calculation but is important in dominance. In fact, doing this would cause a problem in fare calculation, + // because payDifference uses the value field of the transfer allowance under construction. But once we return + // the transfer allowance, payDifference is no longer used. + // This is important for the silver line, which can get you behind gates for less than 2.25. + int subwayFare = (int) Math.round(fares.byId.get(SUBWAY_FARE_ID).fare_attribute.price * 100); + if (transferAllowance.behindGates && transferAllowance.value < subwayFare) { + transferAllowance = new BostonTransferAllowance(subwayFare, transferAllowance.number, transferAllowance.expirationTime, + transferAllowance.transferRuleGroup, transferAllowance.behindGates); + } return new FareBounds(cumulativeFarePaid, transferAllowance.tightenExpiration(maxClockTime)); } From 568070081833f334785c2ab27fafbfc0f8a22019 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 3 Sep 2020 09:43:36 -0400 Subject: [PATCH 38/46] fix(boston-fares): treat SL3 to Blue Line transfer at Airport like a behind-gates transfer --- .../r5/analyst/fare/BostonInRoutingFareCalculator.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 7dc33248e..ce4055d3b 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -72,8 +72,12 @@ private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_FREE, LOCAL_ private static final Set modesWithBehindGateTransfers = new HashSet<>(Arrays.asList(TransferRuleGroup.SUBWAY, TransferRuleGroup.SL_FREE)); private static final String DEFAULT_FARE_ID = LOCAL_BUS_FARE_ID; + // place-aport used to be listed here as well, but was removed because it prevented any discounted transfers + // _at all_ from the SL3 to the Blue Line at Airport. https://www.mbta.com/fares/transfers says that "SL1, SL2, and + // SL3 are subway fares" so we assume that an SL3-Blue Line transfer at Airport is treated the same as a behind gates + // transfer. This may be true at other locations/with other SL variants, but it is not documented. private static final Set stationsWithoutBehindGateTransfers = new HashSet<>(Arrays.asList( - "place-coecl", "place-aport")); + "place-coecl")); private static final Set> stationsConnected = new HashSet<>(Arrays.asList(new HashSet<>(Arrays.asList( "place-dwnxg", "place-pktrm")))); From 640d54766bf2a6a27b5f7c3b75e6b5fafd11f281 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 3 Sep 2020 11:40:20 -0400 Subject: [PATCH 39/46] fix(boston-fares): do not allow virtual behind gates transfer at airport when original boarding was SL_FREE --- .../fare/BostonInRoutingFareCalculator.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index ce4055d3b..87045ecbe 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -78,6 +78,12 @@ private enum TransferRuleGroup { LOCAL_BUS, SUBWAY, EXPRESS_BUS, SL_FREE, LOCAL_ // transfer. This may be true at other locations/with other SL variants, but it is not documented. private static final Set stationsWithoutBehindGateTransfers = new HashSet<>(Arrays.asList( "place-coecl")); + // The transfer system at Airport is not entirely clear, but we are assuming that it is treated as a "virtual + // behind-gates transfer" where even though there is a fare interaction it is not treated as a transfer. However, + // for this to work, you would have had to tap in on another service. So SL1 Airport-SL3-Blue does not provide a free + // transfer to the Blue Line because you never tapped in on the SL1 - the system has no way of knowing that you are + // transferring from the Silver Line rather than starting a new trip. + private static final Set stationsWithVirtualBehindGatesTransfers = new HashSet<>(Arrays.asList("place-aport")); private static final Set> stationsConnected = new HashSet<>(Arrays.asList(new HashSet<>(Arrays.asList( "place-dwnxg", "place-pktrm")))); @@ -237,13 +243,16 @@ private static boolean servicesConnectedBehindFareGates(TransferRuleGroup issuin (receiving == TransferRuleGroup.SUBWAY || receiving == TransferRuleGroup.SL_FREE)); } - private static boolean platformsConnected(int fromStopIndex, String fromStation, int toStopIndex, String toStation){ + private static boolean platformsConnected(int fromStopIndex, String fromStation, int toStopIndex, String toStation, TransferRuleGroup boardGroup){ return (fromStopIndex == toStopIndex || // same platform // different platforms, same station, in stations with behind-gate transfers between platforms (fromStation != null && fromStation.equals(toStation) && // e.g. Copley has same parent station, but no behind-the-gate transfers between platforms - !stationsWithoutBehindGateTransfers.contains(toStation)) || + !stationsWithoutBehindGateTransfers.contains(toStation) && + // If the original service boarded was SL_FREE, you don't get a free transfer at Airport, because + // the system has no way of knowing that you were "behind gates" to begin with. + (boardGroup != TransferRuleGroup.SL_FREE || !stationsWithVirtualBehindGatesTransfers.contains(toStation))) || // different stations connected behind faregates // e.g. Park Street and Downtown Crossing are connected by the Winter Street Concourse stationsConnected.contains(new HashSet<>(Arrays.asList(fromStation, toStation)))); @@ -357,7 +366,7 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat // if the previous alighting stop and this boarding stop are connected behind fare // gates (and without riding a vehicle!), continue to the next ride. There is no CharlieCard tap // and thus for fare purposes these are a single ride. - if (platformsConnected(fromStopIndex, fromStation, boardStopIndex, boardStation)) continue; + if (platformsConnected(fromStopIndex, fromStation, boardStopIndex, boardStation, issuing)) continue; } } @@ -438,7 +447,7 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ if (modesWithBehindGateTransfers.contains(previous)) { // it is possible that we are inside fare gates, because the last vehicle we rode would have left us there String currentStation = transitLayer.parentStationIdForStop.get(state.stop); - boolean behindGates = platformsConnected(prevAlightStopIndex, prevAlightStation, state.stop, currentStation); + boolean behindGates = platformsConnected(prevAlightStopIndex, prevAlightStation, state.stop, currentStation, transferAllowance.transferRuleGroup); transferAllowance = transferAllowance.setBehindGates(behindGates); } else { transferAllowance = transferAllowance.setBehindGates(false); From 6061e387410c5c4015c4a085ee7c4df4f54ffec8 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 3 Sep 2020 14:20:32 -0400 Subject: [PATCH 40/46] fix(boston-fares): correctly handle transfers to SL_FREE --- .../analyst/fare/BostonInRoutingFareCalculator.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 87045ecbe..935fecd70 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -392,7 +392,7 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat transferAllowance = transferAllowance.localBusToSubwayTransferAllowance(); } // Special case: route prefix is (local bus -> subway) - else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ + else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY) { // local bus -> subway -> bus special case if (receiving == TransferRuleGroup.LOCAL_BUS) { //Don't increment cumulativeFarePaid, just clear transferAllowance. Local bus->subway->local bus is a free transfer. @@ -409,6 +409,17 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY){ cumulativeFarePaid += transferAllowance.payDifference(priceToInt(fare.fare_attribute.price)); transferAllowance = noTransferAllowance; } + } else if (receiving == TransferRuleGroup.SL_FREE && issuing != TransferRuleGroup.NONE) { + // when boarding SL_FREE, don't wipe out the transfer allowance from a previous ride. Important for a + // trip that is bus -> SL_FREE -> bus, because the transfer allowance from the first bus can be used + // for the second. + // This will not affect behind-gate transfers, as these are always based on the fare from the previous + // ride, not the issuing ride. + // Example: https://projects.indicatrix.org/fareto-examples/?load=broken-bos-bus-sl-bus&index=1 + // (uncheck Remove non-Pareto-optimal trips) + // Note that this is here and not inside the tryToRedeemTransfer conditional, because SL_FREE is not the + // target of any transfers so tryToRedeemTransfer will always be false. + /* do nothing */ } else { // don't try to use transferValue; pay the full fare for this ride cumulativeFarePaid += payFullFare(fare); transferAllowance = transferAllowance.updateTransferAllowance(fare, boardClockTime); From 48a17ac87ade8a88128133f8b90325244bab8971 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Thu, 3 Sep 2020 17:53:05 -0400 Subject: [PATCH 41/46] fix(boston-fares): kep track of whether rider entered the subway by paying fare or for free --- .../fare/BostonInRoutingFareCalculator.java | 55 +++++++++++++------ 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java index 935fecd70..7575f26d5 100644 --- a/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java +++ b/src/main/java/com/conveyal/r5/analyst/fare/BostonInRoutingFareCalculator.java @@ -133,6 +133,13 @@ public class BostonTransferAllowance extends TransferAllowance { */ public final boolean behindGates; + /** If we managed to get behind the subway gates without paying a fare by boarding the SL1 at Logan, we cannot + * later transfer from SL3 to Blue Line at Airport using the "virtual behind gates" transfer. + * + * cf. https://www.theonion.com/police-apprehends-man-for-repeatedly-failing-to-pay-for-1836794666 + */ + public final boolean enteredSubwayForFree; + /** * No transfer allowance */ @@ -140,6 +147,7 @@ private BostonTransferAllowance () { super(); this.transferRuleGroup = TransferRuleGroup.NONE; this.behindGates = false; + this.enteredSubwayForFree = false; } /** @@ -156,6 +164,7 @@ private BostonTransferAllowance (TransferRuleGroup transferRuleGroup, Fare fare, startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = transferRuleGroup; this.behindGates = false; + this.enteredSubwayForFree = false; } /** @@ -170,14 +179,16 @@ private BostonTransferAllowance(Fare fare, int startTime){ startTime + fare.fare_attribute.transfer_duration); this.transferRuleGroup = fareGroups.get(fare.fare_id); this.behindGates = false; + this.enteredSubwayForFree = false; } /** Used to set whether the rider is still behind the fare gates, and to tighten expiration times */ private BostonTransferAllowance(int value, int number, int expirationTime, TransferRuleGroup transferRuleGroup, - boolean behindGates) { + boolean behindGates, boolean enteredSubwayForFree) { super(value, number, expirationTime); this.transferRuleGroup = transferRuleGroup; this.behindGates = behindGates; + this.enteredSubwayForFree = enteredSubwayForFree; } /** @@ -203,9 +214,9 @@ private BostonTransferAllowance localBusToSubwayTransferAllowance(){ } /** called at the end of the fare calc loop to record whether the last state is behind gates or not. */ - private BostonTransferAllowance setBehindGates (boolean behindGates) { + private BostonTransferAllowance setBehindGates (boolean behindGates, boolean enteredSubwayForFree) { if (behindGates == this.behindGates) return this; - else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, behindGates); + else return new BostonTransferAllowance(this.value, this.number, this.expirationTime, this.transferRuleGroup, behindGates, enteredSubwayForFree); } @Override @@ -214,13 +225,16 @@ public boolean atLeastAsGoodForAllFutureRedemptions(TransferAllowance other) { this.transferRuleGroup == ((BostonTransferAllowance) other).transferRuleGroup && // if this is behind gates, or other is not behind gates, they are comparable // if other is behind gates and this is not, it could possibly be better. - (this.behindGates || !((BostonTransferAllowance) other).behindGates); + (this.behindGates || !((BostonTransferAllowance) other).behindGates) && + // if other entered subway for free, this is as good as or better, because could get free xfer to + // blue line at Airport + (((BostonTransferAllowance) other).enteredSubwayForFree || this.enteredSubwayForFree == ((BostonTransferAllowance) other).enteredSubwayForFree); } public BostonTransferAllowance tightenExpiration (int maxClockTime) { // copied from TransferAllowance but need to override so that everything stays a BostonTransferAllowance int expirationTime = Math.min(this.expirationTime, maxClockTime); - return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates); + return new BostonTransferAllowance(this.value, this.number, expirationTime, this.transferRuleGroup, this.behindGates, this.enteredSubwayForFree); } } @@ -243,16 +257,16 @@ private static boolean servicesConnectedBehindFareGates(TransferRuleGroup issuin (receiving == TransferRuleGroup.SUBWAY || receiving == TransferRuleGroup.SL_FREE)); } - private static boolean platformsConnected(int fromStopIndex, String fromStation, int toStopIndex, String toStation, TransferRuleGroup boardGroup){ + private static boolean platformsConnected(int fromStopIndex, String fromStation, int toStopIndex, String toStation, boolean enteredSubwayForFree){ return (fromStopIndex == toStopIndex || // same platform // different platforms, same station, in stations with behind-gate transfers between platforms (fromStation != null && fromStation.equals(toStation) && // e.g. Copley has same parent station, but no behind-the-gate transfers between platforms !stationsWithoutBehindGateTransfers.contains(toStation) && - // If the original service boarded was SL_FREE, you don't get a free transfer at Airport, because - // the system has no way of knowing that you were "behind gates" to begin with. - (boardGroup != TransferRuleGroup.SL_FREE || !stationsWithVirtualBehindGatesTransfers.contains(toStation))) || + // If the subway was entered for free via SL1 from Logan, you don't get a free transfer at + // Airport, because the system has no way of knowing that you were "behind gates" to begin with. + (!enteredSubwayForFree || !stationsWithVirtualBehindGatesTransfers.contains(toStation))) || // different stations connected behind faregates // e.g. Park Street and Downtown Crossing are connected by the Winter Street Concourse stationsConnected.contains(new HashSet<>(Arrays.asList(fromStation, toStation)))); @@ -307,6 +321,7 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat boardTimes.reverse(); int alightStopIndex = -1; + boolean enteredSubwayForFree = false; // Loop over rides to get to the state in forward-chronological order for (int ride = 0; ride < patterns.size(); ride ++) { @@ -358,22 +373,24 @@ public FareBounds calculateFare(McRaptorSuboptimalPathProfileRouter.McRaptorStat TransferRuleGroup previous = fareGroups.get(prevFare.fare_id); // servicesConnectedBehindFareGates contains an implicit bounds check that ride >= 1 - // this is actually not right, as issuing service could be bus with a free transfer to subway - // Previous is not always the same as issuing, e.g. when there was a previous bus -> subway transfer if (servicesConnectedBehindFareGates(previous, receiving)) { int fromStopIndex = alightStops.get(ride - 1); String fromStation = transitLayer.parentStationIdForStop.get(fromStopIndex); // if the previous alighting stop and this boarding stop are connected behind fare // gates (and without riding a vehicle!), continue to the next ride. There is no CharlieCard tap // and thus for fare purposes these are a single ride. - if (platformsConnected(fromStopIndex, fromStation, boardStopIndex, boardStation, issuing)) continue; + if (platformsConnected(fromStopIndex, fromStation, boardStopIndex, boardStation, enteredSubwayForFree)) continue; } } + // we are no longer in the subway, so it is immaterial if we entered it for free. + enteredSubwayForFree = false; + // Check for transferValue expiration // This is not done on behind-faregate transfers because once you're in the subway, you don't tap your // CharlieCard again, so, if you so desire, you can ride forever 'neath the streets of Boston (or at least // until system closing). + // NB this might not be right for the "virtual" behind-gates transfer at Airport. if (transferAllowance.hasExpiredAt(boardTimes.get(ride))) transferAllowance = noTransferAllowance; // We are doing a transfer that is not behind faregates, check if we might be able to redeem a transfer @@ -424,6 +441,9 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY) { cumulativeFarePaid += payFullFare(fare); transferAllowance = transferAllowance.updateTransferAllowance(fare, boardClockTime); } + + // if we rode the Silver Line for free from Logan, we are entering the subway for free. + if (receiving == TransferRuleGroup.SL_FREE) enteredSubwayForFree = true; } // warning: reams of log output @@ -458,13 +478,14 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY) { if (modesWithBehindGateTransfers.contains(previous)) { // it is possible that we are inside fare gates, because the last vehicle we rode would have left us there String currentStation = transitLayer.parentStationIdForStop.get(state.stop); - boolean behindGates = platformsConnected(prevAlightStopIndex, prevAlightStation, state.stop, currentStation, transferAllowance.transferRuleGroup); - transferAllowance = transferAllowance.setBehindGates(behindGates); + boolean behindGates = platformsConnected(prevAlightStopIndex, prevAlightStation, state.stop, currentStation, + enteredSubwayForFree); + transferAllowance = transferAllowance.setBehindGates(behindGates, behindGates && enteredSubwayForFree); } else { - transferAllowance = transferAllowance.setBehindGates(false); + transferAllowance = transferAllowance.setBehindGates(false, false); } } else { - transferAllowance = transferAllowance.setBehindGates(false); + transferAllowance = transferAllowance.setBehindGates(false, false); } // if we ended up behind gates in the subway, we can get a free transfer to the subway. This is not neeed in @@ -475,7 +496,7 @@ else if (issuing == TransferRuleGroup.LOCAL_BUS_TO_SUBWAY) { int subwayFare = (int) Math.round(fares.byId.get(SUBWAY_FARE_ID).fare_attribute.price * 100); if (transferAllowance.behindGates && transferAllowance.value < subwayFare) { transferAllowance = new BostonTransferAllowance(subwayFare, transferAllowance.number, transferAllowance.expirationTime, - transferAllowance.transferRuleGroup, transferAllowance.behindGates); + transferAllowance.transferRuleGroup, transferAllowance.behindGates, transferAllowance.enteredSubwayForFree); } return new FareBounds(cumulativeFarePaid, transferAllowance.tightenExpiration(maxClockTime)); From adc2d1e57a9559c7fc78a8189ac5dc14957c28b0 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Mon, 16 Nov 2020 19:06:22 -0500 Subject: [PATCH 42/46] feat(transitlayer): allow retaining shapes when building transportnetwork --- .../PointToPointRouterServer.java | 14 +++++++---- .../com/conveyal/r5/transit/TransitLayer.java | 7 +++--- .../conveyal/r5/transit/TransportNetwork.java | 23 +++++++++++++++---- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java b/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java index 4393999e3..fb5dafa6f 100644 --- a/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java +++ b/src/main/java/com/conveyal/r5/point_to_point/PointToPointRouterServer.java @@ -71,7 +71,7 @@ public class PointToPointRouterServer { public static final String BUILDER_CONFIG_FILENAME = "build-config.json"; - private static final String USAGE = "It expects --build [path to directory with GTFS and PBF files] to build the graphs\nor --graphs [path to directory with graph] to start the server with provided graph"; + private static final String USAGE = "It expects --build [path to directory with GTFS and PBF files] to build the graphs\nor --graphs [path to directory with graph] to start the server with provided graph.\n--build --save-shapes [path] will save the shapes in the feed"; public static final int RADIUS_METERS = 200; @@ -82,14 +82,20 @@ public static void main(String[] commandArguments) { final boolean inMemory = false; if ("--build".equals(commandArguments[0])) { - - File dir = new File(commandArguments[1]); + boolean saveShapes = false; + File dir; + if ("--save-shapes".equals(commandArguments[1])) { + saveShapes = true; + dir = new File(commandArguments[2]); + } else { + dir = new File(commandArguments[1]); + } if (!dir.isDirectory() && dir.canRead()) { LOG.error("'{}' is not a readable directory.", dir); } - TransportNetwork transportNetwork = TransportNetwork.fromDirectory(dir); + TransportNetwork transportNetwork = TransportNetwork.fromDirectory(dir, saveShapes); //In memory doesn't save it to disk others do (build, preFlight) if (!inMemory) { try { diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index a993a4055..a0459e837 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -78,8 +78,6 @@ public class TransitLayer implements Serializable, Cloneable { */ public static final int WALK_DISTANCE_LIMIT_METERS = 2000; - public static final boolean SAVE_SHAPES = false; - /** * Distance limit for transfers, meters. Set to 1km which is slightly above OTP's 600m (which was specified as * 1 m/s with 600s max time, which is actually somewhat less than 600m due to extra costs due to steps etc. @@ -126,6 +124,9 @@ public class TransitLayer implements Serializable, Cloneable { public List parentStationIdForStop = new ArrayList<>(); + /** if true, save shapes in graph building */ + public boolean saveShapes = false; + // Inverse map of stopIdForIndex, reconstructed from that list (not serialized). No-entry value is -1. public transient TObjectIntMap indexForStopId; @@ -406,7 +407,7 @@ public void loadFromGtfs (GTFSFeed gtfs, LoadLevel level) throws DuplicateFeedEx tripPattern.routeIndex = routeIndexForRoute.get(trip.route_id); - if (trip.shape_id != null && SAVE_SHAPES) { + if (trip.shape_id != null && saveShapes) { Shape shape = gtfs.getShape(trip.shape_id); if (shape == null) LOG.warn("Shape {} for trip {} was missing", trip.shape_id, trip.trip_id); else { diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java index 2aa95850a..c7be7121e 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java @@ -107,7 +107,7 @@ public void rebuildTransientIndexes() { /** Create a TransportNetwork from gtfs-lib feeds */ public static TransportNetwork fromFeeds (String osmSourceFile, List feeds, TNBuilderConfig config) { - return fromFiles(osmSourceFile, null, feeds, config); + return fromFiles(osmSourceFile, null, feeds, config, false); } /** Legacy method to load from a single GTFS file */ @@ -122,7 +122,8 @@ public static TransportNetwork fromFiles (String osmSourceFile, String gtfsSourc * (due to caching etc.) */ private static TransportNetwork fromFiles (String osmSourceFile, List gtfsSourceFiles, List feeds, - TNBuilderConfig tnBuilderConfig) throws DuplicateFeedException { + TNBuilderConfig tnBuilderConfig, boolean saveShapes) + throws DuplicateFeedException { System.out.println("Summarizing builder config: " + BUILDER_CONFIG_FILENAME); System.out.println(tnBuilderConfig); @@ -154,6 +155,7 @@ private static TransportNetwork fromFiles (String osmSourceFile, List gt // Load transit data TODO remove need to supply street layer at this stage TransitLayer transitLayer = new TransitLayer(); + transitLayer.saveShapes = saveShapes; if (feeds != null) { for (GTFSFeed feed : feeds) { @@ -197,11 +199,17 @@ private static TransportNetwork fromFiles (String osmSourceFile, List gt * distinction should be maintained for various reasons. However, we use the GTFS IDs only for reference, so it * doesn't really matter, particularly for analytics. */ + public static TransportNetwork fromFiles (String osmFile, List gtfsFiles, TNBuilderConfig config, + boolean saveShapes) { + return fromFiles(osmFile, gtfsFiles, null, config, saveShapes); + } + public static TransportNetwork fromFiles (String osmFile, List gtfsFiles, TNBuilderConfig config) { - return fromFiles(osmFile, gtfsFiles, null, config); + // default to not saving shapes + return fromFiles(osmFile, gtfsFiles, config, false); } - public static TransportNetwork fromDirectory (File directory) throws DuplicateFeedException { + public static TransportNetwork fromDirectory (File directory, boolean saveShapes) throws DuplicateFeedException { File osmFile = null; List gtfsFiles = new ArrayList<>(); TNBuilderConfig builderConfig = null; @@ -232,10 +240,15 @@ public static TransportNetwork fromDirectory (File directory) throws DuplicateFe LOG.error("An OSM PBF file is required to build a network."); return null; } else { - return fromFiles(osmFile.getAbsolutePath(), gtfsFiles, builderConfig); + return fromFiles(osmFile.getAbsolutePath(), gtfsFiles, builderConfig, saveShapes); } } + public static final TransportNetwork fromDirectory (File directory) throws DuplicateFeedException { + // default to not saving shapes + return fromDirectory(directory, false); + } + /** * Open and parse the JSON file at the given path into a Jackson JSON tree. Comments and unquoted keys are allowed. * Returns default config if the file does not exist, From 0536add92c84b896bc401465411434de41f57c4d Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Fri, 20 Nov 2020 18:21:20 -0500 Subject: [PATCH 43/46] docs(boston-fares): incomplete boston fare docs --- docs/fares/boston.md | 79 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 docs/fares/boston.md diff --git a/docs/fares/boston.md b/docs/fares/boston.md new file mode 100644 index 000000000..3a91ecf46 --- /dev/null +++ b/docs/fares/boston.md @@ -0,0 +1,79 @@ +# Boston in-routing fare calculation + +The Boston fare calculator is the original fare calculator used as a an example in [our original paper on computing accessibility with fares](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). It handles fares for the [Massachusetts Bay Transportation Authority](https://mbta.com), which provides most of the transit service in the Boston area, including subways, light rail, commuter rail, local and express buses, bus rapid transit, and ferries. As of summer 2018, when we wrote the paper, the fares were as shown in Table 2 of [the original paper](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). This document details the implementation of this fare system in R5. + +## General principles + +The MBTA fare system for all modes _except_ ferry and commuter rail generally allows a single ``pay-the-difference'' transfer from one mode to another. For instance, after a $1.70 bus ride, you can ride the $2.25 subway by paying an upgrade fare of $2.25 - 1.70 = $0.55. If the first mode you paid full fare for was more expensive than the mode you're transferring to, the transfer is free. You generally get only one of these transfers, although after transfering from local bus to subway you then get one more free transfer to a local bus. Note that this single pay-the-difference fare structure can create [negative transfer allowances](https://indicatrix.org/post/regular-2-for-you-3-when-is-a-discount-not-a-discount/), for instance when transferring local -> express -> subway. + +The fares for commuter rail and ferry are simpler. There are no discounted transfers from these modes; the fare for the ride is simply added to the cumulative fare paid, and the + +The Silver Line is [free when boarded at Logan Airport, and allows a free transfer to the subway](http://www.massport.com/logan-airport/to-from-logan/transportation-options/taking-the-t/), as are [the MassPort shuttles](http://www.massport.com/logan-airport/to-from-logan/transportation-options/on-airport-shuttle/), which can create [interesting trips using free airport shuttles and the Silver Line to avoid paying the subway fare](https://projects.indicatrix.org/fareto-examples/?load=bos-eastie-to-revere-sl&index=3). + +## Subway + +The subway fare is $2.25, with [free within-gates transfers](https://projects.indicatrix.org/fareto-examples/?load=bos-red-orange&index=0). However, transfers that require leaving the subway system [require a new fare](https://projects.indicatrix.org/fareto-examples/?load=bos-green-b-to-d&index=3). Transfers are generally assumed to be within-subway if they board at the same parent station as the previous alighting. However, there are exceptions. Park Street and Downtown Crossing are explicitly [connected behind faregates](https://projects.indicatrix.org/fareto-examples/?load=bos-dxc-park&index=0) by the [Winter Street Concourse](https://en.wikipedia.org/wiki/Winter_Street_Concourse). + +Many stations on the MBTA system, most notably [Copley](https://en.wikipedia.org/wiki/Copley_station), do not allow a traveler to transfer to a train traveling in the opposite direction without a new fare payment. Of these, only Copley is treated as not having behind-gate transfers at all, except to trains traveling in the same direction. Copley is treated as such because it is the logical transfer point from an inbound E line train to an outbound train on any other line, or vice-versa, but this transfer is actually not allowed. The router will [recommend a transfer at Arlington, but still present the more expensive transfer at Copley if it is faster](https://projects.indicatrix.org/fareto-examples/?load=bos-green-copley-xfer&index=0). Transfers between trains traveling in the same direction [are allowed at Copley](https://projects.indicatrix.org/fareto-examples/?load=bos-green-copley-same-dir-xfer&index=0). All other stations where multiple lines split apart or cross have behind-gates transfers (Silver Line Way is not considered a station, but two separate stops, in the GTFS we are using). Other stations (e.g. Central) do not have behind-gates transfers, but such a transfer would only be used to change direction on the same line---something we assume is never optimal, since there is no express service on the MBTA subway system. + +Similarly, no opposite-direction _or_ same-direction transfers are allowed at surface Green Line stops; once you leave the Green Line, you cannot reboard. We again assume that these transfers will never be optimal, and don't explicitly disallow them. The only exception we are aware of is if there are short-turns on any of the surface Green Line branches, the router may suggest a transfer at one of the surface stops. However, in this case, the rider could have also just waited at their origin stop for the full route train, and gotten the same arrival time, and in fact the router should find this route anyhow as a no-transfer route will always be found even if there is another multiple-transfer route, due to the RAPTOR search process. Transfers between lines can never occur at surface stops as no lines share surface stops. Trains do occasionally express to relieve bunching, but this is not as far as I know in the schedule, and when it does happen a free transfer is _de facto_ allowed. + +The [Mattapan High-Speed Line](https://en.wikipedia.org/wiki/Ashmont%E2%80%93Mattapan_High-Speed_Line) is a trolley line that is depicted as part of the red line on official MBTA maps; it connects to the Red Line at the Ashmont terminus. Transfers between the Red Line and the Mattapan High-Speed Line [are free (p. 16)](https://cdn.mbta.com/sites/default/files/2020-09/2020-09-01-mbta-combined-tariff.pdf), and are treated by R5 as if they were any other behind-gates transfer in the subway system; that is, a free transfer to local bus [can still be used after riding the Red Line and the Mattapan High-Speed Line](https://projects.indicatrix.org/fareto-examples/?load=bos-red-mattapan-bus&index=1). + +Transfers to local buses from the subway [are free](https://projects.indicatrix.org/fareto-examples/?load=bos-red-orange-bus&index=0), while [transfers from local buses to the subway require an upgrade fare](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-red&index=0). There is an exception to the usual rule of "one pay-the-difference" transfer for local buses and subways; [a rider transferring from a local bus to the subway can then transfer back to another local bus for free](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-subway-bus&index=1). This is true [even when multiple subway lines are ridden in between the bus trips](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-subway-subway-bus&index=1), as long as the rider does not leave the paid area of the system. It is not documented whether this is true for the other bus types, so we assume it is not. + +Transfers to express buses [require an upgrade fare that brings the total in line with the express bus fare](https://projects.indicatrix.org/fareto-examples/?load=bos-subway-inner-express&index=0). Transfers from [express buses to the subway are free](https://projects.indicatrix.org/fareto-examples/?load=bos-express-subway&index=1). Because the amount of discount you get depends on what vehicle you transfer to, this can result in negative transfer allowances in some cases. In these cases, the algorithm may not find the lowest-cost path, when it involves a complex route that requires discarding a ticket or riding an extra "throwaway" transit vehicle, [as described in this blog post](https://indicatrix.org/post/regular-2-for-you-3-when-is-a-discount-not-a-discount/). + +## Bus + +Local buses cost $1.70 [with one free transfer](https://projects.indicatrix.org/fareto-examples/?load=bos-two-bus&index=0). A [third local bus ride requires a new fare payment](https://projects.indicatrix.org/fareto-examples/?load=bos-extra-bus-fare&index=1), but the router will also find [cheaper two-bus options even if they are slower](https://projects.indicatrix.org/fareto-examples/?load=bos-extra-bus-fare&index=0). + +Transfer from [local to express buses require an upgrade fare payment](https://projects.indicatrix.org/fareto-examples/?load=bos-local-express&index=0), while [transfers from express to local buses are free](https://projects.indicatrix.org/fareto-examples/?load=bos-express-local&index=1). + +TODO inner/outer + +## Silver Line + +The Silver Line [SL1, SL2, and SL3 charge subway fares](https://www.mbta.com/fares/subway-fares), while the [SL4 and SL5 charge local bus fares](https://www.mbta.com/fares/bus-fares). This section details the fare system for the SL1, SL2, and SL3. The SL4 and SL5 are simply treated as local buses (described below). + +The [base fare for the Silver Line is $2.25, like the subway](https://projects.indicatrix.org/fareto-examples/?load=bos-sl-base&index=1). However, things are more complicated when transfers are involved. Transfers between the Silver Line and the subway system are [free at South Station](https://projects.indicatrix.org/fareto-examples/?load=bos-red-silver&index=1) because the Silver Line enters a tunnel and platforms are physically connected behind fare gates. Transfers from the Silver Line [to](https://projects.indicatrix.org/fareto-examples/?load=bos-sl-bus&index=0) and [from]() buses are free, just as they are with the subway + +A transfer [from the SL3 to the Blue Line at Airport](https://projects.indicatrix.org/fareto-examples/?load=bos-sl3-blue&index=1) is treated as a behind-gates transfer, even though it is not technically behind gates (the SL3 drops you off in the bus loop on the Massport side of the Airport station, and you have to tag in to board the blue line). The same is true for a [Blue Line to SL3 transfer](https://projects.indicatrix.org/fareto-examples/?load=bos-blue-sl3&index=1). However, this transfer is not allowed when the original boarding was on the Silver Line for free at Logan Airport, as we assume the transfer system cannot recognize this transfer. This is handled by tracking in the transfer allowance whether the subway was entered for free. + +These assumptions about how transfers between the Silver Line and the Blue Line work are as close as we can get to the [documented policy from the MBTA](https://www.mbta.com/fares/transfers) which states that ``transfers at subway stations are free, if you exit the station you will pay the full subway fare to enter another station.'' It is not clear how transfers from the out-of-faregates busway station at Airport are handled, but [the SL3 was intended to connect to the Blue Line](https://blog.mass.gov/transportation/uncategorized/mbta-new-silver-line-3-chelsea-service-between-chelsea-and-south-station/) so it seems unlikely that this transfer would cost. As implemented, the algorithm cannot exactly replicate the CharlieCard system, as the CharlieCard system does not know where a user exited the system, so it may be that subway-subway transfers are simply allowed at Airport regardless of source. However, the implementation aims to reflect the spirit of the fare system. + +When the SL1 from the airport is taken in between two local buses, the transfer allowance is not affected, other than to note that the user is now in the subway system, because there is no fare system interaction. This [allows the user to use the transfer to board another local bus](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-sl1-bus&index=1), [even if a subway was also taken after the SL1](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-sl1-red-bus&index=0). + +## Ferries + +Ferries have a separate fare structure, and [there are no transfer discounts to or from ferries, although the router will trade off ferries with cheaper terrestrial routes](https://projects.indicatrix.org/fareto-examples/?load=bos-ferry-tradeoff&index=0). Transferring to a ferry [does not yield any discount either](https://projects.indicatrix.org/fareto-examples/?load=bos-subway-to-ferry&index=0). This does mean that [riding a local bus, then a ferry, then another local bus only requires _one_ local bus fare, as the transfer from the first bus can be used on the second](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-ferry-bus&index=0). Ferries from [Boston to Logan] or [to the South Shore] cost $9.25, while trips between Logan and the South Shore cost $18.50, regardless of whether [they are undertaken on a single ferry](https://projects.indicatrix.org/fareto-examples/?load=bos-logan-hull&index=0) or [with a transfer in downtown Boston](https://projects.indicatrix.org/fareto-examples/?load=bos-logan-hull-xfer&index=0). The [Charlestown-Downtown Boston ferry costs $3.50](https://projects.indicatrix.org/fareto-examples/?load=bos-ctown-ferry&index=0). + +## Massport shuttles + +[Massport shuttles are free.](https://projects.indicatrix.org/fareto-examples/?load=bos-massport-shuttle&index=0) + +## Commuter rail + +Commuter rail has a zone-based fare system, with fares from each zone to downtown Boston, as well as "interzone" fares for trips that do not start or end in downtown Boston. At the time this paper was written, there were no transfer discounts from commuter rail to other modes, [although some have since been piloted on one line](https://www.bostonglobe.com/2020/05/07/metro/coming-soon-fairmount-line-free-transfers-subway/). + +Two broad classes of one-way commuter rail fares exist: zone fares and interzone fares. Zone fares are fares for trips beginning or ending in Zone 1A, which contains the Downtown Boston terminals as well as several other central stations in Boston, Cambridge, Medford, Malden, and Chelsea; for instance a Zone 5 fare would cover travel from Zone 5 to Zone 1A. Interzone fares are for trips that pass through other zones but not Zone 1A. For instance, an Interzone 3 fare would cover a trip from Zone 6 to Zone 4 (because it passes through three fare zones). The fares are as follows: + +- [Zone 1A <> Zone 1A: $2.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-1a&index=0) +- [Zone 1 -> Zone 1A: $6.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1-1a&index=3) +- [Zone 1A -> Zone 1: $6.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-1&index=0) +- [Zone 2 -> Zone 1A: $6.75]() +- [Zone 1A -> Zone 2: $6.75](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-2&index=4) +- [Zone 1A -> Zone 3: $7.50](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-3&index=1) +- [Zone 1A -> Zone 4: $8.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-4&index=2) +- [Zone 1A -> Zone 5: $9.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-5&index=0) +- [Zone 1A -> Zone 6: $10.00](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-6&index=0) +- [Zone 1A -> Zone 7: $10.50](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-7&index=0) +- [Zone 1A -> Zone 8: $11.50](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-8&index=0) +- [Zone 1A -> Zone 9: $12.00](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-9&index=0) +- Zone 1A to Zone 10 (Wickford Junction) is outside of the analysis area of this project + +Per the fare tariff, there are no discounted transfers at all on commuter rail. However, in practice and since the commuter rail system is a proof-of-payment system, and since some trains express during rush hour, it is possible to transfer and continue a same-direction trip, for instance a trip from [Worcester to Auburndale at rush hour, with a transfer in Framingham to to express service](https://projects.indicatrix.org/fareto-examples/?load=bos-cr-same-dir-xfer&index=0). The router calculates the fare for this as $8—two $4 interzone 4 fares for the two legs of the trip. However, in practice you purchase a $5.50 interzone 7 ticket for this trip. This type of express service is rare in the MBTA commuter rail system, and is thus left unimplemented. + +- Same direction transfers at SL Way should be okay +- Harvard Busway is "behind gates" +- Document assumptions From 5e8a656432737c1e4ce8237012b2366e853690ef Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 24 Nov 2020 16:37:43 -0500 Subject: [PATCH 44/46] docs(boston-fares): finish boston fare docs --- docs/fares/boston.md | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/docs/fares/boston.md b/docs/fares/boston.md index 3a91ecf46..72fadcc1e 100644 --- a/docs/fares/boston.md +++ b/docs/fares/boston.md @@ -1,6 +1,6 @@ # Boston in-routing fare calculation -The Boston fare calculator is the original fare calculator used as a an example in [our original paper on computing accessibility with fares](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). It handles fares for the [Massachusetts Bay Transportation Authority](https://mbta.com), which provides most of the transit service in the Boston area, including subways, light rail, commuter rail, local and express buses, bus rapid transit, and ferries. As of summer 2018, when we wrote the paper, the fares were as shown in Table 2 of [the original paper](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). This document details the implementation of this fare system in R5. +The Boston fare calculator is the original fare calculator used as a an example in [our original paper on computing accessibility with fares](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). It handles fares for the [Massachusetts Bay Transportation Authority](https://mbta.com), which provides most of the transit service in the Boston area, including subways, light rail, commuter rail, local and express buses, bus rapid transit, and ferries. As of summer 2018, when we wrote the paper, the fares were as shown in Table 2 of [the original paper](https://files.indicatrix.org/Conway-Stewart-2019-Charlie-Fare-Constraints.pdf). This document details the implementation of this fare system in R5. The fare system is based on fares and modified GTFS from July 2018. ## General principles @@ -30,7 +30,7 @@ Local buses cost $1.70 [with one free transfer](https://projects.indicatrix.org/ Transfer from [local to express buses require an upgrade fare payment](https://projects.indicatrix.org/fareto-examples/?load=bos-local-express&index=0), while [transfers from express to local buses are free](https://projects.indicatrix.org/fareto-examples/?load=bos-express-local&index=1). -TODO inner/outer +[Inner express buses](https://projects.indicatrix.org/fareto-examples/?load=bos-inner&index=0) and [outer express buses](https://projects.indicatrix.org/fareto-examples/?load=bos-outer&index=1) have different fare characteristics but the same transfer characteristics. ## Silver Line @@ -58,22 +58,50 @@ Commuter rail has a zone-based fare system, with fares from each zone to downtow Two broad classes of one-way commuter rail fares exist: zone fares and interzone fares. Zone fares are fares for trips beginning or ending in Zone 1A, which contains the Downtown Boston terminals as well as several other central stations in Boston, Cambridge, Medford, Malden, and Chelsea; for instance a Zone 5 fare would cover travel from Zone 5 to Zone 1A. Interzone fares are for trips that pass through other zones but not Zone 1A. For instance, an Interzone 3 fare would cover a trip from Zone 6 to Zone 4 (because it passes through three fare zones). The fares are as follows: +### Zone fares + - [Zone 1A <> Zone 1A: $2.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-1a&index=0) - [Zone 1 -> Zone 1A: $6.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1-1a&index=3) - [Zone 1A -> Zone 1: $6.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-1&index=0) -- [Zone 2 -> Zone 1A: $6.75]() +- [Zone 2 -> Zone 1A: $6.75](https://projects.indicatrix.org/fareto-examples/?load=bos-2-1a&index=2) - [Zone 1A -> Zone 2: $6.75](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-2&index=4) +- [Zone 3 -> Zone 1A: $7.50](https://projects.indicatrix.org/fareto-examples/?load=bos-3-1a&index=4) - [Zone 1A -> Zone 3: $7.50](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-3&index=1) +- [Zone 4 -> Zone 1A: $8.25](https://projects.indicatrix.org/fareto-examples/?load=bos-4-1a&index=0) - [Zone 1A -> Zone 4: $8.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-4&index=2) +- [Zone 5 -> Zone 1A: $9.25](https://projects.indicatrix.org/fareto-examples/?load=bos-5-1a&index=2) - [Zone 1A -> Zone 5: $9.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-5&index=0) +- [Zone 6 -> Zone 1A: $10.00](https://projects.indicatrix.org/fareto-examples/?load=bos-6-1a&index=1) - [Zone 1A -> Zone 6: $10.00](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-6&index=0) +- [Zone 7 -> 1A: $10.50](https://projects.indicatrix.org/fareto-examples/?load=bos-7-1a&index=0) - [Zone 1A -> Zone 7: $10.50](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-7&index=0) +- [Zone 8 -> Zone 1A: $11.50](https://projects.indicatrix.org/fareto-examples/?load=bos-8-1a&index=1) - [Zone 1A -> Zone 8: $11.50](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-8&index=0) +- [Zone 9 -> Zone 1A: $12.00](https://projects.indicatrix.org/fareto-examples/?load=bos-9-1a&index=0) - [Zone 1A -> Zone 9: $12.00](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-9&index=0) -- Zone 1A to Zone 10 (Wickford Junction) is outside of the analysis area of this project +- Zone 10 (Wickford Junction) is outside of the analysis area of this project + +### Interzone fares + +Charged by the number of zones passed through, in whole or in part. + +- [Interzone 1: $2.75](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-1&index=0) +- [Interzone 2: $3.25](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-2&index=0) +- [Interzone 3: $3.50](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-3&index=0) +- [Interzone 4: $4.00](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-4&index=0) +- [Interzone 5: $4.50](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-5&index=0) +- [Interzone 6: $5.00](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-6&index=0) +- [Interzone 7: $5.50](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-7&index=0) +- [Interzone 8: $6.00](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-8&index=0) +- [Interzone 9: $6.50](https://projects.indicatrix.org/fareto-examples/?load=bos-iz-9&index=0) +- Interzone 10 is not possible without Wickford Junction, which is outside the analysis area. + +### Boundary zones + +In the baseline, there is one station, Quincy Center, that is in a special 1A/1 boundary zone, which was due to a subway station closure at the time. Fares from Zone 1A/1 to Zone 1A stations [are charged the Zone 1A fare of $2.25](https://projects.indicatrix.org/fareto-examples/?load=bos-1a1-1&index=3), while trips from Zone 1A/1 to other zones [are charged the appropriate interzone fare as if the station was in Zone 1](https://projects.indicatrix.org/fareto-examples/?load=bos-1a1-6&index=0). The opposite is also true, for trips [from Zone 1A](https://projects.indicatrix.org/fareto-examples/?load=bos-1a-1a1&index=) and [from outlying zones](https://projects.indicatrix.org/fareto-examples/?load=bos-6-1a1&index=0). + +### Transfers -Per the fare tariff, there are no discounted transfers at all on commuter rail. However, in practice and since the commuter rail system is a proof-of-payment system, and since some trains express during rush hour, it is possible to transfer and continue a same-direction trip, for instance a trip from [Worcester to Auburndale at rush hour, with a transfer in Framingham to to express service](https://projects.indicatrix.org/fareto-examples/?load=bos-cr-same-dir-xfer&index=0). The router calculates the fare for this as $8—two $4 interzone 4 fares for the two legs of the trip. However, in practice you purchase a $5.50 interzone 7 ticket for this trip. This type of express service is rare in the MBTA commuter rail system, and is thus left unimplemented. +Per the fare tariff, there are no discounted transfers at all on commuter rail. However, in practice and since the commuter rail system is a proof-of-payment system, and since some trains express during rush hour, it is possible to transfer and continue a same-direction trip, for instance a trip from [Worcester to Auburndale at rush hour, with a transfer in Framingham to to express service](https://projects.indicatrix.org/fareto-examples/?load=bos-cr-same-dir-xfer&index=0). The router calculates the fare for this as $8—two $4 interzone 4 fares for the two legs of the trip. However, in practice you purchase a $5.50 interzone 7 ticket for this trip. This type of express service is rare in the MBTA commuter rail system, and the transfer discounts are thus left unimplemented. -- Same direction transfers at SL Way should be okay -- Harvard Busway is "behind gates" -- Document assumptions +There are no discounted transfers [to other modes](https://projects.indicatrix.org/fareto-examples/?load=bos-cr-xfer&index=0) with pay-as-you-go fares on commuter rail, although [the router will trade off a longer trip on commuter rail with disembarking early and changing to local transit when it is cheaper to do so](https://projects.indicatrix.org/fareto-examples/?load=bos-cr-xfer&index=4). Similarly, there are no discounted transfers [from other modes](https://projects.indicatrix.org/fareto-examples/?load=bos-xfer-cr&index=0). Like ferries, when the commuter rail is used in between two other modes, [the transfer allowance from the first mode is preserved and can be used for a discounted transfer on the second mode](https://projects.indicatrix.org/fareto-examples/?load=bos-bus-cr-orange&index=0). From 29affbb3f8472c6afae586a7a6b87b239f73d5c6 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 24 Nov 2020 19:03:41 -0500 Subject: [PATCH 45/46] fix(http-api): remove cors in local mode (see proxy-backend branch of mattwigway/analysis-ui) --- .../conveyal/analysis/components/HttpApi.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/components/HttpApi.java b/src/main/java/com/conveyal/analysis/components/HttpApi.java index 21f635e8a..77febf7e7 100644 --- a/src/main/java/com/conveyal/analysis/components/HttpApi.java +++ b/src/main/java/com/conveyal/analysis/components/HttpApi.java @@ -88,7 +88,11 @@ private spark.Service configureSparkService () { // Or now with non-static Spark we can run two HTTP servers on different ports. // Set CORS headers, to allow requests to this API server from any page. - res.header("Access-Control-Allow-Origin", "*"); + // but do not do this when running offline with no auth, as this may allow a malicious website to use a local + // browser to access the analysis server. + if (!config.offline()) { + res.header("Access-Control-Allow-Origin", "*"); + } // The default MIME type is JSON. This will be overridden by the few controllers that do not return JSON. res.type("application/json"); @@ -120,14 +124,17 @@ private spark.Service configureSparkService () { }); // Handle CORS preflight requests (which are OPTIONS requests). - sparkService.options("/*", (req, res) -> { - res.header("Access-Control-Allow-Methods", "GET,PUT,POST,DELETE,OPTIONS"); - res.header("Access-Control-Allow-Credentials", "true"); - res.header("Access-Control-Allow-Headers", "Accept,Authorization,Content-Type,Origin," + - "X-Requested-With,Content-Length,X-Conveyal-Access-Group" - ); - return "OK"; - }); + // except when running in offline mode (see above comment about auth and CORS) + if (!config.offline()) { + sparkService.options("/*", (req, res) -> { + res.header("Access-Control-Allow-Methods", "GET,PUT,POST,DELETE,OPTIONS"); + res.header("Access-Control-Allow-Credentials", "true"); + res.header("Access-Control-Allow-Headers", "Accept,Authorization,Content-Type,Origin," + + "X-Requested-With,Content-Length,X-Conveyal-Access-Group" + ); + return "OK"; + }); + } // Allow client to fetch information about the backend build version. sparkService.get( From 6c13a807b2e7084e0ac8d3d6e3fb08d7d92872b4 Mon Sep 17 00:00:00 2001 From: Matthew Wigginton Conway Date: Tue, 24 Nov 2020 22:16:51 -0500 Subject: [PATCH 46/46] fix(cors): proxy requests to grids through frontend --- .../java/com/conveyal/analysis/components/LocalComponents.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/analysis/components/LocalComponents.java b/src/main/java/com/conveyal/analysis/components/LocalComponents.java index 0e1aea67f..c8b9fd9d9 100644 --- a/src/main/java/com/conveyal/analysis/components/LocalComponents.java +++ b/src/main/java/com/conveyal/analysis/components/LocalComponents.java @@ -32,7 +32,7 @@ public LocalComponents () { taskScheduler = new TaskScheduler(config); fileStorage = new LocalFileStorage( config.localCacheDirectory(), - String.format("http://localhost:%s/files", config.serverPort()) + "/api/backend/files" // proxied by analysis-ui ); gtfsCache = new GTFSCache(fileStorage, config); osmCache = new OSMCache(fileStorage, config);