Map op issues with conflate command

bwitham commented 7 years ago

blocks #1068

1) Running hoot conflate with the map crop op when prepended (or appended) to conflate.pre.ops and also used with --stats causes the cropper to remove all elements from the map

2) Running hoot conflate with the map crop op in unify.pre.ops causes the cropper to remove all elements from the map (with or without --stats)

bwitham commented 7 years ago

Actually got this to work with conflate.pre.ops, when I was originally trying it with unify.pre.ops and it didn't. Will turn the test script into a unit test to demonstrate.

brianhatchl commented 7 years ago

Nice.

brianhatchl commented 7 years ago

Hmm, my test is still resulting in an empty map and I think I have all the correct parameters.

brianhatchl commented 7 years ago

Attaching my command:

hoot conflate --debug -C RemoveReview2Pre.conf 
-D osm2ogr.ops=hoot::DecomposeBuildingRelationsVisitor 
-D conflate.add.score.tags=yes 
-D hootapi.db.writer.overwrite.map=true 
-D hootapi.db.writer.create.user=true 
-D hootapi.db.writer.email=test@test.com 
-D hootapi.db.reader.email=test@test.com 
-D "map.cleaner.transforms=hoot::ReprojectToPlanarOp;hoot::DuplicateWayRemover;hoot::SuperfluousWayRemover;hoot::IntersectionSplitter;hoot::UnlikelyIntersectionRemover;hoot::DualWaySplitter;hoot::ImpliedDividedMarker;hoot::DuplicateNameRemover;hoot::SmallWayMerger;hoot::RemoveEmptyAreasVisitor;hoot::RemoveDuplicateAreaVisitor;hoot::NoInformationElementRemover" 
-D "small.way.merger.threshold=15" 
-D "unify.optimizer.time.limit=30" 
-D "ogr.split.o2s=false" 
-D "ogr.tds.add.fcsubtype=true" 
-D "ogr.tds.structure=true" 
-D "duplicate.name.case.sensitive=true" 
-D "element.cache.size.node=2000000" 
-D "element.cache.size.relation=200000" 
-D "element.cache.size.way=200000" 
-D "conflate.match.highway.classifier=hoot::HighwayRfClassifier" 
-D "match.creators=hoot::HighwayMatchCreator;hoot::BuildingMatchCreator;hoot::ScriptMatchCreator,PoiGeneric.js;hoot::ScriptMatchCreator,LinearWaterway.js" 
-D "merger.creators=hoot::HighwaySnapMergerCreator;hoot::BuildingMergerCreator;hoot::ScriptMergerCreator" 
-D "search.radius.highway=-1" 
-D "highway.matcher.heading.delta=5.0" 
-D "highway.matcher.max.angle=60" 
-D "way.merger.min.split.size=5" 
-D "conflate.enable.old.roads=false" 
-D "way.subline.matcher=hoot::MaximalNearestSublineMatcher" 
-D "waterway.angle.sample.distance=20.0" 
-D "waterway.matcher.heading.delta=150.0" 
-D "waterway.auto.calc.search.radius=true" 
-D "search.radius.waterway=-1" 
-D "waterway.rubber.sheet.minimum.ties=5" 
-D "waterway.rubber.sheet.ref=true" 
-D "writer.include.debug=false" 
-D convert.bounding.box="-72.4987792839,18.5369085571,-72.4932861198,18.542116671039192" 
-D conflate.use.data.source.ids=true 
-D osm.map.reader.factory.reader=hoot::OsmApiDbAwareHootApiDbReader 
-D osm.map.writer.factory.writer=hoot::OsmApiDbAwareHootApiDbWriter 
-D osmapidb.id.aware.url="osmapidb://vagrant:97iyawbaonaj1TR8@192.168.33.32:5432/openstreetmap" 
-D conflate.pre.ops=hoot::MapCropper 
-D crop.bounds="-72.4987792839,18.5369085571,-72.4932861198,18.542116671039192" 
"osmapidb://vagrant:97iyawbaonaj1TR8@192.168.33.32:5432/openstreetmap" 
"hootapidb://hoot:hoottest@localhost:5432/hoot/PapMgcpClip" 
"hootapidb://hoot:hoottest@localhost:5432/hoot/tmtest7" 
--stats tmtest7.stats

bwitham commented 7 years ago

Ok, I'll try that command out tomorrow.

brianhatchl commented 7 years ago

Here's the output log of another run:

19:08:15.464 INFO  ...pp/hoot/core/cmd/ConflateCmd.h( 127) Conflating osmapidb://vagrant:97iyawbaonaj1TR8@192.168.33.32:5432/openstreetmap with hootapidb://hoot:hoottest@localhost:5432/hoot/PapMgcpClip and writing the output to hootapidb://hoot:hoottest@localhost:5432/hoot/tmtest8
19:08:15.464 INFO  ...pp/hoot/core/cmd/ConflateCmd.h( 130) bytesRead: 116310
19:08:15.465 INFO  .../hoot/core/cmd/BaseCommand.cpp(  78) Loading map data from osmapidb://vagrant:97iyawbaonaj1TR8@192.168.33.32:5432/openstreetmap ...
19:08:15.486 INFO  ...oot/core/io/OsmApiDbReader.cpp( 123) Executing OSM API bounded read query against all element types with bounds -72.4987792839,18.5369085571,-72.4932861198,18.542116671039192...
19:12:23.031 INFO  ...p/hoot/core/ops/MapCropper.cpp( 117) Cropping map.
19:12:23.147 INFO  ...p/hoot/core/ops/MapCropper.cpp( 156)   Removing nodes...
19:12:23.150 INFO  .../hoot/core/cmd/BaseCommand.cpp(  78) Loading map data from hootapidb://hoot:hoottest@localhost:5432/hoot/PapMgcpClip ...
19:12:23.722 INFO  ...pp/hoot/core/cmd/ConflateCmd.h( 143) inputBytes: 400760
19:12:23.722 INFO  .../core/ops/CalculateStatsOp.cpp( 120) Calculating map statistics for input map 1...
Reprojecting 1000 / 1177         
19:12:23.894 INFO  .../hoot/rules/LinearWaterway.js"(  33) "Automatically calculating search radius..."
19:12:23.903 WARN  .../core/conflate/NodeMatcher.cpp(  99) nid: 614348193
19:12:23.903 WARN  .../core/conflate/NodeMatcher.cpp( 100) Node(614348193): x: -726.453 y: 174.474 tags:

 version: 2
 visible: 1
19:12:23.903 WARN  .../core/conflate/NodeMatcher.cpp( 101) wids: [2]{-7, -2}
19:12:23.903 WARN  .../core/conflate/NodeMatcher.cpp( 104) way(-7)
nodes: [8]{-12, 614348189, 614348190, 620012654, 614348191, 614348192, 620012668, 614348193}
tags: operational_status_quality = confirmed
operational_status = open
practicability = 4wd_less_than_3.5mt
source = CNIGS/OIM/FOCS
name = Route ti Goave
highway = unclassified
surface = mud
cached envelope: 0,-1,0,-1
status: unknown1
version: 0
visible: 1
19:12:23.903 WARN  .../core/conflate/NodeMatcher.cpp( 104) way(-2)
nodes: [9]{-2, 620012678, 614348194, 614348193, 615534685, 614348204, 615534684, 614348203, -3}
tags: operational_status_quality = confirmed
operational_status = open
practicability = heavy_truck_less_than_20mt
source = CNIGS/OIM/FOCS
name = 1ère-Ruelle St Antoine
smoothness = bad
highway = residential
surface = dirt
cached envelope: 0,-1,0,-1
status: unknown1
version: 0
visible: 1
19:12:23.904 INFO  ...ate/SearchRadiusCalculator.cpp( 113) An error occurred calculating the rubber sheet transform during automatic search radius calculation.  Cleaning the data and attempting to calculate the transform again...
19:12:23.904 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::ReprojectToPlanarOp
19:12:23.904 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::DuplicateWayRemover
19:12:23.905 INFO  ...nflate/DuplicateWayRemover.cpp(  65) Removing duplicate ways...
19:12:23.911 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::SuperfluousWayRemover
19:12:23.911 INFO  ...late/SuperfluousWayRemover.cpp(  60) Removing superfluous ways...
19:12:23.912 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::IntersectionSplitter
19:12:23.912 INFO  ...itter/IntersectionSplitter.cpp( 131) Splitting intersections...
  Intersection splitter todo: 0       
19:12:23.915 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::UnlikelyIntersectionRemover
19:12:23.915 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::DualWaySplitter
19:12:23.915 INFO  ...e/splitter/DualWaySplitter.cpp(  86) Assuming drives on right.
19:12:23.915 INFO  ...e/splitter/DualWaySplitter.cpp( 249) Splitting divided ways...

19:12:23.915 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::ImpliedDividedMarker
19:12:23.919 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::DuplicateNameRemover
19:12:23.920 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::SmallWayMerger
19:12:23.924 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  74) Applying visitor: hoot::RemoveEmptyAreasVisitor
19:12:23.929 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  74) Applying visitor: hoot::RemoveDuplicateAreaVisitor
19:12:23.935 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::NoInformationElementRemover
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 188) nid1: 614348193
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 189) nid2: 4419195785
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 190) distanceScore: 0.895548
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 191) acc: 28.65
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 192) d: 5.64209
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 193) Normal::phi(d, acc * 1.5): 0.552226
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 194) Normal::phi(d, acc / 2.0): 0.653159
19:12:23.942 INFO  .../core/conflate/NodeMatcher.cpp( 231) result: 1.34103
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 188) nid1: 620012668
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 189) nid2: 4419195785
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 190) distanceScore: 0.183319
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 191) acc: 28.65
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 192) d: 57.1828
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 193) Normal::phi(d, acc * 1.5): 0.908341
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 194) Normal::phi(d, acc / 2.0): 0.999967
19:12:23.943 INFO  .../core/conflate/NodeMatcher.cpp( 231) result: 0.0078073
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 188) nid1: 4419195785
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 189) nid2: 614348193
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 190) distanceScore: 0.895548
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 191) acc: 28.65
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 192) d: 5.64209
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 193) Normal::phi(d, acc * 1.5): 0.552226
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 194) Normal::phi(d, acc / 2.0): 0.653159
19:12:23.944 INFO  .../core/conflate/NodeMatcher.cpp( 231) result: 1.34103
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 188) nid1: 4419195785
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 189) nid2: 620012668
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 190) distanceScore: 0.183319
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 191) acc: 28.65
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 192) d: 57.1828
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 193) Normal::phi(d, acc * 1.5): 0.908341
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 194) Normal::phi(d, acc / 2.0): 0.999967
19:12:23.945 INFO  .../core/conflate/NodeMatcher.cpp( 231) result: 0.0078073
19:12:23.945 INFO  .../core/conflate/RubberSheet.cpp( 326) Found 2 potential ties.
19:12:23.945 INFO  .../core/conflate/RubberSheet.cpp( 364) Found 1 confident ties.
19:12:23.945 INFO  ...ate/SearchRadiusCalculator.cpp( 153) Unable to automatically calculate search radius.  Not enough tie points.  Using default search radius value = 15
19:12:24.895 INFO  .../core/ops/CalculateStatsOp.cpp( 365) Map statistics calculated for input map 1.
19:12:24.895 INFO  .../core/ops/CalculateStatsOp.cpp( 120) Calculating map statistics for input map 2...
19:12:25.712 INFO  .../core/ops/CalculateStatsOp.cpp( 365) Map statistics calculated for input map 2.
19:12:25.712 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::MapCropper
19:12:25.712 INFO  ...p/hoot/core/ops/MapCropper.cpp( 117) Cropping map.
19:12:25.714 INFO  ...p/hoot/core/ops/MapCropper.cpp( 156)   Removing nodes...
19:12:25.715 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  74) Applying visitor: hoot::RemoveElementsVisitor
19:12:25.715 INFO  ...ighway/HighwayMatchCreator.cpp(  96) neighbor counts, max: -1 mean: -nan
19:12:25.715 INFO  ...lygon/BuildingMatchCreator.cpp( 250) *threshold: Thresholds; Match: 0.6 Miss: 0.6 Review: 0.6
19:12:25.716 INFO  ...lygon/BuildingMatchCreator.cpp( 269) Loading model from: /home/vagrant/hoot/conf/BuildingModel.rf
19:12:26.163 INFO  .../hoot/rules/LinearWaterway.js"(  33) "Automatically calculating search radius..."
19:12:26.163 INFO  ...ate/SearchRadiusCalculator.cpp(  96) Unable to automatically calculate search radius.  All features have already been conflated or are invalid.
 Using default search radius value = 15
19:12:26.164 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::SuperfluousNodeRemover
19:12:26.165 INFO  ...ops/SuperfluousNodeRemover.cpp(  54) Removing superfluous nodes...

19:12:26.165 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::SmallWayMerger
19:12:26.165 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  74) Applying visitor: hoot::RemoveMissingElementsVisitor
19:12:26.165 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  74) Applying visitor: hoot::RemoveEmptyReviewRelationsVisitor
19:12:26.165 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::RemoveDuplicateReviewsOp
19:12:26.165 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::BuildingOutlineUpdateOp
19:12:26.165 INFO  .../cpp/hoot/core/ops/NamedOp.cpp(  56) Applying operation: hoot::AddHilbertReviewSortOrderOp

19:12:26.261 INFO  .../core/ops/CalculateStatsOp.cpp( 120) Calculating map statistics for output map...

19:12:27.333 INFO  .../core/ops/CalculateStatsOp.cpp( 365) Map statistics calculated for output map.
stats = (stat) OR (input map 1 stat) (input map 2 stat) (output map stat) in file: tmtest8.stats

bwitham commented 7 years ago

Ok, its the --stats added to 'hoot conflate' causing the problem. Added that to ServiceOsmApiDbHootApiDbConflateCropTest.sh and got empty conflated output...removed it and got the correct conflated output extent. Will see if I can figure out why.

bwitham commented 7 years ago

Figured out that when using --stats the conflated database output table is completely empty. Also, eliminated osm api db data having anything to do with it. Made a new test script which just conflates two hoot api db layers and see the same behavior, an empty output table with --stats. When removing --stats, I see the conflated output data in the hoot api db as expected.

This is very strange to me, b/c I thought we were adding the --stats tag each time the UI launched a conflate job with stats enabled...confused. Will try to figure out why --stats causes empty conflate db table output.

bwitham commented 7 years ago

Well, at least verified that hoot conflate works with --stats for file inputs. There's a starting point.

bwitham commented 7 years ago

Ok, another piece of the puzzle...one that makes a little more sense...when removing the crop pre op and leaving the stats in, conflated output is present in the db. Additionally, when removing the stats option and leaving the crop pre op in, output is also as expected. So, the bug is related to the combination of the crop pre op with the stats option when conflating. Using either by themselves with hoot conflate yields output as expected; using both together kills the output. This behavior is the same for file inputs as it is for db inputs.

bwitham commented 7 years ago

I may have a fix for this by running the pre ops before the beginning of stats calculation....will see how many tests that breaks...

bwitham commented 7 years ago

That change breaks every stats test. This is going to be tough, since moving the pre ops affects the stats collected in that previously stats were calculated on the raw input before any map cleaning and after this change then the stats would be calculated after map cleaning. I'm not sure if we want that...need to think about this some more...

Actually, the change doesn't make much sense the more I think about it anyway...

bwitham commented 7 years ago

One thing that I was doing wrong, which wasn't the cause of this problem, was dropping the other ops from conflate.pre.ops by accident....corrected that.

Anyway, there has to be some kind of weird pointer issue here...when stats is off, cropping works as expected; when stats is on, cropping removes all elements from the map to be conflated.

And as a side note, the unify.pre.ops problem needs to be fixed as well.

bwitham commented 7 years ago

My gut is telling me this could be a projection issue...will see.

bwitham commented 7 years ago

And it does seem to be. Reprojecting inside the cropper fixes this...just need to figure out how to make the workflow right projection-wise for this sequence.

bwitham commented 7 years ago

Going to get around this by doing the cropping in a separate config option just after the data is loaded...that's the best time to do it anyway.

bwitham commented 7 years ago

@brianhatchl Another try at fixing...think I have it this time. Refactored some things to make the cropping work for this workflow. See the latest changes in ServiceOsmApiDbHootApiDbConflateCropTest.sh on the 1076 branch.

Longer term, the better solution here is to add a bounds query capability to hoot core for hoot api db data like we have for osm api db data (services has it for the hoot api db already). Most of the code exists for that change, just a matter of refactoring. That will prevent the reading of unnecessary data....will create a new issue for it.

bwitham commented 7 years ago

Summarizing a conversation from a few moments ago:

still an issue with the cropping...features outside of the bounds are showing up in the cropped data and shouldn't be; going to get a VM from Brian to troubleshoot
line splitting is going on that shouldn't be; #772 should fix this (?)
possibly investigate better visualization for features pulled over from osm api db that don't exist in the hoot dataset that's its being conflated with and just pass through (won't be written back to ME in the final changeset)
decided that pre-filtering on conflatable feature type for data coming from ME isn't really needed b/c iD has capabilities to turn layers selectively off in the view after conflation, if needed; pre-filtering wouldn't help conflation performance much, since unconflatable feature types are ignored; it could help osm api db query performance, but by using HOT we're keeping AOI's pretty small to begin with (hopefully), so probably not an issue there either.

ngageoint / hootenanny

Map op issues with conflate command #1076