From aa2f44cb622acec82f2e8c5ec2304136836c6b3e Mon Sep 17 00:00:00 2001 From: ansons Date: Fri, 15 Mar 2024 16:46:13 -0400 Subject: [PATCH 1/6] feat(grid-dual-access): initial test --- .../results/MultiOriginAssembler.java | 3 +- .../r5/analyst/TemporalDensityResult.java | 31 +++++++++++++++++++ .../r5/analyst/TravelTimeReducer.java | 2 +- .../analyst/cluster/RegionalWorkResult.java | 2 +- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/conveyal/analysis/results/MultiOriginAssembler.java b/src/main/java/com/conveyal/analysis/results/MultiOriginAssembler.java index dc2b0b150..6aa3c9fad 100644 --- a/src/main/java/com/conveyal/analysis/results/MultiOriginAssembler.java +++ b/src/main/java/com/conveyal/analysis/results/MultiOriginAssembler.java @@ -139,7 +139,8 @@ public MultiOriginAssembler (RegionalAnalysis regionalAnalysis, Job job, FileSto // We might want to record a grid of dual accessibility values, but this will require some serious // refactoring of the GridResultWriter. // if (job.templateTask.dualAccessibilityThreshold > 0) { ... } - throw new IllegalArgumentException("Temporal density of opportunities cannot be recorded for gridded origin points."); + // throw new IllegalArgumentException("Temporal density of opportunities cannot be recorded for " + + // "gridded origin points."); } else { // Freeform origins. // Output includes temporal density of opportunities and optionally dual accessibility. diff --git a/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java b/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java index 6ba2294b4..573dd9075 100644 --- a/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java +++ b/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java @@ -1,6 +1,7 @@ package com.conveyal.r5.analyst; import com.conveyal.r5.analyst.cluster.AnalysisWorkerTask; +import com.conveyal.r5.analyst.cluster.RegionalTask; import com.google.common.base.Preconditions; import static com.conveyal.r5.common.Util.notNullOrEmpty; @@ -89,6 +90,36 @@ public int[][] minutesToReachOpportunities(int n) { return result; } + public int[][][] fakeDualAccess (RegionalTask task) { + int nPointSets = task.destinationPointSets.length; + int nCutoffs = task.cutoffsMinutes.length; + int[][][] dualAccess = new int[nPointSets][nPercentiles][nCutoffs]; + for (int d = 0; d < nPointSets; d++) { + for (int p = 0; p < nPercentiles; p++) { + // Hack: use cutoffs as dual access thresholds + for (int c = 0; c < nCutoffs - 1; c++) { + int m = 0; + double sum = 0; + while (sum < task.cutoffsMinutes[c] && m < 120) { + sum += opportunitiesPerMinute[d][p][m]; + m += 1; + } + dualAccess[d][p][c] = m == 0 ? 999 : m; + } + // But hack above won't allow thresholds over 120; so use the dualAccessibilityThreshold instead of + // the last cutoff + int m = 0; + double sum = 0; + while (sum < task.dualAccessibilityThreshold && m < 120) { + sum += opportunitiesPerMinute[d][p][m]; + m += 1; + } + dualAccess[d][p][nCutoffs - 1] = m == 0 ? 999 : m; + } + } + return dualAccess; + } + public int[][] minutesToReachOpportunities() { return minutesToReachOpportunities(opportunityThreshold); } diff --git a/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java b/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java index 213091d56..a4d5c50b2 100644 --- a/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java +++ b/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java @@ -145,7 +145,7 @@ public TravelTimeReducer (AnalysisWorkerTask task, TransportNetwork network) { if (task.includePathResults) { pathResult = new PathResult(task, network.transitLayer); } - if (task.includeTemporalDensity) { + if (task.includeTemporalDensity || task.flags.contains("gridDualAccess")) { temporalDensityResult = new TemporalDensityResult(task); } diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java index c606acc0f..7f10e77b8 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java @@ -63,7 +63,7 @@ public RegionalWorkResult(OneOriginResult result, RegionalTask task) { this.jobId = task.jobId; this.taskId = task.taskId; this.travelTimeValues = result.travelTimes == null ? null : result.travelTimes.values; - this.accessibilityValues = result.accessibility == null ? null : result.accessibility.getIntValues(); + this.accessibilityValues = result.accessibility == null ? null : result.density.fakeDualAccess(task); this.pathResult = result.paths == null ? null : result.paths.summarizeIterations(PathResult.Stat.MINIMUM); this.opportunitiesPerMinute = result.density == null ? null : result.density.opportunitiesPerMinute; // TODO checkTravelTimeInvariants, checkAccessibilityInvariants to verify that values are monotonically increasing From 81e015157334fb516cec29ff76736e558091ea47 Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 17 Mar 2024 21:23:02 -0400 Subject: [PATCH 2/6] feat(grid-dual-access): add comments And return 0 for origins without access (instead of magic number) --- .../r5/analyst/TemporalDensityResult.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java b/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java index 573dd9075..597fe49cd 100644 --- a/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java +++ b/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java @@ -90,6 +90,17 @@ public int[][] minutesToReachOpportunities(int n) { return result; } + /** + * Writes dual access travel time values (in minutes) to our standard access grid format. The value returned (for + * an origin) is the number of minutes required to reach a threshold number of opportunities (specified by + * the cutoffs and task.dualAccessibilityThreshold) in the specified destination layer at a given percentile of + * travel time. If the threshold cannot be reached in less than 120 minutes, returns 0. + * This is a temporary experimental feature, (ab)using existing features in the UI and backend so that grid access + * results can be obtained without any changes to those other components of our system. It uses the supplied + * task.cutoffsMinutes, except for the last one, as dual access thresholds. In place of the last cutoffsMinutes + * value, it uses task.dualAccessibilityThreshold (which is initialized to 0, so this is safe even if a user does + * not supply it). + */ public int[][][] fakeDualAccess (RegionalTask task) { int nPointSets = task.destinationPointSets.length; int nCutoffs = task.cutoffsMinutes.length; @@ -104,17 +115,17 @@ public int[][][] fakeDualAccess (RegionalTask task) { sum += opportunitiesPerMinute[d][p][m]; m += 1; } - dualAccess[d][p][c] = m == 0 ? 999 : m; + dualAccess[d][p][c] = m; } - // But hack above won't allow thresholds over 120; so use the dualAccessibilityThreshold instead of - // the last cutoff + // But the hack above won't allow thresholds over 120 (see validateCutoffsMinutes()); so use the + // dualAccessibilityThreshold instead of the last cutoff. int m = 0; double sum = 0; while (sum < task.dualAccessibilityThreshold && m < 120) { sum += opportunitiesPerMinute[d][p][m]; m += 1; } - dualAccess[d][p][nCutoffs - 1] = m == 0 ? 999 : m; + dualAccess[d][p][nCutoffs - 1] = m; } } return dualAccess; From 3d6041207626b152408bd8f60e7a10660ffe2c2b Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 17 Mar 2024 21:23:38 -0400 Subject: [PATCH 3/6] fix(grid-dual-access): properly trigger with flag --- .../java/com/conveyal/r5/analyst/TravelTimeReducer.java | 2 +- .../conveyal/r5/analyst/cluster/AnalysisWorkerTask.java | 4 ++++ .../conveyal/r5/analyst/cluster/RegionalWorkResult.java | 8 +++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java b/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java index a4d5c50b2..8fe50efc3 100644 --- a/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java +++ b/src/main/java/com/conveyal/r5/analyst/TravelTimeReducer.java @@ -145,7 +145,7 @@ public TravelTimeReducer (AnalysisWorkerTask task, TransportNetwork network) { if (task.includePathResults) { pathResult = new PathResult(task, network.transitLayer); } - if (task.includeTemporalDensity || task.flags.contains("gridDualAccess")) { + if (task.includeTemporalDensity || task.hasFlag("gridDualAccess")) { temporalDensityResult = new TemporalDensityResult(task); } diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java index 2698905fa..59a6936a6 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorkerTask.java @@ -323,4 +323,8 @@ public void validateCutoffsMinutes () { } } + public boolean hasFlag (String flag) { + return this.flags != null && this.flags.contains(flag); + } + } diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java index 7f10e77b8..e57bb09de 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java @@ -63,7 +63,13 @@ public RegionalWorkResult(OneOriginResult result, RegionalTask task) { this.jobId = task.jobId; this.taskId = task.taskId; this.travelTimeValues = result.travelTimes == null ? null : result.travelTimes.values; - this.accessibilityValues = result.accessibility == null ? null : result.density.fakeDualAccess(task); + if (result.accessibility == null) { + this.accessibilityValues = null; + } else if (task.flags.contains("gridDualAccess")) { + this.accessibilityValues = result.density.fakeDualAccess(task); + } else { + this.accessibilityValues = result.accessibility.getIntValues(); + } this.pathResult = result.paths == null ? null : result.paths.summarizeIterations(PathResult.Stat.MINIMUM); this.opportunitiesPerMinute = result.density == null ? null : result.density.opportunitiesPerMinute; // TODO checkTravelTimeInvariants, checkAccessibilityInvariants to verify that values are monotonically increasing From e23fe25d4b792e0e0d8c03fb3601079e28d9680d Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 17 Mar 2024 21:55:04 -0400 Subject: [PATCH 4/6] feat(grid-dual-access): use last cutoff unless nonzero task.dualAccessibilityThreshold value is supplied --- .../r5/analyst/TemporalDensityResult.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java b/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java index 597fe49cd..7651bba0c 100644 --- a/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java +++ b/src/main/java/com/conveyal/r5/analyst/TemporalDensityResult.java @@ -95,11 +95,12 @@ public int[][] minutesToReachOpportunities(int n) { * an origin) is the number of minutes required to reach a threshold number of opportunities (specified by * the cutoffs and task.dualAccessibilityThreshold) in the specified destination layer at a given percentile of * travel time. If the threshold cannot be reached in less than 120 minutes, returns 0. - * This is a temporary experimental feature, (ab)using existing features in the UI and backend so that grid access - * results can be obtained without any changes to those other components of our system. It uses the supplied - * task.cutoffsMinutes, except for the last one, as dual access thresholds. In place of the last cutoffsMinutes - * value, it uses task.dualAccessibilityThreshold (which is initialized to 0, so this is safe even if a user does - * not supply it). + * This is a temporary experimental feature, (ab)using existing features in the UI and backend so that dual access + * results for grid origins can be obtained without any changes to those other components of our system. It uses + * the supplied task.cutoffsMinutes as dual access thresholds. If a nonzero task.dualAccessibility In place of the + * last + * cutoffsMinutes value, it uses task.dualAccessibilityThreshold (which is initialized to 0, so this is safe even + * if a user does not supply it). */ public int[][][] fakeDualAccess (RegionalTask task) { int nPointSets = task.destinationPointSets.length; @@ -108,7 +109,7 @@ public int[][][] fakeDualAccess (RegionalTask task) { for (int d = 0; d < nPointSets; d++) { for (int p = 0; p < nPercentiles; p++) { // Hack: use cutoffs as dual access thresholds - for (int c = 0; c < nCutoffs - 1; c++) { + for (int c = 0; c < nCutoffs; c++) { int m = 0; double sum = 0; while (sum < task.cutoffsMinutes[c] && m < 120) { @@ -117,15 +118,17 @@ public int[][][] fakeDualAccess (RegionalTask task) { } dualAccess[d][p][c] = m; } - // But the hack above won't allow thresholds over 120 (see validateCutoffsMinutes()); so use the - // dualAccessibilityThreshold instead of the last cutoff. - int m = 0; - double sum = 0; - while (sum < task.dualAccessibilityThreshold && m < 120) { - sum += opportunitiesPerMinute[d][p][m]; - m += 1; + // But the hack above won't allow thresholds over 120 (see validateCutoffsMinutes()); so overwrite + // the value for the last cutoff if a nonzero task.dualAccessibilityThreshold value is supplied. + if (task.dualAccessibilityThreshold != 0) { + int m = 0; + double sum = 0; + while (sum < task.dualAccessibilityThreshold && m < 120) { + sum += opportunitiesPerMinute[d][p][m]; + m += 1; + } + dualAccess[d][p][nCutoffs - 1] = m; } - dualAccess[d][p][nCutoffs - 1] = m; } } return dualAccess; From 9d7380fff87097afafa3cf59afd36991a3368e4f Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 17 Mar 2024 22:02:13 -0400 Subject: [PATCH 5/6] fix(grid-dual-access): avoid NPE with flags --- .../com/conveyal/r5/analyst/cluster/RegionalWorkResult.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java index e57bb09de..96755c7fa 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/RegionalWorkResult.java @@ -65,7 +65,7 @@ public RegionalWorkResult(OneOriginResult result, RegionalTask task) { this.travelTimeValues = result.travelTimes == null ? null : result.travelTimes.values; if (result.accessibility == null) { this.accessibilityValues = null; - } else if (task.flags.contains("gridDualAccess")) { + } else if (task.hasFlag("gridDualAccess")) { this.accessibilityValues = result.density.fakeDualAccess(task); } else { this.accessibilityValues = result.accessibility.getIntValues(); From 10a51a836004af3a6d12a87b471c770d98424adb Mon Sep 17 00:00:00 2001 From: ansons Date: Sun, 17 Mar 2024 22:35:39 -0400 Subject: [PATCH 6/6] hack(grid-dual-access): force dual results to be reverted once broker with custom "flags" is deployed --- .../java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java index 2add61702..46bfed946 100644 --- a/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java +++ b/src/main/java/com/conveyal/r5/analyst/cluster/AnalysisWorker.java @@ -41,6 +41,7 @@ import java.util.Arrays; import java.util.List; import java.util.Random; +import java.util.Set; import java.util.UUID; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -407,6 +408,10 @@ protected void handleOneRegionalTask (RegionalTask task) throws Throwable { LOG.debug("Handling regional task {}", task.toString()); + // Force dual access results for grid (remove once broker has been relaunched to support custom flags) + LOG.info("Forcing dual access results"); + task.flags = Set.of("gridDualAccess"); + if (task.injectFault != null) { task.injectFault.considerShutdownOrException(task.taskId); if (task.injectFault.shouldDropTaskBeforeCompute(task.taskId)) {