TODOs to create longer shortcuts #4602
Conversation
| // - most other attributes (other than classification & surface) could also take the "most | ||
| // restrictive" | ||
| // for the entire shortcut |
There was a problem hiding this comment.
actually: if the attribute is used to cost by a factor (i.e. it scales with length of the edge), then we do need to break up. but if it's like destination_only lump penalty we don't need to
There was a problem hiding this comment.
there can't be many (any?) destination_only motorways/trunks. I assume you are looking only at attributes that impact driving costing classes (pretty sure bicycle and pedestrian skip shortcuts).
There was a problem hiding this comment.
there can't be many (any?) destination_only motorways/trunks
that's true. even tertiary would be rare I imagine (the lowest FRC we even contract)
There was a problem hiding this comment.
Was there a change to create shortcuts on secondary and tertiary? Or is my memory bad? I thought we only created shortcuts on level 0 (motorways, trunks, primary).
There was a problem hiding this comment.
That’s why I think it’d make quite an impact
| if (oppdiredge1->sign() || oppdiredge2->sign()) { | ||
| return false; | ||
| } | ||
|
|
||
| // Do not allow a shortcut on a ramp crossing at a traffic signal or where | ||
| // more than 3 edges meet. | ||
| // TODO(nils): why? this doesn't seem important for costing/expansion, also we only allow exactly 2 |
There was a problem hiding this comment.
this seems like the sort of thing that is added to fix some issue that was found without it. There are tons of special cases (as I remember) with ramps as they move from highway to "regular" roads.
There was a problem hiding this comment.
yeah I'm just wondering how that can have any effect on shortcut creation, since if there's a ramp, that node would have at least 3 outgoing edges and we early-abort its contraction
There was a problem hiding this comment.
should I work on this, I guess there's a ton of little scenarios I'll need to test
|
initial results are encouraging: I used an extract with ~ 1 Mio ways (a well balanced extract of cities and rural, Rheinland-Pfalz) and just by removing names & speeds restriction from shortcuts we went down from 10166 shortcuts in level 0 (avg 5 edges/shorcut) to 6978 shortcuts (avg 8 edges/shortcut). even better results for level 1! but frankly this is a shitshow for the tests.. urgh.. |
nilsnolde
force-pushed
the
nn-less-shortcuts
branch
from
8873515
to
454b99a
Compare
| @@ -1570,9 +1570,8 @@ TEST(BiDiAstar, test_recost_path) { | |||
| 3 4 5 | |||
| )"; | |||
| const gurka::ways ways = { | |||
| // make ABC to be a shortcut | |||
| // make ABCDE to be a shortcut | |||
There was a problem hiding this comment.
somehow this test is still failing.. it now takes the supposedly longer route, not ABCDE and I don't understand why yet..
nilsnolde
force-pushed
the
nn-less-shortcuts
branch
from
454b99a
to
ad0d075
Compare
| // if they have access restrictions those must match (for modes that use shortcuts) | ||
| // TODO(nils): same as above: same as access masks | ||
| // IMO, we should take the most restrictive value for the entire shortcut | ||
| // will be a bit harder to determine for timed restrictions though |
There was a problem hiding this comment.
one more comment here: access restrictions are mode specific. but that doesn't matter for most as those modes are "static" (always the same, e.g. height/weight etc). so it still makes sense to only copy the most restrictive one for each type.
only for conditional restrictions modes are dynamic. to not make our life too hard here, I'd propose to simply copy all of the conditional ones onto the shortcut. there's few ones anyways, even in commercial datasets, and if we'd break up shortcuts because of them we'd look at them anyways.
|
getting back to this next week |
|
@nilsnolde definitely should test multiple countries/areas with this change. All of the test files in |
|
|
EDIT: I'll leave this open as draft, it contains all potential todos I could think of. I'll open another PR with the changes to remove names and speed.
relates to #4599 but goes much wider in scope.
for now, I just left a few TODOs in places which I think could be re-investigated. I might miss a few important concepts downstream of creating shortcuts, e.g. I find the
shortcutsmask andsupersededstill a bit hard to make full sense of.I'd like to discuss those TODOs before I jump in to work on it. but I think (and hope) we can create quite a few less and quite a bit longer shortcuts in the future.
I should expand on my thoughts a bit though:
it seems we only contract a node when it has 2 outbound edges on the same hierarchy, but also we even only allow those 2 edges to have the same FRC (for costing purposes). that's the key to most TODOs.
consider these 2 shortcuts we produce today:
where AB and BC differ in access modes and/or access restrictions and/or tolls and/or most other attributes.
note that for an algorithm it doesn't matter what B is connected to otherwise. if it's connected to another edge of the same hierarchy, B wouldn't ever be contracted. so an algorithm would need to traverse BOTH AB and BC in any case, it has no way of turning anywhere from B. hence it doesn't matter if break up the shortcut and we might as well not and assign the most restrictive attributes to the full thing ABC and contract B away.
does that make any sense?