[SinkIslandCheck] sink islands surrounded by pedestrian ways #227
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
December 9, 2019 11:50
seancoulter
force-pushed
the
sinkIslandUpdates
branch
from
6a7dcb4
to
a1cd376
Compare
seancoulter
changed the title
…las-checks into sinkIslandUpdates
seancoulter
changed the title
sayas01
reviewed
Feb 19, 2020
config/configuration.json
Outdated
| @@ -799,6 +799,7 @@ | |||
| "SinkIslandCheck": { | |||
| "tree.size": 50, | |||
| "minimum.highway.type": "service", | |||
| "filter.pedestrian.network": true, | |||
There was a problem hiding this comment.
Is the default config going to be true for "filter.pedestrian.network"?
There was a problem hiding this comment.
It should be false to maintain baseline outputs. Thanks
danielduhh
reviewed
Feb 19, 2020
|
Kudos, SonarCloud Quality Gate passed!
|
danielduhh
approved these changes
Feb 19, 2020
|
@danielduhh updated description with flag count expectations |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR does the following:
--Adds a switch to allow for candidate sink island edges (highway=service+ edges with valid accessibility tags) WITHOUT a motor_vehicle tag to be flagged if those candidate edges are surrounded by pedestrian ways.
--Ignores such candidate sink islands with motor_vehicle=yes tags that are surrounded by pedestrian ways.
Potential Impact:
With the switch on, we should expect to see more flags as a result of sink islands surrounded by pedestrian ways being flagged.
Regardless of the switch, we should anticipate some diffs to account for the fact that we now ignore sink islands with motor_vehicle=yes tags that are surrounded by pedestrian ways.
The check has a known nondeterminism issue that affects flag counts across runs. We estimate the effect on flags to be some additions with mirrored subtractions or just subtractions. Additions and mirrored subtractions means part of the Way was flagged in one run, and another part of the Way was flagged in another run, causing an addition and subtraction on the same Way. We are looking into ways to fix this.
Depending on country size, we can expect flag additions with some subtractions to be an added 20-30% flag count from baseline.
Unit Test Approach:
Added some new unit tests.
Test Results:
Tests pass