From 76ee0dd2303a4e47195f1538e11be912b8a93486 Mon Sep 17 00:00:00 2001 From: tatiana-yan Date: Wed, 3 Jun 2020 11:57:51 +0300 Subject: [PATCH] [search] COMPLEX_POI/SUBPOI minor refactoring. --- search/features_layer_matcher.hpp | 27 +++++++++---------- search/geocoder.cpp | 8 +++--- search/geocoder_context.cpp | 4 +-- search/geocoder_context.hpp | 2 +- search/intersection_result.cpp | 12 ++++----- search/intersection_result.hpp | 4 +-- search/model.cpp | 16 +++++------ search/model.hpp | 8 +++--- search/ranker.cpp | 2 +- search/ranking_info.cpp | 4 +-- search/ranking_info.hpp | 2 +- .../pre_ranker_test.cpp | 2 +- .../processor_test.cpp | 12 ++++----- search/search_quality/scoring_model.py | 10 +++---- search/search_tests/ranking_tests.cpp | 4 +-- 15 files changed, 59 insertions(+), 58 deletions(-) diff --git a/search/features_layer_matcher.hpp b/search/features_layer_matcher.hpp index f9d559dbb8..6c85548e77 100644 --- a/search/features_layer_matcher.hpp +++ b/search/features_layer_matcher.hpp @@ -51,10 +51,10 @@ namespace search // feature-from-parent-layer. Belongs-to is a partial relation on // features, and has different meaning for different search classes: // -// * BUILDING/POI/SUBPOI belongs-to STREET iff it is located on the street; +// * BUILDING/POI belongs-to STREET iff it is located on the street; // * BUILDING belongs-to CITY iff the building is located in the city; -// * POI/SUBPOI belongs-to BUILDING iff the poi is (roughly) located near or inside the building; -// * SUBPOI belongs-to POI iff the poi is (roughly) located near or inside the building; +// * POI belongs-to BUILDING iff the poi is (roughly) located near or inside the building; +// * SUBPOI belongs-to COMPLEX_POI iff the SUBPOI is (roughly) located near or inside the COMPLEX_POI; // * STREET belongs-to CITY iff the street is (roughly) located in the city; // * etc. // @@ -64,7 +64,7 @@ class FeaturesLayerMatcher public: static uint32_t const kInvalidId = std::numeric_limits::max(); static int constexpr kBuildingRadiusMeters = 50; - static int constexpr kBigPoiRadiusMeters = 300; + static int constexpr kComplexPoiRadiusMeters = 300; static int constexpr kStreetRadiusMeters = 100; FeaturesLayerMatcher(DataSource const & dataSource, base::Cancellable const & cancellable); @@ -87,26 +87,25 @@ public: case Model::TYPE_COUNT: ASSERT(false, ("Invalid parent layer type:", parent.m_type)); break; - case Model::TYPE_POI: + case Model::TYPE_COMPLEX_POI: ASSERT_EQUAL(child.m_type, Model::TYPE_SUBPOI, ()); MatchPOIsWithParent(child, parent, std::forward(fn)); break; case Model::TYPE_BUILDING: - ASSERT(child.m_type == Model::TYPE_POI || child.m_type == Model::TYPE_SUBPOI, ()); + ASSERT(Model::IsPoi(child.m_type), ()); MatchPOIsWithParent(child, parent, std::forward(fn)); break; case Model::TYPE_STREET: - ASSERT(child.m_type == Model::TYPE_POI || child.m_type == Model::TYPE_BUILDING || - child.m_type == Model::TYPE_SUBPOI, + ASSERT(Model::IsPoi(child.m_type) || child.m_type == Model::TYPE_BUILDING, ("Invalid child layer type:", child.m_type)); - if (child.m_type == Model::TYPE_POI || child.m_type == Model::TYPE_SUBPOI) + if (Model::IsPoi(child.m_type)) MatchPOIsWithStreets(child, parent, std::forward(fn)); else MatchBuildingsWithStreets(child, parent, std::forward(fn)); break; case Model::TYPE_SUBURB: ASSERT(child.m_type == Model::TYPE_STREET || child.m_type == Model::TYPE_BUILDING || - child.m_type == Model::TYPE_POI || child.m_type == Model::TYPE_SUBPOI, + Model::IsPoi(child.m_type), ()); // Avoid matching buildings to suburb without street. if (child.m_type == Model::TYPE_BUILDING) @@ -131,14 +130,14 @@ private: if (parent.m_type == Model::TYPE_BUILDING) { - ASSERT(child.m_type == Model::TYPE_POI || child.m_type == Model::TYPE_SUBPOI, ()); + ASSERT(Model::IsPoi(child.m_type), ()); parentRadius = kBuildingRadiusMeters; } else { - ASSERT_EQUAL(parent.m_type, Model::TYPE_POI, ()); + ASSERT_EQUAL(parent.m_type, Model::TYPE_COMPLEX_POI, ()); ASSERT_EQUAL(child.m_type, Model::TYPE_SUBPOI, ()); - parentRadius = kBigPoiRadiusMeters; + parentRadius = kComplexPoiRadiusMeters; } auto const & pois = *child.m_sortedFeatures; @@ -234,7 +233,7 @@ private: { BailIfCancelled(); - ASSERT(child.m_type == Model::TYPE_POI || child.m_type == Model::TYPE_SUBPOI, ()); + ASSERT(Model::IsPoi(child.m_type), ()); ASSERT_EQUAL(parent.m_type, Model::TYPE_STREET, ()); auto const & pois = *child.m_sortedFeatures; diff --git a/search/geocoder.cpp b/search/geocoder.cpp index 3f52abf981..4701883c3a 100644 --- a/search/geocoder.cpp +++ b/search/geocoder.cpp @@ -1499,7 +1499,7 @@ bool Geocoder::IsLayerSequenceSane(vector const & layers) const buildingIndex = i; else if (layer.m_type == Model::TYPE_STREET) streetIndex = i; - else if (layer.m_type == Model::TYPE_POI || layer.m_type == Model::TYPE_SUBPOI) + else if (Model::IsPoi(layer.m_type)) poiIndex = i; } @@ -1553,8 +1553,8 @@ void Geocoder::FindPaths(BaseContext & ctx) if (tokenType == BaseContext::TokenType::TOKEN_TYPE_SUBPOI) id = result.m_subpoi; - if (tokenType == BaseContext::TokenType::TOKEN_TYPE_POI) - id = result.m_poi; + if (tokenType == BaseContext::TokenType::TOKEN_TYPE_COMPLEX_POI) + id = result.m_complex_poi; if (tokenType == BaseContext::TokenType::TOKEN_TYPE_STREET) id = result.m_street; @@ -1599,7 +1599,7 @@ void Geocoder::TraceResult(Tracer & tracer, BaseContext const & ctx, MwmSet::Mwm { SCOPE_GUARD(emitParse, [&]() { tracer.EmitParse(ctx.m_tokens); }); - if (type != Model::TYPE_POI && type != Model::TYPE_SUBPOI && type != Model::TYPE_BUILDING) + if (!Model::IsPoi(type) && type != Model::TYPE_BUILDING) return; if (mwmId != m_context->GetId()) diff --git a/search/geocoder_context.cpp b/search/geocoder_context.cpp index 7ba48107c5..bd38801149 100644 --- a/search/geocoder_context.cpp +++ b/search/geocoder_context.cpp @@ -15,7 +15,7 @@ BaseContext::TokenType BaseContext::FromModelType(Model::Type type) switch (type) { case Model::TYPE_SUBPOI: return TOKEN_TYPE_SUBPOI; - case Model::TYPE_POI: return TOKEN_TYPE_POI; + case Model::TYPE_COMPLEX_POI: return TOKEN_TYPE_COMPLEX_POI; case Model::TYPE_BUILDING: return TOKEN_TYPE_BUILDING; case Model::TYPE_STREET: return TOKEN_TYPE_STREET; case Model::TYPE_SUBURB: return TOKEN_TYPE_SUBURB; @@ -91,7 +91,7 @@ string ToString(BaseContext::TokenType type) switch (type) { case BaseContext::TOKEN_TYPE_SUBPOI: return "SUBPOI"; - case BaseContext::TOKEN_TYPE_POI: return "POI"; + case BaseContext::TOKEN_TYPE_COMPLEX_POI: return "COMPLEX_POI"; case BaseContext::TOKEN_TYPE_BUILDING: return "BUILDING"; case BaseContext::TOKEN_TYPE_STREET: return "STREET"; case BaseContext::TOKEN_TYPE_SUBURB: return "SUBURB"; diff --git a/search/geocoder_context.hpp b/search/geocoder_context.hpp index e2a0c65263..906f90451e 100644 --- a/search/geocoder_context.hpp +++ b/search/geocoder_context.hpp @@ -23,7 +23,7 @@ struct BaseContext enum TokenType { TOKEN_TYPE_SUBPOI, - TOKEN_TYPE_POI, + TOKEN_TYPE_COMPLEX_POI, TOKEN_TYPE_BUILDING, TOKEN_TYPE_STREET, TOKEN_TYPE_SUBURB, diff --git a/search/intersection_result.cpp b/search/intersection_result.cpp index 35ccfc340d..dab7ab53cf 100644 --- a/search/intersection_result.cpp +++ b/search/intersection_result.cpp @@ -12,7 +12,7 @@ void IntersectionResult::Set(Model::Type type, uint32_t id) switch (type) { case Model::TYPE_SUBPOI: m_subpoi = id; break; - case Model::TYPE_POI: m_poi = id; break; + case Model::TYPE_COMPLEX_POI: m_complex_poi = id; break; case Model::TYPE_BUILDING: m_building = id; break; case Model::TYPE_STREET: m_street = id; break; case Model::TYPE_SUBURB: m_suburb = id; break; @@ -29,8 +29,8 @@ uint32_t IntersectionResult::InnermostResult() const { if (m_subpoi != kInvalidId) return m_subpoi; - if (m_poi != kInvalidId) - return m_poi; + if (m_complex_poi != kInvalidId) + return m_complex_poi; if (m_building != kInvalidId) return m_building; if (m_street != kInvalidId) @@ -43,7 +43,7 @@ uint32_t IntersectionResult::InnermostResult() const void IntersectionResult::Clear() { m_subpoi = kInvalidId; - m_poi = kInvalidId; + m_complex_poi = kInvalidId; m_building = kInvalidId; m_street = kInvalidId; m_suburb = kInvalidId; @@ -55,8 +55,8 @@ std::string DebugPrint(IntersectionResult const & result) os << "IntersectionResult [ "; if (result.m_subpoi != IntersectionResult::kInvalidId) os << "SUBPOI:" << result.m_subpoi << " "; - if (result.m_poi != IntersectionResult::kInvalidId) - os << "POI:" << result.m_poi << " "; + if (result.m_complex_poi != IntersectionResult::kInvalidId) + os << "COMPLEX_POI:" << result.m_complex_poi << " "; if (result.m_building != IntersectionResult::kInvalidId) os << "BUILDING:" << result.m_building << " "; if (result.m_street != IntersectionResult::kInvalidId) diff --git a/search/intersection_result.hpp b/search/intersection_result.hpp index 561d07adb5..917694bbdc 100644 --- a/search/intersection_result.hpp +++ b/search/intersection_result.hpp @@ -16,7 +16,7 @@ struct IntersectionResult void Set(Model::Type type, uint32_t id); - // Returns the first valid feature among the [SUBPOI, POI, BUILDING, + // Returns the first valid feature among the [SUBPOI, COMPLEX_POI, BUILDING, // STREET]. uint32_t InnermostResult() const; @@ -27,7 +27,7 @@ struct IntersectionResult void Clear(); uint32_t m_subpoi = kInvalidId; - uint32_t m_poi = kInvalidId; + uint32_t m_complex_poi = kInvalidId; uint32_t m_building = kInvalidId; uint32_t m_street = kInvalidId; uint32_t m_suburb = kInvalidId; diff --git a/search/model.cpp b/search/model.cpp index 47ba35ceed..1e6263590e 100644 --- a/search/model.cpp +++ b/search/model.cpp @@ -76,9 +76,9 @@ private: TwoLevelPOIChecker const m_twoLevel; }; -class CanContainSubpoisChecker : public ftypes::BaseChecker +class IsComplexPoiChecker : public ftypes::BaseChecker { - CanContainSubpoisChecker() : ftypes::BaseChecker() + IsComplexPoiChecker() : ftypes::BaseChecker() { Classificator const & c = classif(); vector> const paths = {{"aeroway", "aerodrome"}, @@ -112,9 +112,9 @@ class CanContainSubpoisChecker : public ftypes::BaseChecker } public: - static CanContainSubpoisChecker const & Instance() + static IsComplexPoiChecker const & Instance() { - static CanContainSubpoisChecker const inst; + static IsComplexPoiChecker const inst; return inst; } }; @@ -145,11 +145,11 @@ Model::Type Model::GetType(FeatureType & feature) const static auto const & suburbChecker = IsSuburbChecker::Instance(); static auto const & localityChecker = IsLocalityChecker::Instance(); static auto const & poiChecker = IsPoiChecker::Instance(); - static auto const & canContainSubpoisChecker = CanContainSubpoisChecker::Instance(); + static auto const & complexPoiChecker = IsComplexPoiChecker::Instance(); // Check whether object is POI first to mark POIs with address tags as POI. - if (canContainSubpoisChecker(feature)) - return TYPE_POI; + if (complexPoiChecker(feature)) + return TYPE_COMPLEX_POI; if (poiChecker(feature)) return TYPE_SUBPOI; @@ -185,7 +185,7 @@ string DebugPrint(Model::Type type) switch (type) { case Model::TYPE_SUBPOI: return "SUBPOI"; - case Model::TYPE_POI: return "POI"; + case Model::TYPE_COMPLEX_POI: return "COMPLEX_POI"; case Model::TYPE_BUILDING: return "Building"; case Model::TYPE_STREET: return "Street"; case Model::TYPE_SUBURB: return "Suburb"; diff --git a/search/model.hpp b/search/model.hpp index 4d8b1878bd..4a85f113c0 100644 --- a/search/model.hpp +++ b/search/model.hpp @@ -28,11 +28,11 @@ public: enum Type { // Low-level features such as amenities, offices, shops, buildings without house number, etc. - // Can be located inside another POIs. E.g. cafes/shops inside airports/universities/museums. + // Can be located inside COMPLEX_POIs. E.g. cafes/shops inside airports/universities/museums. TYPE_SUBPOI, - // Big pois which can contain another POIs. E.g. airports, train stations, malls, parks. - TYPE_POI, + // Big pois which can contain SUBPOIs. E.g. airports, train stations, malls, parks. + TYPE_COMPLEX_POI, // All features with set house number. TYPE_BUILDING, @@ -56,6 +56,8 @@ public: return type >= TYPE_VILLAGE && type <= TYPE_COUNTRY; } + static bool IsPoi(Type const type) { return type == TYPE_SUBPOI || type == TYPE_COMPLEX_POI; } + Type GetType(FeatureType & feature) const; }; diff --git a/search/ranker.cpp b/search/ranker.cpp index 19f99551ec..5463b480a8 100644 --- a/search/ranker.cpp +++ b/search/ranker.cpp @@ -354,7 +354,7 @@ class RankerResultMaker info.m_popularity = preInfo.m_popularity; info.m_rating = preInfo.m_rating; info.m_type = preInfo.m_type; - if (info.m_type == Model::TYPE_POI || info.m_type == Model::TYPE_SUBPOI) + if (Model::IsPoi(info.m_type)) info.m_resultType = GetResultType(feature::TypesHolder(ft)); info.m_allTokensUsed = preInfo.m_allTokensUsed; info.m_numTokens = m_params.GetNumTokens(); diff --git a/search/ranking_info.cpp b/search/ranking_info.cpp index 47e9cc9623..f2a35cf817 100644 --- a/search/ranking_info.cpp +++ b/search/ranking_info.cpp @@ -46,7 +46,7 @@ double constexpr kNameScore[NameScore::NAME_SCORE_COUNT] = { }; double constexpr kType[Model::TYPE_COUNT] = { -0.0467816 /* SUBPOI */, - -0.0467816 /* POI */, + -0.0467816 /* COMPLEX_POI */, -0.0467816 /* Building */, -0.0444630 /* Street */, -0.0348396 /* Unclassified */, @@ -254,7 +254,7 @@ double RankingInfo::GetLinearModelRank() const result += kRating * rating; result += m_falseCats * kFalseCats; result += kType[m_type]; - if (m_type == Model::TYPE_POI || m_type == Model::TYPE_SUBPOI) + if (Model::IsPoi(m_type)) result += kResultType[base::Underlying(m_resultType)]; result += kNameScore[nameScore]; result += kErrorsMade * GetErrorsMadePerToken(); diff --git a/search/ranking_info.hpp b/search/ranking_info.hpp index ddcc9f2d0c..6d4a922b13 100644 --- a/search/ranking_info.hpp +++ b/search/ranking_info.hpp @@ -93,7 +93,7 @@ struct RankingInfo // Search type for the feature. Model::Type m_type = Model::TYPE_COUNT; - // Type (food/transport/attraction/etc) for POI/SUBPOI results for non-categorial requests. + // Type (food/transport/attraction/etc) for POI results for non-categorial requests. ResultType m_resultType = ResultType::Count; // True if all of the tokens that the feature was matched by diff --git a/search/search_integration_tests/pre_ranker_test.cpp b/search/search_integration_tests/pre_ranker_test.cpp index 1718b540cb..45a034e57d 100644 --- a/search/search_integration_tests/pre_ranker_test.cpp +++ b/search/search_integration_tests/pre_ranker_test.cpp @@ -153,7 +153,7 @@ UNIT_CLASS_TEST(PreRankerTest, Smoke) fv.GetVector().ForEach([&](FeatureType & ft, uint32_t index) { FeatureID id(mwmId, index); ResultTracer::Provenance provenance; - preRanker.Emplace(id, PreRankingInfo(Model::TYPE_POI, TokenRange(0, 1)), provenance); + preRanker.Emplace(id, PreRankingInfo(Model::TYPE_SUBPOI, TokenRange(0, 1)), provenance); TEST_LESS(index, pois.size(), ()); distances[index] = mercator::DistanceOnEarth(feature::GetCenter(ft), kPivot); diff --git a/search/search_integration_tests/processor_test.cpp b/search/search_integration_tests/processor_test.cpp index 4e9a1a79aa..2b4dddd9ec 100644 --- a/search/search_integration_tests/processor_test.cpp +++ b/search/search_integration_tests/processor_test.cpp @@ -1519,22 +1519,22 @@ UNIT_CLASS_TEST(ProcessorTest, PathsThroughLayers) auto const rulePoi = ExactMatch(countryId, supervisedOffice); auto const ruleSubpoi = ExactMatch(countryId, svmCafe); - // SUBPOI-POI-BUILDING-STREET + // SUBPOI-COMPLEX_POI-BUILDING-STREET TEST(ResultsMatch("computing street 0 supervised cafe ", {ruleSubpoi}), ()); - // SUBPOI-BUILDING-STREET / POI-BUILDING-STREET + // SUBPOI-BUILDING-STREET / COMPLEX_POI-BUILDING-STREET TEST(ResultsMatch("computing street statistical learning cafe ", {ruleSubpoi, ruleStreet}), ()); TEST(ResultsMatch("computing street 0 cafe ", {ruleSubpoi}), ()); TEST(ResultsMatch("computing street statistical learning office ", {rulePoi, ruleStreet}), ()); TEST(ResultsMatch("computing street 0 office ", {rulePoi}), ()); - // POI-BUILDING / SUBPOI-BUILDING is not supported + // COMPLEX_POI-BUILDING / SUBPOI-BUILDING is not supported TEST(ResultsMatch("statistical learning cafe ", {}), ()); TEST(ResultsMatch("0 cafe ", {}), ()); TEST(ResultsMatch("statistical learning supervised ", {}), ()); TEST(ResultsMatch("0 office ", {}), ()); - // POI-STREET / SUBPOI - STREET + // COMPLEX_POI-STREET / SUBPOI - STREET TEST(ResultsMatch("computing street cafe ", {ruleSubpoi, ruleStreet}), ()); TEST(ResultsMatch("computing street office ", {rulePoi, ruleStreet}), ()); @@ -1542,11 +1542,11 @@ UNIT_CLASS_TEST(ProcessorTest, PathsThroughLayers) TEST(ResultsMatch("computing street statistical learning ", {ruleBuilding, ruleStreet}), ()); TEST(ResultsMatch("computing street 0 ", {ruleBuilding}), ()); - // POI / SUBPOI + // COMPLEX_POI / SUBPOI TEST(ResultsMatch("cafe ", {ruleSubpoi}), ()); TEST(ResultsMatch("office ", {rulePoi}), ()); - // POI-SUBPOI + // COMPLEX_POI-SUBPOI TEST(ResultsMatch("supervised cafe ", {ruleSubpoi}), ()); TEST(ResultsMatch("supervised svm ", {ruleSubpoi}), ()); diff --git a/search/search_quality/scoring_model.py b/search/search_quality/scoring_model.py index 9c17305e53..31ddf3223c 100755 --- a/search/search_quality/scoring_model.py +++ b/search/search_quality/scoring_model.py @@ -19,7 +19,7 @@ MAX_RANK = 255.0 MAX_POPULARITY = 255.0 RELEVANCES = {'Harmful': -3, 'Irrelevant': 0, 'Relevant': 1, 'Vital': 3} NAME_SCORES = ['Zero', 'Substring', 'Prefix', 'Full Match'] -SEARCH_TYPES = ['SUBPOI', 'POI', 'Building', 'Street', 'Unclassified', 'Village', 'City', 'State', 'Country'] +SEARCH_TYPES = ['SUBPOI', 'COMPLEX_POI', 'Building', 'Street', 'Unclassified', 'Village', 'City', 'State', 'Country'] RESULT_TYPES = ['TransportMajor', 'TransportLocal', 'Eat', 'Hotel', 'Attraction', 'Service', 'General'] FEATURES = ['DistanceToPivot', 'Rank', 'Popularity', 'Rating', 'FalseCats', 'ErrorsMade', 'MatchedFraction', 'AllTokensUsed', 'ExactCountryOrCapital'] + NAME_SCORES + SEARCH_TYPES + RESULT_TYPES @@ -55,11 +55,11 @@ def normalize_data(data): # Adds dummy variables to data for SEARCH_TYPES. - # We unify BUILDING with POI and SUBPOI here, as we don't have enough + # We unify BUILDING with COMPLEX_POI and SUBPOI here, as we don't have enough # training data to distinguish between them. Remove following # line as soon as the model will be changed or we will have enough # training data. - data['SearchType'] = data['SearchType'].apply(lambda v: v if v != 'Building' and v != 'POI' else 'SUBPOI') + data['SearchType'] = data['SearchType'].apply(lambda v: v if v != 'Building' and v != 'COMPLEX_POI' else 'SUBPOI') for st in SEARCH_TYPES: data[st] = data['SearchType'].apply(lambda v: int(st == v)) @@ -250,7 +250,7 @@ def show_bootstrap_statistics(clf, X, y, features): coefs[i].append(c) subpoi_index = features.index('SUBPOI') - poi_index = features.index('POI') + poi_index = features.index('COMPLEX_POI') building_index = features.index('Building') coefs[building_index] = coefs[subpoi_index] coefs[poi_index] = coefs[subpoi_index] @@ -310,7 +310,7 @@ def main(args): # Following code restores coeffs for merged features. ws[FEATURES.index('Building')] = ws[FEATURES.index('SUBPOI')] - ws[FEATURES.index('POI')] = ws[FEATURES.index('SUBPOI')] + ws[FEATURES.index('COMPLEX_POI')] = ws[FEATURES.index('SUBPOI')] ndcgs = compute_ndcgs_for_ws(data, ws) diff --git a/search/search_tests/ranking_tests.cpp b/search/search_tests/ranking_tests.cpp index 45be1a8090..48d2536285 100644 --- a/search/search_tests/ranking_tests.cpp +++ b/search/search_tests/ranking_tests.cpp @@ -82,9 +82,9 @@ UNIT_TEST(PreferCountry) auto cafe = info; cafe.m_distanceToPivot = 1e3; - cafe.m_tokenRanges[Model::TYPE_POI] = TokenRange(0, 1); + cafe.m_tokenRanges[Model::TYPE_SUBPOI] = TokenRange(0, 1); cafe.m_exactCountryOrCapital = false; - cafe.m_type = Model::TYPE_POI; + cafe.m_type = Model::TYPE_SUBPOI; auto country = info; country.m_distanceToPivot = 1e6;