Add option for allowing POI/Poly cross conflation (#3186)

* Helps solve some of the Building Attribute Conflation problems identified in https://github.com/DigitalGlobe/VGI-team-repo/issues/1846
* Added option poi.polygon.allow.cross.conflation.merging, which allows for merging matches that occur with both POI/Polygon Conflation and some other type of conflation (e.g. Building Conflation). The option is enabled for Attribute Conflation only. The default behavior in hoot is for a review to occur in this situation instead.
* The new config option could also possibly be made to work with conflicts between POI/Poly Conflation and Area Conflation as well, but holding off on that change until there's a need for it.
* Fixed bug with PreserveTypesTagMerger where types were duplicated in the alt_types tag

Author: bwitham

conf/core/AttributeConflation.conf

@@ -11,6 +11,7 @@
   "corner.splitter.rounded.split": "true",
   "duplicate.name.preserve.original.name": "true",
   "highway.merge.tags.only": "true",
+  "poi.polygon.allow.cross.conflation.merging": "true",
   "poi.polygon.auto.merge.many.poi.to.one.poly.matches": "true",
   "reader.conflate.use.data.source.ids.1": "false",
   "remove.elements.visitor.chain.element.criteria": "false",

conf/core/ConfigOptions.asciidoc

@@ -2935,6 +2935,15 @@ is enabled, POI/Polygon conflation will use additional to English language trans
 address normalization.  Disable this only if libpostal's language translation is not adequate for
 your language translation needs.
 
+=== poi.polygon.allow.cross.conflation.merging
+
+* Data Type: bool
+* Default Value: `false`
+
+If false, when a match found by Building Conflation involves the same feature in a match found by
+POI to Polygon Conflation, a review will be generated. If true, then that situation will result in
+a merge between all features involved in the Building and POI to Polygon conflation matches.
+
 === poi.polygon.auto.merge.many.poi.to.one.poly.matches
 
 * Data Type: bool

conf/services/conflateAdvOps.json

@@ -750,12 +750,30 @@
                 "onchange":"true"
             },
             {
-            	"id":"building_review_if_secondary_newer",
-                "elem_type":"bool",
-                "defaultvalue":"false",
-                "description":"If true, any buildings in the secondary layer will be automatically reviewed against potentially matching features in the reference layer if they are marked with a more recent date than that of the reference feature.",
-                "name":"Review If Secondary Layer Building Newer",
-                "hoot_key":"building.review.if.secondary.newer"
+                "id":"building_match_creator",
+                "elem_type":"string",
+                "description":"List of MatchCreators to use during conflation. This can modify what features will be conflated (e.g. buildings, roads, etc.).",
+                "name":"Match Creator",
+                "required":"true",
+                "defaultvalue":"hoot::BuildingMatchCreator",
+                "override":{
+                    "defaultvalue":"hoot::BuildingMatchCreator"
+                },
+                "hoot_key":"match.creators",
+                "required":"true"
+            },
+            {
+                "id":"building_merger_creator",
+                "elem_type":"string",
+                "description":"List of MergerCreators to use during conflation. This can modify what features will be conflated (e.g. buildings, roads, etc.).",
+                "name":"Merger Creator",
+                "required":"true",
+                "defaultvalue":"hoot::BuildingMergerCreator",
+                "override":{
+                    "defaultvalue":"hoot::BuildingMergerCreator"
+                },
+                "hoot_key":"merger.creators",
+                "required":"true"
             },
             {
         	"id":"building_date_format",
@@ -784,30 +802,28 @@
                 "hoot_key":"building.keep.more.complex.geometry.when.auto.merging"
             },
             {
-                "id":"building_match_creator",
-                "elem_type":"string",
-                "description":"List of MatchCreators to use during conflation. This can modify what features will be conflated (e.g. buildings, roads, etc.).",
-                "name":"Match Creator",
-                "required":"true",
-                "defaultvalue":"hoot::BuildingMatchCreator",
-                "override":{
-                    "defaultvalue":"hoot::BuildingMatchCreator"
-                },
-                "hoot_key":"match.creators",
-                "required":"true"
+            	"id":"building_merge_many_to_many_matches",
+                "elem_type":"bool",
+                "defaultvalue":"false",
+                "description":"If false, many to many building matches will result in a review. If true, they will all be merged together when matched.",
+                "name":"Merge Many to Many Matches",
+                "hoot_key":"building.merge.many.to.many.matches"
             },
             {
-                "id":"building_merger_creator",
-                "elem_type":"string",
-                "description":"List of MergerCreators to use during conflation. This can modify what features will be conflated (e.g. buildings, roads, etc.).",
-                "name":"Merger Creator",
-                "required":"true",
-                "defaultvalue":"hoot::BuildingMergerCreator",
-                "override":{
-                    "defaultvalue":"hoot::BuildingMergerCreator"
-                },
-                "hoot_key":"merger.creators",
-                "required":"true"
+            	"id":"building_outline_update_op_remove_building_relations",
+                "elem_type":"bool",
+                "defaultvalue":"false",
+                "description":"If true, when building outlines are updated the updater will remove the source building relations used to create the outline multipolygon relation.  If false, both source building relations and outline multipolygon relations will remain.",
+                "name":"Remove Building Relations",
+                "hoot_key":"building.outline.update.op.remove.building.relations"
+            },
+            {
+            	"id":"building_review_if_secondary_newer",
+                "elem_type":"bool",
+                "defaultvalue":"false",
+                "description":"If true, any buildings in the secondary layer will be automatically reviewed against potentially matching features in the reference layer if they are marked with a more recent date than that of the reference feature.",
+                "name":"Review If Secondary Layer Building Newer",
+                "hoot_key":"building.review.if.secondary.newer"
             },
             {
             	"id":"building_review_matches_other_than_one_to_one",
@@ -914,6 +930,14 @@
         	"hoot_key":"address.additional.tag.keys",
         	"onchange":"true"
     	    },
+            {
+        	"id":"poipolygon_address_allow_lenient_house_number_matching",
+        	"elem_type":"bool",
+        	"defaultvalue":"true",
+        	"description":"If true, POI/Polygon conflation will allow house number subletter mismatches for the addresses with the same house number.  e.g. when enabled, 23a Elm Street matches 23 Elm Street",
+        	"name":"Allow Lenient House Number Matching",
+        	"hoot_key":"poi.polygon.address.allow.lenient.house.number.matching"
+    	    },
             {
         	"id":"poipolygon_address_match_enabled",
         	"elem_type":"bool",
@@ -922,6 +946,14 @@
         	"name":"Address Match Enabled",
         	"hoot_key":"poi.polygon.address.match.enabled"
     	    },
+            {
+        	"id":"poipolygon_allow_cross_conflation_merging",
+        	"elem_type":"bool",
+        	"defaultvalue":"false",
+        	"description":"If false, when a match found by Building Conflation involves the same feature in a match found by POI to Polygon Conflation, a review will be generated. If true, then that situation will result in a merge between all features involved in the Building and POI to Polygon conflation matches.",
+        	"name":"Allow Cross Conflation Merging",
+        	"hoot_key":"poi.polygon.allow.cross.conflation.merging"
+    	    },
             {
         	"id":"poipolygon_auto_merge_many_poi_to_one_poly_matches",
         	"elem_type":"bool",

hoot-core-test/src/test/cpp/hoot/core/conflate/poi-polygon/PoiPolygonMatchCreatorTest.cpp

@@ -94,10 +94,12 @@ class PoiPolygonMatchCreatorTest : public HootTestFixture
       std::shared_ptr<const MatchThreshold> threshold(new MatchThreshold(0.5, 0.5, 0.5));
       uut.createMatches(map, matches, threshold);
       HOOT_STR_EQUALS(2, matches.size());
-      HOOT_STR_EQUALS("PoiPolygonMatch Node(1) Way(-1) P: match: 1 miss: 0 review: 0, distance: 0, close match: 1, type score: 0, name score: 1, address score: -1",
-                      matches[0]->toString());
-      HOOT_STR_EQUALS("PoiPolygonMatch Node(2) Way(-1) P: match: 0 miss: 0 review: 1, distance: 0, close match: 1, type score: 0, name score: 0, address score: 0",
-                      matches[1]->toString());
+      HOOT_STR_EQUALS(
+        "PoiPolygonMatch: POI: Node(1), Poly: Way(-1), P: match: 1 miss: 0 review: 0, distance: 0, close match: 1, type score: 0, name score: 1, address score: -1",
+        matches[0]->toString());
+      HOOT_STR_EQUALS(
+        "PoiPolygonMatch: POI: Node(2), Poly: Way(-1), P: match: 0 miss: 0 review: 1, distance: 0, close match: 1, type score: 0, name score: 0, address score: 0",
+        matches[1]->toString());
     }
   }
 

hoot-core-test/src/test/cpp/hoot/core/conflate/poi-polygon/PoiPolygonMergerCreatorTest.cpp

@@ -33,6 +33,8 @@
 #include <hoot/core/conflate/poi-polygon/PoiPolygonMerger.h>
 #include <hoot/core/conflate/poi-polygon/PoiPolygonMergerCreator.h>
 #include <hoot/core/conflate/polygon/BuildingMatchCreator.h>
+#include <hoot/core/conflate/polygon/BuildingMatch.h>
+#include <hoot/core/conflate/polygon/BuildingRfClassifier.h>
 #include <hoot/core/io/OsmJsonWriter.h>
 #include <hoot/core/util/ConfigOptions.h>
 #include <hoot/core/util/Log.h>
@@ -48,6 +50,7 @@ class PoiPolygonMergerCreatorTest : public HootTestFixture
   CPPUNIT_TEST_SUITE(PoiPolygonMergerCreatorTest);
   CPPUNIT_TEST(basicTest);
   CPPUNIT_TEST(reviewTest);
+  CPPUNIT_TEST(crossConflateMergeTest);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -70,11 +73,10 @@ class PoiPolygonMergerCreatorTest : public HootTestFixture
     conf().set(ConfigOptions().getMergerCreatorsKey(), ConfigOptions().getMergerCreatorsDefaultValue());
   }
 
-  /**
-   * Creates a single match and should result in a PoiPolygonMerger
-   */
   void basicTest()
   {
+    // Creates a single match and should result in a PoiPolygonMerger
+
     OsmMapPtr map(new OsmMap());
 
     Coordinate c1[] = { Coordinate(0.0, 0.0), Coordinate(20.0, 0.0),
@@ -106,11 +108,11 @@ class PoiPolygonMergerCreatorTest : public HootTestFixture
     HOOT_STR_EQUALS(1, (dynamic_cast<PoiPolygonMerger*>(mergers[0]) != 0));
   }
 
-  /**
-   * Creates two matches with overlap and should create a MarkForReviewMerger
-   */
   void reviewTest()
   {
+    // Create a building and poi/poly match with feature overlap and ensure they create reviews and
+    // don't merge together when cross feature conflate merging is not allowed.
+
     OsmMapPtr map(new OsmMap());
 
     Coordinate c1[] = { Coordinate(0.0, 0.0), Coordinate(20.0, 0.0),
@@ -151,18 +153,55 @@ class PoiPolygonMergerCreatorTest : public HootTestFixture
     match2.setMatchEvidenceThreshold(3);
     match2.setReviewEvidenceThreshold(1);
     match2.calculateMatch(w2->getElementId(), n1->getElementId());
+    matchesV.push_back(&match2);
     LOG_VAR(match2);
 
     MatchSet matches;
     matches.insert(matchesV.begin(), matchesV.end());
     vector<Merger*> mergers;
     PoiPolygonMergerCreator uut;
     uut.setOsmMap(map.get());
+
     HOOT_STR_EQUALS(1, uut.createMergers(matches, mergers));
     HOOT_STR_EQUALS(1, mergers.size());
-    LOG_VAR(*mergers[0]);
+    LOG_VART(*mergers[0]);
     HOOT_STR_EQUALS(1, (dynamic_cast<MarkForReviewMerger*>(mergers[0]) != 0));
   }
+
+  void crossConflateMergeTest()
+  {
+    // Create a building and poi/poly match with feature overlap and ensure they all merge together
+    // when cross feature conflate merging is allowed.
+
+    vector<const Match*> matchesV;
+
+    BuildingMatch match1(std::shared_ptr<const MatchThreshold>(new MatchThreshold(0.5, 0.5, 0.5)));
+    match1._p.setMatch();
+    match1._eid1 = ElementId(ElementType::Way, 1);
+    match1._eid2 = ElementId(ElementType::Node, 1);
+    matchesV.push_back(&match1);
+
+    PoiPolygonMatch match2(std::shared_ptr<const MatchThreshold>(new MatchThreshold(0.6, 0.6, 0.6)));
+    match2._class.setMatch();
+    match2._eid1 = ElementId(ElementType::Way, 2);
+    match2._eid2 = ElementId(ElementType::Node, 1);
+    matchesV.push_back(&match2);
+
+    MatchSet matches;
+    matches.insert(matchesV.begin(), matchesV.end());
+    vector<Merger*> mergers;
+    PoiPolygonMergerCreator uut;
+    // Neither of the match types used here actually require a map to calculate isConflicting, but
+    // since the merger creator requires a map we'll pass in an empty one.
+    OsmMapPtr emptyMap(new OsmMap());
+    uut.setOsmMap(emptyMap.get());
+    uut.setAllowCrossConflationMerging(true);
+
+    HOOT_STR_EQUALS(1, uut.createMergers(matches, mergers));
+    HOOT_STR_EQUALS(1, mergers.size());
+    LOG_VART(*mergers[0]);
+    HOOT_STR_EQUALS(1, (dynamic_cast<PoiPolygonMerger*>(mergers[0]) != 0));
+  }
 };
 
 CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(PoiPolygonMergerCreatorTest, "quick");

hoot-core/src/main/cpp/hoot/core/conflate/RfExtractorClassifier.cpp

@@ -69,6 +69,7 @@ MatchClassification RfExtractorClassifier::classify(const ConstOsmMapPtr& map,
   result.setMatchP(scores["match"]);
   result.setReviewP(scores["review"]);
 
+  LOG_VART(result);
   return result;
 }
 
@@ -79,7 +80,6 @@ const vector<std::shared_ptr<const FeatureExtractor>>& RfExtractorClassifier::_g
   {
     _createExtractors();
   }
-
   return _extractors;
 }
 
@@ -94,7 +94,7 @@ map<QString, double> RfExtractorClassifier::getFeatures(const ConstOsmMapPtr& m,
   _getExtractors();
   for (size_t i = 0; i < _extractors.size(); i++)
   {
-    double v = _extractors[i]->extract(*m, e1, e2);
+    const double v = _extractors[i]->extract(*m, e1, e2);
     // if it isn't null then include it.
     if (!FeatureExtractor::isNull(v))
     {

hoot-core/src/main/cpp/hoot/core/conflate/merging/MarkForReviewMerger.cpp

@@ -73,8 +73,8 @@ void MarkForReviewMerger::apply(const OsmMapPtr& map,
   }
   else
   {
-    for (set<pair<ElementId, ElementId>>::const_iterator it = _pairs.begin();
-      it != _pairs.end(); ++it)
+    for (set<pair<ElementId, ElementId>>::const_iterator it = _pairs.begin(); it != _pairs.end();
+         ++it)
     {
       ElementId eid1 = it->first;
       ElementId eid2 = it->second;
@@ -152,7 +152,6 @@ void MarkForReviewMerger::replace(ElementId oldEid, ElementId newEid)
 
 QString MarkForReviewMerger::toString() const
 {
-  //return QString("MarkForReviewMerger");
   return QString("MarkForReviewMerger, pairs: ") + hoot::toString(_pairs);
 }
 

hoot-core/src/main/cpp/hoot/core/conflate/merging/MergerCreator.h

@@ -43,6 +43,9 @@ namespace hoot
 class Match;
 class Merger;
 
+/**
+ * Abstract base class for creating conflation feature mergers
+ */
 class MergerCreator
 {
 public:

hoot-core/src/main/cpp/hoot/core/conflate/merging/MergerFactory.cpp

@@ -30,11 +30,9 @@
 #include <hoot/core/util/Factory.h>
 #include <hoot/core/elements/OsmMapConsumer.h>
 #include <hoot/core/conflate/matching/Match.h>
-#include <hoot/core/conflate/polygon/BuildingMergerCreator.h>
 #include <hoot/core/util/ConfigOptions.h>
 #include <hoot/core/util/HootException.h>
 #include <hoot/core/util/Log.h>
-#include <hoot/core/conflate/matching/MatchGraph.h>
 
 using namespace std;
 
@@ -72,7 +70,6 @@ void MergerFactory::createMergers(const OsmMapPtr& map, const MatchSet& matches,
     PROGRESS_DEBUG("Creating merger " << i + 1 << " / " << _creators.size() << "...");
 
     OsmMapConsumer* omc = dynamic_cast<OsmMapConsumer*>(_creators[i]);
-
     if (omc)
     {
       omc->setOsmMap(map.get());
@@ -81,7 +78,6 @@ void MergerFactory::createMergers(const OsmMapPtr& map, const MatchSet& matches,
     {
       return;
     }
-
     // we don't want the creators to hold onto a map pointer that will go out of scope
     if (omc)
     {
@@ -141,8 +137,8 @@ MergerFactory& MergerFactory::getInstance()
   return *_theInstance;
 }
 
-bool MergerFactory::isConflicting(const ConstOsmMapPtr& map, const Match* m1, const Match* m2)
-  const
+bool MergerFactory::isConflicting(const ConstOsmMapPtr& map, const Match* m1,
+                                  const Match* m2) const
 {
   LOG_VART(_creators.size());
   // if any creator considers a match conflicting then it is a conflict

hoot-core/src/main/cpp/hoot/core/conflate/network/NetworkDetails.cpp

@@ -768,7 +768,7 @@ double NetworkDetails::getPartialEdgeMatchScore(ConstNetworkEdgePtr e1, ConstNet
     {
       if (logWarnCount < Log::getWarnMessageLimit())
       {
-        LOG_WARN("Unable to retieve partial match score.  One or more ways is null.");
+        LOG_WARN("Unable to retrieve partial match score.  One or more ways is null.");
         if (!w1.get())
         {
           LOG_DEBUG("Way 1 is null.");