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 1ad1523cb..bc60772bd 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java @@ -62,7 +62,13 @@ public class PathResult { * as long as every path is being retained. When filtering down to a subset of paths, such as only those passing * through a selected link, we need this additional array to retain the information. */ - private final int[] nUnfilteredIterationsReachingDestination; + private int[] nUnfilteredIterationsReachingDestination; + + /** + * In case the scenario includes multiple select-link modifications, we store separate groups of patterns + * for each selected link, at each destination. + */ + private List>[] iterationsForEachSelectedLink; private final TransitLayer transitLayer; @@ -91,9 +97,12 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) { throw new UnsupportedOperationException("Number of detailed path destinations exceeds limit of " + MAX_PATH_DESTINATIONS); } } - // FIXME should we be allocating these large arrays when not recording paths? iterationsForPathTemplates = new Multimap[nDestinations]; - nUnfilteredIterationsReachingDestination = new int[nDestinations]; + // Only allocate these large arrays when select-link modifications are present. + if (transitLayer.parentNetwork.selectedLinks != null) { + nUnfilteredIterationsReachingDestination = new int[nDestinations]; + iterationsForEachSelectedLink = new ArrayList[nDestinations]; + } this.transitLayer = transitLayer; } @@ -102,21 +111,36 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) { * pattern-based keys */ public void setTarget(int targetIndex, Multimap patterns) { - // The size of a multimap is the number of mappings (number of values), not number of unique keys. - // This size method appears to be O(1), see: com.google.common.collect.AbstractMapBasedMultimap.size - nUnfilteredIterationsReachingDestination[targetIndex] = patterns.size(); - // When selected link analysis is enabled, filter down the PatternSequence-Iteration Multimap to retain only // those keys passing through the selected links. // Maybe selectedLink instance should be on TransitLayer not TransportNetwork. - if (transitLayer.parentNetwork.selectedLink != null) { - patterns = transitLayer.parentNetwork.selectedLink.filterPatterns(patterns); + if (transitLayer.parentNetwork.selectedLinks != null) { + // The size of a multimap is the number of mappings (number of values), not number of unique keys. + // This size method appears to be O(1), see: com.google.common.collect.AbstractMapBasedMultimap.size + nUnfilteredIterationsReachingDestination[targetIndex] = patterns.size(); + // Unfortunate Java "effectively final" silliness, but older iteration style gets ugly here too. + final var finalPatterns = patterns; + iterationsForEachSelectedLink[targetIndex] = transitLayer.parentNetwork.selectedLinks.stream() + .map(selectedLink -> selectedLink.filterPatterns(finalPatterns)) + .map(this::collapsePatternsToRoutes) + .collect(Collectors.toList()); + // Combining all the iterations from many select-links is tricky or nonsensical. They represent different + // locations with different proportions of iterations passing through them, possibly grouped differently + // into routes. It's probably best to list the non-summed iterations separately for each select-link. + } else { + // Standard case: no SelectedLink filtered down the patterns-iterations map. These maps will be left + // null if the filtering did happen, to clearly distinguish between the two cases. + iterationsForPathTemplates[targetIndex] = collapsePatternsToRoutes(patterns); } + } - // The rest of this runs independent of whether a SelectedLink filtered down the patterns-iterations map. - Multimap routes = HashMultimap.create(); + // Cannot be static as it uses the transitlayer field to look up routes. + private Multimap collapsePatternsToRoutes ( + Multimap patterns + ) { + final Multimap routes = HashMultimap.create(); patterns.forEach(((patternSeq, iteration) -> routes.put(new RouteSequence(patternSeq, transitLayer), iteration))); - iterationsForPathTemplates[targetIndex] = routes; + return routes; } /** @@ -131,18 +155,22 @@ public void setTarget(int targetIndex, Multimap patt public ArrayList[] summarizeIterations(Stat stat) { ArrayList[] summary = new ArrayList[nDestinations]; for (int d = 0; d < nDestinations; d++) { + // Hold all the summary CSV rows for this particular destination summary[d] = new ArrayList<>(); - Multimap iterationMap = iterationsForPathTemplates[d]; - if (iterationMap != null) { - // SelectedLink case: collapse all RouteSequences and Iterations for this OD pair into one to simplify. - // iterationMap is empty (not null) for destinations that were reached without using the selected link. - // This could also be done by merging all Iterations under a single RouteSequence with all route IDs. - if (transitLayer.parentNetwork.selectedLink != null) { + // SelectedLink case: collapse all RouteSequences and Iterations for this OD pair into one to simplify. + // iterationMap is empty (not null) for destinations that were reached without using the selected link. + // This could also be done by merging all Iterations under a single RouteSequence with all route IDs. + // Looks like array may be non-null but with null values if transit search fails (no origin linkage). + if (iterationsForEachSelectedLink != null && iterationsForEachSelectedLink[d] != null) { + // TODO transpose to list of arrays instead of array of lists, factor selectedLink iteration outside this method call + int n = 0; // TODO handle this list-zip better (Streams.zip()) + for (var patternsForSelectedLink : iterationsForEachSelectedLink[d]) { + String selectedLinkLabel = transitLayer.parentNetwork.selectedLinks.get(n++).label; int nIterations = 0; TIntSet allRouteIds = new TIntHashSet(); double summedTotalTime = 0; - for (RouteSequence routeSequence: iterationMap.keySet()) { - Collection iterations = iterationMap.get(routeSequence); + for (RouteSequence routeSequence : patternsForSelectedLink.keySet()) { + Collection iterations = patternsForSelectedLink.get(routeSequence); nIterations += iterations.size(); allRouteIds.addAll(routeSequence.routes); summedTotalTime += iterations.stream().mapToInt(i -> i.totalTime).sum(); @@ -161,22 +189,29 @@ public ArrayList[] summarizeIterations(Stat stat) { .mapToObj(ri -> transitLayer.routeString(ri, true)) .collect(Collectors.joining("|")); String iterationProportion = "%.3f".formatted( - nIterations / (double)(nUnfilteredIterationsReachingDestination[d])); - row[0] = allRouteIdsPipeSeparated; + nIterations / (double) (nUnfilteredIterationsReachingDestination[d])); + row[0] = selectedLinkLabel + ": " + allRouteIdsPipeSeparated; row[row.length - 1] = iterationProportion; // Report average of total time over all retained iterations, different than mean/min approach below. row[row.length - 2] = String.format("%.1f", summedTotalTime / nIterations / 60d); summary[d].add(row); - // Fall through to the standard case below, so the summary row is followed by its component parts. - // We could optionally continue to the next loop iteration here, to return only the summary row. + // TODO call the standard case below, so the summary row is followed by its component parts. + // We could optionally continue to the next outer for loop iteration here, to return only the summary row. } - // Standard (non SelectedLink) case. + } + // Standard (non SelectedLink) case. + Multimap iterationMap = iterationsForPathTemplates[d]; + // It looks like this could just be: if (iterationmap == null) continue; (to clarify without indentation) + // When some destinations are reached, any unreached ones are empty maps rather than nulls. They will be + // null if no search happens at all (origin is not connected to network) in which case maybe we should skip + // this step. This makes and stores an array of CSV rows as arrayLists, one for each destination. + if (iterationMap != null) { for (RouteSequence routeSequence: iterationMap.keySet()) { Collection iterations = iterationMap.get(routeSequence); 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/analyst/cluster/SelectedLink.java b/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java index 4cbae63bb..5d9621429 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/SelectedLink.java @@ -160,6 +160,9 @@ public class SelectedLink { private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + /** A unique identifier distinguishing this SelectedLink from all others being used at the same time. */ + public final String label; + /** * Contains all TripPattern inter-stop hops that pass through the selected link area for fast hash-based lookup. * Keys are the index of a TripPattern in the TransitLayer, and values are arrays of stop positions within that @@ -179,7 +182,8 @@ public class SelectedLink { */ private final TransitLayer transitLayer; - public SelectedLink(TransitLayer transitLayer, TIntObjectMap hopsInTripPattern) { + public SelectedLink(String label, TransitLayer transitLayer, TIntObjectMap hopsInTripPattern) { + this.label = label; this.transitLayer = transitLayer; this.hopsInTripPattern = hopsInTripPattern; } diff --git a/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java b/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java index 315e521a8..091acd01b 100644 --- a/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java +++ b/src/main/java/com/conveyal/r5/analyst/scenario/SelectLink.java @@ -20,6 +20,7 @@ import org.slf4j.LoggerFactory; import java.lang.invoke.MethodHandles; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -42,6 +43,8 @@ public class SelectLink extends Modification { /// Public fields supplied on the custom modification. + public String label; // Fall back on comment if not present + public double lon; public double lat; @@ -94,6 +97,9 @@ public boolean resolve(TransportNetwork network) { addError("Heading tolerance must be in the range (0...80]."); } } + if (label == null) { + label = comment; + } return hasErrors(); } @@ -171,7 +177,10 @@ public boolean apply(TransportNetwork network) { }); // Store the resulting precomputed information in a SelectedLink instance on the TransportNetwork. // This could also be on the TransitLayer, but we may eventually want to include street edges in SelectedLink. - network.selectedLink = new SelectedLink(network.transitLayer, hopsInTripPattern); + if (network.selectedLinks == null) { + network.selectedLinks = new ArrayList(); + } + network.selectedLinks.add(new SelectedLink(label, network.transitLayer, hopsInTripPattern)); return hasErrors(); } diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java index ae7f5b028..613d35869 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java @@ -99,7 +99,7 @@ public class TransportNetwork implements Serializable { * potentially fuzzy floating point geometries, and need to be retained across many requests, so it's modeled as a * scenario modification. This is not ideal but it works. This modification can be applied at network build time. */ - public transient SelectedLink selectedLink; + public transient List selectedLinks; /** * Build some simple derived index tables that are not serialized with the network. 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..b88845c88 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 includeName){ 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), includeName)); + boardStopIds.add(transitLayer.stopString(stopSequence.boardStops.get(i), includeName)); + alightStopIds.add(transitLayer.stopString(stopSequence.alightStops.get(i), includeName)); rideTimes.add(String.format("%.1f", stopSequence.rideTimesSeconds.get(i) / 60f)); } String accessTime = stopSequence.access == null ? null : String.format("%.1f", stopSequence.access.time / 60f);