Refactoring and bugfixing of profiles

[mackshot]

• all profiles share a common speed profile, profile_defaults.lua
• the profile logic is encapsulated in file logic.lua
• all parsing logic is encapsulated in file lib/parser.lua

New profiles can easily created by copying profile_car.lua and override the profile_defaults.lua values.

We are working on several truck profiles. If you are interested, just let me know.

The foot and bicycle profile stills needs to be adopted (later) to the logic which we suggest here with this pull request.

[DennisOSRM]

thanks for your patch. Usually, we discuss larger patches like this before in an issue or the mailing list.

Please note that a large'ish number of tests fail.Also, a while back, we chose not to use includes in the default car profile. This lead to a myriad of issues from users missing these includes.

[emiltin]

it would be useful if osrm would handle lua includes better. specifically, it should handle symlinks.

[DennisOSRM]

it would be useful if osrm would handle lua includes better. specifically, it should handle symlinks.

Thought it does follow symlinks as these happen on the filesystem level. Whats the issue?

[emiltin]

hmm it was something with the path parsing that's done in the c++ lua code

[mackshot]

Thanks for your feedback to this patch.

Please let me first explain why we have implemented that changes:
We have run osrm and evaluated the "shipped" car profile. During that evaluation phase, we come (by comparing the driving times with real driving times and those of alternate routing softwares) to the conclusion that the profile isn't working very good (at least in Germany). We decided to take a closer look and found several smaller bugs, which partly explained why the driving times were unrealistic. We started to fix them. During bugfixing we realized that for our need of having different speed profiles for different vehicle types, it would be better to have one central logic file, which can be used for every kind of vehicle (motorized vehicle at least). Thus we split the profile into different parts. The reason to go for a default speed profile is, that many settings are valid for most (motorized) vehicle types. We wanted to easen the process of creating new profiles. And thus we were able to create pretty fast (next to this car profile) four truck profiles - one of them is a profile for bdouble / eurocombi trucks, which is something pretty new for routers.

It was not our aim to undermine any regular process here. We just did, what we thought is useful to get well working profiles easily - which we needed for an application with a strong deadline. Afterwards we thought it might be a good idea to say "thank you" for this project by letting the developers and users benefit from our work.

About the compilation error: I assume the reason for that is, that we fixed a typo in the find_access_tag(source,access_tags_hierarchy) method or that we have removed the namespace of this file. However, if this compilation error is the only problem which lasts we are willing to send in a new pull request without that bug to let the community benefit from our work. As mentioned in the pull request, we have not worked on the foot and bicycle profile, since our focus is on motorized vehicles. Thus our default speed-profile is more a default-motorized-vehicle profile.

I hope I could explain a bit why we had done it that way and that we did not want annoy somebody.

We are no lua experts thus we are not familiar with the mentioned import path problems before. We could not observe any of them.

[emiltin]

it seems there is really two issues here:

1. make sure lua includes work, so people can use them without problems.
   to fix this, we might identify what problems there might be using lua includes, so we can then try to solve them. dennis can you provide more details on why you remove the lua includes in the car profile? one difference between car/bicycle is that there is a symlink in the osmr root folder pointing to the car profile. maybe includes doesn't work if you provide a symlink to a profile to osrm?
2. refactor profiles to make them easier to maintain/modify/extend
   my opinion is that splitting the lua profiles into smaller more managable modules is generelly very useful and will improve profiles and make it easier to build new ones.
   in fact is started this approach already by adding the profiles/lib/ folder: https://github.com/Project-OSRM/osrm-backend/tree/master/profiles/lib. the bicycle profile include the two files in lib/ and this seems to works fine.
   that said i'm not sure i think that the way the current PR reorganizes code is appropriate. for one thing, if we want to refactor, then i think all profiles should be considred, also walk, bike, etc. it should be more like a class hierachy. i also miss a more clear core that structures the handling of access, oneways, speed, etc in a general way.

[DennisOSRM]

We decided to take a closer look and found several smaller bugs, which partly explained why the driving times were unrealistic. We started to fix them

Can you elaborate on what these bugs are?

Afterwards we thought it might be a good idea to say "thank you" for this project by letting the developers and users benefit from our work.

This is very much appreciated. Nevertheless, discussing issues beforehand has the advantage of actually knowing details on what has changed and why.

[emiltin]

note also that the PR breaks the bicycle profile, because profiles/lib/access.lua was modified

[mackshot]

about the bugs:

node_function:

if barrier and "" ~= barrier then
if barrier_whitelist[barrier] then
return!!
else
result.barrier = true
end
end
Due to this return you cannot reach the check for the traffic signal penalty, looks wrong to me.

way_function:

especially block
if result.forward_speed == -1 then
local highway_speed = speed_profile[highway]
...
end
doesn't make sense, sorry. For example, if parse_maxspeed return 0 (for example for german Autobahn), because there is no speed limit, the profile defaults back to the general ("global") motorway speed limit (80 or so) instead of recommended speed of 130 in Germany. Please do not get me wrong, but have you compared the driving speeds of your web application with other routing softwares? We do see a significant difference.
Additionally, general vehicle speed limits are not used correctly: A truck can never drive faster as 80 km/h in Germany and no car can drive with unlimited speed on german Autobahn.
We have redesigned the speed parsing part, which should be align with osm wiki explanations, now!

If you like I can provide a demo access to our application (via email), where you can test our speed profiles.

[DennisOSRM]

Agree that the penalty function should be reachable within node_function. The same holds for any expected travel speeds that are unreasonable.

How about addressing each of the issues you have identified by an issue on its own? To move the changes upstream, it's important to unpack the patch bomb into consumable parts.

One important point is that the splitting of the profile code into separate files will have repercussions. Judging from previous experiences, there will be a fair amount of tickets opened for missing dependencies. Ideally, we'd like to avoid that dependency chain for the default profile.

If you like I can provide a demo access to our application (via email), where you can test our speed profiles.

sure.

[mackshot]

Regarding the demo: Just register here for free: http://business.inTanken.de

Regarding addressing each issue by its own: (As you can see by the time it took until I answered) I do not think that I will find the time to do that, sorry. But of course you can feel free to extract everything useful from our work.

What I really like to understand is, what the problem with including other files is: Is there a bug in lua or are the users not able to put all necessary files in the right spot? The files are already in the right spot, only the default profile in your main directory (which I missed to adapt before creating the pull request) might be a problem, but I am not sure, if this profile must be located there anyway.

[frankvandenbergh]

Hi Markus,

I downloaded your patches for the (base for truck) profiles. Unfortunately I couldn't get it running (it extracts, but no routes whatsoever). In file access.lua I replaced "access_tags_hierarchy" (2x) by "access_tags_hierachy".

Would you be so kind to publish your lua files?

BTW have you considered ADR, special truck prohibits (road closed for trucks) and maut?

Thanks,
Frank

[mackshot]

Hey Frank,

can you please write an email to my business account mp@incs.org? Then we can have a chat about your request and about adding aspects like ADR (we haven't yet).

Thanks,
Markus

[willylambert]

Hello,

I'm also working on OSRM truck profile. In addition to suitable speed profile for truck, we have implemented the following rules based on osm tags:

• Forbid ways with weight < 3.5 T and height < 4m
• Forbid ways with good=no tag
• Penalize roads through residential area (within 100m)
• Penalize ways with good=delivery tag
• Penalize ways next to school amenities (within 50 m)
• Penalize left turn

For some rules we link lua profile to a postgis database loaded with osm data.

I'll be glad to share and discuss this.

[frankvandenbergh]

Hi Markus, Willy,

Thanks for wanting to share your results. I also created a truck profile (as the maxspeed approach in the car.lua sucks for trucks) but not on a detailed level you guys did.

If would be nice to follow Markus approach and try to make the functions shareable/ object oriented. Although his logic.lua is more a motor-vehicle base than a general one.

We better can ask @DennisOSRM what is the best way forward.

BTW Willy did you also test for hgv=no, hazmat:adr_tunnel_cat? Also very nice, the third item.

[mackshot]

Hello Frank (@frankvandenbergh),
Hello Willy (@willylambert),
Hello Dennis (@DennisOSRM),

sorry for the delayed answer but we - as a company - had to discuss in what manner and in which time effort we may want to participate in an open (source) distribution of routing profiles.

Finally we decided that we are willing to share our profiles as long as others share them too and thus all sides benefit from this process. We also discussed where and how to share these profiles. Mainly two ideas come to our mind and we now like to discuss them with you before starting the sharing process:

1. (official) OSRM-RoutingProfile-Repository: We can imagine to have a dedicated OSRM routing profile repository, which is referenced here in OSRM and which can be forked by any git hub user. Of course the OSRM-Team must be happy with that resolution and the question how a standard-profile is distributed to the users who check out the OSRM Repository has to be answered.
2. Independent RoutingProfile-Repository: A shared repository, where we can invite different people/parties to participate and share their profiles.

Please let us know what you think about these options and/or what should be changed to improve these proposals. Also other ideas are pretty welcome.

Andreas (@exciler), a colleague of mine, may step into this discussion as well.

/Markus

[TheMarex]

@mackshot since Dennis is taking a leave from OSRM development, I'm going to jump on this. Why not include them in this repository? How many profiles are are talking about here?
We have a good test suite in form of cucumber tests that we use for the car and bike profile to ensure we don't break any intended behavior. If you want to profit from that, consider adding them here directly.

• We aim to keep profiles somewhat atomic, so I would not split the defaults form the actual profile.
  Also the node_function and way_function should always be part of the main profile file.
• I like the split of the parser logic. We usually try to move more complex utility functions over to C++, since parsing is a lot faster there. But for the moment having that as lua is fine.
• The lua module system is a mess. Luckily @lbud found a way to get it working with all lua versions. Check out her pull request https://github.com/Project-OSRM/osrm-backend/pull/1457/files to see how this works.
• The shared logic should be moved to the lib/ folder

Next steps:

• Fix remaining issues above
• Rebase on develop
• Fix tests
• Split up in smaller commits (I can handle that if necessary)

[TheMarex]

Closing here because there was no response.