Skip to content

Commit 9ad432c

Browse files
mjjbellmjjbell
authored
Fix adding traffic signal penalties during compression (#6419)
Weight and duration penalties are flipped in the lambda function that applies penalties from traffic signals. Duration is in deciseconds, whilst weight is multipled by 10^weight_precision, with weight_precision being 1 by default. Therefore, for default routability profile, the penalties end up being the same, hence why no tests picked this up. If distance weight is used however, it will incorrectly apply an additional penalty to the weight, and not add the traffic signal delay to the duration in the routing graph. To confuse things further, in some API responses the values are correct because they use geometry data instead, but it's still possible that a sub-optimal route was selected. However, given the distance weight is in meters, and the additional penalty per traffic light would be 20, it's unlikely this would have changed the routing results. In any case, we correct the function to apply the arguments correctly.
1 parent c1d2c15 commit 9ad432c

File tree

3 files changed

+61
-29
lines changed

3 files changed

+61
-29
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
- FIXED: Handle snapping parameter for all plugins in NodeJs bindings, but not for Route only. [#6417](https://github.com/Project-OSRM/osrm-backend/pull/6417)
55
- FIXED: Fix annotations=true handling in NodeJS bindings & libosrm. [#6415](https://github.com/Project-OSRM/osrm-backend/pull/6415/)
66
- FIXED: Fix bindings compilation issue on the latest Node. Update NAN to 2.17.0. [#6416](https://github.com/Project-OSRM/osrm-backend/pull/6416)
7+
- Routing:
8+
- FIXED: Fix adding traffic signal penalties during compression [#6419](https://github.com/Project-OSRM/osrm-backend/pull/6419)
79
# 5.27.1
810
- Changes from 5.27.0
911
- Misc:

‎features/car/traffic_light_penalties.feature

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,50 @@ Feature: Car - Handle traffic lights
6666
| k | traffic_signals | backward |
6767

6868
When I route I should get
69-
| from | to | time | # |
70-
| 1 | 2 | 11.1s | no turn with no traffic light |
71-
| 2 | 1 | 11.1s | no turn with no traffic light |
72-
| 3 | 4 | 13.1s | no turn with traffic light |
73-
| 4 | 3 | 13.1s | no turn with traffic light |
74-
| 5 | 6 | 13.1s | no turn with traffic light |
75-
| 6 | 5 | 11.1s | no turn with no traffic light |
76-
| 7 | 8 | 11.1s | no turn with no traffic light |
77-
| 8 | 7 | 13.1s | no turn with traffic light |
69+
| from | to | time | weight | # |
70+
| 1 | 2 | 11.1s | 11.1 | no turn with no traffic light |
71+
| 2 | 1 | 11.1s | 11.1 | no turn with no traffic light |
72+
| 3 | 4 | 13.1s | 13.1 | no turn with traffic light |
73+
| 4 | 3 | 13.1s | 13.1 | no turn with traffic light |
74+
| 5 | 6 | 13.1s | 13.1 | no turn with traffic light |
75+
| 6 | 5 | 11.1s | 11.1 | no turn with no traffic light |
76+
| 7 | 8 | 11.1s | 11.1 | no turn with no traffic light |
77+
| 8 | 7 | 13.1s | 13.1 | no turn with traffic light |
78+
79+
80+
Scenario: Car - Traffic signal direction with distance weight
81+
Given the profile file "car" initialized with
82+
"""
83+
profile.properties.weight_name = 'distance'
84+
profile.properties.traffic_light_penalty = 100000
85+
"""
86+
87+
Given the node map
88+
"""
89+
a---b---c
90+
1 2
91+
| |
92+
| |
93+
| |
94+
| |
95+
| |
96+
d-------f
97+
98+
"""
99+
100+
And the ways
101+
| nodes | highway |
102+
| abc | primary |
103+
| adfc | primary |
104+
105+
And the nodes
106+
| node | highway |
107+
| b | traffic_signals |
108+
109+
When I route I should get
110+
| from | to | time | distances | weight | # |
111+
| 1 | 2 | 100033.2s | 599.9m,0m | 599.8 | goes via the expensive traffic signal |
112+
78113

79114

80115
Scenario: Car - Encounters a traffic light

‎src/extractor/graph_compressor.cpp

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,6 @@ void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
276276
const auto forward_weight2 = fwd_edge_data2.weight;
277277
const auto forward_duration1 = fwd_edge_data1.duration;
278278
const auto forward_duration2 = fwd_edge_data2.duration;
279-
const auto forward_distance2 = fwd_edge_data2.distance;
280279

281280
BOOST_ASSERT(0 != forward_weight1);
282281
BOOST_ASSERT(0 != forward_weight2);
@@ -285,50 +284,46 @@ void GraphCompressor::Compress(const std::unordered_set<NodeID> &barrier_nodes,
285284
const auto reverse_weight2 = rev_edge_data2.weight;
286285
const auto reverse_duration1 = rev_edge_data1.duration;
287286
const auto reverse_duration2 = rev_edge_data2.duration;
288-
const auto reverse_distance2 = rev_edge_data2.distance;
289287

290288
#ifndef NDEBUG
291289
// Because distances are symmetrical, we only need one
292290
// per edge - here we double-check that they match
293291
// their mirrors.
294292
const auto reverse_distance1 = rev_edge_data1.distance;
295293
const auto forward_distance1 = fwd_edge_data1.distance;
294+
const auto forward_distance2 = fwd_edge_data2.distance;
295+
const auto reverse_distance2 = rev_edge_data2.distance;
296296
BOOST_ASSERT(forward_distance1 == reverse_distance2);
297297
BOOST_ASSERT(forward_distance2 == reverse_distance1);
298298
#endif
299299

300300
BOOST_ASSERT(0 != reverse_weight1);
301301
BOOST_ASSERT(0 != reverse_weight2);
302302

303-
auto apply_e2_to_e1 = [&graph](EdgeID edge,
304-
EdgeWeight weight,
305-
EdgeDuration duration,
306-
EdgeDistance distance,
307-
EdgeDuration &duration_penalty,
308-
EdgeWeight &weight_penalty) {
309-
auto &edge_data = graph.GetEdgeData(edge);
310-
edge_data.weight += weight;
311-
edge_data.duration += duration;
312-
edge_data.distance += distance;
303+
auto apply_e2_to_e1 = [&graph](EdgeID edge1,
304+
EdgeID edge2,
305+
EdgeWeight &weight_penalty,
306+
EdgeDuration &duration_penalty) {
307+
auto &edge1_data = graph.GetEdgeData(edge1);
308+
const auto &edge2_data = graph.GetEdgeData(edge2);
309+
edge1_data.weight += edge2_data.weight;
310+
edge1_data.duration += edge2_data.duration;
311+
edge1_data.distance += edge2_data.distance;
313312
if (weight_penalty != INVALID_EDGE_WEIGHT &&
314313
duration_penalty != MAXIMAL_EDGE_DURATION)
315314
{
316-
edge_data.weight += weight_penalty;
317-
edge_data.duration += duration_penalty;
315+
edge1_data.weight += weight_penalty;
316+
edge1_data.duration += duration_penalty;
318317
// Note: no penalties for distances
319318
}
320319
};
321320

322321
apply_e2_to_e1(forward_e1,
323-
forward_weight2,
324-
forward_duration2,
325-
forward_distance2,
322+
forward_e2,
326323
forward_node_weight_penalty,
327324
forward_node_duration_penalty);
328325
apply_e2_to_e1(reverse_e1,
329-
reverse_weight2,
330-
reverse_duration2,
331-
reverse_distance2,
326+
reverse_e2,
332327
reverse_node_weight_penalty,
333328
reverse_node_duration_penalty);
334329

0 commit comments

Comments
 (0)