A set of cucumber tests would help clarify what consequences this change would have, and make sure it does not cause regressions.

I'm not sure it's a good idea to discard a way just because it has an unknown tracktype or smoothness.

How was the actual speeds choosen? (60, 45, 30, 15, etc.)

@emiltin is right. We need a couple of things to proceed with this PR:

• justification of chosen speeds
• tests that check correct behavior of the newly added features
• all previous tests must pass

Right, I'll write the cucumber tests as soon as I can. Is there any specific guideline? I've never really written one, so I'm using as an example this one: https://github.com/DennisOSRM/Project-OSRM/blob/master/features/car/maxspeed.feature

I wonder how these tests are verified, do you run some automated system?

Tracktype and smoothness both have a small, closed and stable set of values. These values are closely followed in practice:
http://taginfo.openstreetmap.org/keys/tracktype#values
http://taginfo.openstreetmap.org/keys/smoothness#values

Major reasons not to follow practice include 2 types of edit accident (both edit mistakes): typos, and concatenation when joining ways. I think the decision here is one of project goal:

• be optimist and hope the way is good to travel through (if not, this may lead the driver to risky situations); or
• be pessimist and confidently avoid situations in which it would not be good to travel through

These situations are very rare (see taginfo). By being pessimist, I believe OSRM can aid in finding such mapping mistakes.

Of course, I'm open to any of those approaches, whichever follows more closely OSRM philosophy.

The "surface tag" does not have a closed and stable set of values. Surface is also less reliable for guessing speeds (particularly values such as surface=unpaved, which I've equated with "dirt" based on discussions on the tagging list such as this one: https://lists.openstreetmap.org/pipermail/tagging/2014-March/016811.html). However, surface can still be a good predictor in most cases (notable exceptions are paved, unpaved, ground and dirt; for these values I have assumed an "average" scenario, also a guess based on the description and related values).

The suggested speeds (which are in kmph) are subjective, based on the descriptions and pictures in the wiki and on personal experience (assuming a low clearance, standard passenger car), and so they would definitely benefit from more opinions and scrutiny. One possible improvement (specially for developing countries) is to lower some of these speeds to just below the speed of residential ways. (30 would become 20, 15 would become 10, then 45 would make a little more sense if it became 40.) Another debatable point is whether we want to route through ways tagged with smoothness=horrible (described in the wiki as equivalent to smoothness=off_road_vehicles). I thought maybe the Humanitarian team may need this in some situations.

To avoid gross guessing errors, I've formulated the following sanity questions to myself and then answered them:

1. Assuming I'm on a low clearance, passenger car, if I had one shortcut rated tracktype=grade5 and an alternative, longer way rated tracktype=grade2, from the descriptions in the wiki and the pictures and my experience, how long could the shortcut be to be worth the trouble? (Speeds are proportional to proportions between such lengths.)
2. I also asked the inverse question: if tracktype=grade5 had some fixed length, how much longer could the tracktype=grade2 way be to still look like an acceptable alternative? This "should" be the inverse value of the previous measure, if not then my subjective perception would be innacurate and I'd need to rethink the situation (improve one or the other answer).
3. The speed for grade1 is considered unbounded by the type of surface (so it gets bounded by other factors). The speed for grade2 is a subjective guess, and for the other values the speed is a comparative guess following the logic above.
4. I also imagined a uniform urban grid where each block is a perfect square (1x1 proportion). Then, for each of the above situations, I imagined a block connecting points A, B, C and D and then imagined the shortcut was on one side (say, A to B) and wondered if I would prefer to take the shortcut or go around the block to get from A to B. This helped refine (pessimistically) the speeds for urban grids. (This is somewhat debatable as different city grids have blocks of different proportions, but it seemed an interesting starting point.)
5. I then did the same self-sanity check for smoothness values.

See https://github.com/DennisOSRM/Project-OSRM/wiki/Cucumber-Test-Suite for info on cuke tests.

I've written an extensive test suite and made some changes to the code. Is there a way to use GitHub to run the tests directly? (I may need to repeat the test.) Or do I have to install the entire software stack on my local machine?

The 47 test scenarios fall in 4 groups:

1. Pairs of values considered "much worse": grade5/grade2, horrible/bad, bad/good. On these combinations, on a square block, the user would be routed along 3 edges instead of directly over a single edge (supposing this is the shortest way through).
2. Broad ranges for expected maximum safe speed for tracktype and smoothness.
3. Sanity checks: grade2 is never a better option than grade1, excellent smoothness is always better than good. And so on.
4. Equivalence of smoothness values: excellent=thin_rollers, good=thin_wheels, etc.

There's also a routability scenario. Is there anything else you think should be tested?

There was one change from the original profile. I was using an absolute speed limit, but while writing the tests I felt the need for preference values to get OSRM to find better routes when the alternative routes are very similar (say, a nearby way with "excellent" smoothness instead of a slighly shorter way with "good" smoothness, both of the same highway type). Now I'm considering both aspects: a preference value (that multiplies way speed calculated by the existing profile) and a maximum absolute speed.

Please run test locally first to make sure they all pass (including all existing tests).
After that, once you push commits to this PR, github will run the test automatically as well.

I have not written any tests for the "surface" tag, as the assignments I made are at best a "guess".

Even though surface speeds are simply assigned, they should probably be tested as well. The lua script is complex and many things potentially influence the final speed.

Trying to build OSRM, I'm having a little problem with cmake:

(...)
-- thread
-- Looking for LuaJIT 5.2
-- Could NOT find LUAJIT (missing: LUAJIT_LIBRARIES)
-- Looking for Luabind...
(...)

Everything else is fine. Since LuaJIT does not come with Debian Wheezy, I've installed it following the instructions in the project site, but CMake seems unable to find it. This is my first time using CMake, any suggestions on how to get it to find the library on /usr/local/bin/?

Some justification (opinions from other people) for lowering speed values at the "bad" end of the scale: https://lists.openstreetmap.org/pipermail/tagging/2014-March/017010.html

On my side I think the job is done. Let me know if anything else is necessary.

Thanks. A bunch of tests are still failing for me:

cucumber features/car/surface.feature:145 # Scenario: Car - Grade2 should be worse than grade1 regardless of highway type
cucumber features/car/surface.feature:165 # Scenario: Car - Grade3 should be worse than grade2 regardless of highway type
cucumber features/car/surface.feature:185 # Scenario: Car - Grade4 should be worse than grade3 regardless of highway type
cucumber features/car/surface.feature:205 # Scenario: Car - Grade5 should be worse than grade4 regardless of highway type
cucumber features/car/surface.feature:285 # Scenario: Car - Smooth for thin wheels should be worse than smooth for thin rollers regardless of highway type
cucumber features/car/surface.feature:305 # Scenario: Car - Smooth for wheels should be worse than smoothness for thin wheels regardless of highway type
cucumber features/car/surface.feature:325 # Scenario: Car - Smooth for robust wheels should be worse than smooth for wheels regardless of highway type
cucumber features/car/surface.feature:345 # Scenario: Car - Smooth for high clearance should be worse than smooth for robust wheels regardless of highway type
cucumber features/car/surface.feature:455 # Scenario: Car - Smooth for off road wheels shouldn't be worse than horrible smoothness
cucumber features/car/surface.feature:465 # Scenario: Car - Horrible smoothness shouldn't be worse than smooth for off road wheels

I suggest reorganizing tests a bit. Instead of testing two surfaces against in each in each scenario, maybe use tables to test the resulting speed of different combinations of highway, surface, smoothness, tracktype, maxspeed etc. We probably also need to check that surface tags don't affect things like oneway and access. Ex make sure smoothness=excellent doesn't make a way with access=no routable.

Great points @emiltin

While rewriting the tests, I had to ask myself whether I would prefer having maxspeed override the safe speed derived from surface quality (surface/tracktype/smoothness). One could expect maxspeed to be a good predictor of a safe speed of travel, particularly on good quality surfaces. Considering that legal maximum speed is determined by the local traffic administration and surface quality is assessed by the local community:

• When the administration is able to do its job well, the maximum speed should not differ significantly from the suggested surface safe speed (we can expect that either maxspeed would responsibly set lower on poorer surfaces, or the surface would be better constructed). In fact, the safe speed should often stay very close to or above maxspeed for the better surfaces in most normal situations (see speeds for grade2 and smoothness=intermediate). This of course might require some community feedback later on.
• When this is not true (as is often the case in developing countries and poorer ones that often need humanitarian aid, and sometimes in developed countries away from major urban centres), the community's assessment of the surface should lead to a more reliable lower safe speed. In fact, most mappers don't even care to apply these surface tags when the surface seems ok.

If people most often tag stuff that lowers speed, doesn't it then make sense to use the lowest speed implied by maxspeed, surface, smoothness, etc?

A way to specify the resulting speed in these kinds of tests would probably be useful and make tests easier to write/read.

"If people most often tag stuff that lowers speed, doesn't it then make sense to use the lowest speed implied by maxspeed, surface, smoothness, etc?" Yes, it does, this is the logic that I followed. I was just explaining that we don't take the minimum of {highway speed, maxspeed, surface speed, smoothness speed, etc.}; instead, we take maxspeed if present, or highway speed if not, and then let this speed value be lowered by surface, smoothness, etc. This is a little different because highway speed is discarded from the minimum calculation when maxspeed is present. My main reason to do this is to keep as much of the current logic untouched as possible. I believe it works this way already because maxspeed is seen as some sort of "official speed", where the speed assigned to each highway is more like a speed guess.

"A way to specify the resulting speed in these kinds of tests would probably be useful and make tests easier to write/read." Absolutely. The Cucumber tests could support an additional "average speed" column which would test against distance divided by time. Some conversion would be needed if speed included "kmph" or "mph". On routability tables, the tests could support average speed syntax too.

Let me know if the tests are passing now or if there's anything else that needs to be fixed or improved.

Did you run the tests yourself?

No. I spent 2 whole days trying to set up the entire infrastructure (reading from OSRM's guides, and from OSRM's dependencies' guides), but I was unable to get OSRM to compile on Debian Wheezy. I wouldn't be able to spend any more days doing that, but I didn't want to give up the knowledge I've gathered after so many discussions in forums and mailing lists.

I understand it can be frustating to get things to compile. But trying to change code and submitting pull requests without being able to run the result yourself is almost impossible.

Well, Emil, if it takes you perhaps one or two minutes to run it, but it takes me days or even over a week to do the same, and all I want is to contribute a small change that pretty much consolidates the knowledge that I've gathered, isn't it worth a try? I really can't spend more time on this, so, if I get not help at all, maybe it's best to just forget about it entirely. It was well researched in my biased opinion. And even though I couldn't run the tests, I did run the Lua code on an interpreter to figure out the times to be tested.

First, we appreciate your work and the pull request. But let me give some additional thoughts on this:

• We understand that bad surface means slower travel. What we need in terms of justification is an explanation of how the parameters you chose were derived and why these make sense. Unfortunately, the link above does not provide this.
• Also, we try to merge only code into the development branch that passes tests. Otherwise tests would be pointless, wouldn't they? Additionally, a brief explanation of how it does what it does is a plus.
• Given that 40+ tests are failing, there is obviously some work left to do before we can actually do the merge.
• Setting up the test environment on Debian wheezy isn't too hard. Our very own Jenkins CI server is running wheezy. Usually, it is just minor things to fix given an actual error message. Debian provides essentially all packages to get going. We are happy to help.

-- Could NOT find LUAJIT (missing: LUAJIT_LIBRARIES)

This is not a critical error. Instead of the JIT compiler the (somewhat) slower interpreter is used.

• Generally, we value cooperation and collaboration.

Let me comment more precisely on what you posted above:

Tracktype and smoothness both have a small, closed and stable set of values. These values are closely followed in practice:
http://taginfo.openstreetmap.org/keys/tracktype#values
http://taginfo.openstreetmap.org/keys/smoothness#values

A small and stable set of values is a great thing.

Major reasons not to follow practice include 2 types of edit accident (both edit mistakes): typos, and concatenation when joining ways. I think the decision here is one of project goal:

be optimist and hope the way is good to travel through (if not, this may lead the driver to risky situations); or
be pessimist and confidently avoid situations in which it would not be good to travel through
These situations are very rare (see taginfo). By being pessimist, I believe OSRM can aid in finding > such mapping mistakes.

Of course, I'm open to any of those approaches, whichever follows more closely OSRM philosophy.

Our philosophy can be summed up with the following: Don't fix the data and don't start splitting hairs. If data is broken by edit mistakes, we expose these problems instead of glossing over. Even if this means that results obviously wrong. Showing issues means fixing issues. The we stay away from quibbling that many discussions on tagging turn into.

The "surface tag" does not have a closed and stable set of values. Surface is also less reliable for guessing speeds (particularly values such as surface=unpaved, which I've equated with "dirt" based on discussions on the tagging list such as this one: https://lists.openstreetmap.org/pipermail/tagging/2014-March/016811.html). However, surface can still be a good predictor in most cases (notable exceptions are paved, unpaved, ground and dirt; for these values I have assumed an "average" scenario, also a guess based on the description and related values).

unstable value sets are usually a thing not to support. It as also an indicator for ongoing hair picking. What's the reasonable subset of smoothness values to support?

The suggested speeds (which are in kmph) are subjective, based on the descriptions and pictures in the wiki and on personal experience (assuming a low clearance, standard passenger car), and so they would definitely benefit from more opinions and scrutiny. One possible improvement (specially for developing countries) is to lower some of these speeds to just below the speed of residential ways. (30 would become 20, 15 would become 10, then 45 would make a little more sense if it became 40.)

This is a fair point. Referencing pages in the wiki is a good building block to make a case for a proposed change set.

Another debatable point is whether we want to route through ways tagged with smoothness=horrible (described in the wiki as equivalent to smoothness=off_road_vehicles). I thought maybe the Humanitarian team may need this in some situations.

That's also a good point.

To avoid gross guessing errors, I've formulated the following sanity questions to myself and then answered them:

1. Assuming I'm on a low clearance, passenger car, if I had one shortcut rated tracktype=grade5 and an alternative, longer way rated tracktype=grade2, from the descriptions in the wiki and the pictures and my experience, how long could the shortcut be to be worth the trouble? (Speeds are proportional to proportions between such lengths.)

What's your answer to this question?

1. I also asked the inverse question: if tracktype=grade5 had some fixed length, how much longer could the tracktype=grade2 way be to still look like an acceptable alternative? This "should" be the inverse value of the previous measure, if not then my subjective perception would be innacurate and I'd need to rethink the situation (improve one or the other answer).

What's your answer to this question?

1. The speed for grade1 is considered unbounded by the type of surface (so it gets bounded by other factors). The speed for grade2 is a subjective guess, and for the other values the speed is a comparative guess following the logic above.

Ok, sounds reasonable.

1. I also imagined a uniform urban grid where each block is a perfect square (1x1 proportion). Then, for each of the above situations, I imagined a block connecting points A, B, C and D and then imagined the shortcut was on one side (say, A to B) and wondered if I would prefer to take the shortcut or go around the block to get from A to B. This helped refine (pessimistically) the speeds for urban grids. (This is somewhat debatable as different city grids have blocks of different proportions, but it seemed an interesting starting point.)

Makes, sense too. Can you elaborate a bit more on the process?

1. I then did the same self-sanity check for smoothness values.

In the hope of supporting the discussion, I've tried to create a simplified verison at: 634ba93.

It's still based on ftrebien's PR and input.

Surface, tracktype and smoothness are considered. Only common tag values are handled. Each value is assigned a max speed, for example surface=pebblestone => max 40 km/h, and smoothness=horrible => 20 km/h. These values are simply assigned by looking at the photos at the wiki, and can of course be adjusted.

The default speed for the way type is then lowered if mandated by any of the surface tags. For example, a way with highway=primary would usually use 65 km/h. But if it has surface=pebblestone, this will be lowered to 40 km/h. If it also has smoothness=horrible, it would instead use 20 km/h, since this is lower that 40.

I've also tried to modify the test to make them a bit clearer. I'm in the process of added the ability to test for average speed in cucumber, but it's not complete. So some tests are marked as @todo, but the idea should be clear.

This approach doesn't multiply anything, or do other advanced combination of the surface tags. As such it wouldn't handle all edge cases, but at least it's pretty easy to follow what's going on.

my simplified version mentioned above can be found in the branch feature/surfaces: https://github.com/DennisOSRM/Project-OSRM/tree/feature/surfaces

Thanks @emiltin. Will check it out.