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

Better escalator duration control: specific duration from OSM duration tag, default speed from build-config.json #6268

Merged
merged 17 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package org.opentripplanner.graph_builder.module.osm;

import java.time.Duration;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.opentripplanner.graph_builder.issue.api.DataImportIssueStore;
import org.opentripplanner.graph_builder.issue.api.Issue;
import org.opentripplanner.osm.model.OsmWay;
import org.opentripplanner.street.model.edge.EscalatorEdge;
import org.opentripplanner.street.model.vertex.IntersectionVertex;
Expand All @@ -13,9 +17,19 @@
class EscalatorProcessor {

private final Map<Long, IntersectionVertex> intersectionNodes;
private final DataImportIssueStore issueStore;

public EscalatorProcessor(Map<Long, IntersectionVertex> intersectionNodes) {
// If an escalator is tagged as moving less than 5 cm/s, or more than 5 m/s,
// assume it's an error and ignore it.
private static final double SLOW_ESCALATOR_ERROR_CUTOFF = 0.05;
private static final double FAST_ESCALATOR_ERROR_CUTOFF = 5.0;

public EscalatorProcessor(
Map<Long, IntersectionVertex> intersectionNodes,
DataImportIssueStore issueStore
) {
this.intersectionNodes = intersectionNodes;
this.issueStore = issueStore;
}

public void buildEscalatorEdge(OsmWay escalatorWay, double length) {
Expand All @@ -27,30 +41,58 @@ public void buildEscalatorEdge(OsmWay escalatorWay, double length) {
.boxed()
.toList();

Optional<Duration> duration = escalatorWay.getDuration(v ->
issueStore.add(
Issue.issue(
"InvalidDuration",
"Duration for osm node {} is not a valid duration: '{}'; the value is ignored.",
escalatorWay.url(),
v
)
)
);
if (duration.isPresent()) {
double speed = length / duration.get().toSeconds();
if (speed < SLOW_ESCALATOR_ERROR_CUTOFF || speed > FAST_ESCALATOR_ERROR_CUTOFF) {
duration = Optional.empty();
tkalvas marked this conversation as resolved.
Show resolved Hide resolved
issueStore.add(
Issue.issue(
"InvalidDuration",
"Duration for osm node {} makes implied speed {} be outside acceptable range.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use URL here as well.

escalatorWay.url(),
speed
)
);
}
}
for (int i = 0; i < nodes.size() - 1; i++) {
if (escalatorWay.isForwardEscalator()) {
EscalatorEdge.createEscalatorEdge(
intersectionNodes.get(nodes.get(i)),
intersectionNodes.get(nodes.get(i + 1)),
length
length,
duration.orElse(null)
);
} else if (escalatorWay.isBackwardEscalator()) {
EscalatorEdge.createEscalatorEdge(
intersectionNodes.get(nodes.get(i + 1)),
intersectionNodes.get(nodes.get(i)),
length
length,
duration.orElse(null)
);
} else {
EscalatorEdge.createEscalatorEdge(
intersectionNodes.get(nodes.get(i)),
intersectionNodes.get(nodes.get(i + 1)),
length
length,
duration.orElse(null)
);

EscalatorEdge.createEscalatorEdge(
intersectionNodes.get(nodes.get(i + 1)),
intersectionNodes.get(nodes.get(i)),
length
length,
duration.orElse(null)
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,10 @@ private void buildBasicGraph() {
long wayCount = osmdb.getWays().size();
ProgressTracker progress = ProgressTracker.track("Build street graph", 5_000, wayCount);
LOG.info(progress.startMessage());
var escalatorProcessor = new EscalatorProcessor(vertexGenerator.intersectionNodes());
var escalatorProcessor = new EscalatorProcessor(
vertexGenerator.intersectionNodes(),
issueStore
);

WAY:for (OsmWay way : osmdb.getWays()) {
WayProperties wayData = way.getOsmProvider().getWayPropertySet().getDataForWay(way);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ protected Collection<KeyValue> map(Edge input) {
List<KeyValue> properties =
switch (input) {
case StreetEdge e -> mapStreetEdge(e);
case EscalatorEdge e -> List.of(kv("distance", e.getDistanceMeters()));
case EscalatorEdge e -> List.of(
kv("distance", e.getDistanceMeters()),
kv("duration", e.getDuration().map(d -> d.toString()).orElse(null))
);
default -> List.of();
};
return ListUtils.combine(baseProps, properties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

import gnu.trove.list.TLongList;
import gnu.trove.list.array.TLongArrayList;
import java.time.Duration;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import org.opentripplanner.graph_builder.module.osm.StreetTraversalPermissionPair;
import org.opentripplanner.street.model.StreetTraversalPermission;

Expand Down Expand Up @@ -130,6 +133,10 @@ public boolean isEscalator() {
return (isTag("highway", "steps") && isOneOfTags("conveying", ESCALATOR_CONVEYING_TAGS));
}

public Optional<Duration> getDuration(Consumer<String> errorHandler) {
return getTagValueAsDuration("duration", errorHandler);
}

public boolean isForwardEscalator() {
return isEscalator() && "forward".equals(this.getTag("conveying"));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.opentripplanner.osm.model;

import java.time.Duration;
import java.time.format.DateTimeParseException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -221,6 +223,101 @@ public OptionalInt getTagAsInt(String tag, Consumer<String> errorHandler) {
return OptionalInt.empty();
}

/**
* Parse an OSM duration tag, which is one of:
tkalvas marked this conversation as resolved.
Show resolved Hide resolved
* mm
* hh:mm
* hh:mm:ss
* and where the leading value is not limited to any maximum.
* See <a href="https://wiki.openstreetmap.org/wiki/Key:duration">OSM wiki definition
* of duration</a>.
*
* @param duration string in format mm, hh:mm, or hh:mm:ss
* @return Duration
* @throws DateTimeParseException on bad input
*/
public static Duration parseOsmDuration(String duration) {
// Unfortunately DateFormatParserBuilder doesn't quite do enough for this case.
// It has the capability for expressing optional parts, so it could express hh(:mm(:ss)?)?
// but it cannot express (hh:)?mm(:ss)? where the existence of (:ss) implies the existence
// of (hh:). Even if it did, it would not be able to handle the cases where hours are
// greater than 23 or (if there is no hours part at all) minutes are greater than 59, which
// are both allowed by the spec and exist in OSM data. Durations are not LocalTimes after
// all, in parsing a LocalTime it makes sense and is correct that hours cannot be more than
// 23 or minutes more than 59, but in durations if you have capped the largest unit, it is
// reasonable for the amount of the largest unit to be as large as it needs to be.
int colonCount = (int) duration.chars().filter(ch -> ch == ':').count();
if (colonCount <= 2) {
try {
int i, j;
long hours, minutes, seconds;
// The first :-separated element can be any width, and has no maximum. It still has
// to be non-negative. The following elements must be 2 characters wide, non-negative,
// and less than 60.
switch (colonCount) {
// case "m"
case 0:
minutes = Long.parseLong(duration);
if (minutes >= 0) {
return Duration.ofMinutes(minutes);
}
break;
// case "h:mm"
case 1:
i = duration.indexOf(':');
hours = Long.parseLong(duration.substring(0, i));
minutes = Long.parseLong(duration.substring(i + 1));
if (duration.length() - i == 3 && hours >= 0 && minutes >= 0 && minutes < 60) {
return Duration.ofHours(hours).plusMinutes(minutes);
}
break;
// case "h:mm:ss"
default:
i = duration.indexOf(':');
j = duration.indexOf(':', i + 1);
hours = Long.parseLong(duration.substring(0, i));
minutes = Long.parseLong(duration.substring(i + 1, j));
seconds = Long.parseLong(duration.substring(j + 1));
if (
j - i == 3 &&
duration.length() - j == 3 &&
hours >= 0 &&
minutes >= 0 &&
minutes < 60 &&
seconds >= 0 &&
seconds < 60
) {
return Duration.ofHours(hours).plusMinutes(minutes).plusSeconds(seconds);
}
break;
}
} catch (NumberFormatException e) {
// fallthrough
}
}
throw new DateTimeParseException("Bad OSM duration", duration, 0);
}

/**
* Gets a tag's value, assumes it is an OSM wiki spesified duration, parses and returns it.
optionsome marked this conversation as resolved.
Show resolved Hide resolved
* If parsing fails, calls the error handler.
*
* @param key
* @param errorHandler
* @return parsed Duration, or empty
*/
public Optional<Duration> getTagValueAsDuration(String key, Consumer<String> errorHandler) {
String value = getTag(key);
if (value != null) {
try {
return Optional.of(parseOsmDuration(value));
} catch (DateTimeParseException e) {
errorHandler.accept(value);
}
}
return Optional.empty();
}

/**
* Some tags are allowed to have values like 55, "true" or "false".
* <p>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package org.opentripplanner.routing.api.request.preference;

import static org.opentripplanner.utils.lang.DoubleUtils.doubleEquals;

import java.io.Serializable;
import java.util.Objects;
import java.util.function.Consumer;
import org.opentripplanner.utils.tostring.ToStringBuilder;

public class EscalatorPreferences implements Serializable {

public static final EscalatorPreferences DEFAULT = new EscalatorPreferences();

private final double reluctance;
private final double speed;

/* Using the angle of 30 degrees and a speed of 0.5 m/s gives a horizontal component
* of approx. 0.43 m/s. This is typical of short escalators like those in shopping
* malls. */
private static final double HORIZONTAL_SPEED = 0.45;

private EscalatorPreferences() {
this.reluctance = 1.5;
this.speed = HORIZONTAL_SPEED;
}

private EscalatorPreferences(Builder builder) {
reluctance = builder.reluctance;
speed = builder.speed;
}

public static Builder of() {
return new Builder(DEFAULT);
}

public Builder copyOf() {
return new Builder(this);
}

public double reluctance() {
return reluctance;
}

public double speed() {
return speed;
}

@Override
public boolean equals(Object o) {
tkalvas marked this conversation as resolved.
Show resolved Hide resolved
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
EscalatorPreferences that = (EscalatorPreferences) o;
return (doubleEquals(that.reluctance, reluctance) && doubleEquals(that.speed, speed));
}

@Override
public int hashCode() {
return Objects.hash(speed, reluctance);
}

@Override
public String toString() {
return ToStringBuilder
.of(EscalatorPreferences.class)
.addNum("speed", speed, DEFAULT.speed)
.addNum("reluctance", reluctance, DEFAULT.reluctance)
.toString();
}

public static class Builder {

private final EscalatorPreferences original;
private double reluctance;
private double speed;

public Builder(EscalatorPreferences original) {
this.original = original;
this.reluctance = original.reluctance;
this.speed = original.speed;
}

public EscalatorPreferences original() {
return original;
}

public double speed() {
return speed;
}

public Builder withSpeed(double speed) {
this.speed = speed;
return this;
}

public double reluctance() {
return reluctance;
}

public Builder withReluctance(double reluctance) {
this.reluctance = reluctance;
return this;
}

public Builder apply(Consumer<Builder> body) {
body.accept(this);
return this;
}

public EscalatorPreferences build() {
var newObj = new EscalatorPreferences(this);
return original.equals(newObj) ? original : newObj;
}
}
}
Loading
Loading