Skip to content

Commit

Permalink
avoid multiple LTS flags on edge in modifications (#843)
Browse files Browse the repository at this point in the history 
add method that clears all LTS flags before setting a new one
use this method in ModifyStreets and ShapefileMatcher
add and clarify code comments
refactoring:
add static imports of enum values for readability
add braces around conditional blocks

the LevelOfTrafficStressLabeler may still set multiple flags during
initial network build, checks and normalization could be added.
  • Loading branch information
@abyrd
abyrd committed Dec 21, 2022
1 parent 76642ab commit bed8ca8
Show file tree
Hide file tree
Showing 7 changed files with 229 additions and 134 deletions.
5 changes: 2 additions & 3 deletions src/main/java/com/conveyal/r5/analyst/scenario/ModifyStreets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.util.EnumSet;
import java.util.List;

import static com.conveyal.r5.labeling.LevelOfTrafficStressLabeler.intToLts;
import static com.conveyal.r5.streets.EdgeStore.intToLts;

/**
* <p>
Expand Down Expand Up @@ -234,8 +234,7 @@ private void handleOneEdge (EdgeStore.Edge newEdge, EdgeStore.Edge oldEdge) {
newEdge.disallowAllModes();
newEdge.allowStreetModes(allowedModes);
if (bikeLts != null) {
// Overwrite the LTS copied in the flags
newEdge.setFlag(intToLts(bikeLts));
newEdge.setLts(bikeLts);
}
if (carSpeedKph != null) {
newEdge.setSpeedKph(carSpeedKph);
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/conveyal/r5/analyst/scenario/ShapefileLts.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ public boolean resolve (TransportNetwork network) {
@Override
public boolean apply (TransportNetwork network) {
// Replicate the entire flags array so we can write to it (following copy-on-write policy).
// Otherwise the TIntAugmentedList only allows extending the base graph.
// Otherwise the TIntAugmentedList only allows extending the base graph. An alternative approach can be seen in
// ModifyStreets, where all affected edges are marked deleted and then recreated in the augmented lists.
// The appraoch here assumes a high percentage of edges changed, while ModifyStreets assumes a small percentage.
network.streetLayer.edgeStore.flags = new TIntArrayList(network.streetLayer.edgeStore.flags);
ShapefileMatcher shapefileMatcher = new ShapefileMatcher(network.streetLayer);
try {
Expand Down
146 changes: 66 additions & 80 deletions src/main/java/com/conveyal/r5/labeling/LevelOfTrafficStressLabeler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static com.conveyal.r5.streets.EdgeStore.EdgeFlag.BIKE_LTS_EXPLICIT;
import static com.conveyal.r5.streets.EdgeStore.EdgeFlag.*;
import static com.conveyal.r5.streets.EdgeStore.intToLts;

/**
* Label streets with a best-guess at their Level of Traffic Stress, as defined in
Expand All @@ -43,11 +44,20 @@ public class LevelOfTrafficStressLabeler {
/**
* Set the LTS for this way in the provided flags (not taking into account any intersection LTS at the moment).
* This sets flags (passed in as the second and third parameters) from the tags on the OSM Way (first parameter).
*
* The general approach in this function is to look at a bunch of OSM tags (highway, cycleway, maxspeed, lanes) and
* edge characteristics (allowed modes) progressing from things implying low to high LTS, and returning early when
* lower LTS has already been established.
*
* One reason to work in order of increasing LTS is that LTS is implemented as non-mutually exclusive flags on the
* edge, and higher LTS flags override lower ones, i.e. if an LTS 4 flag is present the road is seen as LTS 4 even
* if an LTS 2 flag is present. This should really be changed, but it would be a backward-incompatible change to
* the network storage format.
*/
public void label (Way way, EnumSet<EdgeStore.EdgeFlag> forwardFlags, EnumSet<EdgeStore.EdgeFlag> backFlags) {
// the general idea behind this function is that we progress from low-stress to higher-stress, bailing out as we go.

// First, if the input OSM data contains LTS tags, use those rather than estimating LTS from road characteristics.
// First, if the input OSM data contains LTS tags,
// use those rather than estimating LTS from road characteristics and return immediately.
String ltsTagValue = way.getTag("lts");
if (ltsTagValue != null) {
try {
Expand All @@ -67,36 +77,38 @@ public void label (Way way, EnumSet<EdgeStore.EdgeFlag> forwardFlags, EnumSet<Ed
}
}

// Ways that do not permit cars are deemed LTS 1.
if (!forwardFlags.contains(EdgeStore.EdgeFlag.ALLOWS_CAR) && !backFlags.contains(EdgeStore.EdgeFlag.ALLOWS_CAR)) {
// no cars permitted on this way, it is LTS 1
// TODO on street bike lanes/cycletracks digitized as separate infrastructure?
forwardFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_1);
backFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_1);
forwardFlags.add(BIKE_LTS_1);
backFlags.add(BIKE_LTS_1);
return;
}

// leave some unlabeled because we don't really know. Also alleys and parking aisles shouldn't "bleed" high LTS
// into the streets that connect to them
if (way.hasTag("highway", "service"))
// Service roads are left unlabeled because we don't really know how stressful they are.
// Also, alleys and parking aisles shouldn't "bleed" high LTS into the streets that connect to them.
// (Though perhaps the problem there is that streets shouldn't bleed LTS at all, and certainly not from lower
// to higher streets in the hierarchy.)
if (way.hasTag("highway", "service")) {
return;
}

// is this a small, low-stress road?
// Small, low-stress road types are definitively set to LTS 1.
if (way.hasTag("highway", "residential") || way.hasTag("highway", "living_street")) {
forwardFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_1);
backFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_1);
forwardFlags.add(BIKE_LTS_1);
backFlags.add(BIKE_LTS_1);
return;
}

// is there a bike lane?
// we don't have lane widths, so guess from the roadway classification
// If the way has a cycle lane tag, record this fact for later consideration.
boolean hasForwardLane = false;
boolean hasBackwardLane = false;
if (way.hasTag("cycleway", "lane")) {
// there is a bike lane in all directions that cycles are allowed to traverse.
// There is a bike lane in all directions that cycles are allowed to traverse.
hasForwardLane = hasBackwardLane = true;
}

// TODO handle left-hand-drive countries
// TODO handle left-hand-drive countries.
if (way.hasTag("cycleway:left", "lane") || way.hasTag("cycleway", "opposite") || way.hasTag("cycleway:right", "opposite")) {
hasBackwardLane = true;
}
Expand All @@ -107,7 +119,7 @@ public void label (Way way, EnumSet<EdgeStore.EdgeFlag> forwardFlags, EnumSet<Ed
hasForwardLane = true;
}

// extract max speed and lane info
// Extract maximum speed and number of lanes, retaining them for later consideration.
double maxSpeed = Double.NaN;
if (way.hasTag("maxspeed")) {
// parse the max speed tag
Expand All @@ -131,55 +143,52 @@ public void label (Way way, EnumSet<EdgeStore.EdgeFlag> forwardFlags, EnumSet<Ed
}
}

EdgeStore.EdgeFlag defaultLts = EdgeStore.EdgeFlag.BIKE_LTS_3;
// In the absence of other evidence, we will assign LTS 3. This may be revised downward based on tags.
EdgeStore.EdgeFlag defaultLts = BIKE_LTS_3;

// if it's small and slow, LTS 2
if (lanes <= 3 && maxSpeed <= 25 * 1.61) defaultLts = EdgeStore.EdgeFlag.BIKE_LTS_2;
// If it's small and slow, lessen stress to LTS 2.
if (lanes <= 3 && maxSpeed <= 25 * 1.61) defaultLts = BIKE_LTS_2;

// assume that there aren't too many lanes if it's not specified
if (lanes == Integer.MAX_VALUE && maxSpeed <= 25 * 1.61) defaultLts = EdgeStore.EdgeFlag.BIKE_LTS_2;
// Assume that streets with a slow maximum speed aren't too stressful if the number of lanes is not specified.
if (lanes == Integer.MAX_VALUE && maxSpeed <= 25 * 1.61) defaultLts = BIKE_LTS_2;

// TODO arbitrary. Roads up to tertiary with bike lanes are considered LTS 2, roads above tertiary, LTS 3.
// LTS 3 has defined space, but on fast roads
if (way.hasTag("highway", "unclassified") || way.hasTag("highway", "tertiary") || way.hasTag("highway", "tertiary_link")) {
// assume that it's not too fast if it's not specified, but only for these smaller roads
// TODO questionable. Tertiary roads probably tend to be faster than 25 MPH.
if (lanes <= 3 && Double.isNaN(maxSpeed)) defaultLts = EdgeStore.EdgeFlag.BIKE_LTS_2;
if (lanes <= 3 && Double.isNaN(maxSpeed)) defaultLts = BIKE_LTS_2;

if (hasForwardLane) {
forwardFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_2);
}
else {
forwardFlags.add(BIKE_LTS_2);
} else {
forwardFlags.add(defaultLts); // moderate speed single lane street
}

if (hasBackwardLane) {
backFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_2);
}
else {
backFlags.add(BIKE_LTS_2);
} else {
backFlags.add(defaultLts);
}
}
else { // NB includes trunk
// NB this will be LTS 3 unless this street has a low number of lanes and speed limit, or a low speed limit
// and unknown number of lanes
} else {
// This clause includes trunk roads, and will default to LTS 3 unless this street has a low number of lanes
// and speed limit, or a low speed limit and unknown number of lanes.
if (hasForwardLane) {
forwardFlags.add(defaultLts);
}

if (hasBackwardLane) {
backFlags.add(defaultLts);
}
}

// if we've assigned nothing, assign LTS 4
if (!forwardFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_1) && !forwardFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_2) &&
!forwardFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_3) && !forwardFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_4))
forwardFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_4);

if (!backFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_1) && !backFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_2) &&
!backFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_3) && !backFlags.contains(EdgeStore.EdgeFlag.BIKE_LTS_4))
backFlags.add(EdgeStore.EdgeFlag.BIKE_LTS_4);
// If we've assigned nothing, assign LTS 4
if (!forwardFlags.contains(BIKE_LTS_1) && !forwardFlags.contains(BIKE_LTS_2) &&
!forwardFlags.contains(BIKE_LTS_3) && !forwardFlags.contains(BIKE_LTS_4)) {
forwardFlags.add(BIKE_LTS_4);
}
if (!backFlags.contains(BIKE_LTS_1) && !backFlags.contains(BIKE_LTS_2) &&
!backFlags.contains(BIKE_LTS_3) && !backFlags.contains(BIKE_LTS_4)) {
backFlags.add(BIKE_LTS_4);
}
}

/**
Expand All @@ -205,16 +214,16 @@ public void applyIntersectionCosts(StreetLayer streetLayer) {

for (TIntIterator it = streetLayer.incomingEdges.get(v.index).iterator(); it.hasNext();) {
e.seek(it.next());
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_2)) maxLts = Math.max(2, maxLts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_3)) maxLts = Math.max(3, maxLts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_4)) maxLts = Math.max(4, maxLts);
if (e.getFlag(BIKE_LTS_2)) maxLts = Math.max(2, maxLts);
if (e.getFlag(BIKE_LTS_3)) maxLts = Math.max(3, maxLts);
if (e.getFlag(BIKE_LTS_4)) maxLts = Math.max(4, maxLts);
}

for (TIntIterator it = streetLayer.outgoingEdges.get(v.index).iterator(); it.hasNext();) {
e.seek(it.next());
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_2)) maxLts = Math.max(2, maxLts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_3)) maxLts = Math.max(3, maxLts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_4)) maxLts = Math.max(4, maxLts);
if (e.getFlag(BIKE_LTS_2)) maxLts = Math.max(2, maxLts);
if (e.getFlag(BIKE_LTS_3)) maxLts = Math.max(3, maxLts);
if (e.getFlag(BIKE_LTS_4)) maxLts = Math.max(4, maxLts);
}

vertexStresses.put(v.index, maxLts);
Expand All @@ -231,21 +240,13 @@ public void applyIntersectionCosts(StreetLayer streetLayer) {
if (e.getFlag(BIKE_LTS_EXPLICIT)) {
continue;
}

// we do need to check and preserve LTS on this edge, because it can be higher than the intersection
// LTS if the other end of it is connected to a higher-stress intersection.
int lts = it.value();
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_2)) lts = Math.max(2, lts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_3)) lts = Math.max(3, lts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_4)) lts = Math.max(4, lts);

// clear existing markings
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_1);
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_2);
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_3);
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_4);

e.setFlag(intToLts(lts));
if (e.getFlag(BIKE_LTS_2)) lts = Math.max(2, lts);
if (e.getFlag(BIKE_LTS_3)) lts = Math.max(3, lts);
if (e.getFlag(BIKE_LTS_4)) lts = Math.max(4, lts);
e.setLts(lts);
}

// need to set on both incoming and outgoing b/c it is possible to start or end a search at a high-stress intersection
Expand All @@ -254,21 +255,13 @@ public void applyIntersectionCosts(StreetLayer streetLayer) {
if (e.getFlag(BIKE_LTS_EXPLICIT)) {
continue;
}

// we do need to check and preserve LTS on this edge, because it can be higher than the intersection
// LTS if the other end of it is connected to a higher-stress intersection.
int lts = it.value();
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_2)) lts = Math.max(2, lts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_3)) lts = Math.max(3, lts);
if (e.getFlag(EdgeStore.EdgeFlag.BIKE_LTS_4)) lts = Math.max(4, lts);

// clear existing markings
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_1);
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_2);
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_3);
e.clearFlag(EdgeStore.EdgeFlag.BIKE_LTS_4);

e.setFlag(intToLts(lts));
if (e.getFlag(BIKE_LTS_2)) lts = Math.max(2, lts);
if (e.getFlag(BIKE_LTS_3)) lts = Math.max(3, lts);
if (e.getFlag(BIKE_LTS_4)) lts = Math.max(4, lts);
e.setLts(lts);
}
}
}
Expand All @@ -283,13 +276,6 @@ public static Integer ltsToInt (EdgeStore.EdgeFlag flag) {
}
}

public static EdgeStore.EdgeFlag intToLts (int lts) {
if (lts < 2) return EdgeStore.EdgeFlag.BIKE_LTS_1;
else if (lts == 2) return EdgeStore.EdgeFlag.BIKE_LTS_2;
else if (lts == 3) return EdgeStore.EdgeFlag.BIKE_LTS_3;
else return EdgeStore.EdgeFlag.BIKE_LTS_4;
}

/** parse an OSM speed tag */
public static double getSpeedKmh (String maxSpeed) {
try {
Expand Down
15 changes: 6 additions & 9 deletions src/main/java/com/conveyal/r5/shapefile/ShapefileMatcher.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.conveyal.r5.shapefile;

import com.conveyal.osmlib.OSM;
import com.conveyal.r5.streets.EdgeStore;
import com.conveyal.r5.streets.StreetLayer;
import com.conveyal.r5.util.LambdaCounter;
Expand All @@ -10,7 +9,6 @@
import org.locationtech.jts.geom.LineString;
import org.locationtech.jts.geom.MultiLineString;
import org.locationtech.jts.index.strtree.STRtree;
import org.opengis.feature.Property;
import org.opengis.feature.simple.SimpleFeature;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -20,7 +18,7 @@
import java.util.List;
import java.util.stream.IntStream;

import static com.conveyal.r5.labeling.LevelOfTrafficStressLabeler.intToLts;
import static com.conveyal.r5.streets.EdgeStore.intToLts;
import static com.conveyal.r5.streets.EdgeStore.EdgeFlag.BIKE_LTS_EXPLICIT;

/**
Expand Down Expand Up @@ -67,8 +65,8 @@ public ShapefileMatcher (StreetLayer streets) {

/**
* Match each pair of edges in the street layer to a feature in the shapefile. Copy LTS attribute from that feature
* to the pair of edges, setting the BIKE_LTS_EXPLICIT flag. This will prevent Conveyal OSM-inferred LTS from
* overwriting the shapefile-derived LTS.
* to the pair of edges, setting the BIKE_LTS_EXPLICIT flag. This can prevent Conveyal OSM-inferred LTS from
* overwriting the shapefile-derived LTS if this matching process is applied during network build.
* In current usage this is applied after the OSM is already completely loaded and converted to network edges, so
* it overwrites any data from OSM. Perhaps instead of BIKE_LTS_EXPLICIT we should have an LTS source flag:
* OSM_INFERRED, OSM_EXPLICIT, SHAPEFILE_MATCH etc. This could also apply to things like speeds and slopes.
Expand All @@ -93,14 +91,13 @@ public void match (String shapefileName, String attributeName) {
// TODO reuse code from LevelOfTrafficStressLabeler.label()
int lts = ((Number) bestFeature.getAttribute(ltsAttributeIndex)).intValue();
if (lts < 1 || lts > 4) {
LOG.error("Clamping LTS value to range [1...4]. Value in attribute is {}", lts);
LOG.error("LTS should be in range [1...4]. Value in attribute is {}", lts);
}
EdgeStore.EdgeFlag ltsFlag = intToLts(lts);
edge.setFlag(BIKE_LTS_EXPLICIT);
edge.setFlag(ltsFlag);
edge.setLts(lts);
edge.advance();
edge.setFlag(BIKE_LTS_EXPLICIT);
edge.setFlag(ltsFlag);
edge.setLts(lts);
edgePairCounter.increment();
}
});
Expand Down
Loading

0 comments on commit bed8ca8

Please sign in to comment.