From 9a1d11438f74e8aac261135731746a0cc163d736 Mon Sep 17 00:00:00 2001 From: Andrew Byrd Date: Wed, 18 Oct 2023 19:28:13 +0200 Subject: [PATCH] fix test code path for single destination also remove unused opportunity density grids --- .../r5/analyst/TravelTimeComputer.java | 6 ++-- .../r5/analyst/network/GridLayout.java | 24 ------------- .../network/GridSinglePointTaskBuilder.java | 35 ++++++++----------- .../network/RandomFrequencyPhasingTests.java | 1 - .../analyst/network/SimpsonDesertTests.java | 5 --- 5 files changed, 19 insertions(+), 52 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java b/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java index 05851609c..5f8edf7c4 100644 --- a/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java +++ b/src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java @@ -7,6 +7,7 @@ import com.conveyal.r5.analyst.fare.InRoutingFareCalculator; import com.conveyal.r5.analyst.scenario.PickupWaitTimes; import com.conveyal.r5.api.util.LegMode; +import com.conveyal.r5.common.Util; import com.conveyal.r5.point_to_point.builder.PointToPointQuery; import com.conveyal.r5.profile.DominatingList; import com.conveyal.r5.profile.FareDominatingList; @@ -30,6 +31,7 @@ import static com.conveyal.r5.analyst.scenario.PickupWaitTimes.NO_SERVICE_HERE; import static com.conveyal.r5.analyst.scenario.PickupWaitTimes.NO_WAIT_ALL_STOPS; +import static com.conveyal.r5.common.Util.isNullOrEmpty; import static com.conveyal.r5.profile.PerTargetPropagater.MM_PER_METER; /** @@ -87,8 +89,8 @@ public OneOriginResult computeTravelTimes() { ) { // Freeform destinations. Destination PointSet was set by handleOneRequest in the main AnalystWorker. destinations = request.destinationPointSets[0]; - } else if (request.destinationPointSets != null) { - LOG.warn("ONLY VALID IN TESTING: Using PointSet object embedded in request where this is not standard."); + } else if (!isNullOrEmpty(request.destinationPointSets)) { + LOG.warn("ONLY VALID IN TESTING: Using PointSet object embedded in request outside regional analysis."); destinations = request.destinationPointSets[0]; } else { // Gridded (non-freeform) destinations. This method finds them differently for regional and single requests. diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java index 99ef3dbfb..955789964 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java @@ -175,30 +175,6 @@ public Envelope gridEnvelope () { return new Envelope(originPoint.x, farCorner.x, originPoint.y, farCorner.y); } - /** - * Create a gridded pointset coextensive with this grid, with the given number of opportunities in each cell. - * TODO maybe derive this from the network's gridded pointset rather than the GridLayout. - */ - public Grid makeUniformOpportunityDataset (double density) { - Grid grid = new Grid(DEFAULT_ZOOM, this.gridEnvelope()); - for (double[] column : grid.grid) { - Arrays.fill(column, density); - } - return grid; - } - - /** - * Create a gridded pointset coextensive with this grid, with the given number of opportunities in each cell in the - * eastern half of the grid. - */ - public Grid makeRightHalfOpportunityDataset (double density) { - Grid grid = new Grid(DEFAULT_ZOOM, this.gridEnvelope()); - for (int c = grid.grid.length / 2; c < grid.grid.length; c++) { - Arrays.fill(grid.grid[c], density); - } - return grid; - } - public String nextIntegerId() { return Integer.toString(nextIntegerId++); } 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 80403ecbb..9165b6cea 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java +++ b/src/test/java/com/conveyal/r5/analyst/network/GridSinglePointTaskBuilder.java @@ -3,6 +3,7 @@ import com.conveyal.r5.analyst.FreeFormPointSet; import com.conveyal.r5.analyst.Grid; import com.conveyal.r5.analyst.PointSet; +import com.conveyal.r5.analyst.WebMercatorExtents; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; import com.conveyal.r5.analyst.cluster.TravelTimeSurfaceTask; import com.conveyal.r5.analyst.decay.StepDecayFunction; @@ -14,6 +15,7 @@ import java.util.EnumSet; import java.util.stream.IntStream; +import static com.conveyal.r5.analyst.WebMercatorGridPointSet.DEFAULT_ZOOM; import static com.conveyal.r5.analyst.network.GridGtfsGenerator.WEEKDAY_DATE; import static com.conveyal.r5.analyst.network.GridGtfsGenerator.WEEKEND_DATE; @@ -52,6 +54,14 @@ public GridSinglePointTaskBuilder (GridLayout gridLayout) { task.walkSpeed = gridLayout.streetGridSpacingMeters / gridLayout.walkBlockTraversalTimeSeconds; // Record more detailed information to allow comparison to theoretical travel time distributions. task.recordTravelTimeHistograms = true; + // Set the destination grid extents on the task, otherwise if no freeform PointSet is specifid, it will fail + // checks on the grid dimensions and zoom level. + WebMercatorExtents extents = WebMercatorExtents.forWgsEnvelope(gridLayout.gridEnvelope(), DEFAULT_ZOOM); + task.zoom = extents.zoom; + task.north = extents.north; + task.west = extents.west; + task.width = extents.width; + task.height = extents.height; } public GridSinglePointTaskBuilder setOrigin (int gridX, int gridY) { @@ -90,26 +100,6 @@ 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 }; - task.destinationPointSetKeys = new String[] { "GRID" }; - - // In a single point task, the grid of destinations is given with these fields, not from the pointset object. - // The destination point set (containing the opportunity densities) must then match these same dimensions. - task.zoom = grid.extents.zoom; - task.north = grid.extents.north; - task.west = grid.extents.west; - task.width = grid.extents.width; - task.height = grid.extents.height; - - return this; - } - /** * When trying to verify more complex distributions, the Monte Carlo approach may introduce too much noise. * Increasing the number of draws will yield a better approximation of the true travel time distribution @@ -121,6 +111,9 @@ public GridSinglePointTaskBuilder monteCarloDraws (int draws) { } /** + * Create a FreeformPointSet with a single point in it situated at the specified street intersection, and embed + * that PointSet in the request. In normal usage supplying FreeformPointSets as destination is only done for + * regional analysis tasks, but a testing code path exists to handle their presence on single point requests. * This eliminates any difficulty estimating the final segment of egress, walking from the street to a gridded * travel time sample point. Although egress time is something we'd like to test too, it is not part of the transit * routing we're concentrating on here, and will vary as the Simpson Desert street grid does not align with our @@ -129,6 +122,8 @@ public GridSinglePointTaskBuilder monteCarloDraws (int draws) { */ public GridSinglePointTaskBuilder singleFreeformDestination(int x, int y) { FreeFormPointSet ps = new FreeFormPointSet(gridLayout.getIntersectionLatLon(x, y)); + // Downstream code expects to see the same number of keys and PointSet objects so initialize both. + task.destinationPointSetKeys = new String[] { "POINT_SET" }; task.destinationPointSets = new PointSet[] { ps }; return this; } 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 15370f3e7..4f762a631 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/RandomFrequencyPhasingTests.java +++ b/src/test/java/com/conveyal/r5/analyst/network/RandomFrequencyPhasingTests.java @@ -52,7 +52,6 @@ public void testFilteredTripRandomization () throws Exception { .weekendMorningPeak() .setOrigin(20, 20) .monteCarloDraws(1000) - .uniformOpportunityDensity(10) .build(); TravelTimeComputer computer = new TravelTimeComputer(task, network); diff --git a/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java b/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java index 3791493dc..9d60dc301 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java +++ b/src/test/java/com/conveyal/r5/analyst/network/SimpsonDesertTests.java @@ -50,7 +50,6 @@ public void testGridScheduled () throws Exception { AnalysisWorkerTask task = gridLayout.newTaskBuilder() .weekdayMorningPeak() .setOrigin(20, 20) - .uniformOpportunityDensity(10) .singleFreeformDestination(40, 40) .build(); @@ -88,7 +87,6 @@ public void testGridFrequency () throws Exception { AnalysisWorkerTask task = gridLayout.newTaskBuilder() .weekdayMorningPeak() .setOrigin(20, 20) - .uniformOpportunityDensity(10) .singleFreeformDestination(40, 40) .build(); @@ -123,7 +121,6 @@ public void testGridFrequencyAlternatives () throws Exception { AnalysisWorkerTask task = gridLayout.newTaskBuilder() .weekdayMorningPeak() .setOrigin(20, 20) - .uniformOpportunityDensity(10) .singleFreeformDestination(40, 40) .monteCarloDraws(10000) .build(); @@ -187,7 +184,6 @@ public void testOvertakingCases () throws Exception { .departureTimeWindow(7, 0, 5) .maxRides(1) .setOrigin(30, 50) - .uniformOpportunityDensity(10) .singleFreeformDestination(42, 50); // 1. Standard rider: upstream overtaking means Trip B departs origin first and is fastest to destination. @@ -230,7 +226,6 @@ public void testExperiments () throws Exception { AnalysisWorkerTask task = gridLayout.newTaskBuilder() .weekdayMorningPeak() .setOrigin(20, 20) - .uniformOpportunityDensity(10) .singleFreeformDestination(80, 80) .monteCarloDraws(10000) .build();