Include crossing=traffic_signals and use it for walking/cycling penalty on crossings by optionsome · Pull Request #4574 · opentripplanner/OpenTripPlanner

optionsome · 2022-11-04T13:29:14Z

Summary

Set a new "crossing traffic light" value on nodes based on crossing=traffic_signals tag and use it for walking/cycling penalty on crossings

Issue

closes #4573

Unit tests

Updated unit tests.

Documentation

Not needed to be updated

codecov-commenter · 2022-11-17T12:15:34Z

Codecov Report

Attention: Patch coverage is 96.87500% with 2 lines in your changes missing coverage. Please review.

Project coverage is 60.95%. Comparing base (16b16fc) to head (5399b1f).
Report is 7259 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...java/org/opentripplanner/visualizer/ShowGraph.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #4574      +/-   ##
=============================================
+ Coverage      60.86%   60.95%   +0.09%     
- Complexity     12326    12356      +30     
=============================================
  Files           1594     1595       +1     
  Lines          63775    63808      +33     
  Branches        7004     6999       -5     
=============================================
+ Hits           38814    38894      +80     
+ Misses         22780    22738      -42     
+ Partials        2181     2176       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

leonardehrenfried · 2022-11-18T11:05:42Z

BTW, I added car-only test cases for the speed test in #4608. If we merge that before this PR then we have a baseline for comparison of the perf impact.

leonardehrenfried · 2022-12-09T10:05:50Z

  }

-  public boolean hasTrafficLight() {
+  public boolean hasHighwayTrafficLight() {


Can these be renamed into what they are used for? Are they for vehicles and pedestrians respectively?

Highway traffic light is also used for cyclists on car streets, I thought about this too and I ended up naming these like this so how these are used in different types of routing is decided elsewhere.

leonardehrenfried

I have a few requests.

It's actually very hard to predict what the outcome of this will be so I think I will have to trust HSL that they've tested this extensively.

leonardehrenfried · 2022-12-09T10:40:02Z

    final var baseDuration = computeNonDrivingTraversalDuration(from, to, toSpeed);

-    if (isTurnAcrossTraffic(turnAngle)) {
+    if (v.hasCyclingTrafficLight()) {


Not sure if it's possible to have a vertex that is both a traffic light and a turn and even less sure if it matters. I'm happy to keep it that way and experiment with the values a bit.

Yes I wasn't quite sure about this either, thought it might not make sense to apply both of these penalties.

optionsome · 2022-12-09T13:59:42Z

It's actually very hard to predict what the outcome of this will be so I think I will have to trust HSL that they've tested this extensively.

I have tested this with Finland OSM and tag mapping. I would appreciate if someone could also test if this doesn't cause too much problems elsewhere as the tagging conventions etc. can differ.

leonardehrenfried

I have tested this here in Berlin and found the times to be broadly correct.

Update finland walk safety rules

a386b90

optionsome added !Improvement A functional improvement or micro feature +Skip Changelog This is not a relevant change for a product owner since last release. Digitransit Test Feature is under testing in Digitransit environment(s) labels Nov 4, 2022

optionsome added 6 commits November 8, 2022 10:40

Lower safety penalty for some crossing so no walking on car streets

cb1bd9c

Handle highway=cycleway footway=crossing combos

0f84a44

Support crossing=traffic_signals and use it for walking/cycling penalty

3f2f6d6

Disable failing test

91d8ed2

Modify safety for traffic signal ways to adjust for added penalty

c232685

Add comment

0bf2ebd

optionsome changed the title ~~Update finland walk safety rules~~ Include crossing=traffic_signals and use it for walking/cycling penalty on intersections Nov 17, 2022

optionsome removed the +Skip Changelog This is not a relevant change for a product owner since last release. label Nov 17, 2022

Merge remote-tracking branch 'upstream/dev-2.x' into walk-safety-updates

4e4488d

optionsome changed the title ~~Include crossing=traffic_signals and use it for walking/cycling penalty on intersections~~ Include crossing=traffic_signals and use it for walking/cycling penalty on crossings Nov 17, 2022

optionsome added the +Bump Serialization Id Add this label if you want the serialization id automatically bumped after merging the PR label Nov 17, 2022

optionsome commented Nov 17, 2022

View reviewed changes

Comment thread src/ext-test/java/org/opentripplanner/ext/flex/FlexIntegrationTest.java Outdated

leonardehrenfried mentioned this pull request Nov 18, 2022

Fix expectations in Baden-Württemberg speed test, add car-only cases #4608

Merged

optionsome added 5 commits November 18, 2022 15:02

Refactor booleans into short bitset flags

2f0a791

Fix access modifier

c7af285

Simplify logic

6ead892

Make flags final

c454d57

Add tests

ceaba41

hannesj self-requested a review November 22, 2022 09:31

Merge remote-tracking branch 'upstream/dev-2.x' into walk-safety-updates

a664a86

t2gran added this to the 2.3 milestone Dec 6, 2022

hannesj requested review from leonardehrenfried December 8, 2022 15:13

Merge remote-tracking branch 'upstream/dev-2.x' into walk-safety-updates

3698235

optionsome marked this pull request as ready for review December 8, 2022 15:27

optionsome requested a review from a team as a code owner December 8, 2022 15:27

leonardehrenfried reviewed Dec 9, 2022

View reviewed changes

Comment thread src/main/java/org/opentripplanner/street/model/vertex/IntersectionVertex.java Outdated

leonardehrenfried requested changes Dec 9, 2022

View reviewed changes

leonardehrenfried reviewed Dec 9, 2022

View reviewed changes

leonardehrenfried and others added 2 commits December 9, 2022 15:46

Fix flex test

93757c8

Make method static

5399b1f

leonardehrenfried approved these changes Dec 12, 2022

View reviewed changes

hannesj reviewed Dec 12, 2022

View reviewed changes

Comment thread src/main/java/org/opentripplanner/openstreetmap/tagmapping/FinlandMapper.java

hannesj approved these changes Dec 15, 2022

View reviewed changes

optionsome merged commit c7f6b83 into opentripplanner:dev-2.x Dec 15, 2022

optionsome deleted the walk-safety-updates branch December 15, 2022 15:30

t2gran pushed a commit that referenced this pull request Dec 15, 2022

Add changelog entry for #4574 [ci skip]

452946b

t2gran pushed a commit that referenced this pull request Dec 15, 2022

Bump serialization version id for #4574

1536e65

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include crossing=traffic_signals and use it for walking/cycling penalty on crossings#4574

Include crossing=traffic_signals and use it for walking/cycling penalty on crossings#4574
optionsome merged 17 commits into
opentripplanner:dev-2.xfrom
HSLdevcom:walk-safety-updates

optionsome commented Nov 4, 2022 •

edited

Loading

Uh oh!

codecov-commenter commented Nov 17, 2022 •

edited by codecov Bot

Loading

Uh oh!

Uh oh!

leonardehrenfried commented Nov 18, 2022 •

edited

Loading

Uh oh!

leonardehrenfried Dec 9, 2022

Uh oh!

optionsome Dec 9, 2022

Uh oh!

Uh oh!

leonardehrenfried left a comment

Uh oh!

leonardehrenfried Dec 9, 2022

Uh oh!

optionsome Dec 9, 2022

Uh oh!

optionsome commented Dec 9, 2022 •

edited

Loading

Uh oh!

leonardehrenfried left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

optionsome commented Nov 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issue

Unit tests

Documentation

Uh oh!

codecov-commenter commented Nov 17, 2022 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

leonardehrenfried commented Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leonardehrenfried Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

optionsome Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leonardehrenfried left a comment

Choose a reason for hiding this comment

Uh oh!

leonardehrenfried Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

optionsome Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

optionsome commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

leonardehrenfried left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

optionsome commented Nov 4, 2022 •

edited

Loading

codecov-commenter commented Nov 17, 2022 •

edited by codecov Bot

Loading

leonardehrenfried commented Nov 18, 2022 •

edited

Loading

optionsome commented Dec 9, 2022 •

edited

Loading