From 0087e0d899bc90257bcc429074768fa55787ac6c Mon Sep 17 00:00:00 2001 From: ansons Date: Thu, 29 Dec 2022 14:30:05 -0500 Subject: [PATCH 01/18] Increase autoscaling limits for freeform analyses --- .../java/com/conveyal/analysis/components/broker/Broker.java | 5 +++-- 1 file changed, 3 insertions(+), 2 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 d8895e6e3..1eeb93641 100644 --- a/src/main/java/com/conveyal/analysis/components/broker/Broker.java +++ b/src/main/java/com/conveyal/analysis/components/broker/Broker.java @@ -530,8 +530,9 @@ private void requestExtraWorkersIfAppropriate(Job job) { // Do not exceed the limit on workers per category TODO add similar limit per accessGroup or user targetWorkerTotal = Math.min(targetWorkerTotal, MAX_WORKERS_PER_CATEGORY); - // Guardrail until freeform pointsets are tested more thoroughly - if (job.templateTask.originPointSet != null) targetWorkerTotal = Math.min(targetWorkerTotal, 5); + // Guardrails until freeform pointsets are tested more thoroughly + if (job.templateTask.originPointSet != null) targetWorkerTotal = Math.min(targetWorkerTotal, 80); + if (job.templateTask.includePathResults) targetWorkerTotal = Math.min(targetWorkerTotal, 20); int nSpot = targetWorkerTotal - categoryWorkersAlreadyRunning; createWorkersInCategory(job.workerCategory, job.workerTags, 0, nSpot); } From 38290e93571b5a699d4ccad4a926beff361da43a Mon Sep 17 00:00:00 2001 From: ansons Date: Wed, 25 Jan 2023 00:59:35 -0500 Subject: [PATCH 02/18] fix(paths): return correct route info Fixes #855 --- .../r5/analyst/cluster/PathResultSummary.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/PathResultSummary.java b/src/main/java/com/conveyal/r5/analyst/cluster/PathResultSummary.java index 812592eb2..ef9df4ae2 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/PathResultSummary.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/PathResultSummary.java @@ -2,6 +2,7 @@ import com.conveyal.r5.analyst.StreetTimesAndModes; import com.conveyal.r5.transit.TransitLayer; +import com.conveyal.r5.transit.path.RouteSequence; import com.conveyal.r5.transit.path.StopSequence; import gnu.trove.list.TIntList; import gnu.trove.list.array.TIntArrayList; @@ -49,8 +50,8 @@ public PathResultSummary( this.iterations.add(iterationDetails); } List transitLegs = new ArrayList<>(); - for (int routeIndex = 0; routeIndex < pathTemplate.routes.size(); routeIndex++) { - transitLegs.add(new TransitLeg(transitLayer, pathTemplate.stopSequence, routeIndex)); + for (int legIndex = 0; legIndex < pathTemplate.routes.size(); legIndex++) { + transitLegs.add(new TransitLeg(transitLayer, pathTemplate, legIndex)); } itineraries.add(new Itinerary( pathTemplate.stopSequence.access, @@ -115,20 +116,21 @@ static class TransitLeg { public TransitLeg( TransitLayer transitLayer, - StopSequence stopSequence, - int routeIndex + RouteSequence routeSequence, + int legIndex ) { - var routeInfo = transitLayer.routes.get(routeIndex); + var routeInfo = transitLayer.routes.get(routeSequence.routes.get(legIndex)); routeId = routeInfo.route_id; routeName = routeInfo.getName(); + StopSequence stopSequence = routeSequence.stopSequence; - rideTimeSeconds = stopSequence.rideTimesSeconds.get(routeIndex); + rideTimeSeconds = stopSequence.rideTimesSeconds.get(legIndex); - int boardStopIndex = stopSequence.boardStops.get(routeIndex); + int boardStopIndex = stopSequence.boardStops.get(legIndex); boardStopId = getStopId(transitLayer, boardStopIndex); boardStopName = transitLayer.stopNames.get(boardStopIndex); - int alightStopIndex = stopSequence.alightStops.get(routeIndex); + int alightStopIndex = stopSequence.alightStops.get(legIndex); alightStopId = getStopId(transitLayer, alightStopIndex); alightStopName = transitLayer.stopNames.get(alightStopIndex); } From c483a7703918b2b324bd57efd2a11b9e8c89c7a5 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 1 Feb 2023 23:27:29 +0800 Subject: [PATCH 03/18] bail out of ways that reference undefined nodes fixes #851 --- .../com/conveyal/r5/streets/StreetLayer.java | 4 +++ .../conveyal/r5/streets/StreetLayerTest.java | 20 ++++++++++++++ .../com/conveyal/r5/streets/missing-nodes.pbf | Bin 0 -> 592 bytes .../com/conveyal/r5/streets/missing-nodes.xml | 25 ++++++++++++++++++ 4 files changed, 49 insertions(+) create mode 100644 src/test/resources/com/conveyal/r5/streets/missing-nodes.pbf create mode 100644 src/test/resources/com/conveyal/r5/streets/missing-nodes.xml diff --git a/src/main/java/com/conveyal/r5/streets/StreetLayer.java b/src/main/java/com/conveyal/r5/streets/StreetLayer.java index de7a7fd1f..3d781b41b 100644 --- a/src/main/java/com/conveyal/r5/streets/StreetLayer.java +++ b/src/main/java/com/conveyal/r5/streets/StreetLayer.java @@ -310,6 +310,10 @@ void loadFromOsm (OSM osm, boolean removeIslands, boolean saveVertexIndex) { for (int n = 1; n < way.nodes.length; n++) { long nodeId = way.nodes[n]; Node node = osm.nodes.get(nodeId); + if (node == null) { + LOG.warn("Bailing out of OSM way {} that references an undefined node.", entry.getKey()); + break; + } final boolean intersection = osm.intersectionNodes.contains(way.nodes[n]); final boolean lastNode = (n == (way.nodes.length - 1)); if (intersection || lastNode || isImpassable(node)) { diff --git a/src/test/java/com/conveyal/r5/streets/StreetLayerTest.java b/src/test/java/com/conveyal/r5/streets/StreetLayerTest.java index a062bec61..37e7f0c24 100644 --- a/src/test/java/com/conveyal/r5/streets/StreetLayerTest.java +++ b/src/test/java/com/conveyal/r5/streets/StreetLayerTest.java @@ -14,6 +14,7 @@ import java.util.EnumSet; import java.util.List; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -400,4 +401,23 @@ private int connectedVertices(StreetLayer sl, int vertexId) { return r.getReachedVertices().size(); } + /** + * We have decided to tolerate OSM data containing ways that reference missing nodes, because geographic extract + * processes often produce data like this. Load a file containing a way that ends with some missing nodes + * and make sure no exception occurs. The input must contain ways creating intersections such that at least one + * edge is produced, as later steps expect the edge store to be non-empty. The PBF fixture for this test is derived + * from the hand-tweaked XML file of the same name using osmconvert. + */ + @Test + public void testMissingNodes () { + OSM osm = new OSM(null); + osm.intersectionDetection = true; + osm.readFromUrl(StreetLayerTest.class.getResource("missing-nodes.pbf").toString()); + assertDoesNotThrow(() -> { + StreetLayer sl = new StreetLayer(); + sl.loadFromOsm(osm, true, true); + sl.buildEdgeLists(); + }); + } + } diff --git a/src/test/resources/com/conveyal/r5/streets/missing-nodes.pbf b/src/test/resources/com/conveyal/r5/streets/missing-nodes.pbf new file mode 100644 index 0000000000000000000000000000000000000000..6e927f26cd3189882934b0e58af59ee2889347c2 GIT binary patch literal 592 zcmV-W0m~dw0!f1hpjt@(sB_4b@ z(Kwk|;X%WK|C5-N`234=gOf8-a}#yL4D`&DxLi{6ic|gaQ&Nky1cUR7O7uc13sU1t zGE(#6Jzbg@1@nt@lk@Y+Qj1Cy4D>AY3=O&%RWeFS3as??%gf94@(Y0aONvrcOL7wn z^zw_+^%Dy+^?@b>0HDnA5^xf33J=_-&G`ZT%eNy2nzjA43o11V1OWj7 z0TK?DtmAD;0s;U6LJNw` z?1KcQN7V$G3`z}%!{T&^`~dO$p`>XdpI~x7xZiZ literal 0 HcmV?d00001 diff --git a/src/test/resources/com/conveyal/r5/streets/missing-nodes.xml b/src/test/resources/com/conveyal/r5/streets/missing-nodes.xml new file mode 100644 index 000000000..0a9cf4616 --- /dev/null +++ b/src/test/resources/com/conveyal/r5/streets/missing-nodes.xml @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From f36dec3a5005934d62015d37f5c2e54f4fa4aa0f Mon Sep 17 00:00:00 2001 From: Trevor Gerhardt Date: Thu, 2 Feb 2023 08:05:01 +0100 Subject: [PATCH 04/18] Make `OsmCache.getKey` a static method (#857) BundleController now no longer depends on `OSMCache` as a component. --- .../com/conveyal/analysis/controllers/BundleController.java | 5 +---- src/main/java/com/conveyal/r5/streets/OSMCache.java | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/controllers/BundleController.java b/src/main/java/com/conveyal/analysis/controllers/BundleController.java index 269189407..b7fc71cc5 100644 --- a/src/main/java/com/conveyal/analysis/controllers/BundleController.java +++ b/src/main/java/com/conveyal/analysis/controllers/BundleController.java @@ -67,14 +67,11 @@ public class BundleController implements HttpController { private final FileStorage fileStorage; private final GTFSCache gtfsCache; - // FIXME The backend appears to use an osmcache purely to get a file key at which to store incoming OSM. Refactor. - private final OSMCache osmCache; private final TaskScheduler taskScheduler; public BundleController (BackendComponents components) { this.fileStorage = components.fileStorage; this.gtfsCache = components.gtfsCache; - this.osmCache = components.osmCache; this.taskScheduler = components.taskScheduler; } @@ -176,7 +173,7 @@ private Bundle create (Request req, Response res) { osm.close(); checkWgsEnvelopeSize(osmBounds, "OSM data"); // Store the source OSM file. Note that we're not storing the derived MapDB file here. - fileStorage.moveIntoStorage(osmCache.getKey(bundle.osmId), fi.getStoreLocation()); + fileStorage.moveIntoStorage(OSMCache.getKey(bundle.osmId), fi.getStoreLocation()); } if (bundle.feedGroupId == null) { diff --git a/src/main/java/com/conveyal/r5/streets/OSMCache.java b/src/main/java/com/conveyal/r5/streets/OSMCache.java index 77df063ae..e51a6f07c 100644 --- a/src/main/java/com/conveyal/r5/streets/OSMCache.java +++ b/src/main/java/com/conveyal/r5/streets/OSMCache.java @@ -33,11 +33,11 @@ public OSMCache (FileStorage fileStorage) { .maximumSize(10) .build(); - public String cleanId(String id) { + public static String cleanId(String id) { return id.replaceAll("[^A-Za-z0-9]", "-"); } - public FileStorageKey getKey (String id) { + public static FileStorageKey getKey (String id) { // FIXME Transforming IDs each time they're used seems problematic. They should probably only be validated here. String cleanId = cleanId(id); return new FileStorageKey(FileCategory.BUNDLES, cleanId + ".pbf"); From 1f5433b880c505f5d35e7c8065dec198b5645cc0 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 3 Feb 2023 16:17:58 +0800 Subject: [PATCH 05/18] additional street modes in TransportNetworkConfig added comments explaining how only walk stop-to-vertex tables are kept --- .../cluster/TransportNetworkConfig.java | 15 +++++ .../conveyal/r5/streets/EgressCostTable.java | 20 +++--- .../com/conveyal/r5/transit/TransitLayer.java | 4 +- .../conveyal/r5/transit/TransportNetwork.java | 19 +++--- .../r5/transit/TransportNetworkCache.java | 61 ++++++++++++------- 5 files changed, 78 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java b/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java index 54f11ee8c..924939bb0 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java @@ -4,11 +4,13 @@ import com.conveyal.r5.analyst.scenario.Modification; import com.conveyal.r5.analyst.scenario.RasterCost; import com.conveyal.r5.analyst.scenario.ShapefileLts; +import com.conveyal.r5.profile.StreetMode; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.util.List; +import java.util.Set; /** * All inputs and options that describe how to build a particular transport network (except the serialization version). @@ -39,4 +41,17 @@ public class TransportNetworkConfig { /** A list of _R5_ modifications to apply during network build. May be null. */ public List modifications; + /** + * Additional modes other than walk for which to pre-build a full regional grid and distance tables. + * When building a network, by default we build distance tables from transit stops to street vertices, to which we + * connect a grid covering the entire street network at the default zoom level. By default we do this only for the + * walk mode. Pre-building and serializing equivalent data structures for other modes allows workers to start up + * much faster in regional analyses. The work need only be done once when the first single-point worker to builds + * the network. Otherwise, hundreds of workers will each have to build these tables every time they start up. + * Some scenarios, such as those that affect the street layer, may still be slower to apply for modes listed here + * because some intermediate data (stop-to-vertex tables) are only retained for the walk mode. If this proves to be + * a problem it is a candidate for future optimization. + */ + public Set buildGridsForModes; + } diff --git a/src/main/java/com/conveyal/r5/streets/EgressCostTable.java b/src/main/java/com/conveyal/r5/streets/EgressCostTable.java index cb72a65e1..0d4e750b0 100644 --- a/src/main/java/com/conveyal/r5/streets/EgressCostTable.java +++ b/src/main/java/com/conveyal/r5/streets/EgressCostTable.java @@ -262,16 +262,16 @@ public EgressCostTable (LinkedPointSet linkedPointSet, ProgressListener progress GeometryUtils.expandEnvelopeFixed(envelopeAroundStop, linkingDistanceLimitMeters); if (streetMode == StreetMode.WALK) { - // Walking distances from stops to street vertices are saved in the TransitLayer. - // Get the pre-computed walking distance table from the stop to the street vertices, - // then extend that table out from the street vertices to the points in this PointSet. - // TODO reuse the code that computes the walk tables at TransitLayer.buildOneDistanceTable() rather than - // duplicating it below for other modes. + // Distances from stops to street vertices are saved in the TransitLayer, but only for the walk mode. + // Get the pre-computed walking distance table from the stop to the street vertices, then extend that + // table out from the street vertices to the points in this PointSet. It may be possible to reuse the + // code that pre-computes walk tables at TransitLayer.buildOneDistanceTable() rather than duplicating + // it below for other (non-walk) modes. TIntIntMap distanceTableToVertices = transitLayer.stopToVertexDistanceTables.get(stopIndex); return distanceTableToVertices == null ? null : linkedPointSet.extendDistanceTableToPoints(distanceTableToVertices, envelopeAroundStop); } else { - + // For non-walk modes perform a search from each stop, as stop-to-vertex tables are not precomputed. Geometry egressArea = null; // If a pickup delay modification is present for this street mode, egressStopDelaysSeconds is @@ -301,14 +301,14 @@ public EgressCostTable (LinkedPointSet linkedPointSet, ProgressListener progress LOG.warn("Stop unlinked, cannot build distance table: {}", stopIndex); return null; } - // TODO setting the origin point of the router to the stop vertex does not work. - // This is probably because link edges do not allow car traversal. We could traverse them. - // As a stopgap we perform car linking at the geographic coordinate of the stop. + // Setting the origin point of the router to the stop vertex (as follows) does not work. // sr.setOrigin(vertexId); + // This is probably because link edges do not allow car traversal. We could traverse them. + // As a workaround we perform car linking at the geographic coordinate of the stop. VertexStore.Vertex vertex = linkedPointSet.streetLayer.vertexStore.getCursor(vertexId); sr.setOrigin(vertex.getLat(), vertex.getLon()); - // WALK is handled above, this block is exhaustively handling all other modes. + // WALK is handled in the if clause above, this else block is exhaustively handling all other modes. if (streetMode == StreetMode.BICYCLE) { sr.distanceLimitMeters = linkingDistanceLimitMeters; } else if (streetMode == StreetMode.CAR) { diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index e140b0d89..b7c5247ca 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -535,11 +535,13 @@ public void rebuildTransientIndexes () { } /** - * Run a distance-constrained street search from every transit stop in the graph. + * Run a distance-constrained street search from every transit stop in the graph using the walk mode. * Store the distance to every reachable street vertex for each of these origin stops. * If a scenario has been applied, we need to build tables for any newly created stops and any stops within * transfer distance or access/egress distance of those new stops. In that case a rebuildZone geometry should be * supplied. If rebuildZone is null, a complete rebuild of all tables will occur for all stops. + * Note, this rebuilds for the WALK MODE ONLY. The network only has a field for retaining walk distance tables. + * This is a candidate for optimization if car or bicycle scenarios are slow to apply. * @param rebuildZone the zone within which to rebuild tables in FIXED-POINT DEGREES, or null to build all tables. */ public void buildDistanceTables(Geometry rebuildZone) { diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java index c68a8f166..3e3cb3720 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetwork.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetwork.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Stream; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -263,14 +264,14 @@ public static InputFileType forFile(File file) { } /** - * For Analysis purposes, build an efficient implicit grid PointSet for this TransportNetwork. Then, for any modes - * supplied, we also build a linkage that is held permanently in the GridPointSet. This method is called when a - * network is first built. - * The resulting grid PointSet will cover the entire street network layer of this TransportNetwork, which should - * include every point we can route from or to. Any other destination grid (for the same mode, walking) can be made - * as a subset of this one since it includes every potentially accessible point. + * Build a grid PointSet covering the entire street network layer of this TransportNetwork, which should include + * every point we can route from or to. Then for all requested modes build a linkage that is held in the + * GridPointSet. This method is called when a network is first built so these linkages are serialized with it. + * Any other destination grid (at least for the same modes) can be made as a subset of this one since it includes + * every potentially accessible point. Destination grids for other modes will be made on demand, which is a slow + * operation that can occupy hundreds of workers for long periods of time when a regional analysis starts up. */ - public void rebuildLinkedGridPointSet(StreetMode... modes) { + public void rebuildLinkedGridPointSet(Iterable modes) { if (fullExtentGridPointSet != null) { throw new RuntimeException("Linked grid pointset was built more than once."); } @@ -280,6 +281,10 @@ public void rebuildLinkedGridPointSet(StreetMode... modes) { } } + public void rebuildLinkedGridPointSet(StreetMode... modes) { + rebuildLinkedGridPointSet(Set.of(modes)); + } + //TODO: add transit stops to envelope public Envelope getEnvelope() { return streetLayer.getEnvelope(); diff --git a/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java b/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java index d93891ba1..b7f1f1d8f 100644 --- a/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java +++ b/src/main/java/com/conveyal/r5/transit/TransportNetworkCache.java @@ -19,6 +19,7 @@ import com.conveyal.r5.streets.StreetLayer; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; +import com.google.common.collect.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -158,28 +159,52 @@ private static FileStorageKey getR5NetworkFileStorageKey (String networkId) { return new FileStorageKey(BUNDLES, getR5NetworkFilename(networkId)); } + /** @return the network configuration (AKA manifest) for the given network ID, or null if no config file exists. */ + private TransportNetworkConfig loadNetworkConfig (String networkId) { + FileStorageKey configFileKey = new FileStorageKey(BUNDLES, getNetworkConfigFilename(networkId)); + if (!fileStorage.exists(configFileKey)) { + return null; + } + File configFile = fileStorage.getFile(configFileKey); + try { + // Use lenient mapper to mimic behavior in objectFromRequestBody. + return JsonUtilities.lenientObjectMapper.readValue(configFile, TransportNetworkConfig.class); + } catch (IOException e) { + throw new RuntimeException("Error reading TransportNetworkConfig. Does it contain new unrecognized fields?", e); + } + } + /** * If we did not find a cached network, build one from the input files. Should throw an exception rather than * returning null if for any reason it can't finish building one. */ private @Nonnull TransportNetwork buildNetwork (String networkId) { TransportNetwork network; - FileStorageKey networkConfigKey = new FileStorageKey(BUNDLES, GTFSCache.cleanId(networkId) + ".json"); - if (fileStorage.exists(networkConfigKey)) { - network = buildNetworkFromConfig(networkId); - } else { - LOG.warn("Detected old-format bundle stored as single ZIP file"); + TransportNetworkConfig networkConfig = loadNetworkConfig(networkId); + if (networkConfig == null) { + // The switch to use JSON manifests instead of zips occurred in 32a1aebe in July 2016. + // Over six years have passed, buildNetworkFromBundleZip is deprecated and could probably be removed. + LOG.warn("No network config (aka manifest) found. Assuming old-format network inputs bundle stored as a single ZIP file."); network = buildNetworkFromBundleZip(networkId); + } else { + network = buildNetworkFromConfig(networkConfig); } network.scenarioId = networkId; - // Networks created in TransportNetworkCache are going to be used for analysis work. Pre-compute distance tables - // from stops to street vertices, then pre-build a linked grid pointset for the whole region. These linkages - // should be serialized along with the network, which avoids building them when an analysis worker starts. - // The linkage we create here will never be used directly, but serves as a basis for scenario linkages, making - // analysis much faster to start up. + // Pre-compute distance tables from stops out to street vertices, then pre-build a linked grid pointset for the + // whole region covered by the street network. These tables and linkages will be serialized along with the + // network, which avoids building them when every analysis worker starts. The linkage we create here will never + // be used directly, but serves as a basis for scenario linkages, making analyses much faster to start up. + // Note, this retains stop-to-vertex distances for the WALK MODE ONLY, even when they are produced as + // intermediate results while building linkages for other modes. + // This is a candidate for optimization if car or bicycle scenarios are slow to apply. network.transitLayer.buildDistanceTables(null); - network.rebuildLinkedGridPointSet(StreetMode.WALK); + + Set buildGridsForModes = Sets.newHashSet(StreetMode.WALK); + if (networkConfig != null && networkConfig.buildGridsForModes != null) { + buildGridsForModes.addAll(networkConfig.buildGridsForModes); + } + network.rebuildLinkedGridPointSet(buildGridsForModes); // Cache the serialized network on the local filesystem and mirror it to any remote storage. try { @@ -247,17 +272,7 @@ private TransportNetwork buildNetworkFromBundleZip (String networkId) { * This describes the locations of files used to create a bundle, as well as options applied at network build time. * It contains the unique IDs of the GTFS feeds and OSM extract. */ - private TransportNetwork buildNetworkFromConfig (String networkId) { - FileStorageKey configFileKey = new FileStorageKey(BUNDLES, getNetworkConfigFilename(networkId)); - File configFile = fileStorage.getFile(configFileKey); - TransportNetworkConfig config; - - try { - // Use lenient mapper to mimic behavior in objectFromRequestBody. - config = JsonUtilities.lenientObjectMapper.readValue(configFile, TransportNetworkConfig.class); - } catch (IOException e) { - throw new RuntimeException("Error reading TransportNetworkConfig. Does it contain new unrecognized fields?", e); - } + private TransportNetwork buildNetworkFromConfig (TransportNetworkConfig config) { // FIXME duplicate code. All internal building logic should be encapsulated in a method like // TransportNetwork.build(osm, gtfs1, gtfs2...) // We currently have multiple copies of it, in buildNetworkFromConfig and buildNetworkFromBundleZip so you've @@ -265,7 +280,7 @@ private TransportNetwork buildNetworkFromConfig (String networkId) { // Maybe we should just completely deprecate bundle ZIPs and remove those code paths. TransportNetwork network = new TransportNetwork(); - network.scenarioId = networkId; + network.streetLayer = new StreetLayer(); network.streetLayer.loadFromOsm(osmCache.get(config.osmId)); From edacc133a85ef75588316c7400a096609c42cbe9 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 3 Feb 2023 20:34:53 +0800 Subject: [PATCH 06/18] include street mode in linkage/egress log messages --- src/main/java/com/conveyal/r5/streets/EgressCostTable.java | 6 +++--- src/main/java/com/conveyal/r5/streets/LinkedPointSet.java | 4 ++-- src/main/java/com/conveyal/r5/transit/TransitLayer.java | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/conveyal/r5/streets/EgressCostTable.java b/src/main/java/com/conveyal/r5/streets/EgressCostTable.java index 0d4e750b0..60bf5e336 100644 --- a/src/main/java/com/conveyal/r5/streets/EgressCostTable.java +++ b/src/main/java/com/conveyal/r5/streets/EgressCostTable.java @@ -209,7 +209,7 @@ public EgressCostTable (LinkedPointSet linkedPointSet, ProgressListener progress rebuildZone = linkedPointSet.streetLayer.scenarioEdgesBoundingGeometry(linkingDistanceLimitMeters); } - LOG.info("Creating EgressCostTables from each transit stop to PointSet points."); + LOG.info("Creating EgressCostTables from each transit stop to PointSet points for mode {}.", streetMode); if (rebuildZone != null) { LOG.info("Selectively computing tables for only those stops that might be affected by the scenario."); } @@ -232,9 +232,9 @@ public EgressCostTable (LinkedPointSet linkedPointSet, ProgressListener progress progressListener.beginTask(taskDescription, nStops); final LambdaCounter computeCounter = new LambdaCounter(LOG, nStops, computeLogFrequency, - "Computed new stop -> point tables for {} of {} transit stops."); + String.format("Computed new stop-to-point tables from {} of {} transit stops for mode %s.", streetMode)); final LambdaCounter copyCounter = new LambdaCounter(LOG, nStops, copyLogFrequency, - "Copied unchanged stop -> point tables for {} of {} transit stops."); + String.format("Copied unchanged stop-to-point tables from {} of {} transit stops for mode %s.", streetMode)); // Create a distance table from each transit stop to the points in this PointSet in parallel. // Each table is a flattened 2D array. Two values for each point reachable from this stop: (pointIndex, cost) // When applying a scenario, keep the existing distance table for those stops that could not be affected. diff --git a/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java b/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java index c529ced10..01a51a43d 100644 --- a/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java +++ b/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java @@ -135,7 +135,7 @@ public class LinkedPointSet implements Serializable { * the same pointSet and streetMode as the preceding arguments. */ public LinkedPointSet (PointSet pointSet, StreetLayer streetLayer, StreetMode streetMode, LinkedPointSet baseLinkage) { - LOG.info("Linking pointset to street network..."); + LOG.info("Linking pointset to street network for mode {}...", streetMode); this.pointSet = pointSet; this.streetLayer = streetLayer; this.streetMode = streetMode; @@ -301,7 +301,7 @@ public synchronized EgressCostTable getEgressCostTable () { */ private void linkPointsToStreets (boolean all) { LambdaCounter linkCounter = new LambdaCounter(LOG, pointSet.featureCount(), 10000, - "Linked {} of {} PointSet points to streets."); + String.format("Linked {} of {} PointSet points to streets for mode %s.", streetMode)); // Construct a geometry around any edges added by the scenario, or null if there are no added edges. // As it is derived from edge geometries this is a fixed-point geometry and must be intersected with the same. diff --git a/src/main/java/com/conveyal/r5/transit/TransitLayer.java b/src/main/java/com/conveyal/r5/transit/TransitLayer.java index b7c5247ca..871491ff2 100644 --- a/src/main/java/com/conveyal/r5/transit/TransitLayer.java +++ b/src/main/java/com/conveyal/r5/transit/TransitLayer.java @@ -546,7 +546,7 @@ public void rebuildTransientIndexes () { */ public void buildDistanceTables(Geometry rebuildZone) { - LOG.info("Finding distances from transit stops to street vertices."); + LOG.info("Pre-computing distances from transit stops to street vertices (WALK mode only)."); if (rebuildZone != null) { LOG.info("Selectively finding distances for only those stops potentially affected by scenario application."); } From f2d7c32288390f37a6144f4a1b91c0e57a6d6c21 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Tue, 7 Feb 2023 10:46:57 +0800 Subject: [PATCH 07/18] clarify comment on new config option --- .../com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java b/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java index 924939bb0..012e3204a 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/TransportNetworkConfig.java @@ -42,7 +42,7 @@ public class TransportNetworkConfig { public List modifications; /** - * Additional modes other than walk for which to pre-build a full regional grid and distance tables. + * Additional modes other than walk for which to pre-build large data structures (grid linkage and cost tables). * When building a network, by default we build distance tables from transit stops to street vertices, to which we * connect a grid covering the entire street network at the default zoom level. By default we do this only for the * walk mode. Pre-building and serializing equivalent data structures for other modes allows workers to start up From 40c4d758f6eda891526c751c973eab063abb2bdb Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 15 Mar 2023 18:44:49 +0800 Subject: [PATCH 08/18] update build.gradle for gradle 8.x also fixed gradle 9.x deprecation warnings --- build.gradle | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index fee0a0898..3ab0dbb0c 100644 --- a/build.gradle +++ b/build.gradle @@ -1,8 +1,8 @@ plugins { id 'java' - id 'com.github.johnrengelman.shadow' version '6.0.0' + id 'com.github.johnrengelman.shadow' version '8.1.0' id 'maven-publish' - id 'com.palantir.git-version' version '0.12.3' + id 'com.palantir.git-version' version '2.0.0' } group = 'com.conveyal' @@ -72,9 +72,11 @@ tasks.withType(JavaCompile) { options.encoding = 'UTF-8' } -// A task to copy all dependencies of the project into a single directory +// A task to copy all dependency JARs needed at runtime into a single directory task copyDependencies(type: Copy) { - from configurations.default + from(sourceSets.main.runtimeClasspath) { + include '*.jar' + } into 'dependencies' } @@ -82,7 +84,7 @@ task copyDependencies(type: Copy) { task runBackend (type: JavaExec) { dependsOn(build) classpath(sourceSets.main.runtimeClasspath) - main("com.conveyal.analysis.BackendMain") + mainClass = 'com.conveyal.analysis.BackendMain' } // Start up an analysis local backend from a shaded JAR and ask it to shut down immediately. @@ -91,7 +93,7 @@ task runBackend (type: JavaExec) { task testShadowJarRunnable(type: JavaExec) { dependsOn(shadowJar) classpath(shadowJar.archiveFile.get()) - main("com.conveyal.analysis.BackendMain") + mainClass = 'com.conveyal.analysis.BackendMain' jvmArgs("-Dconveyal.immediate.shutdown=true") } From 074c511fde485c750c4ef145f6a6a63cec0892e2 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 15 Mar 2023 19:02:04 +0800 Subject: [PATCH 09/18] use built-in gradle caching in setup-java action --- .github/workflows/gradle.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 22691e6d1..24f08b055 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -33,14 +33,11 @@ jobs: fetch-depth: 0 # Java setup step completes very fast, no need to run in a preconfigured docker container. - name: Set up JDK 11 - uses: actions/setup-java@v1 + uses: actions/setup-java@v3 with: java-version: 11 - - uses: actions/cache@v1 - id: cache - with: - path: ~/.gradle/caches - key: gradle-caches + distribution: temurin + cache: 'gradle' - name: Show version string run: gradle -q printVersion | head -n1 - name: Build and Test From e9d44e1997590d1387623102083e629973a2c795 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Thu, 16 Mar 2023 11:46:25 +0800 Subject: [PATCH 10/18] grow envelope from projected coords, fixes #833 --- .../datasource/ShapefileDataSourceIngester.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/datasource/ShapefileDataSourceIngester.java b/src/main/java/com/conveyal/analysis/datasource/ShapefileDataSourceIngester.java index 03e61c6b7..21b1c06d4 100644 --- a/src/main/java/com/conveyal/analysis/datasource/ShapefileDataSourceIngester.java +++ b/src/main/java/com/conveyal/analysis/datasource/ShapefileDataSourceIngester.java @@ -44,24 +44,25 @@ public void ingest (File file, ProgressListener progressListener) { try { ShapefileReader reader = new ShapefileReader(file); // Iterate over all features to ensure file is readable, geometries are valid, and can be reprojected. - Envelope envelope = reader.wgs84Bounds(); - checkWgsEnvelopeSize(envelope, "Shapefile"); + // Note that the method reader.wgs84Bounds() transforms the envelope in projected coordinates to WGS84, + // which does not necessarily include all the points transformed individually. + Envelope envelope = new Envelope(); reader.wgs84Stream().forEach(f -> { - checkState(envelope.contains(((Geometry)f.getDefaultGeometry()).getEnvelopeInternal())); + envelope.expandToInclude(((Geometry)f.getDefaultGeometry()).getEnvelopeInternal()); }); + checkWgsEnvelopeSize(envelope, "Shapefile"); reader.close(); progressListener.increment(); dataSource.wgsBounds = Bounds.fromWgsEnvelope(envelope); dataSource.attributes = reader.attributes(); dataSource.geometryType = reader.geometryType(); dataSource.featureCount = reader.featureCount(); - dataSource.coordinateSystem = - reader.crs.getName().getCode(); - + dataSource.coordinateSystem = reader.crs.getName().getCode(); progressListener.increment(); } catch (FactoryException | TransformException e) { - throw new DataSourceException("Shapefile transform error. " + - "Try uploading an unprojected WGS84 (EPSG:4326) file.", e); + throw new DataSourceException( + "Shapefile transform error. Try uploading an unprojected WGS84 (EPSG:4326) file.", e + ); } catch (IOException e) { // ShapefileReader throws a checked IOException. throw new DataSourceException("Error parsing shapefile. Ensure the files you uploaded are valid.", e); From eb490b0a5f5c2ee88112aa26774a8067564c40c6 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Thu, 16 Mar 2023 16:12:55 +0800 Subject: [PATCH 11/18] check duplicate and missing GTFS entity keys the same method is applied for all entity types with simple string IDs addresses #792 --- .../gtfs/error/DuplicateKeyError.java | 4 +-- .../conveyal/gtfs/error/MissingKeyError.java | 22 ++++++++++++++++ .../java/com/conveyal/gtfs/model/Agency.java | 15 +++++++---- .../com/conveyal/gtfs/model/Calendar.java | 2 +- .../com/conveyal/gtfs/model/CalendarDate.java | 3 ++- .../java/com/conveyal/gtfs/model/Entity.java | 26 +++++++++++++++++-- .../conveyal/gtfs/model/FareAttribute.java | 2 +- .../java/com/conveyal/gtfs/model/Route.java | 7 ++++- .../java/com/conveyal/gtfs/model/Stop.java | 7 +++-- .../com/conveyal/gtfs/model/StopTime.java | 2 +- .../java/com/conveyal/gtfs/model/Trip.java | 7 ++++- 11 files changed, 80 insertions(+), 17 deletions(-) create mode 100644 src/main/java/com/conveyal/gtfs/error/MissingKeyError.java diff --git a/src/main/java/com/conveyal/gtfs/error/DuplicateKeyError.java b/src/main/java/com/conveyal/gtfs/error/DuplicateKeyError.java index 9630449f4..50dca9c59 100644 --- a/src/main/java/com/conveyal/gtfs/error/DuplicateKeyError.java +++ b/src/main/java/com/conveyal/gtfs/error/DuplicateKeyError.java @@ -8,8 +8,8 @@ public class DuplicateKeyError extends GTFSError implements Serializable { public static final long serialVersionUID = 1L; - public DuplicateKeyError(String file, long line, String field) { - super(file, line, field); + public DuplicateKeyError(String file, long line, String field, String id) { + super(file, line, field, id); } @Override public String getMessage() { diff --git a/src/main/java/com/conveyal/gtfs/error/MissingKeyError.java b/src/main/java/com/conveyal/gtfs/error/MissingKeyError.java new file mode 100644 index 000000000..5a1ccdaeb --- /dev/null +++ b/src/main/java/com/conveyal/gtfs/error/MissingKeyError.java @@ -0,0 +1,22 @@ +package com.conveyal.gtfs.error; + +import com.conveyal.gtfs.validator.model.Priority; + +import java.io.Serializable; + +/** Indicates that a GTFS entity was not added to a table because it had no primary key. */ +public class MissingKeyError extends GTFSError implements Serializable { + public static final long serialVersionUID = 1L; + + public MissingKeyError(String file, long line, String field) { + super(file, line, field); + } + + @Override public String getMessage() { + return "Missing primary key."; + } + + @Override public Priority getPriority() { + return Priority.MEDIUM; + } +} diff --git a/src/main/java/com/conveyal/gtfs/model/Agency.java b/src/main/java/com/conveyal/gtfs/model/Agency.java index a2e301329..fd1e6031c 100644 --- a/src/main/java/com/conveyal/gtfs/model/Agency.java +++ b/src/main/java/com/conveyal/gtfs/model/Agency.java @@ -20,6 +20,11 @@ public class Agency extends Entity { public URL agency_branding_url; public String feed_id; + @Override + public String getId() { + return agency_id; + } + public static class Loader extends Entity.Loader { public Loader(GTFSFeed feed) { @@ -45,11 +50,11 @@ public void loadOneRow() throws IOException { a.agency_fare_url = getUrlField("agency_fare_url", false); a.agency_branding_url = getUrlField("agency_branding_url", false); a.feed_id = feed.feedId; - - // TODO clooge due to not being able to have null keys in mapdb - if (a.agency_id == null) a.agency_id = "NONE"; - - feed.agency.put(a.agency_id, a); + // Kludge because mapdb does not support null keys + if (a.agency_id == null) { + a.agency_id = "NONE"; + } + insertCheckingDuplicateKey(feed.agency, a, "agency_id"); } } diff --git a/src/main/java/com/conveyal/gtfs/model/Calendar.java b/src/main/java/com/conveyal/gtfs/model/Calendar.java index e0f0be87f..8ac72a000 100644 --- a/src/main/java/com/conveyal/gtfs/model/Calendar.java +++ b/src/main/java/com/conveyal/gtfs/model/Calendar.java @@ -52,7 +52,7 @@ public void loadOneRow() throws IOException { String service_id = getStringField("service_id", true); // TODO service_id can reference either calendar or calendar_dates. Service service = services.computeIfAbsent(service_id, Service::new); if (service.calendar != null) { - feed.errors.add(new DuplicateKeyError(tableName, row, "service_id")); + feed.errors.add(new DuplicateKeyError(tableName, row, "service_id", service_id)); } else { Calendar c = new Calendar(); c.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index diff --git a/src/main/java/com/conveyal/gtfs/model/CalendarDate.java b/src/main/java/com/conveyal/gtfs/model/CalendarDate.java index 081f4c8b3..db4a1e9f3 100644 --- a/src/main/java/com/conveyal/gtfs/model/CalendarDate.java +++ b/src/main/java/com/conveyal/gtfs/model/CalendarDate.java @@ -52,7 +52,8 @@ public void loadOneRow() throws IOException { Service service = services.computeIfAbsent(service_id, Service::new); LocalDate date = getDateField("date", true); if (service.calendar_dates.containsKey(date)) { - feed.errors.add(new DuplicateKeyError(tableName, row, "(service_id, date)")); + String keyString = String.format("(%s,%s)", service_id, date.toString()); + feed.errors.add(new DuplicateKeyError(tableName, row, "(service_id, date)", keyString)); } else { CalendarDate cd = new CalendarDate(); cd.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index diff --git a/src/main/java/com/conveyal/gtfs/model/Entity.java b/src/main/java/com/conveyal/gtfs/model/Entity.java index c445f2a41..215d08072 100644 --- a/src/main/java/com/conveyal/gtfs/model/Entity.java +++ b/src/main/java/com/conveyal/gtfs/model/Entity.java @@ -1,6 +1,8 @@ package com.conveyal.gtfs.model; import com.beust.jcommander.internal.Sets; +import com.conveyal.gtfs.error.DuplicateKeyError; +import com.conveyal.gtfs.error.MissingKeyError; import com.conveyal.r5.analyst.progress.ProgressInputStream; import com.conveyal.gtfs.GTFSFeed; import com.conveyal.gtfs.error.DateParseError; @@ -57,8 +59,10 @@ public abstract class Entity implements Serializable { * with a sequence number. For example stop_times and shapes have no single field that uniquely * identifies a row, and the stop_sequence or shape_pt_sequence must also be considered. */ - public String getId () { - return null; + public String getId() { + // Several entities have compound keys which are handled as tuple objects, not strings. + // Fail fast if anything tries to fetch a string ID for those Entity types. + throw new UnsupportedOperationException(); } /** @@ -301,6 +305,24 @@ public void loadTable (ZipFile zip) throws IOException { } } + /** + * Insert the given value into the map, checking whether a value already exists with its key. + * The entity type must override getId() for this to work. We have to pass in the name of the key field for + * error reporting purposes because although there is a method to get the ID of an Entity there is not a method + * to get the name of the field(s) it is taken from. + */ + protected void insertCheckingDuplicateKey (Map map, E value, String keyField) { + String key = value.getId(); + if (key == null) { + feed.errors.add(new MissingKeyError(tableName, value.sourceFileLine, keyField)); + return; + } + // Map returns previous value if one was already present + E previousValue = map.put(key, value); + if (previousValue != null) { + feed.errors.add(new DuplicateKeyError(tableName, value.sourceFileLine, keyField, key)); + } + } } /** diff --git a/src/main/java/com/conveyal/gtfs/model/FareAttribute.java b/src/main/java/com/conveyal/gtfs/model/FareAttribute.java index 35afcbc97..9ea58bede 100644 --- a/src/main/java/com/conveyal/gtfs/model/FareAttribute.java +++ b/src/main/java/com/conveyal/gtfs/model/FareAttribute.java @@ -39,7 +39,7 @@ public void loadOneRow() throws IOException { String fareId = getStringField("fare_id", true); Fare fare = fares.computeIfAbsent(fareId, Fare::new); if (fare.fare_attribute != null) { - feed.errors.add(new DuplicateKeyError(tableName, row, "fare_id")); + feed.errors.add(new DuplicateKeyError(tableName, row, "fare_id", fareId)); } else { FareAttribute fa = new FareAttribute(); fa.sourceFileLine = row + 1; // offset line number by 1 to account for 0-based row index diff --git a/src/main/java/com/conveyal/gtfs/model/Route.java b/src/main/java/com/conveyal/gtfs/model/Route.java index cb9c208b8..f4965861b 100644 --- a/src/main/java/com/conveyal/gtfs/model/Route.java +++ b/src/main/java/com/conveyal/gtfs/model/Route.java @@ -32,6 +32,11 @@ public class Route extends Entity { // implements Entity.Factory public URL route_branding_url; public String feed_id; + @Override + public String getId() { + return route_id; + } + public static class Loader extends Entity.Loader { public Loader(GTFSFeed feed) { @@ -72,7 +77,7 @@ public void loadOneRow() throws IOException { r.route_text_color = getStringField("route_text_color", false); r.route_branding_url = getUrlField("route_branding_url", false); r.feed_id = feed.feedId; - feed.routes.put(r.route_id, r); + insertCheckingDuplicateKey(feed.routes, r, "route_id"); } } diff --git a/src/main/java/com/conveyal/gtfs/model/Stop.java b/src/main/java/com/conveyal/gtfs/model/Stop.java index 580030bef..26a5fabb3 100644 --- a/src/main/java/com/conveyal/gtfs/model/Stop.java +++ b/src/main/java/com/conveyal/gtfs/model/Stop.java @@ -1,10 +1,12 @@ package com.conveyal.gtfs.model; import com.conveyal.gtfs.GTFSFeed; +import com.conveyal.gtfs.error.DuplicateKeyError; import java.io.IOException; import java.net.URL; import java.util.Iterator; +import java.util.Map; public class Stop extends Entity { @@ -61,12 +63,13 @@ public void loadOneRow() throws IOException { s.stop_timezone = getStringField("stop_timezone", false); s.wheelchair_boarding = getStringField("wheelchair_boarding", false); s.feed_id = feed.feedId; - /* TODO check ref integrity later, this table self-references via parent_station */ - feed.stops.put(s.stop_id, s); + // Referential integrity is checked later after fully loaded. Stops self-reference via parent_station. + insertCheckingDuplicateKey(feed.stops, s, "stop_id"); } } + public static class Writer extends Entity.Writer { public Writer (GTFSFeed feed) { super(feed, "stops"); diff --git a/src/main/java/com/conveyal/gtfs/model/StopTime.java b/src/main/java/com/conveyal/gtfs/model/StopTime.java index b15f7b46d..daabfec14 100644 --- a/src/main/java/com/conveyal/gtfs/model/StopTime.java +++ b/src/main/java/com/conveyal/gtfs/model/StopTime.java @@ -28,7 +28,7 @@ public class StopTime extends Entity implements Cloneable, Serializable { @Override public String getId() { - return trip_id; // Needs sequence number to be unique + return trip_id; // Concatenate with sequence number to make unique } @Override diff --git a/src/main/java/com/conveyal/gtfs/model/Trip.java b/src/main/java/com/conveyal/gtfs/model/Trip.java index 601d4d60d..d0a16380e 100644 --- a/src/main/java/com/conveyal/gtfs/model/Trip.java +++ b/src/main/java/com/conveyal/gtfs/model/Trip.java @@ -20,6 +20,11 @@ public class Trip extends Entity { public int wheelchair_accessible; public String feed_id; + @Override + public String getId() { + return trip_id; + } + public static class Loader extends Entity.Loader { public Loader(GTFSFeed feed) { @@ -47,7 +52,7 @@ public void loadOneRow() throws IOException { t.bikes_allowed = getIntField("bikes_allowed", false, 0, 2); t.wheelchair_accessible = getIntField("wheelchair_accessible", false, 0, 2); t.feed_id = feed.feedId; - feed.trips.put(t.trip_id, t); + insertCheckingDuplicateKey(feed.trips, t, "trip_id"); /* Check referential integrity without storing references. Trip cannot directly reference Services or From a7fb443cc6a3a1e5d0027ff91ebdc4bad542a5d4 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 17 Mar 2023 18:38:07 +0800 Subject: [PATCH 12/18] always checkGridSize when creating grids (#844) --- .../analysis/models/AnalysisRequest.java | 19 ------- .../java/com/conveyal/r5/analyst/Grid.java | 5 ++ .../r5/analyst/WebMercatorExtents.java | 50 +++++++++++++++++-- .../com/conveyal/r5/analyst/GridTest.java | 40 ++++++++------- .../network/GridSinglePointTaskBuilder.java | 4 ++ .../network/RandomFrequencyPhasingTests.java | 1 + 6 files changed, 78 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java index daad437e1..10d326656 100644 --- a/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java +++ b/src/main/java/com/conveyal/analysis/models/AnalysisRequest.java @@ -28,9 +28,6 @@ * sends/forwards to R5 workers (see {@link AnalysisWorkerTask}), though it has many of the same fields. */ public class AnalysisRequest { - private static int MIN_ZOOM = 9; - private static int MAX_ZOOM = 12; - private static int MAX_GRID_CELLS = 5_000_000; /** * These three IDs are redundant, and just help reduce the number of database lookups necessary. @@ -207,7 +204,6 @@ public void populateTask (AnalysisWorkerTask task, UserPermissions userPermissio // TODO define class with static factory function WebMercatorGridBounds.fromLatLonBounds(). // Also include getIndex(x, y), getX(index), getY(index), totalTasks() WebMercatorExtents extents = WebMercatorExtents.forWgsEnvelope(bounds.envelope(), zoom); - checkGridSize(extents); task.height = extents.height; task.north = extents.north; task.west = extents.west; @@ -271,21 +267,6 @@ public void populateTask (AnalysisWorkerTask task, UserPermissions userPermissio } } - private static void checkGridSize (WebMercatorExtents extents) { - if (extents.zoom < MIN_ZOOM || extents.zoom > MAX_ZOOM) { - throw AnalysisServerException.badRequest(String.format( - "Requested zoom (%s) is outside valid range (%s - %s)", extents.zoom, MIN_ZOOM, MAX_ZOOM - )); - } - if (extents.height * extents.width > MAX_GRID_CELLS) { - throw AnalysisServerException.badRequest(String.format( - "Requested number of destinations (%s) exceeds limit (%s). " + - "Set smaller custom geographic bounds or a lower zoom level.", - extents.height * extents.width, MAX_GRID_CELLS - )); - } - } - private EnumSet getEnumSetFromString (String s) { if (s != null && !"".equals(s)) { return EnumSet.copyOf(Arrays.stream(s.split(",")).map(LegMode::valueOf).collect(Collectors.toList())); diff --git a/src/main/java/com/conveyal/r5/analyst/Grid.java b/src/main/java/com/conveyal/r5/analyst/Grid.java index 7a11f595e..60bb526c6 100644 --- a/src/main/java/com/conveyal/r5/analyst/Grid.java +++ b/src/main/java/com/conveyal/r5/analyst/Grid.java @@ -107,6 +107,11 @@ public Grid (int west, int north, int width, int height, int zoom) { this(new WebMercatorExtents(west, north, width, height, zoom)); } + /** + * Other constructors and factory methods all call this one, and Grid has a WebMercatorExtents field, which means + * Grid construction always follows construction of a WebMercatorExtents, whose constructor always performs a + * check on the size of the grid. + */ public Grid (WebMercatorExtents extents) { this.extents = extents; this.grid = new double[extents.width][extents.height]; diff --git a/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java b/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java index 150043b91..0d75a79ca 100644 --- a/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java +++ b/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java @@ -1,5 +1,6 @@ package com.conveyal.r5.analyst; +import com.conveyal.analysis.AnalysisServerException; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; import org.geotools.geometry.jts.ReferencedEnvelope; import org.geotools.referencing.CRS; @@ -17,8 +18,8 @@ import static com.google.common.base.Preconditions.checkState; /** - * Really we should be embedding one of these in the tasks, grids, etc. to factor out all the common fields. - * Equals and hashcode are semantic, for use as or within hashtable keys. + * Really we should have a field pointing to an instance of this in tasks, grids, etc. to factor out all the common + * fields. Equals and hashcode are semantic, for use as or within hashtable keys. * * TODO may want to distinguish between WebMercatorExtents, WebMercatorGrid (adds lat/lon conversion functions), * and OpportunityGrid (AKA Grid) which adds opportunity counts. These can compose, not necessarily subclass. @@ -26,6 +27,10 @@ */ public class WebMercatorExtents { + private static final int MIN_ZOOM = 9; + private static final int MAX_ZOOM = 12; + private static final int MAX_GRID_CELLS = 5_000_000; + /** The pixel number of the westernmost pixel (smallest x value). */ public final int west; @@ -44,12 +49,17 @@ public class WebMercatorExtents { /** Web mercator zoom level. */ public final int zoom; + /** + * All factory methods or constructors for WebMercatorExtents should eventually call this constructor, + * as it will enforce size constraints that prevent straining or stalling the system. + */ public WebMercatorExtents (int west, int north, int width, int height, int zoom) { this.west = west; this.north = north; this.width = width; this.height = height; this.zoom = zoom; + checkGridSize(); } /** @@ -84,7 +94,11 @@ public static WebMercatorExtents forPointsets (PointSet[] pointSets) { } } - /** Create the minimum-size immutable WebMercatorExtents containing both this one and the other one. */ + /** + * Create the minimum-size immutable WebMercatorExtents containing both this one and the other one. + * Note that WebMercatorExtents fields are immutable, and this method does not modify the instance in place. + * It creates a new instance. This behavior differs from GeoTools / JTS Envelopes. + */ public WebMercatorExtents expandToInclude (WebMercatorExtents other) { checkState(this.zoom == other.zoom, "All grids supplied must be at the same zoom level."); final int thisEast = this.west + this.width; @@ -207,4 +221,34 @@ public static Coordinate mercatorPixelToMeters (double xPixel, double yPixel, in return new Coordinate(xMeters, yMeters); } + /** + * Users may create very large grids in various ways. For example, by setting large custom analysis bounds or by + * uploading spatial data sets with very large extents. This method checks some limits on the zoom level and total + * number of cells to avoid straining or stalling the system. + * + * The fields of WebMercatorExtents are immutable and are not typically deserialized from incoming HTTP API + * requests. As all instances are created through a constructor, so we can perform this check every time a grid is + * created. If we do eventually deserialize these from HTTP API requests, we'll have to call the check explicitly. + * The Grid class internally uses a WebMercatorExtents field, so dimensions are also certain to be checked while + * constructing a Grid. + * + * An analysis destination grid might become problematic at a smaller size than an opportunity data grid. But until + * we have a reason to distinguish different situations, MAX_GRID_CELLS is defined as a constant in this class. + * If needed, we can make the max size a method parameter and pass in different limits in different contexts. + */ + public void checkGridSize () { + if (this.zoom < MIN_ZOOM || this.zoom > MAX_ZOOM) { + throw AnalysisServerException.badRequest(String.format( + "Requested zoom (%s) is outside valid range (%s - %s)", this.zoom, MIN_ZOOM, MAX_ZOOM + )); + } + if (this.height * this.width > MAX_GRID_CELLS) { + throw AnalysisServerException.badRequest(String.format( + "Requested number of destinations (%s) exceeds limit (%s). " + + "Set smaller custom geographic bounds or a lower zoom level.", + this.height * this.width, MAX_GRID_CELLS + )); + } + } + } diff --git a/src/test/java/com/conveyal/r5/analyst/GridTest.java b/src/test/java/com/conveyal/r5/analyst/GridTest.java index 6798f548e..03149d1c2 100644 --- a/src/test/java/com/conveyal/r5/analyst/GridTest.java +++ b/src/test/java/com/conveyal/r5/analyst/GridTest.java @@ -26,31 +26,33 @@ public class GridTest { @Test public void testGetMercatorEnvelopeMeters() throws Exception { // Southeastern Australia - // Correct meter coordinates from http://www.maptiler.org/google-maps-coordinates-tile-bounds-projection/ - int zoom = 4; - int xTile = 14; - int yTile = 9; + // Reference coordinates in meters from http://www.maptiler.org/google-maps-coordinates-tile-bounds-projection/ + int zoom = 9; + int xTile = 465; + int yTile = 312; Grid grid = new Grid(256 * xTile, 256 * yTile, 256, 256, zoom); ReferencedEnvelope envelope = grid.getWebMercatorExtents().getMercatorEnvelopeMeters(); - assertEquals(15028131.257091936, envelope.getMinX(), 0.1); - assertEquals(-5009377.085697312, envelope.getMinY(), 0.1); - assertEquals(17532819.79994059, envelope.getMaxX(), 0.1); - assertEquals(-2504688.542848654, envelope.getMaxY(), 0.1); - - // Cutting through Paris - zoom = 5; - xTile = 16; - yTile = 11; + assertEquals(16358747.0, envelope.getMinX(), 1); + assertEquals(-4461476.0, envelope.getMinY(), 1); + assertEquals(16437019.0, envelope.getMaxX(), 1); + assertEquals(-4383205.0, envelope.getMaxY(), 1); + + // The tile east of Greenwich + // Reference coordinates in meters from http://www.maptiler.org/google-maps-coordinates-tile-bounds-projection/ + // Expect the zero edge to be more exact, others to within one meter. + zoom = 12; + xTile = 2048; + yTile = 1362; grid = new Grid(256 * xTile, 256 * yTile, 256, 256, zoom); envelope = grid.getWebMercatorExtents().getMercatorEnvelopeMeters(); - assertEquals(0, envelope.getMinX(), 0.1); - assertEquals(5009377.085697312, envelope.getMinY(), 0.1); - assertEquals(1252344.271424327, envelope.getMaxX(), 0.1); - assertEquals(6261721.357121639, envelope.getMaxY(), 0.1); + assertEquals(0.0, envelope.getMinX(), 0.01); + assertEquals(6701999.0, envelope.getMinY(), 1); + assertEquals(9784.0, envelope.getMaxX(), 1); + assertEquals(6711783.0, envelope.getMaxY(), 1); // /** -// * Make sure the Mercator projection works properly. Open the resulting file in GIS and -// * compare with http://www.maptiler.org/google-maps-coordinates-tile-bounds-projection/ +// * Uncomment to manually verify that the Mercator projection works properly. Open the resulting file in GIS +// * and compare with http://www.maptiler.org/google-maps-coordinates-tile-bounds-projection/ // */ // OutputStream outputStream = new FileOutputStream("test.tiff"); // grid.writeGeotiff(outputStream); diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java b/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java index a4b71fd9c..431dfcfbc 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java @@ -108,6 +108,10 @@ public GridSinglePointTaskBuilder maxRides(int rides) { return this; } + /** + * Even if you're not actually using the opportunity count, you should call this to set the grid extents on the + * resulting task. Otherwise it will fail checks on the grid dimensions and zoom level. + */ public GridSinglePointTaskBuilder uniformOpportunityDensity (double density) { Grid grid = gridLayout.makeUniformOpportunityDataset(density); task.destinationPointSets = new PointSet[] { grid }; diff --git a/src/test/java/com/conveyal/r5/analyst/network/RandomFrequencyPhasingTests.java b/src/test/java/com/conveyal/r5/analyst/network/RandomFrequencyPhasingTests.java index 4f762a631..15370f3e7 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/RandomFrequencyPhasingTests.java +++ b/src/test/java/com/conveyal/r5/analyst/network/RandomFrequencyPhasingTests.java @@ -52,6 +52,7 @@ public void testFilteredTripRandomization () throws Exception { .weekendMorningPeak() .setOrigin(20, 20) .monteCarloDraws(1000) + .uniformOpportunityDensity(10) .build(); TravelTimeComputer computer = new TravelTimeComputer(task, network); From 2817bd4de2f21b10a3e6aaf1b34b34e72f5aada6 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 17 Mar 2023 19:45:27 +0800 Subject: [PATCH 13/18] correct comments about polyline encoder --- src/main/java/com/conveyal/gtfs/model/Pattern.java | 10 ++++++++-- .../conveyal/r5/util/EncodedPolylineSerializer.java | 12 +++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/model/Pattern.java b/src/main/java/com/conveyal/gtfs/model/Pattern.java index 3cd86456c..c2eeae69f 100644 --- a/src/main/java/com/conveyal/gtfs/model/Pattern.java +++ b/src/main/java/com/conveyal/gtfs/model/Pattern.java @@ -24,9 +24,15 @@ public class Pattern implements Serializable { // TODO: add set of shapes // public Set associatedShapes; - // This is the only place in the library we are still using the old JTS package name. These objects are - // serialized into MapDB files. We want to read and write MapDB files that old workers can understand. + // This is the only place we are still using the old JTS package name. + // Hopefully we can get rid of this - it's the only thing still using JTS objects under the obsolete vividsolutions + // package name so is pulling in extra dependencies and requiring conversions (toLegacyLineString). + // Unfortunately these objects are serialized into MapDB files, and we want to read and write MapDB files that + // old workers can understand. This cannot be migrated to newer JTS package names without deprecating all older + // workers, then deleting all MapDB representations of GTFS data from S3 and the local cache directory, forcing + // them to all be rebuilt the next time they're used. public com.vividsolutions.jts.geom.LineString geometry; + public String name; public String route_id; public static Joiner joiner = Joiner.on("-").skipNulls(); diff --git a/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java b/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java index ee2d4ffdb..eb440fc26 100644 --- a/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java +++ b/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java @@ -11,9 +11,15 @@ import java.io.IOException; /** - * Serialize to Google encoded polyline. - * Hopefully we can get rid of this - it's the only thing still using JTS objects under the vividsolutions package name - * so is pulling in extra dependencies and requiring conversions (toLegacyLineString). + * Serialize JTS LineString to Google encoded polyline. + * + * This class is the only use of dependency com.axiomalaska:polyline-encoder, and it is only used in + * com.conveyal.r5.transitive.TransitivePattern, which is in turn only used in + * com.conveyal.r5.transitive.TransitiveNetwork, which is in turn only used in + * com.conveyal.r5.analyst.cluster.AnalysisWorker#saveTauiMetadata. + * That dependency has required maintainance on a few occasions and is hosted at a repo outside Maven Central which has + * become unavailable on a few occations. We should evaluate licence compatibility (LGPL) for copying it into this repo + * ("vendoring" it). */ public class EncodedPolylineSerializer extends JsonSerializer { From 92cb3a3d4857acfe5f49896e0866fe9e1a9991aa Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 17 Mar 2023 20:43:22 +0800 Subject: [PATCH 14/18] avoid hard NPE failure hiding underlying problems Skip making patterns when severe problems like referential integrity errors are present, so those errors are visible instead of just NPE. --- src/main/java/com/conveyal/gtfs/GTFSFeed.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/GTFSFeed.java b/src/main/java/com/conveyal/gtfs/GTFSFeed.java index f1f380601..83236ad01 100644 --- a/src/main/java/com/conveyal/gtfs/GTFSFeed.java +++ b/src/main/java/com/conveyal/gtfs/GTFSFeed.java @@ -1,6 +1,7 @@ package com.conveyal.gtfs; import com.conveyal.gtfs.error.GTFSError; +import com.conveyal.gtfs.error.ReferentialIntegrityError; import com.conveyal.gtfs.model.Agency; import com.conveyal.gtfs.model.Calendar; import com.conveyal.gtfs.model.CalendarDate; @@ -19,6 +20,7 @@ import com.conveyal.gtfs.model.StopTime; import com.conveyal.gtfs.model.Transfer; import com.conveyal.gtfs.model.Trip; +import com.conveyal.gtfs.validator.model.Priority; import com.conveyal.gtfs.validator.service.GeoUtils; import com.conveyal.r5.analyst.progress.ProgressListener; import com.google.common.collect.HashMultimap; @@ -253,7 +255,15 @@ public void loadFromFile(ZipFile zip, String fid) throws Exception { // There are conceivably cases where the extra step of identifying and naming patterns is not necessary. // In current usage we do always need them, and performing this step during load allows enforcing subsequent // read-only access. - findPatterns(); + // Find patterns only if there are no referential integrity errors or other severe problems. Those problems + // can cause pattern finding to fail hard with null pointer exceptions, causing detailed error messages to be + // lost and hiding underlying problems from the user. If high-priority problems are present, the feed should be + // presented to the user as unuseable anyway. + if (errors.stream().anyMatch(e -> e.getPriority() == Priority.HIGH)) { + LOG.warn("Feed contains high priority errors, not finding patterns. It will be useless for routing."); + } else { + findPatterns(); + } // Prevent loading additional feeds into this MapDB. loaded = true; @@ -576,11 +586,13 @@ private void namePatterns(Collection patterns) { public LineString getStraightLineForStops(String trip_id) { CoordinateList coordinates = new CoordinateList(); - LineString ls = null; + LineString lineString = null; Trip trip = trips.get(trip_id); Iterable stopTimes; stopTimes = getOrderedStopTimesForTrip(trip.trip_id); + // lineString must remain null if there are less than two stopTimes to avoid + // an exception when creating linestring. if (Iterables.size(stopTimes) > 1) { for (StopTime stopTime : stopTimes) { Stop stop = stops.get(stopTime.stop_id); @@ -588,13 +600,9 @@ public LineString getStraightLineForStops(String trip_id) { Double lon = stop.stop_lon; coordinates.add(new Coordinate(lon, lat)); } - ls = geometryFactory.createLineString(coordinates.toCoordinateArray()); + lineString = geometryFactory.createLineString(coordinates.toCoordinateArray()); } - // set ls equal to null if there is only one stopTime to avoid an exception when creating linestring - else{ - ls = null; - } - return ls; + return lineString; } /** From 45d6bdcd82e341d44626988bb3134fe4ce7b6f6b Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Fri, 17 Mar 2023 21:08:59 +0800 Subject: [PATCH 15/18] consistently set error line number from row field this is zero-based and will not match values in the Entity objects themselves, or values produced in post-validation. Adjusting them all to be one-based is a separate task. --- src/main/java/com/conveyal/gtfs/model/Entity.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/gtfs/model/Entity.java b/src/main/java/com/conveyal/gtfs/model/Entity.java index 215d08072..a2395b473 100644 --- a/src/main/java/com/conveyal/gtfs/model/Entity.java +++ b/src/main/java/com/conveyal/gtfs/model/Entity.java @@ -314,13 +314,13 @@ public void loadTable (ZipFile zip) throws IOException { protected void insertCheckingDuplicateKey (Map map, E value, String keyField) { String key = value.getId(); if (key == null) { - feed.errors.add(new MissingKeyError(tableName, value.sourceFileLine, keyField)); + feed.errors.add(new MissingKeyError(tableName, row, keyField)); return; } // Map returns previous value if one was already present E previousValue = map.put(key, value); if (previousValue != null) { - feed.errors.add(new DuplicateKeyError(tableName, value.sourceFileLine, keyField, key)); + feed.errors.add(new DuplicateKeyError(tableName, row, keyField, key)); } } } From 8545de9f2ee12f240e910d42a944c96e57459348 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Sat, 18 Mar 2023 01:25:33 +0800 Subject: [PATCH 16/18] get polyline encoder 0.2 from Conveyal Maven repo --- build.gradle | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 3ab0dbb0c..618a7bb88 100644 --- a/build.gradle +++ b/build.gradle @@ -125,10 +125,8 @@ repositories { // Put Open Source Geospatial before Maven Central to get JAI core, see https://stackoverflow.com/a/26993223 maven { url 'https://repo.osgeo.org/repository/release/' } mavenCentral() - // TODO review whether we really need the repositories below + // Polyline encoder 0.2 is now in Maven repo maven { url 'https://maven.conveyal.com' } - // For the polyline encoder - maven { url 'https://nexus.axiomalaska.com/nexus/content/repositories/public-releases' } } // Exclude all JUnit 4 transitive dependencies - IntelliJ bug causes it to think we're using Junit 4 instead of 5. From 9079edf191caabd4cf6b093a562bf013a79bfff1 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Sat, 18 Mar 2023 01:25:33 +0800 Subject: [PATCH 17/18] get polyline encoder 0.2 from Conveyal Maven repo --- build.gradle | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 3ab0dbb0c..618a7bb88 100644 --- a/build.gradle +++ b/build.gradle @@ -125,10 +125,8 @@ repositories { // Put Open Source Geospatial before Maven Central to get JAI core, see https://stackoverflow.com/a/26993223 maven { url 'https://repo.osgeo.org/repository/release/' } mavenCentral() - // TODO review whether we really need the repositories below + // Polyline encoder 0.2 is now in Maven repo maven { url 'https://maven.conveyal.com' } - // For the polyline encoder - maven { url 'https://nexus.axiomalaska.com/nexus/content/repositories/public-releases' } } // Exclude all JUnit 4 transitive dependencies - IntelliJ bug causes it to think we're using Junit 4 instead of 5. From c8adfeca7ce80454bf3da4313318447f31bae38e Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Tue, 21 Mar 2023 09:11:08 +0800 Subject: [PATCH 18/18] Update javadoc about dependency --- .../java/com/conveyal/r5/util/EncodedPolylineSerializer.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java b/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java index eb440fc26..ee5e8ba35 100644 --- a/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java +++ b/src/main/java/com/conveyal/r5/util/EncodedPolylineSerializer.java @@ -17,9 +17,8 @@ * com.conveyal.r5.transitive.TransitivePattern, which is in turn only used in * com.conveyal.r5.transitive.TransitiveNetwork, which is in turn only used in * com.conveyal.r5.analyst.cluster.AnalysisWorker#saveTauiMetadata. - * That dependency has required maintainance on a few occasions and is hosted at a repo outside Maven Central which has - * become unavailable on a few occations. We should evaluate licence compatibility (LGPL) for copying it into this repo - * ("vendoring" it). + * That dependency has required maintainance on a few occasions and was hosted at a repo outside Maven Central which has + * become unavailable on a few occations. We have copied the artifact to our S3-backed Conveyal Maven repo. */ public class EncodedPolylineSerializer extends JsonSerializer {