Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Can/should we handle relations in diff conflate w/ tags? #3449

Closed
bwitham opened this issue Sep 6, 2019 · 4 comments
Closed

Can/should we handle relations in diff conflate w/ tags? #3449

bwitham opened this issue Sep 6, 2019 · 4 comments
Labels
Category: Core Priority: Medium Status: New/Undefined Type: Bug

Comments

@bwitham
Copy link
Contributor

bwitham commented Sep 6, 2019 •
edited
Loading

https://github.com/DigitalGlobe/VGI-team-repo/issues/1917#issuecomment-528388817

hootenanny/hoot-core/src/main/cpp/hoot/core/conflate/DiffConflator.cpp

Line 327 in faba854

else if (ElementType::Relation == c.getElement()->getElementType().getEnum())

@bwitham bwitham self-assigned this Oct 16, 2019
@bwitham
Copy link
Contributor Author

bwitham commented Oct 28, 2019

holding off on this until I can find a dataset that triggers the warning

@bwitham bwitham removed their assignment Oct 28, 2019
@bwitham
Copy link
Contributor Author

bwitham commented Oct 28, 2019

Found a relation to look at in the data from #3387.

@bwitham bwitham self-assigned this Oct 28, 2019
@bwitham
Copy link
Contributor Author

bwitham commented Oct 28, 2019

will do this in the 3387 branch, if it ends up being feasible for the examples in that input data.

@bwitham bwitham changed the title Can we handle relations in diff conflate? Can/should we handle relations in diff conflate? Oct 28, 2019
@bwitham bwitham changed the title Can/should we handle relations in diff conflate? Can/should we handle relations in diff conflate w/ tags? Oct 31, 2019
@bwitham
Copy link
Contributor Author

bwitham commented Oct 31, 2019

Maybe not good testing data after all, since all the relations I'm seeing are in the ref and not the sec, so they wouldn't make it to output anyway. Will close this out and re-open if we find a good use case to support doing it.

Its odd to me though that only ways and not nodes are handled in DiffConflator::addChangesToMap, but it may just be I don't understand all of the logic. That could be something else to look into eventually.

@bwitham bwitham closed this as completed Oct 31, 2019
@bwitham bwitham removed their assignment Oct 31, 2019
@bwitham bwitham mentioned this issue Oct 31, 2019
Merged
bwitham added a commit that referenced this issue Oct 31, 2019
@bwitham
3387 - Add option to diff conflate to keep reviews in output and more (…
dc87c60 
…#3588)

* Added an option, `differential.treat.reviews.as.matches`, that allows for not treating reviews as matches in Differential Conflation, which will let them pass to the diff output. The use case for this was a situation where a one to many POI review was preventing several secondary POIs from being added to the diff. Since some of these reviews were questionable, the longer term solution to this is some reworking of POI reviews (#3579). In the meantime, this config option may be useful.
* Disabled the removal/replacement of roundabouts during Differential Conflation. in #3580 there was a situation where roundabouts were being mangled badly during roundabout replacement. The original point of roundabout removal/replacement was to preserve them, since we don't conflate them very well some of the time. However with diff conflate, there's no chance to mangle them since we're only keeping non-matches in the secondary data and not actually trying to merge ref and secondary roundabouts...so removing the logic was the simplest solution.
* Downgraded warning logged when diff conflate w/ tags encounters a relation to a debug statement, since that is normal operation for now. #3449 can be re-opened to handle relations if need be.
* Fixed a NPE in the Network alg when running ref conflate with the same data from #3387
* Fixed issue where diff conflate case tests were not actually running diff conflate
* Fxied handling of output size limit in `MapComparator::_printIdDiff`
* Added in some utilities to `Roundabout`, `OsmUtils`, `Node`, and `Way` that make debugging roundabout conflate problems easier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Category: Core Priority: Medium Status: New/Undefined Type: Bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bwitham