-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Implement more flexible turn weight function #4775
Comments
Preliminaries
1) There is the
|
Attribute | Read/write? | Type | Notes |
---|---|---|---|
direction_modifier | Read | Enum | Geometry of turn. Defined in include/extractor/guidance/turn_instruction.hpp |
turn_type | Read | Enum | Priority of turn. Defined in include/extractor/guidance/turn_instruction.hpp |
has_traffic_light | Read | Boolean | Is a traffic light present at this turn? |
source_restricted | Read | Boolean | Is it from a restricted access road? (See definition in process_way ) |
target_restricted | Read | Boolean | Is it to a restricted access road? (See definition in process_way ) |
angle | Read | Float | Angle of turn in degrees (0-360 : 0 =u-turn, 180 =straight on) |
duration | Read/write | Float | Penalty to be applied for this turn (duration in deciseconds) |
weight | Read/write | Float | Penalty to be applied for this turn (routing weight) |
- The different lua functions
setup
,process_node
,process_way
andprocess_turn
have different accesses to osm tags.
- in short,
setup
is used when osrm still has access to osm tags.- tags are whitelisted, restricted, included
- speeds depending on surface, way type etc is set
- the
process
functions do not have access to osm tags anymore but only to the data that is provided by the API
2) There is cpp
land in which process_function
is fed
- Depending on version number, turn_function is called here : https://github.com/Project-OSRM/osrm-backend/blob/master/src/extractor/scripting_environment_lua.cpp#L968
- Input parameters are two objects
context.profile_table
andExtractionTurn turn
contest.profile_table
is asol::table
object and seem to be values set in thesetup
functions oflua
profiles. not completely sure hereExtractionTurn
is a handy object https://github.com/Project-OSRM/osrm-backend/blob/master/include/extractor/extraction_turn.hpp#L15 storing all information such ashas_traffic_light
that can be used inlua
world later on
- the
graph_compressor
does something I don't quite understand, feedingProcessTurn
for every node here https://github.com/Project-OSRM/osrm-backend/blob/master/src/extractor/graph_compressor.cpp#L234 - Edge based graph factory generates
ExtractionTurn
objects here https://github.com/Project-OSRM/osrm-backend/blob/master/src/extractor/edge_based_graph_factory.cpp#L601 and callsProcessTurn
afterwards 🎉 looks like the right place to be
Enough digging. This should cover the most I need to know about turn functions. (Maybe...)
Open questions
- What does
graph_compressor
do regarding the turn function - At what point do we process the actual OSM tags?
taginfo.json
?
This is a hack that was introduced in the wake of the via-way-turn-restriction work to extract the traffic light turn penalty. We should probably think about a better implementation there. |
Prior work
PR 3009
PR 2912
|
Beware❗️ Docs were outdated. Updating docs first, then revisiting this comment. Theory
This is probably the part that should be use-case/request/community driven. Important implementation background information:Exposing more information to These are found in
We should consider this when making choices.
|
@emiltin regarding the issues you mentioned:
edit: apparently travel mode is already available in |
the number of lanes is rarely tagged explicitly, and alone it is therefore not enough to describe the road type. the use case described in #477 is that you don't want to suggest crossing (either turning across, or going straight across) a bigger road, if there is no traffic light. this includes motorway, trunk and primary roads, also in cases where number of lanes is not specified. probably also secondary and tertiary probably with a lower penalty. other things to consider are tags like oneway, maxspeed, access, vehicle and motor_vehicle which affect the amount, density and speed of traffic you ned to cross. note that you probably also need sub tags, eg. maxspeed:forward, oneway:car, etc. you also need to know if there's traffic lights at the intersection. if there is, there should probably not be any additional penalty beyond what the the traffic light itself incurs. if there's no traffic light, it would be useful to know about stop and yiels signs. tags related to bus, tram and pedestrian crossings could be useful. if live traffic data is used, that information would also be useful. higher speeds probably mean it's harder to cross. of course live traffic volumes would also be very useful, if it was available. |
for mode change i think it's ok, since mode before and after the turn is available. |
Considering all discussions I am proposing this approach for this ticket. Open for discussion/feedback please ❤️ New API Proposal
|
Attribute | Read/write? | Type | Notes |
---|---|---|---|
angle | Read | Float | Angle of turn in degrees (0-360 : 0 =u-turn, 180 =straight on) |
number_of_roads | Read | Integer | Number of ways at the intersection of the turn |
is_u_turn | Read | Boolean | Is the turn a u-turn? |
has_traffic_light | Read | Boolean | Is a traffic light present at this turn? |
weight | Read/write | Float | Penalty to be applied for this turn (routing weight) |
duration | Read/write | Float | Penalty to be applied for this turn (duration in deciseconds) |
source
andtarget
are information on the road involved in the turn and include
Attribute | Read/write? | Type | Notes |
---|---|---|---|
restricted | Read | Boolean | Is a restricted access road? (See definition in process_way ) |
mode | Read | Enum | Travel mode. Defined in include/extractor/travel_mode.hpp |
highway_user_class | Read | Integer | Classification based on highway tag defined by user during setup |
access_user_class | Read | Integer | Classification based on access tag defined by user during setup |
is_motorway | Read | Boolean | way is a motorway |
is_link | Read | Boolean | way is a slip/link road |
number_of_lanes | Read | Unsigned | total number of lanes in way |
Usage of new API
- divebombs can be prevented by adjusting weight using
road_classification.motorway_class
androad_classification.link_class
information - mode changes detectable by comparing
source.mode
andtarget.mode
- turn duration can be set considering
road_classification.num_lanes
,number_of_roads
andhas_traffic_light
- using way types to calculate turn penalties #477 @emiltin adjustments for crossing can be then done using
highway_user_class
and/oraccess_user_class
- e.g. for a walking profile we might consider motorway, trunk and primary as
[1] big roads
, secondary and tertiary as[2] middle-sized roads
, and residential, living_street and pedestrian as[3] normal roads
- we can then set the following during
setup
:
highway_user_class = { motorway = 1, trunk = 1, primary = 1, secondary = 2, tertiary = 2, residential = 3, living_street = 3, pedestrian = 3 }
- depending on the highway tag,
source.highway_user_class
andtarget.highway_user_class
will be set to1
,2
or3
and can be used to set a turn weight in the lua profile.
- e.g. for a walking profile we might consider motorway, trunk and primary as
Open Question
- Should we add additional
is_stop
andis_give_way
to theturn
parameter?
this would result in a breaking API change
Feedback highly, highly appreciated @TheMarex @emiltin @daniel-j-h
Also, thanks for the guidance until now.
With this approach it seems you only have information about the source and destination leg of the turn. So if you cross straight over a road (say a you cross a trunk road as a pedestrian) you would not get any information about the road you cross? You get the turn angle and the number of legs in the intersection, but you actually don't know about the geometry and thus which legs/roads you cross. I'm a bit concerned that classification of just two tags (highway and access) will be too limited. A more general approach where you can use more tags would be preferable, but I understand there are resource limitations. The naming of motorway_user_class is confusing since it's based on the 'highway' tag. Also the naming *_class seems a bit odd for Boolean values? |
facepalm Yes, of course you would need it too. Will add information about the street that needs to be crossed
I will take another look at the extraction phase. I still haven't got a full overview of when we have or lose access to certain information/tags. Let me see whether I can make it more general.
Ugh. I keep confusing highway tag and motorway key. I meant highway all the way through! Thanks!
I agree. It is actually the same values that we have in Thanks for the super rapid fast feedback expect a second version of the proposal by the end of the day |
What you might want to think about is having an abstraction between the source of the values (various guidance related structs) and the API you actually expose in Lua. For example you can save the value of
We currently version the Lua API so if we need to break the API we can do that. Though maybe we can think about what it would mean to avoid breaking the API for these changes? Since most of the issues mentioned here are quite generic, I would try to find a few examples for each heuristic you want to implement. That should give you an idea how often these cases occur and maybe help you to prioritize. |
some other things that would be nice to have:
|
Good point. I'll consider it. I need to makes tests to see how much storage consumption will increase when I add new attributes/keys and I am afraid that traffic_calming is very specific. Is it? I am not sure.
Currently, you can set the turn weight to infinity by setting
So as long as the turn function is able to identify the turns you want to exclude, you can exclude it by setting it's turn weight very high. I will think about an exclude method for turn functions but I am afraid it's not easily designable. In contrary to the way function for example, where you can just exclude certain
I see! Being able to identify kerbs for wheelchairs seems a legit/important case. From the design point of few, I feel like this shouldn't be a I think,
If you agree I can write up a ticket thinking about how |
Exclude class: the one that I can set with another examples except for virtual turns: maybe a better example is to add some penalty on any traffic calming for car profile. the traffic calming is sometimes on a crossing, sometimes in the middle of a road (if you look at #3862, there was some info on the implementation). maybe we could remove |
on the crossed road(s), which is needed for bicycle/foot routing: maybe and extra array of highways with the same info as a very nice feature would be to have also non-routable lines, like removed highways (like primary highways with |
First ApproachI started implementation in PR #4789. I haven't decided on a final API yet but I started making changes to expose information to the turn function that does not really cost us more memory during the process (see the table in a former comment). I also left the parameters as they are to prevent a breaking API change. The information exposed to the turn function as attributes of
The difference to the current version is The two vectors
They are ordered geometrically counter-clockwise and are split to two vectors depending on whether they are to the right or left, when coming from
The PR is still WIP but it's a start. If you want you can use it to play around and see whether it can already solve one/some of your use cases (maybe partly). Next StepsFor now I prioritized the attributes I would like to expose in the context of this issue as follows:
I might try implementing all of them and see whether it is feasible or not. edit: stops are not easy. might be out of scope for now. will also take a second look at the PR that targeted stops before. Open issuesAs for the many other things we have discussed here:
I'll keep you all updated 😄 |
The concept of crossing roads is a bit tricky, since all ways meet at a central node. Consider this simple situation:
If 'anc' is a big road, then going from d to c mean you cross a big road. If 'anc' is oneway, then one of the legs would be incoming and thus not included in the two vectors roads_on_the_right and roads_on_the_left. I think this shows we also need incoming legs. If 'anc' is big and 'dn' and 'nb' are smaller sideroads, then going from d to c does not require crossing a big road (assuming right-side driving). To determine this you need to check if there's a big road on both sides. Another case would be 'na' an d 'nb' being bigger roads but both oneways away from n. In that case you don't really cross a big road when going from d to c. Turn restrictions would really also need to be considered. |
PR #4789 is now open for review. What the PR does:
What the PR does not do:
|
Closing and update top comment to summarize |
- Changes from 5.15.2: - Guidance - ADDED Project-OSRM#4676: Support for maneuver override relation, allowing data-driven overrides for turn-by-turn instructions [Project-OSRM#4676](Project-OSRM#4676) - CHANGED Project-OSRM#4830: Announce reference change if names are empty - CHANGED Project-OSRM#4835: MAXIMAL_ALLOWED_SEPARATION_WIDTH increased to 12 meters - CHANGED Project-OSRM#4842: Lower priority links from a motorway now are used as motorway links [Project-OSRM#4842](Project-OSRM#4842) - CHANGED Project-OSRM#4895: Use ramp bifurcations as fork intersections [Project-OSRM#4895](Project-OSRM#4895) - CHANGED Project-OSRM#4893: Handle motorway forks with links as normal motorway intersections[Project-OSRM#4893](Project-OSRM#4893) - FIXED Project-OSRM#4905: Check required tags of `maneuver` relations [Project-OSRM#4905](Project-OSRM#4905) - Profile: - FIXED: `highway=service` will now be used for restricted access, `access=private` is still disabled for snapping. - ADDED Project-OSRM#4775: Exposes more information to the turn function, now being able to set turn weights with highway and access information of the turn as well as other roads at the intersection [Project-OSRM#4775](Project-OSRM#4775) - FIXED Project-OSRM#4763: Add support for non-numerical units in car profile for maxheight [Project-OSRM#4763](Project-OSRM#4763) - ADDED Project-OSRM#4872: Handling of `barrier=height_restrictor` nodes [Project-OSRM#4872](Project-OSRM#4872)
Goal
Have a turn weight function that allows more flexible setting of turn weights.
This takes places during the
extraction
phase and (probably?) in thelua
profiles.The function should enable implementing turn weights that are able to regard
and others such as slip roads (#3133), traffic lights (#96), turn lanes (#607), stop signs (#3009).
Implementation completed in PR #4789
What the PR does:
What the PR does not do:
traffic_calming
. A related issue would be Take Traffic Calming Road Furniture Into Account #3021 .Prior Work
Prior work exists that got stale at some point and were never merged:
Approach
Implementation probably involves changing the turn function such that osm tags are available. Better description/information will be inserted here as soon as I have a better scope of things.New API Proposal
Old and deprecated approach:
The user can set an arbitrary integer/classification to edges based on
highway
tag andaccess
tag.process_turn
process_turn(profile, turn, source, target)
profile
is the profile table setup duringsetup
turn
gives specific information about the turn and includes0-360
:0
=u-turn,180
=straight on)source
andtarget
are information on the road involved in the turn and includeprocess_way
)include/extractor/travel_mode.hpp
this would result in a breaking API change
Docs are here
New accessible attributes in
process_turn
ExtractionTurnLegs
have the propertiesThe order of the roads in roads_on_the_right and roads_on_the_left are counter clockwise. If the turn is a u turn, all other connected roads will be in roads_on_the_right.
Todos
profiles.md
Anyone with hints/great ideas/comments will be appreciated ❤️
@TheMarex @daniel-j-h
The text was updated successfully, but these errors were encountered: