[routing] Fixed surface factors to fit constraint tertiary+gravel is better than track.

Signed-off-by: Viktor Govako <viktor.govako@gmail.com>

generator/generator_tests/osm_type_test.cpp

@@ -555,8 +555,16 @@ UNIT_CLASS_TEST(TestWithClassificator, OsmType_Surface)
   TestSurfaceTypes("asphalt", "bad", "", "paved_bad");
   TestSurfaceTypes("asphalt", "", "0", "paved_bad");
 
+  TestSurfaceTypes("fine_gravel", "", "", "paved_bad");
   TestSurfaceTypes("fine_gravel", "intermediate", "", "paved_bad");
+
+  // We definitely should store more than 4 surface options.
+  // Gravel (widely used tag) always goes to unpaved_bad which is strange sometimes.
+  // At the same time, we can't definitely say that it's unpaved_good :)
+  TestSurfaceTypes("gravel", "excellent", "", "unpaved_good");
+  TestSurfaceTypes("gravel", "", "", "unpaved_bad");
   TestSurfaceTypes("gravel", "intermediate", "", "unpaved_bad");
+
   TestSurfaceTypes("paved", "intermediate", "", "paved_good");
   TestSurfaceTypes("", "intermediate", "", "unpaved_good");
 

routing/routing_integration_tests/route_test.cpp

@@ -662,4 +662,17 @@ using namespace std;
         mercator::FromLatLon(34.7068976, 33.1199084), {0., 0.},
         mercator::FromLatLon(34.7070505, 33.1198391), 670.077);
   }
+
+  UNIT_TEST(Crimea_UseGravelTertiary)
+  {
+    integration::CalculateRouteAndTestRouteLength(
+        integration::GetVehicleComponents(VehicleType::Car),
+        mercator::FromLatLon(45.362591, 36.471533), {0., 0.},
+        mercator::FromLatLon(45.475055, 36.341766), 18815.6);
+
+    integration::CalculateRouteAndTestRouteLength(
+        integration::GetVehicleComponents(VehicleType::Car),
+        mercator::FromLatLon(45.470764, 36.331289), {0., 0.},
+        mercator::FromLatLon(45.424964, 36.080336), 55220.2);
+  }
 } // namespace route_test

routing_common/car_model.cpp

@@ -46,12 +46,6 @@ VehicleModel::LimitsInitList const kCarOptionsDefault = {
     {{"highway", "living_street"}, true},
     {{"highway", "road"}, true},
     {{"highway", "track"}, true}
-    /// @todo: Add to classificator
-    //{ {"highway", "shuttle_train"},  10 },
-    //{ {"highway", "ferry"},          5  },
-    //{ {"highway", "default"},        10 },
-    /// @todo: Check type
-    //{ {"highway", "construction"},   40 },
 };
 
 VehicleModel::LimitsInitList const kCarOptionsNoPassThroughLivingStreet = {
@@ -134,12 +128,14 @@ VehicleModel::AdditionalRoadsList const kAdditionalRoads = {
     {{"route", "ferry"}, kHighwayBasedSpeeds.Get(HighwayType::RouteFerry)},
     {{"man_made", "pier"}, kHighwayBasedSpeeds.Get(HighwayType::ManMadePier)}};
 
+/// @todo Should make some compare constrains (like in CarModel_TrackVsGravelTertiary test)
+/// to better fit these factors with reality. I have no idea, how they were set.
 VehicleModel::SurfaceInitList const kCarSurface = {
   // {{surfaceType, surfaceType}, {weightFactor, etaFactor}}
   {{"psurface", "paved_good"}, {1.0, 1.0}},
   {{"psurface", "paved_bad"}, {0.5, 0.5}},
   {{"psurface", "unpaved_good"}, {0.4, 0.8}},
-  {{"psurface", "unpaved_bad"}, {0.1, 0.3}}
+  {{"psurface", "unpaved_bad"}, {0.2, 0.3}}
 };
 
 // Names must be the same with country names from countries.txt

routing_common/car_model_coefs.hpp

@@ -48,6 +48,7 @@ HighwayBasedSpeeds const kHighwayBasedSpeeds = {
     {HighwayType::HighwaySecondary, InOutCitySpeedKMpH(60.00 /* in city */, 70.00 /* out city */)},
     {HighwayType::HighwaySecondaryLink, InOutCitySpeedKMpH(48.00 /* in city */, 56.00 /* out city */)},
     {HighwayType::HighwayService, InOutCitySpeedKMpH({15.00, 15.00} /* in city */, {15.00, 15.00} /* out city */)},
+    /// @todo Why tertiary is the only road with inCity speed _bigger_ than outCity?
     {HighwayType::HighwayTertiary, InOutCitySpeedKMpH(60.00 /* in city */, 50.00 /* out city */)},
     {HighwayType::HighwayTertiaryLink, InOutCitySpeedKMpH({40.95, 34.97} /* in city */, {45.45, 39.73} /* out city */)},
     {HighwayType::HighwayTrack, InOutCitySpeedKMpH({5.00, 5.00} /* in city */, {5.00, 5.00} /* out city */)},

routing_common/routing_common_tests/vehicle_model_test.cpp

@@ -3,19 +3,16 @@
 #include "routing_common/car_model_coefs.hpp"
 #include "routing_common/maxspeed_conversion.hpp"
 #include "routing_common/vehicle_model.hpp"
+#include "routing_common/car_model.hpp"
 
 #include "indexer/classificator.hpp"
 #include "indexer/classificator_loader.hpp"
-#include "indexer/feature.hpp"
+#include "indexer/feature_data.hpp"
 
 #include "platform/measurement_utils.hpp"
 
-#include "base/macros.hpp"
 #include "base/math.hpp"
 
-#include <cstdint>
-#include <vector>
-
 namespace vehicle_model_test
 {
 using namespace routing;
@@ -54,52 +51,53 @@ VehicleModel::SurfaceInitList const kCarSurface = {
 class VehicleModelTest
 {
 public:
-  VehicleModelTest() { classificator::Load(); }
+  VehicleModelTest()
+  {
+    classificator::Load();
+    auto const & c = classif();
+
+    primary = c.GetTypeByPath({"highway", "primary"});
+    secondary = c.GetTypeByPath({"highway", "secondary"});
+    secondaryBridge = c.GetTypeByPath({"highway", "secondary", "bridge"});
+    secondaryTunnel = c.GetTypeByPath({"highway", "secondary", "tunnel"});
+    residential = c.GetTypeByPath({"highway", "residential"});
+
+    oneway = c.GetTypeByPath({"hwtag", "oneway"});
+    pavedGood = c.GetTypeByPath({"psurface", "paved_good"});
+    pavedBad = c.GetTypeByPath({"psurface", "paved_bad"});
+    unpavedGood = c.GetTypeByPath({"psurface", "unpaved_good"});
+    unpavedBad = c.GetTypeByPath({"psurface", "unpaved_bad"});
+  }
+
+  uint32_t primary, secondary, secondaryTunnel, secondaryBridge, residential;
+  uint32_t oneway, pavedGood, pavedBad, unpavedGood, unpavedBad;
 };
 
-class TestVehicleModel : public VehicleModel
+class VehicleModelStub : public VehicleModel
 {
-  friend void CheckOneWay(initializer_list<uint32_t> const & types, bool expectedValue);
-  friend void CheckPassThroughAllowed(initializer_list<uint32_t> const & types, bool expectedValue);
-  friend void CheckSpeedWithParams(initializer_list<uint32_t> const & types,
-                                   SpeedParams const & params, SpeedKMpH const & expectedSpeed);
-
 public:
-  TestVehicleModel()
+  VehicleModelStub()
     : VehicleModel(classif(), kTestLimits, kCarSurface, {kDefaultSpeeds, kDefaultFactors})
   {
   }
 
   // We are not going to use offroad routing in these tests.
-  SpeedKMpH const & GetOffroadSpeed() const override { return kDummy; }
-
-private:
-  static SpeedKMpH const kDummy;
+  SpeedKMpH const & GetOffroadSpeed() const override
+  {
+    static SpeedKMpH offroad{0.0 /* weight */, 0.0 /* eta */};
+    return offroad;
+  }
 };
 
-SpeedKMpH const TestVehicleModel::kDummy = {0.0 /* weight */, 0.0 /* eta */};
-
-uint32_t GetType(char const * s0, char const * s1 = 0, char const * s2 = 0)
-{
-  char const * const t[] = {s0, s1, s2};
-  size_t const size = (s0 != 0) + size_t(s1 != 0) + size_t(s2 != 0);
-  return classif().GetTypeByPath(vector<string>(t, t + size));
-}
-
-uint32_t GetOnewayType()
-{
-  return GetType("hwtag", "oneway");
-}
-
 void CheckSpeedWithParams(initializer_list<uint32_t> const & types, SpeedParams const & params,
                           SpeedKMpH const & expectedSpeed)
 {
-  TestVehicleModel vehicleModel;
+  VehicleModelStub model;
   feature::TypesHolder h;
   for (uint32_t t : types)
     h.Add(t);
 
-  TEST_EQUAL(vehicleModel.GetTypeSpeed(h, params), expectedSpeed, ());
+  TEST_EQUAL(model.GetTypeSpeed(h, params), expectedSpeed, ());
 }
 
 void CheckSpeed(initializer_list<uint32_t> const & types, InOutCitySpeedKMpH const & expectedSpeed)
@@ -112,98 +110,81 @@ void CheckSpeed(initializer_list<uint32_t> const & types, InOutCitySpeedKMpH con
 
 void CheckOneWay(initializer_list<uint32_t> const & types, bool expectedValue)
 {
-  TestVehicleModel vehicleModel;
+  VehicleModelStub model;
   feature::TypesHolder h;
   for (uint32_t t : types)
     h.Add(t);
 
-  TEST_EQUAL(vehicleModel.HasOneWayType(h), expectedValue, ());
+  TEST_EQUAL(model.HasOneWayType(h), expectedValue, ());
 }
 
 void CheckPassThroughAllowed(initializer_list<uint32_t> const & types, bool expectedValue)
 {
-  TestVehicleModel vehicleModel;
+  VehicleModelStub model;
   feature::TypesHolder h;
   for (uint32_t t : types)
     h.Add(t);
 
-  TEST_EQUAL(vehicleModel.HasPassThroughType(h), expectedValue, ());
+  TEST_EQUAL(model.HasPassThroughType(h), expectedValue, ());
 }
 
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_MaxSpeed)
+UNIT_CLASS_TEST(VehicleModelStub, MaxSpeed)
 {
-  TestVehicleModel vehicleModel;
-  TEST_EQUAL(vehicleModel.GetMaxWeightSpeed(), 150.0, ());
+  TEST_EQUAL(GetMaxWeightSpeed(), 150.0, ());
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_Speed)
+UNIT_CLASS_TEST(VehicleModelTest, Speed)
 {
   {
-    CheckSpeed({GetType("highway", "secondary", "bridge")}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-    CheckSpeed({GetType("highway", "secondary", "tunnel")}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-    CheckSpeed({GetType("highway", "secondary")}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+    CheckSpeed({secondaryBridge}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+    CheckSpeed({secondaryTunnel}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+    CheckSpeed({secondary}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
   }
 
-  CheckSpeed({GetType("highway", "trunk")},
+  CheckSpeed({classif().GetTypeByPath({"highway", "trunk"})},
              {SpeedKMpH(100.0 /* weight */, 100.0 /* eta */) /* in city */,
               SpeedKMpH(150.0 /* weight */, 150.0 /* eta */) /* out of city */});
-  CheckSpeed({GetType("highway", "primary")}, {SpeedKMpH(90.0, 90.0), SpeedKMpH(120.0, 120.0)});
-  CheckSpeed({GetType("highway", "residential")}, {SpeedKMpH(22.5, 27.5), SpeedKMpH(25.0, 30.0)});
+  CheckSpeed({primary}, {SpeedKMpH(90.0, 90.0), SpeedKMpH(120.0, 120.0)});
+  CheckSpeed({residential}, {SpeedKMpH(22.5, 27.5), SpeedKMpH(25.0, 30.0)});
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_Speed_MultiTypes)
+UNIT_CLASS_TEST(VehicleModelTest, Speed_MultiTypes)
 {
-  uint32_t const typeTunnel = GetType("highway", "secondary", "tunnel");
-  uint32_t const typeSecondary = GetType("highway", "secondary");
-  uint32_t const typeHighway = GetType("highway");
+  uint32_t const typeHighway = classif().GetTypeByPath({"highway"});
 
-  CheckSpeed({typeTunnel, typeSecondary}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-  CheckSpeed({typeTunnel, typeHighway}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-  CheckSpeed({typeHighway, typeTunnel}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+  CheckSpeed({secondaryTunnel, secondary}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+  CheckSpeed({secondaryTunnel, typeHighway}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+  CheckSpeed({typeHighway, secondaryTunnel}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_OneWay)
+UNIT_CLASS_TEST(VehicleModelTest, OneWay)
 {
-  uint32_t const typeBridge = GetType("highway", "secondary", "bridge");
-  uint32_t const typeOneway = GetOnewayType();
+  CheckSpeed({secondaryBridge, oneway}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+  CheckOneWay({secondaryBridge, oneway}, true);
+  CheckSpeed({oneway, secondaryBridge}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+  CheckOneWay({oneway, secondaryBridge}, true);
 
-  CheckSpeed({typeBridge, typeOneway}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-  CheckOneWay({typeBridge, typeOneway}, true);
-  CheckSpeed({typeOneway, typeBridge}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-  CheckOneWay({typeOneway, typeBridge}, true);
-
-  CheckOneWay({typeOneway}, true);
+  CheckOneWay({oneway}, true);
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_DifferentSpeeds)
+UNIT_CLASS_TEST(VehicleModelTest, DifferentSpeeds)
 {
-  uint32_t const typeSecondary = GetType("highway", "secondary");
-  uint32_t const typePrimary = GetType("highway", "primary");
-  uint32_t const typeOneway = GetOnewayType();
-
-  CheckSpeed({typeSecondary, typePrimary}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-
-  CheckSpeed({typeSecondary, typePrimary, typeOneway}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
-  CheckOneWay({typePrimary, typeOneway, typeSecondary}, true);
+  // What is the purpose of this artificial test with several highway types? To show that order is important?
+  CheckSpeed({secondary, primary}, kDefaultSpeeds.Get(HighwayType::HighwaySecondary));
+  CheckSpeed({oneway, primary, secondary}, kDefaultSpeeds.Get(HighwayType::HighwayPrimary));
+  CheckOneWay({primary, oneway, secondary}, true);
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_PassThroughAllowed)
+UNIT_CLASS_TEST(VehicleModelTest, PassThroughAllowed)
 {
-  CheckPassThroughAllowed({GetType("highway", "secondary")}, true);
-  CheckPassThroughAllowed({GetType("highway", "primary")}, true);
-  CheckPassThroughAllowed({GetType("highway", "service")}, false);
+  CheckPassThroughAllowed({secondary}, true);
+  CheckPassThroughAllowed({primary}, true);
+  CheckPassThroughAllowed({classif().GetTypeByPath({"highway", "service"})}, false);
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_SpeedFactor)
+UNIT_CLASS_TEST(VehicleModelTest, SpeedFactor)
 {
-  uint32_t const secondary = GetType("highway", "secondary");
-  uint32_t const residential = GetType("highway", "residential");
-  uint32_t const pavedGood = GetType("psurface", "paved_good");
-  uint32_t const pavedBad = GetType("psurface", "paved_bad");
-  uint32_t const unpavedGood = GetType("psurface", "unpaved_good");
-  uint32_t const unpavedBad = GetType("psurface", "unpaved_bad");
-
   CheckSpeed({secondary, pavedGood},
              {SpeedKMpH(64.0 /* weight */, 63.0 /* eta */) /* in city */,
               SpeedKMpH(64.0 /* weight */, 63.0 /* eta */) /* out of city */});
@@ -217,14 +198,8 @@ UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_SpeedFactor)
   CheckSpeed({residential, unpavedBad}, {SpeedKMpH(4.5, 5.5), SpeedKMpH(5.0, 6.0)});
 }
 
-UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_MaxspeedFactor)
+UNIT_CLASS_TEST(VehicleModelTest, MaxspeedFactor)
 {
-  uint32_t const secondary = GetType("highway", "secondary");
-  uint32_t const residential = GetType("highway", "residential");
-  uint32_t const primary = GetType("highway", "primary");
-  uint32_t const pavedGood = GetType("psurface", "paved_good");
-  uint32_t const unpavedBad = GetType("psurface", "unpaved_bad");
-
   Maxspeed const maxspeed90 =
       Maxspeed(measurement_utils::Units::Metric, 90 /* forward speed */, kInvalidSpeed);
   CheckSpeedWithParams({secondary, unpavedBad},
@@ -251,6 +226,46 @@ UNIT_CLASS_TEST(VehicleModelTest, VehicleModel_MaxspeedFactor)
                        SpeedKMpH(24.0, 27.0));
 }
 
+namespace
+{
+bool LessSpeed(SpeedKMpH const & l, SpeedKMpH const & r)
+{
+  TEST(l.IsValid() && r.IsValid(), (l, r));
+  return l.m_weight < r.m_weight && l.m_eta < r.m_eta;
+}
+
+#define TEST_LESS_SPEED(l, r) TEST(LessSpeed(l, r), (l, r))
+} // namespace
+
+UNIT_CLASS_TEST(VehicleModelTest, CarModel_TrackVsGravelTertiary)
+{
+  auto const & model = CarModel::AllLimitsInstance();
+
+  auto const & c = classif();
+  feature::TypesHolder h1;
+  h1.Add(c.GetTypeByPath({"highway", "track"}));
+
+  feature::TypesHolder h2;
+  h2.Add(c.GetTypeByPath({"highway", "tertiary"}));
+  h2.Add(unpavedBad);   // from OSM surface=gravel
+
+  // https://www.openstreetmap.org/#map=19/45.43640/36.39689
+  // Obvious that gravel tertiary (moreover with maxspeed=60kmh) should be better than track.
+
+  {
+    SpeedParams p1({}, kInvalidSpeed, false /* inCity */);
+    SpeedParams p2({measurement_utils::Units::Metric, 60, 60}, kInvalidSpeed, false /* inCity */);
+    TEST_LESS_SPEED(model.GetTypeSpeed(h1, p1), model.GetTypeSpeed(h2, p2));
+  }
+
+  {
+    SpeedParams p({}, kInvalidSpeed, false /* inCity */);
+    TEST_LESS_SPEED(model.GetTypeSpeed(h1, p), model.GetTypeSpeed(h2, p));
+  }
+}
+
+#undef TEST_LESS_SPEED
+
 UNIT_TEST(VehicleModel_MultiplicationOperatorTest)
 {
   SpeedKMpH const speed(90 /* weight */, 100 /* eta */);
@@ -258,13 +273,13 @@ UNIT_TEST(VehicleModel_MultiplicationOperatorTest)
   SpeedKMpH const lResult = speed * factor;
   SpeedKMpH const rResult = factor * speed;
   TEST_EQUAL(lResult, rResult, ());
-  TEST(base::AlmostEqualAbs(lResult.m_weight, 90.0, 1e-7), ());
-  TEST(base::AlmostEqualAbs(lResult.m_eta, 110.0, 1e-7), ());
+  TEST(base::AlmostEqualULPs(lResult.m_weight, 90.0), ());
+  TEST(base::AlmostEqualULPs(lResult.m_eta, 110.0), ());
 }
 
 UNIT_TEST(VehicleModel_CarModelValidation)
 {
-  vector<HighwayType> const carRoadTypes = {
+  HighwayType const carRoadTypes[] = {
       HighwayType::HighwayLivingStreet,  HighwayType::HighwayMotorway,
       HighwayType::HighwayMotorwayLink,  HighwayType::HighwayPrimary,
       HighwayType::HighwayPrimaryLink,   HighwayType::HighwayResidential,

routing_common/vehicle_model.hpp

@@ -68,7 +68,7 @@ using HighwayBasedSpeeds = base::SmallMap<HighwayType, InOutCitySpeedKMpH>;
 /// \brief Params for calculation of an approximate speed on a feature.
 struct SpeedParams
 {
-  /// For unit tests compatibility.
+  /// @deprecated For unit tests compatibility.
   SpeedParams(bool forward, bool inCity, Maxspeed const & maxspeed)
     : m_maxspeed(maxspeed), m_defSpeedKmPH(kInvalidSpeed), m_inCity(inCity), m_forward(forward)
   {
@@ -291,6 +291,7 @@ public:
   bool IsPassThroughAllowed(FeatureType & f) const override;
   /// @}
 
+  // Made public to have simple access from unit tests.
 public:
   /// @returns true if |m_highwayTypes| or |m_addRoadTypes| contains |type| and false otherwise.
   bool IsRoadType(uint32_t type) const;
@@ -314,14 +315,6 @@ public:
            (m_onewayType == rhs.m_onewayType);
   }
 
-protected:
-  /// @returns a special restriction which is set to the feature.
-  virtual RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const;
-
-  void AddAdditionalRoadTypes(Classificator const & classif, AdditionalRoadsList const & roads);
-
-  uint32_t PrepareToMatchType(uint32_t type) const;
-
   /// \returns true if |types| is a oneway feature.
   /// \note According to OSM, tag "oneway" could have value "-1". That means it's a oneway feature
   /// with reversed geometry. In that case at map generation the geometry of such features
@@ -331,6 +324,14 @@ protected:
 
   bool HasPassThroughType(feature::TypesHolder const & types) const;
 
+protected:
+  /// @returns a special restriction which is set to the feature.
+  virtual RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const;
+
+  void AddAdditionalRoadTypes(Classificator const & classif, AdditionalRoadsList const & roads);
+
+  uint32_t PrepareToMatchType(uint32_t type) const;
+
   SpeedKMpH GetSpeedWihtoutMaxspeed(FeatureType & f, SpeedParams params) const;
 
   /// \brief Maximum within all the speed limits set in a model (car model, bicycle model and so on).