From 7060665fc46ce6004668727345e3847348f11a7d Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 20 Dec 2023 14:03:38 +0800 Subject: [PATCH 01/19] fix final and static field modifiers in broker Many constants (as indicated by capitalization) were not static. One field always set in the constructor was not final. The list of resultAssemblers WAS static. This may be a remnant of pre- component design. This was not problematic because we never construct more than one Broker, but it should be possible to do (no global state). --- .../analysis/components/broker/Broker.java | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/components/broker/Broker.java b/src/main/java/com/conveyal/analysis/components/broker/Broker.java index 2aea67dc2..d9f78a6e0 100644 --- a/src/main/java/com/conveyal/analysis/components/broker/Broker.java +++ b/src/main/java/com/conveyal/analysis/components/broker/Broker.java @@ -95,15 +95,14 @@ public interface Config { boolean testTaskRedelivery (); } - private Config config; + private final Config config; // Component Dependencies private final FileStorage fileStorage; private final EventBus eventBus; private final WorkerLauncher workerLauncher; - private final ListMultimap jobs = - MultimapBuilder.hashKeys().arrayListValues().build(); + private final ListMultimap jobs = MultimapBuilder.hashKeys().arrayListValues().build(); /** * The most tasks to deliver to a worker at a time. Workers may request less tasks than this, and the broker should @@ -114,24 +113,24 @@ public interface Config { * The value should eventually be tuned. The current value of 16 is just the value used by the previous sporadic * polling system (WorkerStatus.LEGACY_WORKER_MAX_TASKS) which may not be ideal but is known to work. */ - public final int MAX_TASKS_PER_WORKER = 16; + public static final int MAX_TASKS_PER_WORKER = 16; /** * Used when auto-starting spot instances. Set to a smaller value to increase the number of * workers requested automatically */ - public final int TARGET_TASKS_PER_WORKER_TRANSIT = 800; - public final int TARGET_TASKS_PER_WORKER_NONTRANSIT = 4_000; + public static final int TARGET_TASKS_PER_WORKER_TRANSIT = 800; + public static final int TARGET_TASKS_PER_WORKER_NONTRANSIT = 4_000; /** * We want to request spot instances to "boost" regional analyses after a few regional task * results are received for a given workerCategory. Do so after receiving results for an * arbitrary task toward the beginning of the job */ - public final int AUTO_START_SPOT_INSTANCES_AT_TASK = 42; + public static final int AUTO_START_SPOT_INSTANCES_AT_TASK = 42; /** The maximum number of spot instances allowable in an automatic request */ - public final int MAX_WORKERS_PER_CATEGORY = 250; + public static final int MAX_WORKERS_PER_CATEGORY = 250; /** * How long to give workers to start up (in ms) before assuming that they have started (and @@ -139,15 +138,11 @@ public interface Config { */ public static final long WORKER_STARTUP_TIME = 60 * 60 * 1000; - /** Keeps track of all the workers that have contacted this broker recently asking for work. */ private WorkerCatalog workerCatalog = new WorkerCatalog(); - /** - * These objects piece together results received from workers into one regional analysis result - * file per job. - */ - private static Map resultAssemblers = new HashMap<>(); + /** These objects piece together results received from workers into one regional analysis result file per job. */ + private Map resultAssemblers = new HashMap<>(); /** * keep track of which graphs we have launched workers on and how long ago we launched them, so From 4a5b72120fee108d941b459ff78e3f5e97be222c Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 23 Jan 2024 15:20:23 -0500 Subject: [PATCH 02/19] Append feedId values to alighting stops --- src/main/java/com/conveyal/r5/transit/TransitLayer.java | 4 ++++ src/main/java/com/conveyal/r5/transit/path/RouteSequence.java | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 871491ff2..b4c00af55 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -842,4 +842,8 @@ public String stopString(int stopIndex, boolean includeName) { if (includeName) stop += " (" + stopNames.get(stopIndex) + ")"; return stop; } + + public String feedFromStop(int stopIndex) { + return stopIdForIndex.get(stopIndex) == null ? "[new]" : stopIdForIndex.get(stopIndex).split(":")[0]; + } } diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 6ed2eb73c..d4bb9dc85 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -37,6 +37,8 @@ public String[] detailsWithGtfsIds(TransitLayer transitLayer){ routeIds.add(transitLayer.routeString(routes.get(i), false)); boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), false)); alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), false)); + alightStopIds.add(":"); + alightStopIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); From 990a542ae1a1e36fd7578535604ce99305128ae7 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 23 Jan 2024 15:22:08 -0500 Subject: [PATCH 03/19] Write feedIds as separate column --- src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java | 1 + src/main/java/com/conveyal/r5/transit/path/RouteSequence.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index c2366ac1a..eaf7d5084 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -53,6 +53,7 @@ public class PathResult { "routes", "boardStops", "alightStops", + "feedIds", "rideTimes", "accessTime", "egressTime", diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index d4bb9dc85..2f20a6505 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -32,13 +32,13 @@ public String[] detailsWithGtfsIds(TransitLayer transitLayer){ StringJoiner routeIds = new StringJoiner("|"); StringJoiner boardStopIds = new StringJoiner("|"); StringJoiner alightStopIds = new StringJoiner("|"); + StringJoiner feedIds = new StringJoiner("|"); StringJoiner rideTimes = new StringJoiner("|"); for (int i = 0; i < routes.size(); i++) { routeIds.add(transitLayer.routeString(routes.get(i), false)); boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), false)); alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), false)); - alightStopIds.add(":"); - alightStopIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); + feedIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); From f71a72ccbe2685b8ce4c0736f5379a97e5231045 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 23 Jan 2024 16:35:38 -0500 Subject: [PATCH 04/19] Append feedId values to alighting stop list --- .../java/com/conveyal/r5/transit/path/RouteSequence.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 2f20a6505..6efe23dcb 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -38,9 +38,12 @@ public String[] detailsWithGtfsIds(TransitLayer transitLayer){ routeIds.add(transitLayer.routeString(routes.get(i), false)); boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), false)); alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), false)); - feedIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } + alightStopIds.add("|"); + for (int i = 0; i < routes.size(); i++) { + alightStopIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); + } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); String egressTime = stopSequence.egress == null ? null : String.format("%.1f", stopSequence.egress.time / 60f); return new String[]{ From c9be778fb71dd1ec0da1090c94237e6327674a84 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 23 Jan 2024 16:48:54 -0500 Subject: [PATCH 05/19] Revert "Append feedId values to alighting stop list" This reverts commit f71a72ccbe2685b8ce4c0736f5379a97e5231045. --- .../java/com/conveyal/r5/transit/path/RouteSequence.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 6efe23dcb..2f20a6505 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -38,12 +38,9 @@ public String[] detailsWithGtfsIds(TransitLayer transitLayer){ routeIds.add(transitLayer.routeString(routes.get(i), false)); boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), false)); alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), false)); + feedIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } - alightStopIds.add("|"); - for (int i = 0; i < routes.size(); i++) { - alightStopIds.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); - } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); String egressTime = stopSequence.egress == null ? null : String.format("%.1f", stopSequence.egress.time / 60f); return new String[]{ From 32eb1dc49cb27868445004df68557e914f457700 Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Fri, 26 Jan 2024 11:51:44 +0100 Subject: [PATCH 06/19] Exclude modifications for regional analysis lookup (#926) This change adds a new helper method to pass a `fields` object to `MongoMap#findByIdIfPermitted` and utilizes that to exclude modifications while looking up a regional analysis to generate a `scenarioJsonUrl`. This will prevent errors from occurring if custom modifications are used in a worker version that do not exist in the server. --- .../RegionalAnalysisController.java | 7 +++++-- .../analysis/persistence/MongoMap.java | 19 +++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/controllers/RegionalAnalysisController.java b/src/main/java/com/conveyal/analysis/controllers/RegionalAnalysisController.java index 32336fd7c..b08cb8a9b 100644 --- a/src/main/java/com/conveyal/analysis/controllers/RegionalAnalysisController.java +++ b/src/main/java/com/conveyal/analysis/controllers/RegionalAnalysisController.java @@ -527,8 +527,11 @@ private RegionalAnalysis updateRegionalAnalysis (Request request, Response respo * the given regional analysis. */ private JsonNode getScenarioJsonUrl (Request request, Response response) { - RegionalAnalysis regionalAnalysis = Persistence.regionalAnalyses - .findByIdIfPermitted(request.params("_id"), UserPermissions.from(request)); + RegionalAnalysis regionalAnalysis = Persistence.regionalAnalyses.findByIdIfPermitted( + request.params("_id"), + DBProjection.exclude("request.scenario.modifications"), + UserPermissions.from(request) + ); // In the persisted objects, regionalAnalysis.scenarioId seems to be null. Get it from the embedded request. final String networkId = regionalAnalysis.bundleId; final String scenarioId = regionalAnalysis.request.scenarioId; diff --git a/src/main/java/com/conveyal/analysis/persistence/MongoMap.java b/src/main/java/com/conveyal/analysis/persistence/MongoMap.java index 6001a82d2..4f34353d1 100644 --- a/src/main/java/com/conveyal/analysis/persistence/MongoMap.java +++ b/src/main/java/com/conveyal/analysis/persistence/MongoMap.java @@ -43,12 +43,11 @@ public int size() { return (int) wrappedCollection.getCount(); } - public V findByIdFromRequestIfPermitted(Request request) { - return findByIdIfPermitted(request.params("_id"), UserPermissions.from(request)); - } - - public V findByIdIfPermitted(String id, UserPermissions userPermissions) { - V result = wrappedCollection.findOneById(id); + /** + * `fields` is nullable. + */ + public V findByIdIfPermitted(String id, DBObject fields, UserPermissions userPermissions) { + V result = wrappedCollection.findOneById(id, fields); if (result == null) { throw AnalysisServerException.notFound(String.format( @@ -61,6 +60,14 @@ public V findByIdIfPermitted(String id, UserPermissions userPermissions) { } } + public V findByIdIfPermitted(String id, UserPermissions userPermissions) { + return findByIdIfPermitted(id, null, userPermissions); + } + + public V findByIdFromRequestIfPermitted(Request request) { + return findByIdIfPermitted(request.params("_id"), UserPermissions.from(request)); + } + public V get(String key) { return wrappedCollection.findOneById(key); } From 06d9beed01daae96a45780ba73a9a3af70954704 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Mon, 6 Nov 2023 17:43:07 +0800 Subject: [PATCH 07/19] include route, stop names in regional path results --- .../java/com/conveyal/r5/analyst/cluster/PathResult.java | 2 +- .../java/com/conveyal/r5/transit/path/RouteSequence.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index c2366ac1a..99601291e 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -108,7 +108,7 @@ public ArrayList[] summarizeIterations(Stat stat) { int nIterations = iterations.size(); checkState(nIterations > 0, "A path was stored without any iterations"); String waits = null, transfer = null, totalTime = null; - String[] path = routeSequence.detailsWithGtfsIds(transitLayer); + String[] path = routeSequence.detailsWithGtfsIds(transitLayer, true); double targetValue; IntStream totalWaits = iterations.stream().mapToInt(i -> i.waitTimes.sum()); if (stat == Stat.MINIMUM) { diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 6ed2eb73c..a5bdd71ae 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -28,15 +28,15 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer) } /** Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. */ - public String[] detailsWithGtfsIds(TransitLayer transitLayer){ + public String[] detailsWithGtfsIds(TransitLayer transitLayer, boolean includeNames){ StringJoiner routeIds = new StringJoiner("|"); StringJoiner boardStopIds = new StringJoiner("|"); StringJoiner alightStopIds = new StringJoiner("|"); StringJoiner rideTimes = new StringJoiner("|"); for (int i = 0; i < routes.size(); i++) { - routeIds.add(transitLayer.routeString(routes.get(i), false)); - boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), false)); - alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), false)); + routeIds.add(transitLayer.routeString(routes.get(i), includeNames)); + boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), includeNames)); + alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), includeNames)); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); From d5f5becc8d2f9d0b14e8cafdc4468775f75cf9ba Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Sat, 3 Feb 2024 01:06:20 +0800 Subject: [PATCH 08/19] enable/disable ids and names csv with string flags --- .../analysis/models/AnalysisRequest.java | 7 +++ .../analyst/cluster/AnalysisWorkerTask.java | 7 +++ .../r5/analyst/cluster/PathResult.java | 19 ++++++- .../com/conveyal/r5/transit/TransitLayer.java | 57 ++++++++++++++----- .../r5/transit/path/RouteSequence.java | 17 +++--- 5 files changed, 86 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java index 80d5e507a..f2249c7fe 100644 --- a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java @@ -21,6 +21,7 @@ import java.util.Collection; import java.util.EnumSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; /** @@ -175,6 +176,11 @@ public class AnalysisRequest { */ public int dualAccessibilityThreshold = 0; + /** + * Freeform (untyped) flags for enabling experimental, undocumented, or arcane behavior in backend or workers. + * This should be used to replace all previous special behavior flags that were embedded inside analysis names etc. + */ + public Set flags; /** * Create the R5 `Scenario` from this request. @@ -281,6 +287,7 @@ public void populateTask (AnalysisWorkerTask task, UserPermissions userPermissio task.includeTemporalDensity = includeTemporalDensity; task.dualAccessibilityThreshold = dualAccessibilityThreshold; + task.flags = flags; } private EnumSet getEnumSetFromString (String s) { diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java index 30266f171..d64996a15 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java @@ -15,6 +15,7 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import java.util.Arrays; +import java.util.Set; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @@ -177,6 +178,12 @@ public abstract class AnalysisWorkerTask extends ProfileRequest { */ public ChaosParameters injectFault; + /** + * Freeform (untyped) flags for enabling experimental, undocumented, or arcane worker behavior. + * This should be used to replace all previous special behavior flags that were embedded inside analysis names etc. + */ + public Set flags; + /** * Is this a single point or regional request? Needed to encode types in JSON serialization. Can that type field be * added automatically with a serializer annotation instead of by defining a getter method and two dummy methods? diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index 99601291e..36e71699f 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -19,6 +19,9 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_AND_NAME; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_ONLY; import static com.google.common.base.Preconditions.checkState; /** @@ -47,8 +50,11 @@ public class PathResult { * With additional changes, patterns could be collapsed further to route combinations or modes. */ public final Multimap[] iterationsForPathTemplates; + private final TransitLayer transitLayer; + private TransitLayer.EntityRepresentation nameOrId; + public static final String[] DATA_COLUMNS = new String[]{ "routes", "boardStops", @@ -76,6 +82,17 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) { } iterationsForPathTemplates = new Multimap[nDestinations]; this.transitLayer = transitLayer; + if (task.flags == null) { + nameOrId = ID_ONLY; + } else { + boolean includeEntityNames = task.flags.contains("csv_names"); + boolean includeEntityIds = !task.flags.contains("csv_no_ids"); + if (includeEntityIds && includeEntityNames) { + this.nameOrId = ID_AND_NAME; + } else if (includeEntityNames) { + this.nameOrId = NAME_ONLY; + } + } } /** @@ -108,7 +125,7 @@ public ArrayList[] summarizeIterations(Stat stat) { int nIterations = iterations.size(); checkState(nIterations > 0, "A path was stored without any iterations"); String waits = null, transfer = null, totalTime = null; - String[] path = routeSequence.detailsWithGtfsIds(transitLayer, true); + String[] path = routeSequence.detailsWithGtfsIds(transitLayer, nameOrId); double targetValue; IntStream totalWaits = iterations.stream().mapToInt(i -> i.waitTimes.sum()); if (stat == Stat.MINIMUM) { diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index 871491ff2..a025ba4d0 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -54,6 +54,9 @@ import java.util.stream.IntStream; import java.util.stream.StreamSupport; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_ONLY; + /** * A key simplifying factor is that we don't handle overnight trips. This is fine for analysis at usual times of day. @@ -815,31 +818,59 @@ public TIntSet findStopsInGeometry (Geometry geometry) { return stops; } + public enum EntityRepresentation { + ID_ONLY, NAME_ONLY, ID_AND_NAME + } + /** * For the given pattern index, returns the GTFS routeId. If includeName is true, the returned string will * also include a route_short_name or route_long_name (if they are not null). */ - public String routeString(int routeIndex, boolean includeName) { + public String routeString (int routeIndex, EntityRepresentation nameOrId) { RouteInfo routeInfo = routes.get(routeIndex); - String route = routeInfo.route_id; - if (includeName) { - if (routeInfo.route_short_name != null) { - route += " (" + routeInfo.route_short_name + ")"; - } else if (routeInfo.route_long_name != null){ - route += " (" + routeInfo.route_long_name + ")"; + String name = routeInfo.route_short_name; + String id = routeInfo.route_id; + // If we might actually use the name, check some fallbacks. + if (nameOrId != ID_ONLY) { + if (name == null) { + name = routeInfo.route_long_name; + } + if (name == null) { + name = routeInfo.route_id; } } - return route; + return switch (nameOrId) { + case NAME_ONLY -> name; + case ID_AND_NAME -> id + " (" + name + ")"; + default -> id; + }; } /** * For the given stop index, returns the GTFS stopId (stripped of R5's feedId prefix) and, if includeName is true, * stopName. */ - public String stopString(int stopIndex, boolean includeName) { - // TODO use a compact feed index, instead of splitting to remove feedIds - String stop = stopIdForIndex.get(stopIndex) == null ? "[new]" : stopIdForIndex.get(stopIndex).split(":")[1]; - if (includeName) stop += " (" + stopNames.get(stopIndex) + ")"; - return stop; + public String stopString(int stopIndex, EntityRepresentation nameOrId) { + String stopId = stopIdForIndex.get(stopIndex); + String stopName = stopNames.get(stopIndex); + // I'd trust the JVM JIT to optimize out these assignments on different code paths, but not the split call. + if (nameOrId != NAME_ONLY) { + if (stopId == null) { + stopId = "[new]"; + } else { + // TODO use a compact feed ID instead of splitting to remove feedIds (or put feedId into another CSV field) + stopId = stopId.split(":")[1]; + } + } + if (nameOrId != ID_ONLY) { + if (stopName == null) { + stopName = "[new]"; + } + } + return switch (nameOrId) { + case NAME_ONLY -> stopName; + case ID_AND_NAME -> stopId + " (" + stopName + ")"; + default -> stopId; + }; } } diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index a5bdd71ae..3bb4f512c 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -1,6 +1,7 @@ package com.conveyal.r5.transit.path; import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.TransitLayer.EntityRepresentation; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; @@ -9,6 +10,8 @@ import java.util.Objects; import java.util.StringJoiner; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_AND_NAME; + /** A door-to-door path that includes the routes ridden between stops */ public class RouteSequence { @@ -28,15 +31,15 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer) } /** Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. */ - public String[] detailsWithGtfsIds(TransitLayer transitLayer, boolean includeNames){ + public String[] detailsWithGtfsIds (TransitLayer transitLayer, EntityRepresentation nameOrId){ StringJoiner routeIds = new StringJoiner("|"); StringJoiner boardStopIds = new StringJoiner("|"); StringJoiner alightStopIds = new StringJoiner("|"); StringJoiner rideTimes = new StringJoiner("|"); for (int i = 0; i < routes.size(); i++) { - routeIds.add(transitLayer.routeString(routes.get(i), includeNames)); - boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), includeNames)); - alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), includeNames)); + routeIds.add(transitLayer.routeString(routes.get(i), nameOrId)); + boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), nameOrId)); + alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), nameOrId)); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); @@ -55,9 +58,9 @@ public String[] detailsWithGtfsIds(TransitLayer transitLayer, boolean includeNam public Collection transitLegs(TransitLayer transitLayer) { Collection transitLegs = new ArrayList<>(); for (int i = 0; i < routes.size(); i++) { - String routeString = transitLayer.routeString(routes.get(i), true); - String boardStop = transitLayer.stopString(stopSequence.boardStops.get(i), true); - String alightStop = transitLayer.stopString(stopSequence.alightStops.get(i), true); + String routeString = transitLayer.routeString(routes.get(i), ID_AND_NAME); + String boardStop = transitLayer.stopString(stopSequence.boardStops.get(i), ID_AND_NAME); + String alightStop = transitLayer.stopString(stopSequence.alightStops.get(i), ID_AND_NAME); transitLegs.add(new TransitLeg(routeString, stopSequence.rideTimesSeconds.get(i), boardStop, alightStop)); } return transitLegs; From 2376c4edd56504f9d7eec4ceb1c211b96322a2f8 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 14 Feb 2024 00:06:29 +0800 Subject: [PATCH 09/19] keepRoutingOnFoot only in transit access searches --- .../conveyal/r5/analyst/TravelTimeComputer.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java b/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java index 6af5dbcf7..0237a636e 100644 --- a/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java +++ b/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java @@ -167,14 +167,16 @@ public OneOriginResult computeTravelTimes() { // The generalized cost calculations currently increment time and weight by the same amount. sr.quantityToMinimize = StreetRouter.State.RoutingVariable.DURATION_SECONDS; sr.route(); - // Change to walking in order to reach transit stops in pedestrian-only areas like train stations. - // This implies you are dropped off or have a very easy parking spot for your vehicle. - // This kind of multi-stage search should also be used when building egress distance cost tables. - if (accessMode != StreetMode.WALK) { - sr.keepRoutingOnFoot(); - } if (request.hasTransit()) { + // Change to walking in order to reach transit stops in pedestrian-only areas like train stations. + // This implies you are dropped off or have a very easy parking spot for your vehicle. + // This kind of multi-stage search should also be used when building egress distance cost tables. + // Note that this can take up to twice as long as the initial car/bike search. Do it only when the + // walking is necessary, and when the radius of the car/bike search is limited, as for transit access. + if (accessMode != StreetMode.WALK) { + sr.keepRoutingOnFoot(); + } // Find access times to transit stops, keeping the minimum across all access street modes. // Note that getReachedStops() returns the routing variable units, not necessarily seconds. // TODO add logic here if linkedStops are specified in pickupDelay? From e11fc19e5af413da331bce40c6ced687be5ba8c2 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Sat, 17 Feb 2024 00:45:23 +0800 Subject: [PATCH 10/19] nested CsvResultOptions in requests reverse order of name and ID in output and enum. string flags are maintained in request but no longer used. deleted code serves as example of how to use flags. --- .../analysis/models/AnalysisRequest.java | 6 ++++++ .../analysis/models/CsvResultOptions.java | 18 +++++++++++++++++ .../analyst/cluster/AnalysisWorkerTask.java | 4 ++++ .../r5/analyst/cluster/PathResult.java | 20 ++++--------------- .../com/conveyal/r5/transit/TransitLayer.java | 6 +++--- .../r5/transit/path/RouteSequence.java | 17 ++++++++-------- 6 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 src/main/java/com/conveyal/analysis/models/CsvResultOptions.java diff --git a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java index f2249c7fe..3bff888e8 100644 --- a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java @@ -12,8 +12,10 @@ import com.conveyal.r5.analyst.scenario.Scenario; import com.conveyal.r5.api.util.LegMode; import com.conveyal.r5.api.util.TransitModes; +import com.conveyal.r5.transit.TransitLayer; import com.mongodb.QueryBuilder; import org.apache.commons.codec.digest.DigestUtils; +import org.checkerframework.checker.units.qual.C; import java.time.LocalDate; import java.util.ArrayList; @@ -182,6 +184,9 @@ public class AnalysisRequest { */ public Set flags; + /** Control the details of CSV regional analysis output, including whether to output IDs, names, or both. */ + public CsvResultOptions csvResultOptions = new CsvResultOptions(); + /** * Create the R5 `Scenario` from this request. */ @@ -288,6 +293,7 @@ public void populateTask (AnalysisWorkerTask task, UserPermissions userPermissio task.includeTemporalDensity = includeTemporalDensity; task.dualAccessibilityThreshold = dualAccessibilityThreshold; task.flags = flags; + task.csvResultOptions = csvResultOptions; } private EnumSet getEnumSetFromString (String s) { diff --git a/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java b/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java new file mode 100644 index 000000000..c21b52aa3 --- /dev/null +++ b/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java @@ -0,0 +1,18 @@ +package com.conveyal.analysis.models; + +import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.TransitLayer.EntityRepresentation; + +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY; + +/** + * API model type included in analysis requests to control details of CSV regional analysis output. + * This type is shared between AnalysisRequest (Frontend -> Broker) and AnalysisWorkerTask (Broker -> Workers). + * There is precedent for nested compound types shared across those top level request types (see DecayFunction). + */ +public class CsvResultOptions { + public EntityRepresentation routeRepresentation = ID_ONLY; + public EntityRepresentation stopRepresentation = ID_ONLY; + // Only feed ID representation is allowed to be null (no feed IDs at all, the default). + public EntityRepresentation feedRepresentation = null; +} diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java index d64996a15..2698905fa 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java @@ -1,5 +1,6 @@ package com.conveyal.r5.analyst.cluster; +import com.conveyal.analysis.models.CsvResultOptions; import com.conveyal.r5.analyst.FreeFormPointSet; import com.conveyal.r5.analyst.Grid; import com.conveyal.r5.analyst.GridTransformWrapper; @@ -184,6 +185,9 @@ public abstract class AnalysisWorkerTask extends ProfileRequest { */ public Set flags; + /** Control the details of CSV regional analysis output, including whether to output IDs, names, or both. */ + public CsvResultOptions csvResultOptions; + /** * Is this a single point or regional request? Needed to encode types in JSON serialization. Can that type field be * added automatically with a serializer annotation instead of by defining a getter method and two dummy methods? diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index 36e71699f..dd42972b1 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -1,5 +1,6 @@ package com.conveyal.r5.analyst.cluster; +import com.conveyal.analysis.models.CsvResultOptions; import com.conveyal.r5.analyst.StreetTimesAndModes; import com.conveyal.r5.transit.TransitLayer; import com.conveyal.r5.transit.path.Path; @@ -19,9 +20,6 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY; -import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_AND_NAME; -import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_ONLY; import static com.google.common.base.Preconditions.checkState; /** @@ -53,7 +51,7 @@ public class PathResult { private final TransitLayer transitLayer; - private TransitLayer.EntityRepresentation nameOrId; + private final CsvResultOptions csvOptions; public static final String[] DATA_COLUMNS = new String[]{ "routes", @@ -82,17 +80,7 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) { } iterationsForPathTemplates = new Multimap[nDestinations]; this.transitLayer = transitLayer; - if (task.flags == null) { - nameOrId = ID_ONLY; - } else { - boolean includeEntityNames = task.flags.contains("csv_names"); - boolean includeEntityIds = !task.flags.contains("csv_no_ids"); - if (includeEntityIds && includeEntityNames) { - this.nameOrId = ID_AND_NAME; - } else if (includeEntityNames) { - this.nameOrId = NAME_ONLY; - } - } + this.csvOptions = task.csvResultOptions; } /** @@ -125,7 +113,7 @@ public ArrayList[] summarizeIterations(Stat stat) { int nIterations = iterations.size(); checkState(nIterations > 0, "A path was stored without any iterations"); String waits = null, transfer = null, totalTime = null; - String[] path = routeSequence.detailsWithGtfsIds(transitLayer, nameOrId); + String[] path = routeSequence.detailsWithGtfsIds(transitLayer, csvOptions); double targetValue; IntStream totalWaits = iterations.stream().mapToInt(i -> i.waitTimes.sum()); if (stat == Stat.MINIMUM) { diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index a025ba4d0..7ec85e6fa 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -819,7 +819,7 @@ public TIntSet findStopsInGeometry (Geometry geometry) { } public enum EntityRepresentation { - ID_ONLY, NAME_ONLY, ID_AND_NAME + ID_ONLY, NAME_ONLY, NAME_AND_ID } /** @@ -841,7 +841,7 @@ public String routeString (int routeIndex, EntityRepresentation nameOrId) { } return switch (nameOrId) { case NAME_ONLY -> name; - case ID_AND_NAME -> id + " (" + name + ")"; + case NAME_AND_ID -> name + " (" + id + ")"; default -> id; }; } @@ -869,7 +869,7 @@ public String stopString(int stopIndex, EntityRepresentation nameOrId) { } return switch (nameOrId) { case NAME_ONLY -> stopName; - case ID_AND_NAME -> stopId + " (" + stopName + ")"; + case NAME_AND_ID -> stopName + " (" + stopId + ")"; default -> stopId; }; } diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 3bb4f512c..1e60478f3 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -1,5 +1,6 @@ package com.conveyal.r5.transit.path; +import com.conveyal.analysis.models.CsvResultOptions; import com.conveyal.r5.transit.TransitLayer; import com.conveyal.r5.transit.TransitLayer.EntityRepresentation; import gnu.trove.list.TIntList; @@ -10,7 +11,7 @@ import java.util.Objects; import java.util.StringJoiner; -import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_AND_NAME; +import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_AND_ID; /** A door-to-door path that includes the routes ridden between stops */ public class RouteSequence { @@ -31,15 +32,15 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer) } /** Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. */ - public String[] detailsWithGtfsIds (TransitLayer transitLayer, EntityRepresentation nameOrId){ + public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions csvOptions){ StringJoiner routeIds = new StringJoiner("|"); StringJoiner boardStopIds = new StringJoiner("|"); StringJoiner alightStopIds = new StringJoiner("|"); StringJoiner rideTimes = new StringJoiner("|"); for (int i = 0; i < routes.size(); i++) { - routeIds.add(transitLayer.routeString(routes.get(i), nameOrId)); - boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), nameOrId)); - alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), nameOrId)); + routeIds.add(transitLayer.routeString(routes.get(i), csvOptions.routeRepresentation)); + boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), csvOptions.stopRepresentation)); + alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), csvOptions.stopRepresentation)); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); @@ -58,9 +59,9 @@ public String[] detailsWithGtfsIds (TransitLayer transitLayer, EntityRepresentat public Collection transitLegs(TransitLayer transitLayer) { Collection transitLegs = new ArrayList<>(); for (int i = 0; i < routes.size(); i++) { - String routeString = transitLayer.routeString(routes.get(i), ID_AND_NAME); - String boardStop = transitLayer.stopString(stopSequence.boardStops.get(i), ID_AND_NAME); - String alightStop = transitLayer.stopString(stopSequence.alightStops.get(i), ID_AND_NAME); + String routeString = transitLayer.routeString(routes.get(i), NAME_AND_ID); + String boardStop = transitLayer.stopString(stopSequence.boardStops.get(i), NAME_AND_ID); + String alightStop = transitLayer.stopString(stopSequence.alightStops.get(i), NAME_AND_ID); transitLegs.add(new TransitLeg(routeString, stopSequence.rideTimesSeconds.get(i), boardStop, alightStop)); } return transitLegs; From 119ffc149a5c7ebe0b2b0e033f9560652f10b2c9 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Thu, 29 Feb 2024 11:46:55 +0900 Subject: [PATCH 11/19] Update src/main/java/com/conveyal/analysis/models/AnalysisRequest.java Co-authored-by: Anson Stewart --- src/main/java/com/conveyal/analysis/models/AnalysisRequest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java index 3bff888e8..95944f262 100644 --- a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java @@ -12,7 +12,6 @@ import com.conveyal.r5.analyst.scenario.Scenario; import com.conveyal.r5.api.util.LegMode; import com.conveyal.r5.api.util.TransitModes; -import com.conveyal.r5.transit.TransitLayer; import com.mongodb.QueryBuilder; import org.apache.commons.codec.digest.DigestUtils; import org.checkerframework.checker.units.qual.C; From c92bf7ba245d25c83e35055e045c4b0f81a47b39 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Thu, 29 Feb 2024 11:48:40 +0900 Subject: [PATCH 12/19] Update src/main/java/com/conveyal/analysis/models/CsvResultOptions.java Co-authored-by: Anson Stewart --- src/main/java/com/conveyal/analysis/models/CsvResultOptions.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java b/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java index c21b52aa3..e925e5ff3 100644 --- a/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java +++ b/src/main/java/com/conveyal/analysis/models/CsvResultOptions.java @@ -1,6 +1,5 @@ package com.conveyal.analysis.models; -import com.conveyal.r5.transit.TransitLayer; import com.conveyal.r5.transit.TransitLayer.EntityRepresentation; import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY; From eca7051f1e59db545bff9249ad9d2aafd6c12ccc Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Thu, 29 Feb 2024 11:48:46 +0900 Subject: [PATCH 13/19] Update src/main/java/com/conveyal/analysis/models/AnalysisRequest.java Co-authored-by: Anson Stewart --- src/main/java/com/conveyal/analysis/models/AnalysisRequest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java index 95944f262..cda79c9b8 100644 --- a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java @@ -14,7 +14,6 @@ import com.conveyal.r5.api.util.TransitModes; import com.mongodb.QueryBuilder; import org.apache.commons.codec.digest.DigestUtils; -import org.checkerframework.checker.units.qual.C; import java.time.LocalDate; import java.util.ArrayList; From b90a9f7064d1161a3688ff25b3adb808499984c7 Mon Sep 17 00:00:00 2001 From: ansons Date: Wed, 28 Feb 2024 22:04:47 -0500 Subject: [PATCH 14/19] Add "group" column in CSV path results For future use (see #924, for example) --- .../java/com/conveyal/r5/analyst/cluster/PathResult.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java index 2fe797dcc..43e617920 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -64,7 +64,8 @@ public class PathResult { "transferTime", "waitTimes", "totalTime", - "nIterations" + "nIterations", + "group" }; public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) { @@ -141,7 +142,10 @@ public ArrayList[] summarizeIterations(Stat stat) { score = thisScore; } } - String[] row = ArrayUtils.addAll(path, transfer, waits, totalTime, String.valueOf(nIterations)); + String group = ""; // Reserved for future use + String[] row = ArrayUtils.addAll( + path, transfer, waits, totalTime, String.valueOf(nIterations), group + ); checkState(row.length == DATA_COLUMNS.length); summary[d].add(row); } From 9d3b3ffc50570055047e34a9d9ae6bce53caeb00 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 5 Mar 2024 16:20:17 -0500 Subject: [PATCH 15/19] feat(path details): revise javadoc --- src/main/java/com/conveyal/r5/transit/TransitLayer.java | 3 +++ .../java/com/conveyal/r5/transit/path/RouteSequence.java | 5 ++++- 2 files changed, 7 insertions(+), 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 8d77d890a..f978d5a5e 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -874,6 +874,9 @@ public String stopString(int stopIndex, EntityRepresentation nameOrId) { }; } + /** + * For a supplied stopIndex in the transit layer, return the feed id (which we prepend to the GTFS stop id). + */ public String feedFromStop(int stopIndex) { return stopIdForIndex.get(stopIndex) == null ? "[new]" : stopIdForIndex.get(stopIndex).split(":")[0]; } diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index dd269cbf0..05e923225 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -31,7 +31,10 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer) } } - /** Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. */ + /** + * Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. + * @param csvOptions indicates whether names or IDs should be returned for certain fields. + */ public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions csvOptions){ StringJoiner routeString = new StringJoiner("|"); StringJoiner boardStops = new StringJoiner("|"); From 63ee1e950cb14d5144ac074b29f297aea5f6eea0 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 5 Mar 2024 16:21:31 -0500 Subject: [PATCH 16/19] refactor(path details): consistent variable naming --- .../r5/transit/path/RouteSequence.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 05e923225..b40ac7c0f 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -36,27 +36,26 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer) * @param csvOptions indicates whether names or IDs should be returned for certain fields. */ public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions csvOptions){ - StringJoiner routeString = new StringJoiner("|"); - StringJoiner boardStops = new StringJoiner("|"); - StringJoiner alightStops = new StringJoiner("|"); - StringJoiner feedString = new StringJoiner("|"); - StringJoiner rideTimes = new StringJoiner("|"); + StringJoiner routeJoiner = new StringJoiner("|"); + StringJoiner boardStopJoiner = new StringJoiner("|"); + StringJoiner alightStopJoiner = new StringJoiner("|"); + StringJoiner feedJoiner = new StringJoiner("|"); + StringJoiner rideTimeJoiner = new StringJoiner("|"); for (int i = 0; i < routes.size(); i++) { - routeString.add(transitLayer.routeString(routes.get(i), csvOptions.routeRepresentation)); - boardStops.add(transitLayer.stopString(stopSequence.boardStops.get(i), csvOptions.stopRepresentation)); - alightStops.add(transitLayer.stopString(stopSequence.alightStops.get(i), csvOptions.stopRepresentation)); - if (csvOptions.feedRepresentation != null) { - feedString.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); + routeJoiner.add(transitLayer.routeString(routes.get(i), csvOptions.routeRepresentation)); + boardStopJoiner.add(transitLayer.stopString(stopSequence.boardStops.get(i), csvOptions.stopRepresentation)); + alightStopJoiner.add(transitLayer.stopString(stopSequence.alightStops.get(i), csvOptions.stopRepresentation)); + feedJoiner.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); } - rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); + rideTimeJoiner.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f); String egressTime = stopSequence.egress == null ? null : String.format("%.1f", stopSequence.egress.time / 60f); return new String[]{ - routeString.toString(), - boardStops.toString(), - alightStops.toString(), - rideTimes.toString(), + routeJoiner.toString(), + boardStopJoiner.toString(), + alightStopJoiner.toString(), + rideTimeJoiner.toString(), accessTime, egressTime }; From d739e256b800ed1ea945d331bc517819ab7c20fe Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 5 Mar 2024 16:21:57 -0500 Subject: [PATCH 17/19] fix(path details): write feed values --- src/main/java/com/conveyal/r5/transit/path/RouteSequence.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index b40ac7c0f..09818f051 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -56,6 +56,7 @@ public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions boardStopJoiner.toString(), alightStopJoiner.toString(), rideTimeJoiner.toString(), + feedJoiner.toString(), accessTime, egressTime }; From 8999be6c6020b3de690ed8da6b55e03f28f30237 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 5 Mar 2024 16:29:22 -0500 Subject: [PATCH 18/19] feat(path details): revise javadoc --- .../java/com/conveyal/r5/transit/path/RouteSequence.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 09818f051..75bc3cb5d 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -34,6 +34,12 @@ public RouteSequence(PatternSequence patternSequence, TransitLayer transitLayer) /** * Returns details summarizing this route sequence, using GTFS ids stored in the supplied transitLayer. * @param csvOptions indicates whether names or IDs should be returned for certain fields. + * @return array of pipe-concatenated strings, with the route, board stop, alight stop, ride time, and feed for + * each transit leg, as well as the access and egress time. + * + * If csvOptions.feedRepresentation is not null, the feed values will be R5-generated UUID for boarding stop of + * each leg. We are grabbing the feed ID from the stop rather than the route (which might seem like a better + * representative of the leg) because stops happen to have a readily available feed ID. */ public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions csvOptions){ StringJoiner routeJoiner = new StringJoiner("|"); From 7b6e513b78b26b209ea28f76a5f16b8534d79bb8 Mon Sep 17 00:00:00 2001 From: ansons Date: Tue, 5 Mar 2024 16:37:06 -0500 Subject: [PATCH 19/19] fix(path details): add back conditional mistakenly deleted --- src/main/java/com/conveyal/r5/transit/path/RouteSequence.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java index 75bc3cb5d..62e6cfd34 100644 --- a/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java +++ b/src/main/java/com/conveyal/r5/transit/path/RouteSequence.java @@ -51,6 +51,7 @@ public String[] detailsWithGtfsIds (TransitLayer transitLayer, CsvResultOptions routeJoiner.add(transitLayer.routeString(routes.get(i), csvOptions.routeRepresentation)); boardStopJoiner.add(transitLayer.stopString(stopSequence.boardStops.get(i), csvOptions.stopRepresentation)); alightStopJoiner.add(transitLayer.stopString(stopSequence.alightStops.get(i), csvOptions.stopRepresentation)); + if (csvOptions.feedRepresentation != null) { feedJoiner.add(transitLayer.feedFromStop(stopSequence.boardStops.get(i))); } rideTimeJoiner.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f));