valhalla / valhalla

Open Source Routing Engine for OpenStreetMap
https://valhalla.github.io/valhalla/
Other
4.44k stars 674 forks source link

Support `lit` OSM key #3848

Closed nilsnolde closed 11 months ago

nilsnolde commented 1 year ago

For pedestrian safety.

Easy:

Medium-easy:

ImreSamu commented 1 year ago

https://wiki.openstreetmap.org/wiki/Key:lit

add one bit on directed edge for lit

I would also store the "missing/unknown" information.

For pedestrian safety.

extreme example: I have 2 route proposals:

Route_A :

Route_B :

in this case: I will prefer the "Route_B" ( if all other information is the same. )

nilsnolde commented 1 year ago

That's a good point @ImreSamu . Didn't think that far yet! Not sure if we want to afford another bit for lit. Or maybe there's another way to do it?! What do you think @kevinkreiser / @dnesbitt61 ?

nilsnolde commented 1 year ago

In the end it’s similar to all „optional“ tags right.. We can’t store „maybe“ for everything, nor can the community tag everything that’s missing (bike lane/sidewalk on roads are other examples and your same logic could be applied there but isn’t for that reason). Isn’t the convention rather to tag when a feature does exist instead of tagging non-existent features? For pure data size/parsing performance I surely prefer that. Then it boils down to OSM completeness, smth we will have to live with.

Wanna say, I think in the end it’s fine to use a boolean and assume „no“ if it’s not present.

kevinkreiser commented 1 year ago

in the end it’s fine to use a boolean and assume „no“ if it’s not present

agreed

Not sure if we want to afford another bit for lit. Or maybe there's another way to do it?!

taking a bit would be unfortunate as we have so few remaining. using the extended structure is possible but very costly to the final tileset size. we could also store it in the edgeinfo but this makes costing less performant. another option is to find another property that can only be true when lit isnt true and vice versa and then use a union to allow them to share the bit. this only works if there is another property on the edge that lets you determine which case could be true. we have an example of this for stopimpact. im struggling to think of a case where something couldnt be lit though..

nilsnolde commented 1 year ago

Yeah, neither edgeinfo nor the extension is really nice. In my mind, the extension is a very nice sledge hammer help for forks that don't want to lose potentially upstream mergeabiility. (was told it's not much different from the penalty of using edgeinfo) I'll already have to use it for one use case. Of course we can use it at some point for really urgent cases without going v4.

Not sure if that'll ever be properly addressed, IMO OSM should not encourage to tag any street lit=no that's not lit, that carries a very long tail of other tags that would've to be tagged "no" as well, and that'd be a huge space issue for extracts. And then software has to assume "no" if it's missing. But again, I'm not much of an OSM editor..

ImreSamu commented 1 year ago

Wanna say, I think in the end it’s fine to use a boolean and assume „no“ if it’s not present.

:tiger2: :smile:

Individual risk assessment is based on subjective factors so not easy to create a perfect data model.

for me "pedestrian safety" == Avoiding KNOWN risk

"known" RISK

so for me, the lit=no is more valuable information than the lit=yes or lit is unknown

In other words,

nilsnolde commented 1 year ago

I see. Your examples make sense, and thankfully they also work with one bit. The exact logic while costing is, as I said initially, still TBD. More important for feasibility is how much space will it take. And there we’re at 1 bit now and that makes us happy enough to proceed.

I’d say let’s wait with the costing logic until someone tackles it (unless you’re interested?). Looks we should do smth more complex though.

EDIT: more complex, e.g. general penalty if lit=no and extra penalty if lit=no and tunnel=yes. Important is just to remember that also ways with tunnel=yes and no lit tag will also be considered unlit by default. So a mapper must explicitly tag lit=yes for us to consider it a lit edge.

nilsnolde commented 1 year ago

@chrstnbwnkl and me are about to PR adding the lit key to the graph and will implement it in EdgeCost in the next PR. That raises again the question how we want to use it for costing..

I'd suggest to use it as-is, i.e. expose a use_unlit factor for pedestrian (& bicycle?), ranging from 0 - 1. use_unlit somehow fits the bill better than use_lit, but I don't feel strongly about it. After thinking about it a bit more carefully, IMO it's not very practical to somehow conflate multiple properties into a use_safe or so parameter, as it's very subjective what is exactly perceived as "safe" and surely depends on the context as well. Should we add more safety-related features at some point and decide it's sensible to conflate some into a new parameter, we could still do that and keep use_unlit without deprecating it (my main worry when deciding on new parameters).

I tend to use similar logic as we do for use_ferry: have a very high factor of around 10.0 when use_unlit == 0.0, converge at 1.0 for use_unlit == 0.5 and favor it for values above 0.5. But we can discuss those details in the actual PR.

Any opinions/suggestions from others?

chrstnbwnkl commented 1 year ago

I agree with using it "as is" as opposed to grouping it because of how the definition of "safe" differs for each individual. As discussed previously, I don't, however, agree with using it in a negated way, but prefer to stick to use_lit.

Regarding the value range, I think in the case of use_lit == 0.5 meaning no preference, I'm wondering if there's any use case for use_lit < 0.5 (except maybe in connection with dubious criminal activity? :smile:). That's why I would propose use_lit == 0 meaning no preference and use_lit == 1 meaning strong preference for lit edges.

nilsnolde commented 1 year ago

I'm wondering if there's any use case for use_lit < 0.5 (except maybe in connection with dubious criminal activity?

hahah yeah that sounds reasonable, didn't think about it enough apparently.

I agree it's more intuitive using use_lit and lower the costing for values > 0.0 (and maybe default to 0.5, i.e. slight preference), an actual argument (that I didn't really think about until right now) (the argument is still true, but it's not really an argument in the first place, see EDIT) to go with use_unlit would be that our algorithm is much better at penalizing segments rather than preferring them. I remember Dave mentioning that a few times, and I guess the reason is that you have a lower difference in costing, compared to not applying the factor, when you prefer rather than avoid, e.g. if you imagine an edge being lit with a before-cost of 1 and you apply a favoring of factor 2, that makes it an after-cost of 0.5. But if you do it the other way around (avoid rather than prefer), you'll end up with an after-cost of 2 with a factor of 2.

Actually I don't think we have any such factor yet, which is only to be preferred/avoided and not the other way around, so there's no good precedence I think. And while I do agree (now) that use_lit is more intuitive, use_unlit is potentially more effective and I tend to prefer efficiency over intuition:)

What do the others think?

EDIT: sorry, slight correction: of course we could still use use_lit and just turn the user-provided factor into the inverse and use it to avoid. So all fine, let's use use_lit as @chrstnbwnkl suggests. Urgh, I should finish for today, sorry for the noise..

nilsnolde commented 1 year ago

Or asked differently: how did you historically deal with deciding on the range of costing penalties or favoring? The way I think about it is (when penalizing): if the penalty factor is 10, it basically means in my head, to avoid unlit roads, as a user I'd be cool with walking a detour of factor 10 (e.g. 10 times the same road under the same conditions otherwise, instead of just once). While that sounds a bit strong, I wonder if you constructed some test maps of reasonable & unreasonable cases or went into real data in an area where you know the tag is pretty prevalent and see what kind of things are happening with different value ranges?

nilsnolde commented 11 months ago

huch, this was done ages ago