From 857ccb37f1d909d001c6f3b955cf267d47110215 Mon Sep 17 00:00:00 2001 From: Viktor Govako Date: Fri, 14 Oct 2022 14:34:53 +0300 Subject: [PATCH] [search] Fixed m_matchedLength calculation for category. Signed-off-by: Viktor Govako --- .../generator_tests_support/test_feature.cpp | 30 +++++++++-------- .../generator_tests_support/test_feature.hpp | 20 ++++------- search/ranker.cpp | 1 + search/ranking_utils.cpp | 25 +++++++------- .../processor_test.cpp | 33 +++++++++++++++++-- 5 files changed, 68 insertions(+), 41 deletions(-) diff --git a/generator/generator_tests_support/test_feature.cpp b/generator/generator_tests_support/test_feature.cpp index f01010da47..73d57ca048 100644 --- a/generator/generator_tests_support/test_feature.cpp +++ b/generator/generator_tests_support/test_feature.cpp @@ -34,6 +34,11 @@ uint64_t GenUniqueId() static atomic id; return id.fetch_add(1); } + +vector MakePoly(m2::RectD const & rect) +{ + return { rect.LeftBottom(), rect.RightBottom(), rect.RightTop(), rect.LeftTop(), rect.LeftBottom() }; +} } // namespace // TestFeature ------------------------------------------------------------------------------------- @@ -70,6 +75,11 @@ TestFeature::TestFeature(m2::PointD const & center, StringUtf8Multilang const & Init(); } +TestFeature::TestFeature(m2::RectD const & boundary, std::string const & name, std::string const & lang) + : TestFeature(MakePoly(boundary), name, lang) +{ +} + TestFeature::TestFeature(vector const & boundary, string const & name, string const & lang) : m_id(GenUniqueId()), m_center(0, 0), m_boundary(boundary), m_type(Type::Area) { @@ -279,7 +289,7 @@ string TestStreet::ToDebugString() const // TestSquare -------------------------------------------------------------------------------------- TestSquare::TestSquare(m2::RectD const & rect, string const & name, string const & lang) - : TestFeature(name, lang), m_rect(rect) + : TestFeature(rect, name, lang) { } @@ -289,19 +299,12 @@ void TestSquare::Serialize(FeatureBuilder & fb) const auto const & classificator = classif(); fb.SetType(classificator.GetTypeByPath({"place", "square"})); - - fb.AddPoint(m_rect.LeftBottom()); - fb.AddPoint(m_rect.RightBottom()); - fb.AddPoint(m_rect.RightTop()); - fb.AddPoint(m_rect.LeftTop()); - fb.AddPoint(m_rect.LeftBottom()); - fb.SetArea(); } string TestSquare::ToDebugString() const { ostringstream os; - os << "TestSquare [" << DebugPrint(m_names) << ", " << m_rect << "]"; + os << "TestSquare [" << DebugPrint(m_names) << "]"; return os.str(); } @@ -418,7 +421,7 @@ TestBuilding::TestBuilding(m2::PointD const & center, string const & name, { } -TestBuilding::TestBuilding(vector const & boundary, string const & name, +TestBuilding::TestBuilding(m2::RectD const & boundary, string const & name, string const & houseNumber, string_view street, string const & lang) : TestFeature(boundary, name, lang) , m_houseNumber(houseNumber) @@ -435,10 +438,9 @@ void TestBuilding::Serialize(FeatureBuilder & fb) const if (!m_streetName.empty()) params.AddStreet(m_streetName); - auto const & classificator = classif(); - fb.AddType(classificator.GetTypeByPath({"building"})); - for (auto const & type : m_types) - fb.AddType(classificator.GetTypeByPath(type)); + fb.AddType(classif().GetTypeByPath({"building"})); + for (uint32_t const type : m_types) + fb.AddType(type); } string TestBuilding::ToDebugString() const diff --git a/generator/generator_tests_support/test_feature.hpp b/generator/generator_tests_support/test_feature.hpp index 4c37b612ce..dde4110c55 100644 --- a/generator/generator_tests_support/test_feature.hpp +++ b/generator/generator_tests_support/test_feature.hpp @@ -10,10 +10,8 @@ #include "geometry/point2d.hpp" #include "geometry/rect2d.hpp" -#include #include #include -#include #include class FeatureType; @@ -32,7 +30,7 @@ public: bool Matches(FeatureType & feature) const; void SetPostcode(std::string const & postcode) { m_postcode = postcode; } uint64_t GetId() const { return m_id; } -// StringUtf8Multilang const & GetNames() const { return m_names; } + std::string_view GetName(std::string_view lang) const { std::string_view res; @@ -61,8 +59,8 @@ protected: TestFeature(StringUtf8Multilang const & name); TestFeature(m2::PointD const & center, std::string const & name, std::string const & lang); TestFeature(m2::PointD const & center, StringUtf8Multilang const & name); - TestFeature(std::vector const & boundary, std::string const & name, - std::string const & lang); + TestFeature(m2::RectD const & boundary, std::string const & name, std::string const & lang); + TestFeature(std::vector const & boundary, std::string const & name, std::string const & lang); uint64_t const m_id; m2::PointD const m_center; @@ -153,9 +151,6 @@ public: // TestFeature overrides: void Serialize(feature::FeatureBuilder & fb) const override; std::string ToDebugString() const override; - -private: - m2::RectD m_rect; }; class TestPOI : public TestFeature @@ -205,21 +200,20 @@ public: std::string const & lang); TestBuilding(m2::PointD const & center, std::string const & name, std::string const & houseNumber, std::string_view street, std::string const & lang); - TestBuilding(std::vector const & boundary, std::string const & name, - std::string const & houseNumber, std::string_view street, - std::string const & lang); + TestBuilding(m2::RectD const & boundary, std::string const & name, std::string const & houseNumber, + std::string_view street, std::string const & lang); // TestFeature overrides: void Serialize(feature::FeatureBuilder & fb) const override; std::string ToDebugString() const override; - void AddType(std::vector const & path) { m_types.push_back(path); } + void AddType(uint32_t type) { m_types.push_back(type); } private: std::string const m_houseNumber; std::string const m_streetName; - std::vector> m_types; + std::vector m_types; }; class TestPark : public TestFeature diff --git a/search/ranker.cpp b/search/ranker.cpp index 0c9bb00a92..620b922b1b 100644 --- a/search/ranker.cpp +++ b/search/ranker.cpp @@ -628,6 +628,7 @@ private: if (m_params.GetNumTokens() == preInfo.InnermostTokenRange().Size()) info.m_nameScore = NameScore::FULL_PREFIX; + ASSERT_LESS_OR_EQUAL(categoriesInfo.GetMatchedLength(), totalLength, (featureTypes)); info.m_matchedFraction = std::max(info.m_matchedFraction, categoriesInfo.GetMatchedLength() / static_cast(totalLength)); if (!info.m_errorsMade.IsValid()) diff --git a/search/ranking_utils.cpp b/search/ranking_utils.cpp index dcdf7a11d0..f8e5e1632e 100644 --- a/search/ranking_utils.cpp +++ b/search/ranking_utils.cpp @@ -16,21 +16,19 @@ namespace search using namespace std; using namespace strings; -namespace -{ -struct TokenInfo -{ - bool m_isCategoryToken = false; - bool m_inFeatureTypes = false; -}; -} // namespace - // CategoriesInfo ---------------------------------------------------------------------------------- CategoriesInfo::CategoriesInfo(feature::TypesHolder const & holder, TokenSlice const & tokens, Locales const & locales, CategoriesHolder const & categories) { + struct TokenInfo + { + bool m_isCategoryToken = false; + bool m_inFeatureTypes = false; + }; + QuerySlice slice(tokens); vector infos(slice.Size()); + ForEachCategoryType(slice, locales, categories, [&](size_t i, uint32_t t) { ASSERT_LESS(i, infos.size(), ()); @@ -38,12 +36,15 @@ CategoriesInfo::CategoriesInfo(feature::TypesHolder const & holder, TokenSlice c info.m_isCategoryToken = true; if (holder.HasWithSubclass(t)) - { - m_matchedLength += slice.Get(i).size(); info.m_inFeatureTypes = true; - } }); + for (size_t i = 0; i < slice.Size(); ++i) + { + if (infos[i].m_inFeatureTypes) + m_matchedLength += slice.Get(i).size(); + } + // Note that m_inFeatureTypes implies m_isCategoryToken. m_pureCategories = all_of(infos.begin(), infos.end(), [](TokenInfo const & info) diff --git a/search/search_integration_tests/processor_test.cpp b/search/search_integration_tests/processor_test.cpp index 3fa035e022..11a09f456d 100644 --- a/search/search_integration_tests/processor_test.cpp +++ b/search/search_integration_tests/processor_test.cpp @@ -127,8 +127,8 @@ UNIT_CLASS_TEST(ProcessorTest, Smoke) TestBuilding feynmanHouse({10, 10}, "Feynman house", "1 unit 1", feynmanStreet.GetName("en"), "en"); TestBuilding bohrHouse({10, 10}, "Bohr house", "1 unit 1", bohrStreet1.GetName("en"), "en"); - TestBuilding hilbertHouse({{10.0005, 10.0005}, {10.0006, 10.0005}, {10.0006, 10.0006}, {10.0005, 10.0006}}, - "Hilbert house", "1 unit 2", bohrStreet1.GetName("en"), "en"); + TestBuilding hilbertHouse({10.0005, 10.0005, 10.0006, 10.0006}, + "Hilbert house", "1 unit 2", bohrStreet1.GetName("en"), "en"); TestBuilding descartesHouse({10, 10}, "Descartes house", "2", "en"); TestBuilding bornHouse({14.999, 15}, "Born house", "8", firstAprilStreet.GetName("en"), "en"); @@ -3167,4 +3167,33 @@ UNIT_CLASS_TEST(ProcessorTest, PoiStreetCity_FancyMatch) TEST(ResultsMatch({results[2]}, {ExactMatch(countryId, minskStreet)}), ()); } +UNIT_CLASS_TEST(ProcessorTest, ComplexPoi_Rank) +{ + auto const & cl = classif(); + + TestBuilding landuse({-1, -1, 1, 1}, "Telekom", "5", "xxx", "de"); + landuse.AddType(cl.GetTypeByPath({"landuse", "commercial"})); + TestPOI poiInMall({0, 0}, "yyy", "de"); + poiInMall.SetType(cl.GetTypeByPath({"shop", "clothes"})); + TestPOI telekom({2, 2}, "Telekom shop", "de"); + telekom.SetType(cl.GetTypeByPath({"shop", "mobile_phone"})); + + auto countryId = BuildCountry("Wonderland", [&](TestMwmBuilder & builder) + { + builder.Add(landuse); + builder.Add(poiInMall); + builder.Add(telekom); + }); + + SetViewport({-0.5, -0.5, 0.5, 0.5}); + + auto request = MakeRequest("Telekom shop"); + auto const & results = request->Results(); + + TEST_EQUAL(results.size(), 2, ()); + + TEST(ResultsMatch({results[0]}, {ExactMatch(countryId, telekom)}), ()); + TEST(ResultsMatch({results[1]}, {ExactMatch(countryId, poiInMall)}), ()); +} + } // namespace processor_test