Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feed id column to path results #927

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7060665
fix final and static field modifiers in broker
abyrd Dec 20, 2023
4a5b721
Append feedId values to alighting stops
ansoncfit Jan 23, 2024
990a542
Write feedIds as separate column
ansoncfit Jan 23, 2024
f71a72c
Append feedId values to alighting stop list
ansoncfit Jan 23, 2024
c9be778
Revert "Append feedId values to alighting stop list"
ansoncfit Jan 23, 2024
32eb1dc
Exclude modifications for regional analysis lookup (#926)
trevorgerhardt Jan 26, 2024
d1b202d
Merge branch 'dev' into broker-static-final
abyrd Jan 26, 2024
12a1b7a
Merge pull request #922 from conveyal/broker-static-final
abyrd Jan 26, 2024
06d9bee
include route, stop names in regional path results
abyrd Nov 6, 2023
d5f5bec
enable/disable ids and names csv with string flags
abyrd Feb 2, 2024
2376c4e
keepRoutingOnFoot only in transit access searches
abyrd Feb 13, 2024
e11fc19
nested CsvResultOptions in requests
abyrd Feb 16, 2024
d12f763
Merge pull request #931 from conveyal/conditional-foot-routing
ansoncfit Feb 16, 2024
119ffc1
Update src/main/java/com/conveyal/analysis/models/AnalysisRequest.java
abyrd Feb 29, 2024
c92bf7b
Update src/main/java/com/conveyal/analysis/models/CsvResultOptions.java
abyrd Feb 29, 2024
eca7051
Update src/main/java/com/conveyal/analysis/models/AnalysisRequest.java
abyrd Feb 29, 2024
4f4985b
Merge branch 'dev' into path-route-names
abyrd Feb 29, 2024
5aa45f5
Merge pull request #930 from conveyal/path-route-names
abyrd Feb 29, 2024
a55fa2d
Merge branch 'results-with-feedids' of https://github.com/conveyal/r5…
ansoncfit Feb 29, 2024
b90a9f7
Add "group" column in CSV path results
ansoncfit Feb 29, 2024
9d3b3ff
feat(path details): revise javadoc
ansoncfit Mar 5, 2024
63ee1e9
refactor(path details): consistent variable naming
ansoncfit Mar 5, 2024
d739e25
fix(path details): write feed values
ansoncfit Mar 5, 2024
8999be6
feat(path details): revise javadoc
ansoncfit Mar 5, 2024
7b6e513
fix(path details): add back conditional
ansoncfit Mar 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions src/main/java/com/conveyal/analysis/components/broker/Broker.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,14 @@ public interface Config {
boolean testTaskRedelivery ();
}

private Config config;
private final Config config;

// Component Dependencies
private final FileStorage fileStorage;
private final EventBus eventBus;
private final WorkerLauncher workerLauncher;

private final ListMultimap<WorkerCategory, Job> jobs =
MultimapBuilder.hashKeys().arrayListValues().build();
private final ListMultimap<WorkerCategory, Job> jobs = MultimapBuilder.hashKeys().arrayListValues().build();

/**
* The most tasks to deliver to a worker at a time. Workers may request less tasks than this, and the broker should
Expand All @@ -114,40 +113,36 @@ public interface Config {
* The value should eventually be tuned. The current value of 16 is just the value used by the previous sporadic
* polling system (WorkerStatus.LEGACY_WORKER_MAX_TASKS) which may not be ideal but is known to work.
*/
public final int MAX_TASKS_PER_WORKER = 16;
public static final int MAX_TASKS_PER_WORKER = 16;

/**
* Used when auto-starting spot instances. Set to a smaller value to increase the number of
* workers requested automatically
*/
public final int TARGET_TASKS_PER_WORKER_TRANSIT = 800;
public final int TARGET_TASKS_PER_WORKER_NONTRANSIT = 4_000;
public static final int TARGET_TASKS_PER_WORKER_TRANSIT = 800;
public static final int TARGET_TASKS_PER_WORKER_NONTRANSIT = 4_000;

/**
* We want to request spot instances to "boost" regional analyses after a few regional task
* results are received for a given workerCategory. Do so after receiving results for an
* arbitrary task toward the beginning of the job
*/
public final int AUTO_START_SPOT_INSTANCES_AT_TASK = 42;
public static final int AUTO_START_SPOT_INSTANCES_AT_TASK = 42;

/** The maximum number of spot instances allowable in an automatic request */
public final int MAX_WORKERS_PER_CATEGORY = 250;
public static final int MAX_WORKERS_PER_CATEGORY = 250;

/**
* How long to give workers to start up (in ms) before assuming that they have started (and
* starting more on a given graph if they haven't.
*/
public static final long WORKER_STARTUP_TIME = 60 * 60 * 1000;


/** Keeps track of all the workers that have contacted this broker recently asking for work. */
private WorkerCatalog workerCatalog = new WorkerCatalog();

/**
* These objects piece together results received from workers into one regional analysis result
* file per job.
*/
private static Map<String, MultiOriginAssembler> resultAssemblers = new HashMap<>();
/** These objects piece together results received from workers into one regional analysis result file per job. */
private Map<String, MultiOriginAssembler> resultAssemblers = new HashMap<>();

/**
* keep track of which graphs we have launched workers on and how long ago we launched them, so
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,11 @@ private RegionalAnalysis updateRegionalAnalysis (Request request, Response respo
* the given regional analysis.
*/
private JsonNode getScenarioJsonUrl (Request request, Response response) {
RegionalAnalysis regionalAnalysis = Persistence.regionalAnalyses
.findByIdIfPermitted(request.params("_id"), UserPermissions.from(request));
RegionalAnalysis regionalAnalysis = Persistence.regionalAnalyses.findByIdIfPermitted(
request.params("_id"),
DBProjection.exclude("request.scenario.modifications"),
UserPermissions.from(request)
);
// In the persisted objects, regionalAnalysis.scenarioId seems to be null. Get it from the embedded request.
final String networkId = regionalAnalysis.bundleId;
final String scenarioId = regionalAnalysis.request.scenarioId;
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/com/conveyal/analysis/models/AnalysisRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -175,6 +176,14 @@ public class AnalysisRequest {
*/
public int dualAccessibilityThreshold = 0;

/**
* Freeform (untyped) flags for enabling experimental, undocumented, or arcane behavior in backend or workers.
* This should be used to replace all previous special behavior flags that were embedded inside analysis names etc.
*/
public Set<String> flags;

/** Control the details of CSV regional analysis output, including whether to output IDs, names, or both. */
public CsvResultOptions csvResultOptions = new CsvResultOptions();

/**
* Create the R5 `Scenario` from this request.
Expand Down Expand Up @@ -281,6 +290,8 @@ public void populateTask (AnalysisWorkerTask task, UserPermissions userPermissio

task.includeTemporalDensity = includeTemporalDensity;
task.dualAccessibilityThreshold = dualAccessibilityThreshold;
task.flags = flags;
task.csvResultOptions = csvResultOptions;
}

private EnumSet<LegMode> getEnumSetFromString (String s) {
Expand Down
17 changes: 17 additions & 0 deletions src/main/java/com/conveyal/analysis/models/CsvResultOptions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.conveyal.analysis.models;

import com.conveyal.r5.transit.TransitLayer.EntityRepresentation;

import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY;

/**
* API model type included in analysis requests to control details of CSV regional analysis output.
* This type is shared between AnalysisRequest (Frontend -> Broker) and AnalysisWorkerTask (Broker -> Workers).
* There is precedent for nested compound types shared across those top level request types (see DecayFunction).
*/
public class CsvResultOptions {
public EntityRepresentation routeRepresentation = ID_ONLY;
public EntityRepresentation stopRepresentation = ID_ONLY;
// Only feed ID representation is allowed to be null (no feed IDs at all, the default).
public EntityRepresentation feedRepresentation = null;
}
19 changes: 13 additions & 6 deletions src/main/java/com/conveyal/analysis/persistence/MongoMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,11 @@ public int size() {
return (int) wrappedCollection.getCount();
}

public V findByIdFromRequestIfPermitted(Request request) {
return findByIdIfPermitted(request.params("_id"), UserPermissions.from(request));
}

public V findByIdIfPermitted(String id, UserPermissions userPermissions) {
V result = wrappedCollection.findOneById(id);
/**
* `fields` is nullable.
*/
public V findByIdIfPermitted(String id, DBObject fields, UserPermissions userPermissions) {
V result = wrappedCollection.findOneById(id, fields);

if (result == null) {
throw AnalysisServerException.notFound(String.format(
Expand All @@ -61,6 +60,14 @@ public V findByIdIfPermitted(String id, UserPermissions userPermissions) {
}
}

public V findByIdIfPermitted(String id, UserPermissions userPermissions) {
return findByIdIfPermitted(id, null, userPermissions);
}

public V findByIdFromRequestIfPermitted(Request request) {
return findByIdIfPermitted(request.params("_id"), UserPermissions.from(request));
}

public V get(String key) {
return wrappedCollection.findOneById(key);
}
Expand Down
14 changes: 8 additions & 6 deletions src/main/java/com/conveyal/r5/analyst/TravelTimeComputer.java
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,16 @@ public OneOriginResult computeTravelTimes() {
// The generalized cost calculations currently increment time and weight by the same amount.
sr.quantityToMinimize = StreetRouter.State.RoutingVariable.DURATION_SECONDS;
sr.route();
// Change to walking in order to reach transit stops in pedestrian-only areas like train stations.
// This implies you are dropped off or have a very easy parking spot for your vehicle.
// This kind of multi-stage search should also be used when building egress distance cost tables.
if (accessMode != StreetMode.WALK) {
sr.keepRoutingOnFoot();
}

if (request.hasTransit()) {
// Change to walking in order to reach transit stops in pedestrian-only areas like train stations.
// This implies you are dropped off or have a very easy parking spot for your vehicle.
// This kind of multi-stage search should also be used when building egress distance cost tables.
// Note that this can take up to twice as long as the initial car/bike search. Do it only when the
// walking is necessary, and when the radius of the car/bike search is limited, as for transit access.
if (accessMode != StreetMode.WALK) {
sr.keepRoutingOnFoot();
}
// Find access times to transit stops, keeping the minimum across all access street modes.
// Note that getReachedStops() returns the routing variable units, not necessarily seconds.
// TODO add logic here if linkedStops are specified in pickupDelay?
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.conveyal.r5.analyst.cluster;

import com.conveyal.analysis.models.CsvResultOptions;
import com.conveyal.r5.analyst.FreeFormPointSet;
import com.conveyal.r5.analyst.Grid;
import com.conveyal.r5.analyst.GridTransformWrapper;
Expand All @@ -15,6 +16,7 @@
import com.fasterxml.jackson.annotation.JsonTypeInfo;

import java.util.Arrays;
import java.util.Set;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
Expand Down Expand Up @@ -177,6 +179,15 @@ public abstract class AnalysisWorkerTask extends ProfileRequest {
*/
public ChaosParameters injectFault;

/**
* Freeform (untyped) flags for enabling experimental, undocumented, or arcane worker behavior.
* This should be used to replace all previous special behavior flags that were embedded inside analysis names etc.
*/
public Set<String> flags;

/** Control the details of CSV regional analysis output, including whether to output IDs, names, or both. */
public CsvResultOptions csvResultOptions;

/**
* Is this a single point or regional request? Needed to encode types in JSON serialization. Can that type field be
* added automatically with a serializer annotation instead of by defining a getter method and two dummy methods?
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/com/conveyal/r5/analyst/cluster/PathResult.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.conveyal.r5.analyst.cluster;

import com.conveyal.analysis.models.CsvResultOptions;
import com.conveyal.r5.analyst.StreetTimesAndModes;
import com.conveyal.r5.transit.TransitLayer;
import com.conveyal.r5.transit.path.Path;
Expand Down Expand Up @@ -47,19 +48,24 @@ public class PathResult {
* With additional changes, patterns could be collapsed further to route combinations or modes.
*/
public final Multimap<RouteSequence, Iteration>[] iterationsForPathTemplates;

private final TransitLayer transitLayer;

private final CsvResultOptions csvOptions;

public static final String[] DATA_COLUMNS = new String[]{
"routes",
"boardStops",
"alightStops",
"feedIds",
"rideTimes",
"accessTime",
"egressTime",
"transferTime",
"waitTimes",
"totalTime",
"nIterations"
"nIterations",
"group"
};

public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) {
Expand All @@ -76,6 +82,7 @@ public PathResult(AnalysisWorkerTask task, TransitLayer transitLayer) {
}
iterationsForPathTemplates = new Multimap[nDestinations];
this.transitLayer = transitLayer;
this.csvOptions = task.csvResultOptions;
}

/**
Expand Down Expand Up @@ -108,7 +115,7 @@ public ArrayList<String[]>[] summarizeIterations(Stat stat) {
int nIterations = iterations.size();
checkState(nIterations > 0, "A path was stored without any iterations");
String waits = null, transfer = null, totalTime = null;
String[] path = routeSequence.detailsWithGtfsIds(transitLayer);
String[] path = routeSequence.detailsWithGtfsIds(transitLayer, csvOptions);
double targetValue;
IntStream totalWaits = iterations.stream().mapToInt(i -> i.waitTimes.sum());
if (stat == Stat.MINIMUM) {
Expand All @@ -135,7 +142,10 @@ public ArrayList<String[]>[] summarizeIterations(Stat stat) {
score = thisScore;
}
}
String[] row = ArrayUtils.addAll(path, transfer, waits, totalTime, String.valueOf(nIterations));
String group = ""; // Reserved for future use
String[] row = ArrayUtils.addAll(
path, transfer, waits, totalTime, String.valueOf(nIterations), group
);
checkState(row.length == DATA_COLUMNS.length);
summary[d].add(row);
}
Expand Down
64 changes: 51 additions & 13 deletions src/main/java/com/conveyal/r5/transit/TransitLayer.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
import java.util.stream.IntStream;
import java.util.stream.StreamSupport;

import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.ID_ONLY;
import static com.conveyal.r5.transit.TransitLayer.EntityRepresentation.NAME_ONLY;


/**
* A key simplifying factor is that we don't handle overnight trips. This is fine for analysis at usual times of day.
Expand Down Expand Up @@ -815,31 +818,66 @@ public TIntSet findStopsInGeometry (Geometry geometry) {
return stops;
}

public enum EntityRepresentation {
ID_ONLY, NAME_ONLY, NAME_AND_ID
}

/**
* For the given pattern index, returns the GTFS routeId. If includeName is true, the returned string will
* also include a route_short_name or route_long_name (if they are not null).
*/
public String routeString(int routeIndex, boolean includeName) {
public String routeString (int routeIndex, EntityRepresentation nameOrId) {
RouteInfo routeInfo = routes.get(routeIndex);
String route = routeInfo.route_id;
if (includeName) {
if (routeInfo.route_short_name != null) {
route += " (" + routeInfo.route_short_name + ")";
} else if (routeInfo.route_long_name != null){
route += " (" + routeInfo.route_long_name + ")";
String name = routeInfo.route_short_name;
String id = routeInfo.route_id;
// If we might actually use the name, check some fallbacks.
if (nameOrId != ID_ONLY) {
if (name == null) {
name = routeInfo.route_long_name;
}
if (name == null) {
name = routeInfo.route_id;
}
}
return route;
return switch (nameOrId) {
case NAME_ONLY -> name;
case NAME_AND_ID -> name + " (" + id + ")";
default -> id;
};
}

/**
* For the given stop index, returns the GTFS stopId (stripped of R5's feedId prefix) and, if includeName is true,
* stopName.
*/
public String stopString(int stopIndex, boolean includeName) {
// TODO use a compact feed index, instead of splitting to remove feedIds
String stop = stopIdForIndex.get(stopIndex) == null ? "[new]" : stopIdForIndex.get(stopIndex).split(":")[1];
if (includeName) stop += " (" + stopNames.get(stopIndex) + ")";
return stop;
public String stopString(int stopIndex, EntityRepresentation nameOrId) {
String stopId = stopIdForIndex.get(stopIndex);
String stopName = stopNames.get(stopIndex);
// I'd trust the JVM JIT to optimize out these assignments on different code paths, but not the split call.
if (nameOrId != NAME_ONLY) {
if (stopId == null) {
stopId = "[new]";
} else {
// TODO use a compact feed ID instead of splitting to remove feedIds (or put feedId into another CSV field)
stopId = stopId.split(":")[1];
}
}
if (nameOrId != ID_ONLY) {
if (stopName == null) {
stopName = "[new]";
}
}
return switch (nameOrId) {
case NAME_ONLY -> stopName;
case NAME_AND_ID -> stopName + " (" + stopId + ")";
default -> stopId;
};
}

/**
* For a supplied stopIndex in the transit layer, return the feed id (which we prepend to the GTFS stop id).
*/
public String feedFromStop(int stopIndex) {
return stopIdForIndex.get(stopIndex) == null ? "[new]" : stopIdForIndex.get(stopIndex).split(":")[0];
}
}
Loading
Loading