-
Notifications
You must be signed in to change notification settings - Fork 665
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
Simple legal default speeds #4739
base: master
Are you sure you want to change the base?
Conversation
…urban edge speeds
// Speed assignment | ||
speed_assigner.UpdateSpeed(directededge, density, infer_turn_channels, end_node_code, | ||
end_node_state_code); | ||
end_node_state_code, | ||
found_speed == 0 || !update_speed_from_legal_speeds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test for speed limit if speed assigner overrides legal default speeds
src/mjolnir/legal_speed.h
Outdated
} | ||
|
||
return speed; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra semicolon
I ran some route tests on a Switzerland extract and compared routes on a graph with legal speeds set as edge speeds vs a graph with no extra speed assignments, and the results are not very convincing (results from the default speed graph in light blue, default graph in purple): Routes via rural areas tend to deviate from arterial roads onto lower road classes. Without looking at this in more detail, it seems very likely that these smaller roads are assigned high speeds because the general rural legal speed for Switzerland is 80 kph (which is higher than what Valhalla assumes for these road classes if no maxspeed tag exists). In urban areas, I observed almost no differences, presumably in part due to the high occurence of I think @nilsnolde was right, the purely density based assignment does not make much sense for setting edge speeds, maybe that should only be applied in cases where we have legal speeds that are based on some more specific rule (form of way, road use, etc.). |
This PR adds support for simple legal default speeds.
Fixes #3334 and #1069.
Following the discussions in #1069 and #3334 this PR attempts to facilitate assignment of country specific legal speeds based on simple rules. The idea is to allow the user to provide a JSON file that follows the schema proposed in this repo (see example JSON here). If present, valhalla will try to set edge speeds based on the edges' admin info where no tagged speeds were present.
Since in most countries there are rules simply on the notion of whether a road segment is in an urban or rural area, this assignment of default speeds happens in the enhance stage of the graph build, where the density estimate is available.
The proposed logic further attempts to set speeds according to rules specific to a certain road class or road use. While the aforementioned JSON schema allows for much more complicated rules, this logic only implements a few of these for now. I think it is possible to extend this in the future by adding another legal speed assignment stage in earlier stages of the graph build where more information from the original way tags are available. Though it won't be possible to implement all of these rules, since some are dependent on vehicle information only available in costing.
WRT the customizable speed config added in #3055, that config, if present, will still override any speeds set by this logic.
Tasklist