Invalid Access Tag Check #46

Bentleysb · 2018-05-23T18:27:35Z

This check flags Edges and Lines where the access tag is invalid due to an improper use or lack of supporting tags.

To be flagged all of the following must be true:

A highway tag is present and above a minimum priority
An access tag is present with a value of no
There are no supporting tags for access=no, such as public_transport=yes
The Edge or Line is not in an Area or Relation with a landuse=military or military=* tag

This PR includes flag logic, configuration, unit tests, and documentation.

OSM Example:
Way id:28957025 has the access=no tag without any supporting tags, such as public_transport=yes.

…Fixed )'s in validCheckForObject.

…g names, changed congig defaults to use tag classes, refactored Math.abs() to Edge.isMasterEdgeId().

…fig minimum.highway.type to residential

…linear to checks.validation.tag, updated javadocs.

… moved boolean checks around

…ags.

mertemin · 2018-06-01T17:26:39Z

src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java

+    {
+        for (final Area area : object.getAtlas()
+                .areas(area -> Validators.isOfType(area, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(area, MilitaryTag.class)))


This method is iterating over all the areas. Please use a method like areasIntersecting that would limit the search.

The issue with using areasIntersecting is that it requires a polygon, and this check is working with LineItems. As far as I am aware, LineItems can be converted to polygons by either using their bounding box, or by connecting there first and last nodes. If this was done, false negatives could result if a road that would be flagged was near a military tagged area.
areasCovering could be used, but would require iteration over all the Locations in the polyline of a LineItem. That is the best option I can come up with. I would love to hear if there is an even better alternative that you can think of.

LineItem has a method called bounds, which will give you a Polygon. You can retrieve areas intersecting that and filter them down. e.g. atlas.areasIntersecting(line.bounds(), area -> (line.intersects(areaPolygon) || areaPolygon.fullyGeometricallyEncloses(line.asPolyLine())) && (Validators.isOfType(area, LandUseTag.class, LandUseTag.MILITARY) || Validators.hasValuesFor(area, MilitaryTag.class))).

You should alsoe cache the result of object.asPolyLine(), because that method would be returning a new PolyLine object per call.

mertemin · 2018-06-01T17:26:58Z

src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java

+        for (final Relation relation : object.getAtlas().relations(
+                relation -> (Validators.isOfType(relation, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(relation, MilitaryTag.class))
+                        && relation.isMultiPolygon()))


Again this method is iterating over all the relations.

This, unfortunately, does not seem to have as easy a fix as with areas. There does not appear to be an equivalent to areasCovering for relations. The only thing I could find was relationsWithEntitiesIntersecting. Using this would create false positive flags for anything that was completely contained in a relation, if I am not mistaken. I do not see an alternative to iterating over all relations, but would love to consider anything you might be able to come up with. I only need to test multipolygon relations for the intersection or containment of the current check object.

See my other comment please. Retrieve relations intersecting line bounds and then filter them down.

Thanks for the guidance. Your solution for areas is great. Unfortunately, it does not work in the case of relations. Using relationsWithEntitiesIntersecting will create false positive flags for items with bounds that are completely contained by the relation.

Example:
https://www.openstreetmap.org/way/251110595 contained by
https://www.openstreetmap.org/relation/7437523

Iterating over each and every relation should be avoided.

You can further filter relations returned from relationsWithEntitiesIntersecting method. That method accepts a second optional parameter: Predicate<Relation>. Something like:

object.getAtlas().relationsWithEntitiesIntersecting(object.bounds(), relation -> (Validators.isOfType(relation, LandUseTag.class, LandUseTag.MILITARY) || Validators.hasValuesFor(relation, MilitaryTag.class) && relation.isMultiPolygon()))

should give multi polygon relations with intersecting entities. Then MultiPolygon could be created from relation and checked for intersection.

I agree that this should be avoided, but after all this I do not see an alternative. I thank you very much for all your suggestions, unfortunately, the use of relationsWithEntitiesIntersecting will create false positive flags. The LineItems in question do not intersect any of the members of the military relations that contain them, even when the bounds of the LineItems are used. Thus, no relations are passed to the predicate for further filtering.

To completely substitute this loop, there would need to be a method that checks for containment of an item in a relation, not intersection.

As another example:
Unit test accessNoInHighwayEdgesLanduseMilitaryRelation simulates this scenario.

Sure, but that sounds like a problem in atlas - possibly missing an API that could check for intersection against the relation altogether. Feel free to create an issue on atlas project, document it here and then move forward. False positives are acceptable. However, iterating over all relations should be avoided given that the number of relations in an Atlas could be huge and such iteration will be repeated potentially for each Edge in the same Atlas.

Alternatively, you can edit this check to go through military areas rather than Edges and Lines. That way you can iterate military areas first and mark Edges and Lines.

Missing atlas relation method issue: Relation Covering Method #137

mertemin · 2018-06-01T17:30:29Z

src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java

+ * This check flags {@link Edge}s and {@link Line}s that include an access tag with a value of no,
+ * and does not have any supporting tags. Supporting tags declare what is or is not included in
+ * {@code access=no}. For example a supporting tag of {@code public_transport=yes} would mean only
+ * public transport vehicles are allowed.


Please document the use of tags.filter configuration. Otherwise, this check's implementation doesn't have a direct use of the tag example mentioned above. It'd be confusing.

mertemin · 2018-06-01T17:32:44Z

src/test/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheckTestRule.java

+                    @Node(coordinates = @Loc(value = TEST_7)),
+                    @Node(coordinates = @Loc(value = TEST_8)),
+                    @Node(coordinates = @Loc(value = TEST_9)),
+                    @Node(coordinates = @Loc(value = TEST_10)) },


You don't need to create a node for each shape point in an edge. You only need nodes for edge's end points.

mertemin · 2018-06-01T17:34:13Z

src/test/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheckTestRule.java

+    private static final String TEST_11 = "0.2,-0.1";
+    private static final String TEST_12 = "-0.2,-0.1";
+    private static final String TEST_13 = "-0.2,1.0";
+    private static final String TEST_14 = "0.2,1.0";


Although it is OK to use these values, I'd suggest using locations that are closer to real life lat/long values.

…ry unit test nodes.

mertemin · 2018-06-04T22:28:18Z

src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java

+    {
+        for (final Area area : object.getAtlas()
+                .areas(area -> Validators.isOfType(area, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(area, MilitaryTag.class)))


LineItem has a method called bounds, which will give you a Polygon. You can retrieve areas intersecting that and filter them down. e.g. atlas.areasIntersecting(line.bounds(), area -> (line.intersects(areaPolygon) || areaPolygon.fullyGeometricallyEncloses(line.asPolyLine())) && (Validators.isOfType(area, LandUseTag.class, LandUseTag.MILITARY) || Validators.hasValuesFor(area, MilitaryTag.class))).

You should alsoe cache the result of object.asPolyLine(), because that method would be returning a new PolyLine object per call.

mertemin · 2018-06-04T22:29:05Z

src/main/java/org/openstreetmap/atlas/checks/validation/tag/InvalidAccessTagCheck.java

+        for (final Relation relation : object.getAtlas().relations(
+                relation -> (Validators.isOfType(relation, LandUseTag.class, LandUseTag.MILITARY)
+                        || Validators.hasValuesFor(relation, MilitaryTag.class))
+                        && relation.isMultiPolygon()))


See my other comment please. Retrieve relations intersecting line bounds and then filter them down.

jklamer · 2018-06-26T16:50:23Z

@Bentleysb sorry for the long merge delays, can you fix the conflicting files and we'll go ahead and merge

* public/dev: (37 commits) Check resource loader update/Global Polygon Filter (osmlab#52) Invalid Lanes Tag Check (osmlab#55) Conflicting Area Tag Combination check (osmlab#53) Invalid Access Tag Check (osmlab#46) Sharp Angle Check Enhancement (osmlab#56) Intersecting Building Check Enhancments (osmlab#48) Batch Size sized by numer of buckets, but never less than 100) (osmlab#54) Log time to execute message as info (osmlab#51) Building Road Intersection Enhancements (osmlab#41) Fixing issue that was causing exception in MapRouletteClient when Def… (osmlab#49) create required maproulette challenge json object for check configs without defaultPriority (osmlab#47) MalformedRoundabout Configuration Bug (osmlab#45) Waterbody And Island Size Check (osmlab#44) Malformed Roundabout Check (osmlab#42) Update README and gradlew (osmlab#43) One Member Relation Check (osmlab#40) Sink Island Check Enhancements (osmlab#36) Update flagged point (osmlab#39) Address Point Match Check Bug Fix (osmlab#38) Updating atlas-generator dependency ... # Conflicts: # dependencies.gradle # gradle.properties # gradle/wrapper/gradle-wrapper.properties # src/main/java/org/openstreetmap/atlas/checks/base/BaseCheck.java # src/main/java/org/openstreetmap/atlas/checks/base/CheckResourceLoader.java # src/main/java/org/openstreetmap/atlas/checks/validation/points/AddressPointMatchCheck.java # src/test/java/org/openstreetmap/atlas/checks/base/CheckResourceLoaderTest.java

Bentleysb added 29 commits May 10, 2018 08:37

Created HighwayAccessTag check. getConnectedLineItems v1.0.

3a5532e

HighwayAccessTagCheck v1 complete, including unit tests and config.

b098abd

Delete HighwayAccessTagCheckTest.java

ac8e873

Fixed private gate unit test, added hasGate(), fixed config.

1f25a24

fiddled with config

3c7417b

Merge branch 'dev' of https://github.com/Bentleysb/atlas-checks into dev

69ff95c

Fixed non-highway connections and minimum highway priority.

1ea4dd3

Fixed config input handler, and updated config

8b1d80c

Only creates one tag per osm way. Fixed Atlas 2way edge based error. …

fc70197

…Fixed )'s in validCheckForObject.

Removed access=private handling. Added do not flag key/value config.

6554e32

Added java docs, changed unintuitive variabel names, refactored confi…

97f8513

…g names, changed congig defaults to use tag classes, refactored Math.abs() to Edge.isMasterEdgeId().

Added area/realtion landuse=military do not flag, changed default con…

644a907

…fig minimum.highway.type to residential

updated unit test to reflect configurable minimum.highway.type

7b483f2

Moved landuse=military check call to increase run speed.

f120910

Fixed landuse=military for relations, and added military=* check.

fc303bc

Updated isInMilitaryArea() java doc.

0fd6e2b

Added contians check to relation portion of isInMilitaryArea()

1e9ac36

Fixed area contains portion of isInMilitaryArea()

d2e0f2f

Removed connected highways.

1278759

Replaced do-not-flag with tags.filter

3b2bb44

Refactored to be InvalidAccessTagCheck, moved from checks.validation.…

a2709d5

…linear to checks.validation.tag, updated javadocs.

Changed isInMilitaryArea() to use Validator, removed outdated config,…

66be2f8

… moved boolean checks around

Added InvalidAccessTag.md, updated comments, added MR challenge info.

d8e85e9

Added serial version uid, updated MD headings, updated MR challange t…

a0c08fa

…ags.

Updated InvalidAccessTagCheck.md

a1f26fb

Updated InvalidAccessTagCheck instruction

d802f97

fixed typo in InvalidAccessTagCheck.md

214427a

spotless

53fb2bf

fixed InvalidAccessTagHCeck.md heading, updated MR instruction config.

e9f8a31

mertemin suggested changes Jun 1, 2018

View reviewed changes

Bentleysb added 2 commits June 1, 2018 13:13

Changed unit test coordinates to be more realistic, removed unnecessa…

4f50136

…ry unit test nodes.

Added reference to tags.filter in InvalidTagCheck java doc.

e219f0a

mertemin suggested changes Jun 4, 2018

View reviewed changes

Bentleysb added 2 commits June 4, 2018 16:17

Changed isInMilitary() to use areasIntersecting() instead of areas().

b298c00

changed isInMilitaryArea to use relationsWithEntitiesIntersecting

e669d7b

jklamer approved these changes Jun 12, 2018

View reviewed changes

Bentleysb added 2 commits June 26, 2018 09:58

Merge branch 'dev' into dev

6500bcc

Update configuration.json

c62aace

jklamer merged commit ec4e6c1 into osmlab:dev Jun 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Invalid Access Tag Check #46

Invalid Access Tag Check #46

Bentleysb commented May 23, 2018

mertemin Jun 1, 2018

Bentleysb Jun 2, 2018

mertemin Jun 4, 2018

mertemin Jun 1, 2018

Bentleysb Jun 2, 2018

mertemin Jun 4, 2018

Bentleysb Jun 5, 2018

mertemin Jun 7, 2018

Bentleysb Jun 7, 2018

mertemin Jun 7, 2018

mertemin Jun 7, 2018

Bentleysb Jun 11, 2018

mertemin Jun 1, 2018

mertemin Jun 1, 2018

mertemin Jun 1, 2018

mertemin Jun 4, 2018

mertemin Jun 4, 2018

jklamer commented Jun 26, 2018

Invalid Access Tag Check #46

Invalid Access Tag Check #46

Conversation

Bentleysb commented May 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklamer commented Jun 26, 2018