bicycle profile does not handle forward/backward/oneway properly

[danpat]

If we have a motorway (implied oneway), and it has both foot:forward=yes and foot:backward=yes, then we end up with only the forward direction routable.

The logic in bicycle.lua is incorrect.

These lines should be independent if statements: https://github.com/Project-OSRM/osrm-backend/blob/master/profiles/bicycle.lua#L307-L314 not elseif, as having both tags on a way is valid. We should not assume that the opposite direction is inaccessible either.

Also, https://github.com/Project-OSRM/osrm-backend/blob/master/profiles/bicycle.lua#L312 should be backward_speed not forward_speed.

ALSO, the logic here is difficult to follow because we initially default the travel mode to bicycle here: https://github.com/Project-OSRM/osrm-backend/blob/master/profiles/bicycle.lua#L189-L190

which means that instead of "deny everything, allow some things", we're defaulting to "allow everything, deny some things", but the forward/backward logic isn't working like that.

[karenzshea]

Failing test in question :

osrm-backend/features/bicycle/pushing.feature

Lines 68 to 74 in 61bdb65

	@todo
	Scenario: Bike - Pushing bikes on ways with foot=yes in one direction
	Then routability should be
	| highway | foot:forward | foot:backward | forw | backw |
	| motorway | | | | |
	| motorway | yes | | foot | |
	| motorway | | yes | | foot |

[emiltin]

in the profile refactor PR #2032 a number of these edge case were fixed, because the logic was cleaned up a lot. the logic the profiles are currently quite hard to follow and reason about.

[daniel-j-h]

Argh we never incorporated that profile refactoring - can you open a new pull request so we can discuss in a ticket. You're right the profiles are getting out of hand.

[emiltin]

You mean an issue or a PR?

[daniel-j-h]

Pull Request, sorry.

The Lua profiles are here to stay for the time being. We need to make them maintainable.

[danpat]

The Lua profiles are here to stay for the time being.

Yeah. @kkaefer did some experiments integrating NodeJS and v8 as alternatives to Lua. We achieved good performance with v8 directly, but it lacks any I/O facilities, so it'd be a big step backwards there. We also looked at using NodeJS, but it wasn't clear how to call out to it in a multi-threaded manner, it's designed to be the parent process (i.e. if we used Node, then we'd call osrm-extract from Node scripts which would have profile-like callbacks), and performance is limited because of the single-threaded nature of Node.

So yes, after all that, Lua is here to stay for a while we think. Improvements in build script Lua detection should alleviate some of the build pains we feel, which are one of the main hangups.

As a side note: @joto has been experimenting with https://github.com/bitfunnel/nativejit/ in https://github.com/osmcode/osmium-filter. If we want really fast profiles, we could explore that as an option.

[emiltin]

There are existing related test marked as @todo here:

https://github.com/Project-OSRM/osrm-backend/blob/master/features/bicycle/access.feature#L38
https://github.com/Project-OSRM/osrm-backend/blob/master/features/bicycle/access.feature#L69

They are not entirely correct though. Also leaving the highway tag empty will cause 'primary' to be used by default by the cucumber support code. To really leave it out '(nil)' must be used.

I'm currently looking at refactoring the bicycle profile, in line with what I did with the car profile.