From 1f3a8d4538c6ac89318477d37f311b565963dc60 Mon Sep 17 00:00:00 2001 From: karenzshea Date: Thu, 9 Feb 2017 10:54:39 +0100 Subject: [PATCH] remove units from rate columns in routability testing --- features/car/access.feature | 8 +-- features/car/maxspeed.feature | 76 ++++++++++----------- features/car/service.feature | 8 +-- features/car/traffic_turn_penalties.feature | 72 +++++++++---------- features/car/weight.feature | 8 +-- features/step_definitions/routability.js | 27 ++++++-- features/support/shared_steps.js | 2 +- profiles/lib/handlers.lua | 10 --- unit_tests/library/route.cpp | 2 +- unit_tests/library/tile.cpp | 2 +- 10 files changed, 110 insertions(+), 105 deletions(-) diff --git a/features/car/access.feature b/features/car/access.feature index 1d94b54d0..bcc33c96a 100644 --- a/features/car/access.feature +++ b/features/car/access.feature @@ -179,12 +179,12 @@ Feature: Car - Restricted access | primary | | | | no | x | @hov - Scenario: Car - only designated HOV ways are ignored by default + Scenario: Car - designated HOV ways are rated low Then routability should be | highway | hov | bothw | forw_rate | - | primary | designated | x | 52.63 km/h | - | primary | yes | x | | - | primary | no | x | | + | primary | designated | x | 2 | + | primary | yes | x | 18 | + | primary | no | x | 18 | @hov Scenario: Car - a way with all lanes HOV-designated is inaccessible by default (similar to hov=designated) diff --git a/features/car/maxspeed.feature b/features/car/maxspeed.feature index db270a44e..1ae1a666a 100644 --- a/features/car/maxspeed.feature +++ b/features/car/maxspeed.feature @@ -86,54 +86,54 @@ OSRM will use 4/5 of the projected free-flow speed. Then routability should be | highway | maxspeed | width | maxspeed:forward | maxspeed:backward | forw | backw | forw_rate | backw_rate | - | primary | | | | | 64 km/h | 64 km/h | 65 km/h | 65 km/h | - | primary | | 3 | | | 64 km/h | 64 km/h | 32 km/h | 32 km/h | - | primary | 60 | | | | 47 km/h | 47 km/h | 48 km/h | 48 km/h | - | primary | 60 | 3 | | | 47 km/h | 47 km/h | 24 km/h | 24 km/h | - | primary | | | 60 | | 47 km/h | 64 km/h | 48 km/h | 65 km/h | - | primary | | 3 | 60 | | 47 km/h | 64 km/h | 24 km/h | 32 km/h | - | primary | | | | 60 | 64 km/h | 47 km/h | 65 km/h | 48 km/h | - | primary | | 3 | | 60 | 64 km/h | 47 km/h | 32 km/h | 24 km/h | - | primary | 15 | | 60 | | 47 km/h | 11 km/h | 48 km/h | 12 km/h | - | primary | 15 | 3 | 60 | | 48 km/h | 12 km/h | 24 km/h | 6 km/h | - | primary | 15 | | | 60 | 12 km/h | 47 km/h | 12 km/h | 48 km/h | - | primary | 15 | 3 | | 60 | 12 km/h | 47 km/h | 6 km/h | 24 km/h | - | primary | 15 | | 30 | 60 | 23 km/h | 47 km/h | 24 km/h | 48 km/h | - | primary | 15 | 3 | 30 | 60 | 23 km/h | 47 km/h | 12 km/h | 24 km/h | + | primary | | | | | 64 km/h | 64 km/h | 18 | 18 | + | primary | | 3 | | | 64 km/h | 64 km/h | 9 | 9 | + | primary | 60 | | | | 47 km/h | 47 km/h | 13 | 13 | + | primary | 60 | 3 | | | 47 km/h | 47 km/h | 7 | 7 | + | primary | | | 60 | | 47 km/h | 64 km/h | 13 | 18 | + | primary | | 3 | 60 | | 47 km/h | 64 km/h | 7 | 9 | + | primary | | | | 60 | 64 km/h | 47 km/h | 18 | 13 | + | primary | | 3 | | 60 | 64 km/h | 47 km/h | 9 | 7 | + | primary | 15 | | 60 | | 47 km/h | 11 km/h | 13 | 3 | + | primary | 15 | 3 | 60 | | 48 km/h | 12 km/h | 7 | 2 | + | primary | 15 | | | 60 | 12 km/h | 47 km/h | 3 | 13 | + | primary | 15 | 3 | | 60 | 12 km/h | 47 km/h | 2 | 7 | + | primary | 15 | | 30 | 60 | 23 km/h | 47 km/h | 7 | 13 | + | primary | 15 | 3 | 30 | 60 | 23 km/h | 47 km/h | 3 | 7 | Scenario: Car - Single lane streets be ignored or incur a penalty Then routability should be | highway | maxspeed | lanes | maxspeed:forward | maxspeed:backward | forw | backw | forw_rate | backw_rate | - | primary | | | | | 64 km/h | 64 km/h | 65 km/h | 65 km/h | - | primary | | 1 | | | 64 km/h | 64 km/h | 32 km/h | 32 km/h | - | primary | 60 | | | | 47 km/h | 47 km/h | 48 km/h | 48 km/h | - | primary | 60 | 1 | | | 47 km/h | 47 km/h | 24 km/h | 24 km/h | - | primary | | | 60 | | 47 km/h | 64 km/h | 48 km/h | 65 km/h | - | primary | | 1 | 60 | | 47 km/h | 64 km/h | 24 km/h | 32 km/h | - | primary | | | | 60 | 64 km/h | 47 km/h | 65 km/h | 48 km/h | - | primary | | 1 | | 60 | 64 km/h | 47 km/h | 32 km/h | 24 km/h | - | primary | 15 | | 60 | | 47 km/h | 11 km/h | 48 km/h | 12 km/h | - | primary | 15 | 1 | 60 | | 48 km/h | 12 km/h | 24 km/h | 6 km/h | - | primary | 15 | | | 60 | 12 km/h | 47 km/h | 12 km/h | 48 km/h | - | primary | 15 | 1 | | 60 | 12 km/h | 47 km/h | 6 km/h | 24 km/h | - | primary | 15 | | 30 | 60 | 23 km/h | 47 km/h | 24 km/h | 48 km/h | - | primary | 15 | 1 | 30 | 60 | 23 km/h | 47 km/h | 12 km/h | 24 km/h | + | primary | | | | | 64 km/h | 64 km/h | 18 | 18 | + | primary | | 1 | | | 64 km/h | 64 km/h | 9 | 9 | + | primary | 60 | | | | 47 km/h | 47 km/h | 13 | 13 | + | primary | 60 | 1 | | | 47 km/h | 47 km/h | 7 | 7 | + | primary | | | 60 | | 47 km/h | 64 km/h | 13 | 18 | + | primary | | 1 | 60 | | 47 km/h | 64 km/h | 7 | 9 | + | primary | | | | 60 | 64 km/h | 47 km/h | 18 | 13 | + | primary | | 1 | | 60 | 64 km/h | 47 km/h | 9 | 7 | + | primary | 15 | | 60 | | 47 km/h | 11 km/h | 13 | 3 | + | primary | 15 | 1 | 60 | | 48 km/h | 12 km/h | 7 | 2 | + | primary | 15 | | | 60 | 12 km/h | 47 km/h | 3 | 13 | + | primary | 15 | 1 | | 60 | 12 km/h | 47 km/h | 2 | 7 | + | primary | 15 | | 30 | 60 | 23 km/h | 47 km/h | 7 | 13 | + | primary | 15 | 1 | 30 | 60 | 23 km/h | 47 km/h | 3 | 7 | Scenario: Car - Single lane streets only incur a penalty for two-way streets Then routability should be | highway | maxspeed | lanes | oneway | forw | backw | forw_rate | backw_rate | - | primary | 30 | 1 | yes | 23 km/h | | 24 km/h | | - | primary | 30 | 1 | -1 | | 23 km/h | | 24 km/h | - | primary | 30 | 1 | | 23 km/h | 23 km/h | 12 km/h | 12 km/h | - | primary | 30 | 2 | | 23 km/h | 23 km/h | 24 km/h | 24 km/h | + | primary | 30 | 1 | yes | 23 km/h | | 7 | | + | primary | 30 | 1 | -1 | | 23 km/h | | 7 | + | primary | 30 | 1 | | 23 km/h | 23 km/h | 3 | 3 | + | primary | 30 | 2 | | 23 km/h | 23 km/h | 7 | 7 | Scenario: Car - Forward/backward maxspeed on reverse oneways Then routability should be | highway | maxspeed | maxspeed:forward | maxspeed:backward | oneway | forw | backw | forw_rate | backw_rate | - | primary | | | | -1 | | 64 km/h | | 65 km/h | - | primary | 30 | | | -1 | | 23 km/h | | 24 km/h | - | primary | | 30 | | -1 | | 64 km/h | | 65 km/h | - | primary | | | 30 | -1 | | 23 km/h | | 24 km/h | - | primary | 20 | 30 | | -1 | | 15 km/h | | 16 km/h | - | primary | 20 | | 30 | -1 | | 23 km/h | | 24 km/h | + | primary | | | | -1 | | 64 km/h | | 18 | + | primary | 30 | | | -1 | | 23 km/h | | 7 | + | primary | | 30 | | -1 | | 64 km/h | | 18 | + | primary | | | 30 | -1 | | 23 km/h | | 7 | + | primary | 20 | 30 | | -1 | | 15 km/h | | 4 | + | primary | 20 | | 30 | -1 | | 23 km/h | | 7 | diff --git a/features/car/service.feature b/features/car/service.feature index 453eedde0..f294652be 100644 --- a/features/car/service.feature +++ b/features/car/service.feature @@ -7,8 +7,8 @@ Feature: Car - Surfaces Scenario: Car - Ways tagged service should reduce speed Then routability should be | highway | service | forw | backw | forw_rate | - | service | alley | 15 km/h +-1 | 15 km/h +-1 | 12.05 km/h | + | service | alley | 15 km/h +-1 | 15 km/h +-1 | 2 | | service | emergency_access | | | | - | service | driveway | 15 km/h +-1 | 15 km/h +-1 | 12.05 km/h | - | service | drive-through | 15 km/h +-1 | 15 km/h +-1 | 12.05 km/h | - | service | parking | 15 km/h +-1 | 15 km/h +-1 | 12.05 km/h | + | service | driveway | 15 km/h +-1 | 15 km/h +-1 | 2 | + | service | drive-through | 15 km/h +-1 | 15 km/h +-1 | 2 | + | service | parking | 15 km/h +-1 | 15 km/h +-1 | 2 | diff --git a/features/car/traffic_turn_penalties.feature b/features/car/traffic_turn_penalties.feature index d2a996367..7769b1eca 100644 --- a/features/car/traffic_turn_penalties.feature +++ b/features/car/traffic_turn_penalties.feature @@ -52,23 +52,23 @@ Feature: Traffic - turn penalties Scenario: Weighting not based on turn penalty file When I route I should get - | from | to | route | speed | time | - | a | h | ad,dhk,dhk | 65 km/h | 11s +-1 | - # straight - | i | g | fim,fg,fg | 55 km/h | 13s +-1 | - # right - | a | e | ad,def,def | 44 km/h | 16.3s +-1 | - # left - | c | g | cd,def,fg,fg | 65 km/h | 22s +-1 | - # double straight - | p | g | mp,fim,fg,fg | 60 km/h | 24s +-1 | - # straight-right - | a | l | ad,dhk,klm,klm | 53 km/h | 27s +-1 | - # straight-left - | l | e | klm,dhk,def,def | 55 km/h | 26s +-1 | - # double right - | g | n | fg,fim,mn,mn | 44 km/h | 32s +-1 | - # double left + | from | to | route | speed | weight | time | + | a | h | ad,dhk,dhk | 65 km/h | 11s +-1 | 11s +-1 | + # straight + | i | g | fim,fg,fg | 55 km/h | 13s +-1 | 13s +-1 | + # right + | a | e | ad,def,def | 44 km/h | 16.3s +-1 | 16.3s +-1 | + # left + | c | g | cd,def,fg,fg | 65 km/h | 22s +-1 | 22s +-1 | + # double straight + | p | g | mp,fim,fg,fg | 60 km/h | 24s +-1 | 24s +-1 | + # straight-right + | a | l | ad,dhk,klm,klm | 53 km/h | 27s +-1 | 27s +-1 | + # straight-left + | l | e | klm,dhk,def,def | 55 km/h | 26s +-1 | 26s +-1 | + # double right + | g | n | fg,fim,mn,mn | 44 km/h | 32s +-1 | 32s +-1 | + # double left Scenario: Weighting based on turn penalty file Given the turn penalty file @@ -88,25 +88,25 @@ Feature: Traffic - turn penalties # ade left turn And the contract extra arguments "--turn-penalty-file {penalties_file}" When I route I should get - | from | to | route | speed | time | - | a | h | ad,dhk,dhk | 65 km/h | 11s +-1 | - # straight - | i | g | fim,fg,fg | 56 km/h | 15s +-1 | - # right - ifg penalty - | a | e | ad,def,def | 53 km/h | 14s +-1 | - # left - faster because of negative ade penalty - | c | g | cd,def,fg,fg | 52 km/h | 27s +-1 | - # double straight - | p | g | mp,fim,fg,fg | 49 km/h | 29s +-1 | - # straight-right - ifg penalty - | a | l | ad,def,fim,klm,klm | 48 km/h | 45s +-1 | - # was straight-left - forced around by hkl penalty - | l | e | klm,fim,def,def | 38 km/h | 38s +-1 | - # double right - forced left by lkh penalty - | g | n | fg,fim,mn,mn | 25 km/h | 57s +-1 | - # double left - imn penalty - | j | c | jk,klm,fim,def,cd,cd | 44 km/h | 65.8s +-1 | - # double left - hdc penalty ever so slightly higher than imn; forces all the way around + | from | to | route | speed | weight | time | + | a | h | ad,dhk,dhk | 65 km/h | 11 | 11s +-1 | + # straight + | i | g | fim,fg,fg | 56 km/h | 12.8 | 12s +-1 | + # right - ifg penalty + | a | e | ad,def,def | 67 km/h | 10.8 | 10s +-1 | + # left - faster because of negative ade penalty + | c | g | cd,def,fg,fg | 65 km/h | 22 | 22s +-1 | + # double straight + | p | g | mp,fim,fg,fg | 61 km/h | 23.8 | 23s +-1 | + # straight-right - ifg penalty + | a | l | ad,def,fim,klm,klm | 58 km/h | 37 | 37s +-1 | + # was straight-left - forced around by hkl penalty + | l | e | klm,fim,def,def | 44 km/h | 32.6 | 32s +-1 | + # double right - forced left by lkh penalty + | g | n | fg,fim,mn,mn | 28 km/h | 51.8 | 51s +-1 | + # double left - imn penalty + | j | c | jk,klm,fim,def,cd,cd | 53 km/h | 54.6 | 54s +-1 | + # double left - hdc penalty ever so slightly higher than imn; forces all the way around Scenario: Too-negative penalty clamps, but does not fail Given the contract extra arguments "--turn-penalty-file {penalties_file}" diff --git a/features/car/weight.feature b/features/car/weight.feature index 5d7f4b201..5ca4e205e 100644 --- a/features/car/weight.feature +++ b/features/car/weight.feature @@ -21,8 +21,8 @@ Feature: Car - weights | bdf | service | When I route I should get | from | to | route | speed | weight | - | a | e | abc,cg,efg,efg | 28 km/h | 38 +-1 | - | a | d | abc,bdf,bdf | 18 km/h | 21 +-1 | + | a | e | abc,cg,efg,efg | 28 km/h | 126.6 | + | a | d | abc,bdf,bdf | 18 km/h | 71.7 | Scenario: Does not jump off the highway to go down service road Given the node map @@ -59,5 +59,5 @@ Feature: Car - weights """ When I route I should get | from | to | route | speed | weight | - | a | d | ab,bc,cd,cd | 65 km/h | 12 +-1 | - | a | e | ab,be,be | 14 km/h | 104 | + | a | d | ab,bc,cd,cd | 65 km/h | 44.4 | + | a | e | ab,be,be | 14 km/h | 112 | diff --git a/features/step_definitions/routability.js b/features/step_definitions/routability.js index e3ecd3185..4fdc53b11 100644 --- a/features/step_definitions/routability.js +++ b/features/step_definitions/routability.js @@ -6,9 +6,10 @@ module.exports = function () { this.Then(/^routability should be$/, (table, callback) => { this.buildWaysFromTable(table, () => { var directions = ['forw','backw','bothw'], + testedHeaders = ['forw','backw','bothw','forw_rate','backw_rate','bothw_rate'], headers = new Set(Object.keys(table.hashes()[0])); - if (!['forw','backw','bothw','forw_rate','backw_rate','bothw_rate'].some(k => !!headers.has(k))) { + if (!testedHeaders.some(k => !!headers.has(k))) { throw new Error('*** routability table must contain either "forw", "backw", "bothw", "forw_rate" or "backw_rate" column'); } @@ -16,19 +17,33 @@ module.exports = function () { if (e) return callback(e); var testRow = (row, i, cb) => { var outputRow = Object.assign({}, row); + // clear the fields that are tested for in the copied response object + for (var field in outputRow) { + if (testedHeaders.indexOf(field) != -1) + outputRow[field] = ''; + } testRoutabilityRow(i, (err, result) => { if (err) return cb(err); directions.filter(d => headers.has(d + '_rate')).forEach((direction) => { - var want = row[direction + '_rate']; - if (/^\d+ km\/h/.test(want)) { + var rate = direction + '_rate'; + var want = row[rate]; + switch (true) { + case '' === want: + outputRow[rate] = result[direction].status ? + result[direction].status.toString() : ''; + break; + case /^\d+$/.test(want): if (result[direction].rate) { - outputRow[direction + '_rate'] = !isNaN(result[direction].rate) ? - result[direction].rate.toString()+' km/h' : + outputRow[rate] = !isNaN(result[direction].rate) ? + result[direction].rate.toString() : result[direction].rate.toString() || ''; } else { - outputRow[direction + '_rate'] = ''; + outputRow[rate] = ''; } + break; + default: + throw new Error(util.format('*** Unknown expectation format: %s for header %s', want, rate)); } }); diff --git a/features/support/shared_steps.js b/features/support/shared_steps.js index 2ed49d87d..27044375b 100644 --- a/features/support/shared_steps.js +++ b/features/support/shared_steps.js @@ -109,7 +109,7 @@ module.exports = function () { if (row.weight.length) { if (!row.weight.match(/[\d\.]+/)) return cb(new Error('*** Weight must be specified as a numeric value. (ex: 8)')); - got.weight = instructions ? util.format('%dm', weight) : ''; + got.weight = instructions ? util.format('%d', weight) : ''; } else { got.weight = ''; } diff --git a/profiles/lib/handlers.lua b/profiles/lib/handlers.lua index c108463bb..7af7e1685 100644 --- a/profiles/lib/handlers.lua +++ b/profiles/lib/handlers.lua @@ -486,16 +486,6 @@ function Handlers.run(handlers,way,result,data,profile) for i,handler in ipairs(handlers) do if Handlers[handler](way,result,data,profile) == false then return false - else - if handler == 'handle_penalties' then --- io.write('handler: ', handler, '\n') --- io.write('weight ', result.weight, '\n') --- io.write('rate ', result.forward_rate, '\n') --- io.write('duration ', result.duration, '\n') --- io.write('speed ', result.forward_speed, '\n') - io.write('forward speed ', result.forward_speed) - io.write('forward rate ', result.forward_rate) - end end end end diff --git a/unit_tests/library/route.cpp b/unit_tests/library/route.cpp index bcbfb55c8..b6cc49adf 100644 --- a/unit_tests/library/route.cpp +++ b/unit_tests/library/route.cpp @@ -51,7 +51,7 @@ BOOST_AUTO_TEST_CASE(test_route_same_coordinates_fixture) {{"distance", 0.}, {"duration", 0.}, {"weight", 0.}, - {"weight_name", "duration"}, + {"weight_name", "routability"}, {"geometry", "yw_jGupkl@??"}, {"legs", json::Array{{json::Object{ diff --git a/unit_tests/library/tile.cpp b/unit_tests/library/tile.cpp index 2e0f14ee9..af5109e87 100644 --- a/unit_tests/library/tile.cpp +++ b/unit_tests/library/tile.cpp @@ -32,7 +32,7 @@ BOOST_AUTO_TEST_CASE(test_tile) const auto rc = osrm.Tile(params, result); BOOST_CHECK(rc == Status::Ok); - BOOST_CHECK_EQUAL(result.size(), 114033); + BOOST_CHECK_EQUAL(result.size(), 113824); protozero::pbf_reader tile_message(result); tile_message.next();