From efd08d272be719844d0d69d48135a498e0108775 Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Mon, 16 Nov 2015 10:38:50 +0300 Subject: [PATCH 1/4] Routing consistency integration tests. --- omim.pro | 4 + .../routing_consistency_tests.cpp | 191 ++++++++++++++++++ .../routing_consistency_tests.pro | 22 ++ .../routing_test_tools.cpp | 26 --- .../routing_test_tools.hpp | 32 ++- 5 files changed, 248 insertions(+), 27 deletions(-) create mode 100644 routing/routing_consistency_tests/routing_consistency_tests.cpp create mode 100644 routing/routing_consistency_tests/routing_consistency_tests.pro diff --git a/omim.pro b/omim.pro index ff5012df0f..efdee28228 100644 --- a/omim.pro +++ b/omim.pro @@ -138,6 +138,10 @@ SUBDIRS = 3party base coding geometry indexer routing routing_integration_tests.depends = $$MapDepLibs routing SUBDIRS *= routing_integration_tests + routing_consistency_tests.subdir = routing/routing_consistency_tests + routing_consistency_tests.depends = $$SUBDIRS + SUBDIRS *= routing_consistency_tests + # TODO(AlexZ): Move pedestrian tests into routing dir. pedestrian_routing_tests.depends = $$MapDepLibs routing SUBDIRS *= pedestrian_routing_tests diff --git a/routing/routing_consistency_tests/routing_consistency_tests.cpp b/routing/routing_consistency_tests/routing_consistency_tests.cpp new file mode 100644 index 0000000000..a2daf0c80a --- /dev/null +++ b/routing/routing_consistency_tests/routing_consistency_tests.cpp @@ -0,0 +1,191 @@ +#include "testing/testing.hpp" +#include "../routing_integration_tests/routing_test_tools.hpp" + +#include "generator/borders_loader.hpp" + +#include "storage/country_decl.hpp" + +#include "indexer/mercator.hpp" + +#include "geometry/point2d.hpp" + +#include "base/logging.hpp" + +#include "std/string.hpp" +#include "std/fstream.hpp" + +#include "3party/gflags/src/gflags/gflags.h" + + +using namespace routing; +using storage::CountryInfo; + +double constexpr kMinimumRouteDistance = 10000.; +double constexpr kRouteLengthAccuracy = 0.2; + +// Testing stub to make routing test tools linkable. +static CommandLineOptions g_options; +CommandLineOptions const & GetTestingOptions() {return g_options;} + +DEFINE_string(input_file, "", "File with statistics output."); +DEFINE_string(data_path, "", "Working directory, 'path_to_exe/../../data' if empty."); +DEFINE_bool(verbose, false, "Output processed lines to log."); +DEFINE_uint64(confidence, 5, "Maximum test count for each single mwm file."); + +// Information about successful user routing. +struct UserRoutingRecord +{ + m2::PointD start; + m2::PointD stop; + double distance; +}; + +// Parsing value from statistics. +double GetDouble(string const & incomeString, string const & key) +{ + auto it = incomeString.find(key); + if (it == string::npos) + return 0; + auto end = incomeString.find(" ", it); + string number = incomeString.substr(it + key.size() + 1 /* key= */, end); + return stod(number); +} + +// Decoding statistics line. Returns true if the incomeString is the valid record about +// OSRM routing attempt. +bool ParseUserString(string const & incomeString, UserRoutingRecord & result) +{ + // Check if it is a proper routing record. + auto it = incomeString.find("Routing_CalculatingRoute"); + if (it == string::npos) + return false; + it = incomeString.find("result=NoError"); + if (it == string::npos) + return false; + it = incomeString.find("name=vehicle"); + if (it == string::npos) + return false; + if (GetDouble(incomeString, "startDirectionX") != 0 && GetDouble(incomeString, "startDirectionX") != 0) + return false; + + // Extract numbers from a record. + result.distance = GetDouble(incomeString, "distance"); + result.start = MercatorBounds::FromLatLon(GetDouble(incomeString, "startLat"), GetDouble(incomeString, "startLon")); + result.stop = MercatorBounds::FromLatLon(GetDouble(incomeString, "finalLat"), GetDouble(incomeString, "finalLon")); + return true; +} + +class RouteTester +{ +public: + RouteTester(string const & baseDir) : m_components(integration::GetOsrmComponents()) + { + CHECK(borders::LoadCountriesList(baseDir, m_countries), + ("Error loading country polygons files")); + } + + bool BuildRoute(UserRoutingRecord const & record) + { + auto const result = integration::CalculateRoute(m_components, record.start, m2::PointD::Zero(), record.stop); + if (result.second != IRouter::NoError) + { + LOG(LINFO, ("Can't build the route. Code:", result.second)); + return false; + } + auto const delta = record.distance * kRouteLengthAccuracy; + auto const routeLength = result.first->GetFollowedPolyline().GetDistanceToEndM(); + if (std::abs(routeLength - record.distance) < delta) + return true; + + LOG(LINFO, ("Route has invalid length. Expected:", record.distance, "have:", routeLength)); + return false; + } + + bool CheckRecord(UserRoutingRecord const & record) + { + CountryInfo startCountry, finishCountry; + m_components.GetCountryInfoGetter().GetRegionInfo(record.start, startCountry); + m_components.GetCountryInfoGetter().GetRegionInfo(record.stop, finishCountry); + if (startCountry.m_name != finishCountry.m_name || startCountry.m_name.empty()) + return false; + + if (record.distance < kMinimumRouteDistance) + return false; + + if (m_checkedCountries[startCountry.m_name] > FLAGS_confidence) + return false; + + return true; + } + + bool BuildRouteByRecord(UserRoutingRecord const & record) + { + CountryInfo startCountry; + m_components.GetCountryInfoGetter().GetRegionInfo(record.start, startCountry); + m_checkedCountries[startCountry.m_name] += 1; + if (!BuildRoute(record)) + { + m_errors[startCountry.m_name] += 1; + return false; + } + return true; + } + + void PrintStatistics() + { + LOG(LINFO, ("Checked", m_checkedCountries.size(), "countries.")); + LOG(LINFO, ("Found", m_errors.size(), "maps with errors.")); + for (auto const & record : m_errors) + { + if (record.second == m_checkedCountries[record.first]) + LOG(LINFO, ("ERROR!", record.first, " seems to be broken!")); + else + LOG(LINFO, ("Warning! Country:",record.first, "has", record.second,"errors on", m_checkedCountries[record.first], "checks")); + } + } + +private: + borders::CountriesContainerT m_countries; + integration::IRouterComponents & m_components; + + map m_checkedCountries; + map m_errors; +}; + +void ReadInput(istream & stream, RouteTester & tester) +{ + string line; + while (stream.good()) + { + getline(stream, line); + strings::Trim(line); + if (line.empty()) + continue; + UserRoutingRecord record; + if (ParseUserString(line, record)) + if (tester.CheckRecord(record)) + { + if (FLAGS_verbose) + LOG(LINFO, ("Checked", line)); + tester.BuildRoute(record); + } + } + tester.PrintStatistics(); +} + +int main(int argc, char ** argv) +{ + google::SetUsageMessage("Check mwm and routing files consistency. Calculating roads from a user statistics."); + + google::ParseCommandLineFlags(&argc, &argv, true); + + g_options.m_dataPath = FLAGS_data_path.c_str(); + if (FLAGS_input_file.empty()) + return 1; + + RouteTester tester(FLAGS_data_path); + ifstream stream(FLAGS_input_file); + ReadInput(stream, tester); + + return 0; +} diff --git a/routing/routing_consistency_tests/routing_consistency_tests.pro b/routing/routing_consistency_tests/routing_consistency_tests.pro new file mode 100644 index 0000000000..16fa1ecf7f --- /dev/null +++ b/routing/routing_consistency_tests/routing_consistency_tests.pro @@ -0,0 +1,22 @@ +# This subproject implements routing consistency tests. +# This tests are launched on the whole world dataset. + +TARGET = routing_consistency +CONFIG += console warn_on +CONFIG -= app_bundle +TEMPLATE = app + +ROOT_DIR = ../.. +DEPENDENCIES = map routing search storage indexer platform geometry coding base osrm jansson protobuf tomcrypt succinct stats_client generator gflags + +include($$ROOT_DIR/common.pri) + +QT *= core + +INCLUDEPATH *= $$ROOT_DIR/3party/gflags/src + +SOURCES += \ + ../routing_integration_tests/routing_test_tools.cpp \ + routing_consistency_tests.cpp \ + +HEADERS += \ diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index e78a32cafd..17ba6beeef 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -15,8 +15,6 @@ #include "indexer/index.hpp" -#include "storage/country_info_getter.hpp" - #include "platform/local_country_file.hpp" #include "platform/local_country_file_utils.hpp" #include "platform/platform.hpp" @@ -30,7 +28,6 @@ using namespace routing; -using platform::LocalCountryFile; namespace { @@ -94,29 +91,6 @@ namespace integration return shared_ptr(move(router)); } - class IRouterComponents - { - public: - IRouterComponents(vector const & localFiles) - : m_featuresFetcher(CreateFeaturesFetcher(localFiles)) - , m_infoGetter(CreateCountryInfoGetter()) - { - } - - virtual ~IRouterComponents() = default; - - virtual IRouter * GetRouter() const = 0; - - storage::CountryInfoGetter const & GetCountryInfoGetter() const noexcept - { - return *m_infoGetter; - } - - protected: - shared_ptr m_featuresFetcher; - unique_ptr m_infoGetter; - }; - class OsrmRouterComponents : public IRouterComponents { public: diff --git a/routing/routing_integration_tests/routing_test_tools.hpp b/routing/routing_integration_tests/routing_test_tools.hpp index 355b6fc4aa..028d7845c9 100644 --- a/routing/routing_integration_tests/routing_test_tools.hpp +++ b/routing/routing_integration_tests/routing_test_tools.hpp @@ -10,6 +10,10 @@ #include "platform/local_country_file.hpp" +#include "storage/country_info_getter.hpp" + +#include "map/feature_vec_model.hpp" + /* * These tests are developed to simplify routing integration tests writing. * You can use the interface bellow however you want but there are some hints. @@ -36,12 +40,38 @@ */ using namespace routing; using namespace turns; +using platform::LocalCountryFile; typedef pair, IRouter::ResultCode> TRouteResult; namespace integration { - class IRouterComponents; +shared_ptr CreateFeaturesFetcher(vector const & localFiles); + +unique_ptr CreateCountryInfoGetter(); + + class IRouterComponents + { + public: + IRouterComponents(vector const & localFiles) + : m_featuresFetcher(CreateFeaturesFetcher(localFiles)) + , m_infoGetter(CreateCountryInfoGetter()) + { + } + + virtual ~IRouterComponents() = default; + + virtual IRouter * GetRouter() const = 0; + + storage::CountryInfoGetter const & GetCountryInfoGetter() const noexcept + { + return *m_infoGetter; + } + + protected: + shared_ptr m_featuresFetcher; + unique_ptr m_infoGetter; + }; void TestOnlineCrosses(ms::LatLon const & startPoint, ms::LatLon const & finalPoint, vector const & expected, IRouterComponents & routerComponents); From 64f157916e11c863b41b625567aaf4a697c9a65a Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Fri, 20 Nov 2015 15:49:44 +0300 Subject: [PATCH 2/4] Routing single node features order bugfix. --- routing/osrm2feature_map.cpp | 26 ++++--------------- routing/osrm2feature_map.hpp | 6 ++--- routing/osrm_helpers.cpp | 18 +++++-------- routing/osrm_helpers.hpp | 2 +- .../osrm_route_test.cpp | 10 ++++++- .../routing_tests/routing_mapping_test.cpp | 12 +++------ 6 files changed, 28 insertions(+), 46 deletions(-) diff --git a/routing/osrm2feature_map.cpp b/routing/osrm2feature_map.cpp index f42038d8c7..661b64a2de 100644 --- a/routing/osrm2feature_map.cpp +++ b/routing/osrm2feature_map.cpp @@ -122,36 +122,20 @@ bool IsInside(FtSeg const & bigSeg, FtSeg const & smallSeg) (segmentLeft <= smallSeg.m_pointStart && segmentRight >= smallSeg.m_pointEnd); } -FtSeg SplitSegment(FtSeg const & segment, FtSeg const & splitter, bool const resultFromLeft) +FtSeg SplitSegment(FtSeg const & segment, FtSeg const & splitter) { FtSeg resultSeg; resultSeg.m_fid = segment.m_fid; if (segment.m_pointStart < segment.m_pointEnd) { - if (resultFromLeft) - { - resultSeg.m_pointStart = segment.m_pointStart; - resultSeg.m_pointEnd = splitter.m_pointEnd; - } - else - { - resultSeg.m_pointStart = splitter.m_pointStart; - resultSeg.m_pointEnd = segment.m_pointEnd; - } + resultSeg.m_pointStart = segment.m_pointStart; + resultSeg.m_pointEnd = splitter.m_pointEnd; } else { - if (resultFromLeft) - { - resultSeg.m_pointStart = segment.m_pointStart; - resultSeg.m_pointEnd = splitter.m_pointStart; - } - else - { - resultSeg.m_pointStart = splitter.m_pointEnd; - resultSeg.m_pointEnd = segment.m_pointEnd; - } + resultSeg.m_pointStart = segment.m_pointStart; + resultSeg.m_pointEnd = splitter.m_pointStart; } return resultSeg; } diff --git a/routing/osrm2feature_map.hpp b/routing/osrm2feature_map.hpp index 217540d26b..5144ce3dbb 100644 --- a/routing/osrm2feature_map.hpp +++ b/routing/osrm2feature_map.hpp @@ -93,9 +93,9 @@ namespace OsrmMappingTypes bool IsInside(FtSeg const & bigSeg, FtSeg const & smallSeg); /// Splits segment by splitter segment and takes part of it. -/// Warning this function includes a whole splitter segment to a result segment described by the -/// resultFromLeft variable. -FtSeg SplitSegment(FtSeg const & segment, FtSeg const & splitter, bool const resultFromLeft); +/// Warning! This function returns part from the start of the segment to the splitter, including it. +/// Splitter segment points must be ordered. +FtSeg SplitSegment(FtSeg const & segment, FtSeg const & splitter); } // namespace OsrmMappingTypes class OsrmFtSegMapping; diff --git a/routing/osrm_helpers.cpp b/routing/osrm_helpers.cpp index 8a2662e242..9ee1dc7793 100644 --- a/routing/osrm_helpers.cpp +++ b/routing/osrm_helpers.cpp @@ -68,7 +68,7 @@ double Point2PhantomNode::CalculateDistance(FeatureType const & ft, void Point2PhantomNode::CalculateWeight(OsrmMappingTypes::FtSeg const & seg, m2::PointD const & segPt, NodeID const & nodeId, - bool calcFromRight, int & weight, int & offset) const + int & weight, int & offset) const { // nodeId can be INVALID_NODE_ID when reverse node is absent. This node has no weight. if (nodeId == INVALID_NODE_ID) @@ -92,13 +92,8 @@ void Point2PhantomNode::CalculateWeight(OsrmMappingTypes::FtSeg const & seg, auto const range = m_routingMapping.m_segMapping.GetSegmentsRange(nodeId); OsrmMappingTypes::FtSeg segment; - size_t const startIndex = calcFromRight ? range.second - 1 : range.first; - size_t const endIndex = calcFromRight ? range.first - 1 : range.second; - int const indexIncrement = calcFromRight ? -1 : 1; - bool foundSeg = false; - m2::PointD lastPoint; - for (size_t segmentIndex = startIndex; segmentIndex != endIndex; segmentIndex += indexIncrement) + for (size_t segmentIndex = range.first; segmentIndex != range.second; ++segmentIndex) { m_routingMapping.m_segMapping.GetSegmentByIndex(segmentIndex, segment); if (!segment.IsValid()) @@ -120,11 +115,10 @@ void Point2PhantomNode::CalculateWeight(OsrmMappingTypes::FtSeg const & seg, if (segment.m_fid == seg.m_fid && OsrmMappingTypes::IsInside(segment, seg)) { - auto const splittedSegment = OsrmMappingTypes::SplitSegment(segment, seg, !calcFromRight); + auto const splittedSegment = OsrmMappingTypes::SplitSegment(segment, seg); distanceM += CalculateDistance(ft, splittedSegment.m_pointStart, splittedSegment.m_pointEnd); // node.m_seg always forward ordered (m_pointStart < m_pointEnd) - distanceM -= MercatorBounds::DistanceOnEarth( - ft.GetPoint(calcFromRight ? seg.m_pointStart : seg.m_pointEnd), segPt); + distanceM -= MercatorBounds::DistanceOnEarth(ft.GetPoint(seg.m_pointEnd), segPt); foundSeg = true; } @@ -274,9 +268,9 @@ void Point2PhantomNode::CalculateWeights(FeatureGraphNode & node) const // Need to initialize weights for correct work of PhantomNode::GetForwardWeightPlusOffset // and PhantomNode::GetReverseWeightPlusOffset. CalculateWeight(node.segment, node.segmentPoint, node.node.forward_node_id, - false /* calcFromRight */, node.node.forward_weight, node.node.forward_offset); + node.node.forward_weight, node.node.forward_offset); CalculateWeight(node.segment, node.segmentPoint, node.node.reverse_node_id, - true /* calcFromRight */, node.node.reverse_weight, node.node.reverse_offset); + node.node.reverse_weight, node.node.reverse_offset); } void Point2Node::operator()(FeatureType const & ft) diff --git a/routing/osrm_helpers.hpp b/routing/osrm_helpers.hpp index 97168e0c3e..61de9e54a2 100644 --- a/routing/osrm_helpers.hpp +++ b/routing/osrm_helpers.hpp @@ -59,7 +59,7 @@ private: /// Calculates part of a node weight in the OSRM format. Projection point @segPt divides node on /// two parts. So we find weight of a part, set by the @calcFromRight parameter. void CalculateWeight(OsrmMappingTypes::FtSeg const & seg, m2::PointD const & segPt, - NodeID const & nodeId, bool calcFromRight, int & weight, int & offset) const; + NodeID const & nodeId, int & weight, int & offset) const; /// Returns minimal weight of the node. EdgeWeight GetMinNodeWeight(NodeID node, m2::PointD const & point) const; diff --git a/routing/routing_integration_tests/osrm_route_test.cpp b/routing/routing_integration_tests/osrm_route_test.cpp index 45534062c8..55f96eb39c 100644 --- a/routing/routing_integration_tests/osrm_route_test.cpp +++ b/routing/routing_integration_tests/osrm_route_test.cpp @@ -120,6 +120,14 @@ namespace {11.327927635052676, 48.166256203616726}, 2870710.); } + UNIT_TEST(PeruSingleRoadTest) + { + integration::CalculateRouteAndTestRouteLength( + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(-14.22061, -73.35969), {0., 0.}, + MercatorBounds::FromLatLon(-14.22389, -73.44281), 15900.); + } + UNIT_TEST(RussiaMoscowFranceParisCenterRouteTest) { integration::CalculateRouteAndTestRouteLength( @@ -245,6 +253,6 @@ namespace IRouter::ResultCode const result = routeResult.second; TEST_EQUAL(result, IRouter::NoError, ()); - integration::TestRouteTime(route, 909.); + integration::TestRouteTime(route, 900.); } } // namespace diff --git a/routing/routing_tests/routing_mapping_test.cpp b/routing/routing_tests/routing_mapping_test.cpp index 619af953c7..313abf6250 100644 --- a/routing/routing_tests/routing_mapping_test.cpp +++ b/routing/routing_tests/routing_mapping_test.cpp @@ -118,14 +118,10 @@ UNIT_TEST(FtSegSplitSegmentiTest) OsrmMappingTypes::FtSeg bseg(123, 5, 1); OsrmMappingTypes::FtSeg splitter(123, 2, 3); - OsrmMappingTypes::FtSeg res1(123, 2, 5); - TEST_EQUAL(res1, OsrmMappingTypes::SplitSegment(seg, splitter, false), ()); - OsrmMappingTypes::FtSeg res2(123, 1, 3); - TEST_EQUAL(res2, OsrmMappingTypes::SplitSegment(seg, splitter, true), ()); + OsrmMappingTypes::FtSeg res1(123, 1, 3); + TEST_EQUAL(res1, OsrmMappingTypes::SplitSegment(seg, splitter), ()); - OsrmMappingTypes::FtSeg res3(123, 3, 1); - TEST_EQUAL(res3, OsrmMappingTypes::SplitSegment(bseg, splitter, false), ()); - OsrmMappingTypes::FtSeg res4(123, 5, 2); - TEST_EQUAL(res4, OsrmMappingTypes::SplitSegment(bseg, splitter, true), ()); + OsrmMappingTypes::FtSeg res2(123, 5, 2); + TEST_EQUAL(res2, OsrmMappingTypes::SplitSegment(bseg, splitter), ()); } } // namespace From 57d4b734aa4956bc783e1df27690f6575e5a29ef Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Fri, 20 Nov 2015 16:15:51 +0300 Subject: [PATCH 3/4] PR fixes. --- omim.pro | 2 +- .../routing_consistency_tests.cpp | 16 ++++++---------- .../osrm_route_test.cpp | 7 +++++++ .../routing_test_tools.hpp | 16 ++++++++-------- routing/turns_generator.cpp | 1 + std/cmath.hpp | 2 ++ 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/omim.pro b/omim.pro index efdee28228..58ecaa9533 100644 --- a/omim.pro +++ b/omim.pro @@ -139,7 +139,7 @@ SUBDIRS = 3party base coding geometry indexer routing SUBDIRS *= routing_integration_tests routing_consistency_tests.subdir = routing/routing_consistency_tests - routing_consistency_tests.depends = $$SUBDIRS + routing_consistency_tests.depends = $$MapDepLibs routing SUBDIRS *= routing_consistency_tests # TODO(AlexZ): Move pedestrian tests into routing dir. diff --git a/routing/routing_consistency_tests/routing_consistency_tests.cpp b/routing/routing_consistency_tests/routing_consistency_tests.cpp index a2daf0c80a..eb80cd6f23 100644 --- a/routing/routing_consistency_tests/routing_consistency_tests.cpp +++ b/routing/routing_consistency_tests/routing_consistency_tests.cpp @@ -16,7 +16,6 @@ #include "3party/gflags/src/gflags/gflags.h" - using namespace routing; using storage::CountryInfo; @@ -65,7 +64,7 @@ bool ParseUserString(string const & incomeString, UserRoutingRecord & result) it = incomeString.find("name=vehicle"); if (it == string::npos) return false; - if (GetDouble(incomeString, "startDirectionX") != 0 && GetDouble(incomeString, "startDirectionX") != 0) + if (GetDouble(incomeString, "startDirectionX") != 0 && GetDouble(incomeString, "startDirectionY") != 0) return false; // Extract numbers from a record. @@ -78,10 +77,8 @@ bool ParseUserString(string const & incomeString, UserRoutingRecord & result) class RouteTester { public: - RouteTester(string const & baseDir) : m_components(integration::GetOsrmComponents()) + RouteTester() : m_components(integration::GetOsrmComponents()) { - CHECK(borders::LoadCountriesList(baseDir, m_countries), - ("Error loading country polygons files")); } bool BuildRoute(UserRoutingRecord const & record) @@ -93,8 +90,8 @@ public: return false; } auto const delta = record.distance * kRouteLengthAccuracy; - auto const routeLength = result.first->GetFollowedPolyline().GetDistanceToEndM(); - if (std::abs(routeLength - record.distance) < delta) + auto const routeLength = result.first->GetTotalDistanceMeters(); + if (abs(routeLength - record.distance) < delta) return true; LOG(LINFO, ("Route has invalid length. Expected:", record.distance, "have:", routeLength)); @@ -145,7 +142,6 @@ public: } private: - borders::CountriesContainerT m_countries; integration::IRouterComponents & m_components; map m_checkedCountries; @@ -167,7 +163,7 @@ void ReadInput(istream & stream, RouteTester & tester) { if (FLAGS_verbose) LOG(LINFO, ("Checked", line)); - tester.BuildRoute(record); + tester.BuildRouteByRecord(record); } } tester.PrintStatistics(); @@ -183,7 +179,7 @@ int main(int argc, char ** argv) if (FLAGS_input_file.empty()) return 1; - RouteTester tester(FLAGS_data_path); + RouteTester tester; ifstream stream(FLAGS_input_file); ReadInput(stream, tester); diff --git a/routing/routing_integration_tests/osrm_route_test.cpp b/routing/routing_integration_tests/osrm_route_test.cpp index 55f96eb39c..e17e11a3e2 100644 --- a/routing/routing_integration_tests/osrm_route_test.cpp +++ b/routing/routing_integration_tests/osrm_route_test.cpp @@ -8,6 +8,13 @@ using namespace routing; namespace { + UNIT_TEST(StrangeCaseInAfrica) + { + integration::CalculateRouteAndTestRouteLength( + integration::GetOsrmComponents(), MercatorBounds::FromLatLon(19.207890000000002573, 30.506630000000001246), {0., 0.}, + MercatorBounds::FromLatLon(19.172889999999998878, 30.473150000000000404), 7250.); + } + // Node filtering test. SVO has many restricted service roads that absent in a OSRM index. UNIT_TEST(MoscowToSVOAirport) { diff --git a/routing/routing_integration_tests/routing_test_tools.hpp b/routing/routing_integration_tests/routing_test_tools.hpp index 028d7845c9..93e505e022 100644 --- a/routing/routing_integration_tests/routing_test_tools.hpp +++ b/routing/routing_integration_tests/routing_test_tools.hpp @@ -1,19 +1,19 @@ #pragma once +#include "routing/osrm_router.hpp" + +#include "storage/country_info_getter.hpp" + +#include "map/feature_vec_model.hpp" + +#include "platform/local_country_file.hpp" + #include "std/set.hpp" #include "std/shared_ptr.hpp" #include "std/string.hpp" #include "std/utility.hpp" #include "std/vector.hpp" -#include "routing/osrm_router.hpp" - -#include "platform/local_country_file.hpp" - -#include "storage/country_info_getter.hpp" - -#include "map/feature_vec_model.hpp" - /* * These tests are developed to simplify routing integration tests writing. * You can use the interface bellow however you want but there are some hints. diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 8bd59eb4ec..07febdd437 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -13,6 +13,7 @@ #include "3party/osrm/osrm-backend/data_structures/internal_route_result.hpp" +#include "std/cmath.hpp" #include "std/numeric.hpp" #include "std/string.hpp" diff --git a/std/cmath.hpp b/std/cmath.hpp index dbd007bf76..0601e73eaa 100644 --- a/std/cmath.hpp +++ b/std/cmath.hpp @@ -11,6 +11,8 @@ #define new DEBUG_NEW #endif +using std::abs; + namespace math { double constexpr pi = 3.14159265358979323846; From aed82c800c44e12eaeb8829aa779d2801b5781a0 Mon Sep 17 00:00:00 2001 From: Lev Dragunov Date: Tue, 24 Nov 2015 16:39:11 +0300 Subject: [PATCH 4/4] Review fixes. --- .../routing_consistency_tests.cpp | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/routing/routing_consistency_tests/routing_consistency_tests.cpp b/routing/routing_consistency_tests/routing_consistency_tests.cpp index eb80cd6f23..7247119bb8 100644 --- a/routing/routing_consistency_tests/routing_consistency_tests.cpp +++ b/routing/routing_consistency_tests/routing_consistency_tests.cpp @@ -1,5 +1,5 @@ #include "testing/testing.hpp" -#include "../routing_integration_tests/routing_test_tools.hpp" +#include "routing/routing_integration_tests/routing_test_tools.hpp" #include "generator/borders_loader.hpp" @@ -19,8 +19,8 @@ using namespace routing; using storage::CountryInfo; -double constexpr kMinimumRouteDistance = 10000.; -double constexpr kRouteLengthAccuracy = 0.2; +double constexpr kMinimumRouteDistanceM = 10000.; +double constexpr kRouteLengthAccuracy = 0.15; // Testing stub to make routing test tools linkable. static CommandLineOptions g_options; @@ -40,13 +40,15 @@ struct UserRoutingRecord }; // Parsing value from statistics. -double GetDouble(string const & incomeString, string const & key) +double GetDouble(string const & incomingString, string const & key) { - auto it = incomeString.find(key); + auto it = incomingString.find(key); if (it == string::npos) return 0; - auto end = incomeString.find(" ", it); - string number = incomeString.substr(it + key.size() + 1 /* key= */, end); + // Skip "key=" + it += key.size() + 1; + auto end = incomingString.find(" ", it); + string number = incomingString.substr(it, end - it); return stod(number); } @@ -55,14 +57,11 @@ double GetDouble(string const & incomeString, string const & key) bool ParseUserString(string const & incomeString, UserRoutingRecord & result) { // Check if it is a proper routing record. - auto it = incomeString.find("Routing_CalculatingRoute"); - if (it == string::npos) + if (incomeString.find("Routing_CalculatingRoute") == string::npos) return false; - it = incomeString.find("result=NoError"); - if (it == string::npos) + if (incomeString.find("result=NoError") == string::npos) return false; - it = incomeString.find("name=vehicle"); - if (it == string::npos) + if (incomeString.find("name=vehicle") == string::npos) return false; if (GetDouble(incomeString, "startDirectionX") != 0 && GetDouble(incomeString, "startDirectionY") != 0) return false; @@ -106,7 +105,7 @@ public: if (startCountry.m_name != finishCountry.m_name || startCountry.m_name.empty()) return false; - if (record.distance < kMinimumRouteDistance) + if (record.distance < kMinimumRouteDistanceM) return false; if (m_checkedCountries[startCountry.m_name] > FLAGS_confidence) @@ -137,7 +136,8 @@ public: if (record.second == m_checkedCountries[record.first]) LOG(LINFO, ("ERROR!", record.first, " seems to be broken!")); else - LOG(LINFO, ("Warning! Country:",record.first, "has", record.second,"errors on", m_checkedCountries[record.first], "checks")); + LOG(LINFO, ("Warning! Country:", record.first, "has", record.second, "errors on", + m_checkedCountries[record.first], "checks")); } } @@ -158,13 +158,14 @@ void ReadInput(istream & stream, RouteTester & tester) if (line.empty()) continue; UserRoutingRecord record; - if (ParseUserString(line, record)) - if (tester.CheckRecord(record)) - { - if (FLAGS_verbose) - LOG(LINFO, ("Checked", line)); - tester.BuildRouteByRecord(record); - } + if (!ParseUserString(line, record)) + continue; + if (tester.CheckRecord(record)) + { + if (FLAGS_verbose) + LOG(LINFO, ("Checked", line)); + tester.BuildRouteByRecord(record); + } } tester.PrintStatistics(); }