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.

Sign up for GitHub

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

Jump to bottom

Take stop and give way signs into account during routing #6426

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Take stop and give way signs into account during routing #6426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
8a4af59
wip
SiarheiFedartsou Oct 27, 2022
7f635f2
Take stop signs into account during routing
SiarheiFedartsou Oct 27, 2022
72ea34f
Take stop signs into account during routing
SiarheiFedartsou Oct 28, 2022
36f3a5e
Take stop signs into account during routing
SiarheiFedartsou Oct 28, 2022
3558ec9
Take stop signs into account during routing
SiarheiFedartsou Oct 28, 2022
4432756
Take stop signs into account during routing
SiarheiFedartsou Oct 28, 2022
350862d
Take stop signs into account during routing
SiarheiFedartsou Oct 28, 2022
9788317
Take stop signs into account during routing
SiarheiFedartsou Oct 28, 2022
c28a683
Take stop signs into account during routing
SiarheiFedartsou Oct 30, 2022
62298cf
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
be1b866
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
b887e72
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
eb72ca4
Merge branch 'master' into sf-stop-sign
SiarheiFedartsou Oct 31, 2022
a7142ee
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
3080be5
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
ee008e7
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
7508262
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
3f7a629
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
04ee126
Take stop signs into account during routing
SiarheiFedartsou Oct 31, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- CHANGED: Make edge metrics strongly typed [#6420](https://github.com/Project-OSRM/osrm-backend/pull/6420)
- FIXED: Typo in file name src/util/timed_historgram.cpp -> src/util/timed_histogram.cpp [#6428](https://github.com/Project-OSRM/osrm-backend/issues/6428)
- Routing:
- ADDED: Stop and give way signs are now taken into account in car profile. [#6426](https://github.com/Project-OSRM/osrm-backend/pull/6426)
- FIXED: Fix adding traffic signal penalties during compression [#6419](https://github.com/Project-OSRM/osrm-backend/pull/6419)
# 5.27.1
- Changes from 5.27.0
Expand Down
77 changes: 77 additions & 0 deletions features/car/give_way_sign_penalties.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
@routing @car @give_way_sign
Feature: Car - Handle give way signs

Background:
Given the profile "car"

Scenario: Car - Encounters a give way sign
Copy link
Member Author

@SiarheiFedartsou SiarheiFedartsou Oct 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It is almost copy-paste of existing tests for traffic lights

osrm-backend/features/car/traffic_light_penalties.feature

Line 7 in afe3445

Scenario: Car - Encounters a traffic light

Probably worth having here all tests we have for traffic lights, but is there way to reduce copy-paste here somehow? E.g. somehow run the same test multiple times, but with different highway tag values for each run?

Copy link
Member

Choose a reason for hiding this comment

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

I expect there's some support for parametrization of tests, but we're on an ancient Cucumber version, so might take a lot of effort. Plus, having a separate file per penalty feature will allow the functionality to diverge if required.

Given the node map
"""
a-1-b-2-c

d-3-e-4-f

g-h-i k-l-m
| |
j n

"""

And the ways
| nodes | highway |
| abc | primary |
| def | primary |
| ghi | primary |
| klm | primary |
| hj | primary |
| ln | primary |

And the nodes
| node | highway |
| e | give_way |
| l | give_way |

# TODO: give way signs with no direction has no any impact on routing at the moment
When I route I should get
| from | to | time | # |
| 1 | 2 | 11.1s | no turn with no give way |
| 3 | 4 | 11.1s | no turn with give way |
| g | j | 18.7s | turn with no give way |
| k | n | 18.7s | turn with give way |


Scenario: Car - Give way direction
Given the node map
"""
a-1-b-2-c

d-3-e-4-f

g-5-h-6-i

j-7-k-8-l

"""

And the ways
| nodes | highway |
| abc | primary |
| def | primary |
| ghi | primary |
| jkl | primary |

And the nodes
| node | highway | direction |
| e | give_way | |
| h | give_way | forward |
| k | give_way | backward |
When I route I should get
| from | to | time | weight | # |
| 1 | 2 | 11.1s | 11.1 | no turn with no give way |
| 2 | 1 | 11.1s | 11.1 | no turn with no give way |
| 3 | 4 | 11.1s | 11.1 | no turn with give way |
| 4 | 3 | 11.1s | 11.1 | no turn with give way |
| 5 | 6 | 12.6s | 12.6 | no turn with give way |
| 6 | 5 | 11.1s | 11.1 | no turn with no give way |
| 7 | 8 | 11.1s | 11.1 | no turn with no give way |
| 8 | 7 | 12.6s | 12.6 | no turn with give way |
77 changes: 77 additions & 0 deletions features/car/stop_sign_penalties.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
@routing @car @stop_sign
Copy link
Member Author

@SiarheiFedartsou SiarheiFedartsou Oct 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @mjjbell !

May I ask you to take a look here?
What I actually did is took our existing traffic lights logic and completely repeated it for stop signs. There is a lot of things to be done here(at least we need to get rid of copy-paste), but before starting doing it, would be great to have your opinion here. Don't you see any fundamental problems in this approach? I am not very familiar with OSRM internals yet(one of the goals of this PR is to educate myself :) ) and tbh some parts of PR were "blindly" copy-pasted from traffic lights, so asking.

Copy link
Member

Choose a reason for hiding this comment

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

Yes if we're viewing all of them as node penalties applicable by direction, then it's basically the same code.

If we wanted to model the impact of each sign/traffic light more accurately (e.g. junction size, unprotected turns, all the other stuff in #2652, etc) then we'd probably want to handle them differently, but I don't think we're at that stage yet.

Feature: Car - Handle stop signs

Background:
Given the profile "car"

Scenario: Car - Encounters a stop sign
Given the node map
"""
a-1-b-2-c

d-3-e-4-f

g-h-i k-l-m
| |
j n

"""

And the ways
| nodes | highway |
| abc | primary |
| def | primary |
| ghi | primary |
| klm | primary |
| hj | primary |
| ln | primary |

And the nodes
| node | highway |
| e | stop |
| l | stop |

# TODO: stop signs with no direction has no any impact on routing at the moment
When I route I should get
| from | to | time | # |
| 1 | 2 | 11.1s | no turn with no stop sign |
| 3 | 4 | 11.1s | no turn with stop sign |
| g | j | 18.7s | turn with no stop sign |
| k | n | 18.7s | turn with stop sign |

Scenario: Car - Stop sign direction
Given the node map
"""
a-1-b-2-c

d-3-e-4-f

g-5-h-6-i

j-7-k-8-l

"""

And the ways
| nodes | highway |
| abc | primary |
| def | primary |
| ghi | primary |
| jkl | primary |

And the nodes
| node | highway | direction |
| e | stop | |
| h | stop | forward |
| k | stop | backward |

When I route I should get
| from | to | time | weight | # |
| 1 | 2 | 11.1s | 11.1 | no turn with no stop sign |
| 2 | 1 | 11.1s | 11.1 | no turn with no stop sign |
| 3 | 4 | 11.1s | 11.1 | no turn with stop sign |
| 4 | 3 | 11.1s | 11.1 | no turn with stop sign |
| 5 | 6 | 13.1s | 13.1 | no turn with stop sign |
| 6 | 5 | 11.1s | 11.1 | no turn with no stop sign |
| 7 | 8 | 11.1s | 11.1 | no turn with no stop sign |
| 8 | 7 | 13.1s | 13.1 | no turn with stop sign |
1 change: 0 additions & 1 deletion features/options/extract/turn_function.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
@routing @testbot @turn_function
Feature: Turn Function Information


Background:
Given the profile file
"""
Expand Down
11 changes: 8 additions & 3 deletions include/extractor/edge_based_graph_factory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "util/typedefs.hpp"

#include "storage/io.hpp"
#include "traffic_signals.hpp"
#include "traffic_flow_control_nodes.hpp"

#include <algorithm>
#include <cstddef>
Expand Down Expand Up @@ -69,7 +69,9 @@ class EdgeBasedGraphFactory
EdgeBasedNodeDataContainer &node_data_container,
const CompressedEdgeContainer &compressed_edge_container,
const std::unordered_set<NodeID> &barrier_nodes,
const TrafficSignals &traffic_signals,
const TrafficFlowControlNodes &traffic_signals,
const TrafficFlowControlNodes &stop_signs,
const TrafficFlowControlNodes &give_way_signs,
const std::vector<util::Coordinate> &coordinates,
const NameTable &name_table,
const std::unordered_set<EdgeID> &segregated_edges,
Expand Down Expand Up @@ -135,7 +137,10 @@ class EdgeBasedGraphFactory
const util::NodeBasedDynamicGraph &m_node_based_graph;

const std::unordered_set<NodeID> &m_barrier_nodes;
const TrafficSignals &m_traffic_signals;
const TrafficFlowControlNodes &m_traffic_signals;
const TrafficFlowControlNodes &m_stop_signs;
const TrafficFlowControlNodes &m_give_way_signs;

const CompressedEdgeContainer &m_compressed_edge_container;

const NameTable &name_table;
Expand Down
29 changes: 21 additions & 8 deletions include/extractor/extraction_containers.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
#include "extractor/scripting_environment.hpp"

#include "storage/tar_fwd.hpp"
#include "traffic_lights.hpp"
#include "traffic_signals.hpp"
#include "traffic_flow_control_nodes.hpp"

#include <unordered_map>
#include <unordered_set>
Expand All @@ -27,20 +26,29 @@ namespace extractor
*/
class ExtractionContainers
{
public:
using InputTrafficFlowControlNode = std::pair<OSMNodeID, TrafficFlowControlNodeDirection>;

private:
using ReferencedWays = std::unordered_map<OSMWayID, NodesOfWay>;
using ReferencedTrafficSignals =
using ReferencedTrafficFlowControlNodes =
std::pair<std::unordered_set<OSMNodeID>, std::unordered_multimap<OSMNodeID, OSMNodeID>>;
// The relationship between way and nodes is lost during node preparation.
// We identify the ways and nodes relevant to restrictions/overrides/signals prior to
// node processing so that they can be referenced in the preparation phase.
ReferencedWays IdentifyRestrictionWays();
ReferencedWays IdentifyManeuverOverrideWays();
ReferencedTrafficSignals IdentifyTrafficSignals();

ReferencedTrafficFlowControlNodes IdentifyTrafficFlowControlNodes(
const std::vector<InputTrafficFlowControlNode> &external_traffic_control_nodes);

void PrepareNodes();
void PrepareManeuverOverrides(const ReferencedWays &maneuver_override_ways);
void PrepareRestrictions(const ReferencedWays &restriction_ways);
void PrepareTrafficSignals(const ReferencedTrafficSignals &referenced_traffic_signals);
void PrepareTrafficFlowControlNodes(
const ReferencedTrafficFlowControlNodes &referenced_traffic_control_nodes,
TrafficFlowControlNodes &internal_traffic_control_nodes);

void PrepareEdges(ScriptingEnvironment &scripting_environment);

void WriteCharData(const std::string &file_name);
Expand All @@ -54,7 +62,6 @@ class ExtractionContainers
using NameOffsets = std::vector<size_t>;
using WayIDVector = std::vector<OSMWayID>;
using WayNodeIDOffsets = std::vector<size_t>;
using InputTrafficSignal = std::pair<OSMNodeID, TrafficLightClass::Direction>;

std::vector<OSMNodeID> barrier_nodes;
NodeIDVector used_node_id_list;
Expand All @@ -69,8 +76,14 @@ class ExtractionContainers

unsigned max_internal_node_id;

std::vector<InputTrafficSignal> external_traffic_signals;
TrafficSignals internal_traffic_signals;
std::vector<InputTrafficFlowControlNode> external_traffic_signals;
TrafficFlowControlNodes internal_traffic_signals;

std::vector<InputTrafficFlowControlNode> external_stop_signs;
TrafficFlowControlNodes internal_stop_signs;

std::vector<InputTrafficFlowControlNode> external_give_ways;
TrafficFlowControlNodes internal_give_way_signs;

std::vector<NodeBasedEdge> used_edges;

Expand Down
14 changes: 10 additions & 4 deletions include/extractor/extraction_node.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#ifndef EXTRACTION_NODE_HPP
#define EXTRACTION_NODE_HPP

#include "traffic_lights.hpp"
#include "traffic_flow_control_nodes.hpp"
#include <cstdint>
Copy link
Member

Choose a reason for hiding this comment

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

Needed?


namespace osrm
{
Expand All @@ -10,14 +11,19 @@ namespace extractor

struct ExtractionNode
{
ExtractionNode() : traffic_lights(TrafficLightClass::NONE), barrier(false) {}
ExtractionNode() : traffic_lights(TrafficFlowControlNodeDirection::NONE), barrier(false) {}
void clear()
{
traffic_lights = TrafficLightClass::NONE;
traffic_lights = TrafficFlowControlNodeDirection::NONE;
stop_sign = TrafficFlowControlNodeDirection::NONE;
give_way = TrafficFlowControlNodeDirection::NONE;
barrier = false;
}
TrafficLightClass::Direction traffic_lights;
TrafficFlowControlNodeDirection traffic_lights;
bool barrier;

TrafficFlowControlNodeDirection stop_sign;
TrafficFlowControlNodeDirection give_way;
};
} // namespace extractor
} // namespace osrm
Expand Down
7 changes: 6 additions & 1 deletion include/extractor/extraction_turn.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ struct ExtractionTurn
int number_of_roads,
bool is_u_turn,
bool has_traffic_light,
bool has_stop_sign,
bool has_give_way_sign,
bool is_left_hand_driving,
bool source_restricted,
TravelMode source_mode,
Expand All @@ -75,7 +77,8 @@ struct ExtractionTurn
const std::vector<ExtractionTurnLeg> &roads_on_the_right,
const std::vector<ExtractionTurnLeg> &roads_on_the_left)
: angle(180. - angle), number_of_roads(number_of_roads), is_u_turn(is_u_turn),
has_traffic_light(has_traffic_light), is_left_hand_driving(is_left_hand_driving),
has_traffic_light(has_traffic_light), has_stop_sign(has_stop_sign),
has_give_way_sign(has_give_way_sign), is_left_hand_driving(is_left_hand_driving),

source_restricted(source_restricted), source_mode(source_mode),
source_is_motorway(source_is_motorway), source_is_link(source_is_link),
Expand All @@ -100,6 +103,8 @@ struct ExtractionTurn
const int number_of_roads;
const bool is_u_turn;
const bool has_traffic_light;
const bool has_stop_sign;
const bool has_give_way_sign;
const bool is_left_hand_driving;

// source info
Expand Down
10 changes: 7 additions & 3 deletions include/extractor/extractor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#include "util/guidance/turn_lanes.hpp"

#include "restriction_graph.hpp"
#include "traffic_signals.hpp"
#include "traffic_flow_control_nodes.hpp"
#include "util/typedefs.hpp"

namespace osrm
Expand All @@ -66,7 +66,9 @@ class Extractor
LaneDescriptionMap turn_lane_map;
std::vector<TurnRestriction> turn_restrictions;
std::vector<UnresolvedManeuverOverride> unresolved_maneuver_overrides;
TrafficSignals traffic_signals;
TrafficFlowControlNodes traffic_signals;
TrafficFlowControlNodes stop_signs;
TrafficFlowControlNodes give_way_signs;
std::unordered_set<NodeID> barriers;
std::vector<util::Coordinate> osm_coordinates;
extractor::PackedOSMIDs osm_node_ids;
Expand All @@ -86,7 +88,9 @@ class Extractor
const std::vector<util::Coordinate> &coordinates,
const CompressedEdgeContainer &compressed_edge_container,
const std::unordered_set<NodeID> &barrier_nodes,
const TrafficSignals &traffic_signals,
const TrafficFlowControlNodes &traffic_signals,
const TrafficFlowControlNodes &stop_signs,
const TrafficFlowControlNodes &give_way_signs,
const RestrictionGraph &restriction_graph,
const std::unordered_set<EdgeID> &segregated_edges,
const NameTable &name_table,
Expand Down
1 change: 1 addition & 0 deletions include/extractor/extractor_callbacks.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include <string>
#include <unordered_map>
#include <unordered_set>

namespace osmium
{
Expand Down
6 changes: 4 additions & 2 deletions include/extractor/graph_compressor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include "extractor/scripting_environment.hpp"
#include "util/typedefs.hpp"

#include "traffic_signals.hpp"
#include "traffic_flow_control_nodes.hpp"
#include "util/node_based_graph.hpp"

#include <memory>
Expand All @@ -26,7 +26,9 @@ class GraphCompressor

public:
void Compress(const std::unordered_set<NodeID> &barrier_nodes,
const TrafficSignals &traffic_signals,
const TrafficFlowControlNodes &traffic_signals,
const TrafficFlowControlNodes &stop_signs,
const TrafficFlowControlNodes &give_way_signs,
ScriptingEnvironment &scripting_environment,
std::vector<TurnRestriction> &turn_restrictions,
std::vector<UnresolvedManeuverOverride> &maneuver_overrides,
Expand Down
Loading