Modify

Opened 4 months ago

Last modified 4 months ago

#18364 new enhancement

[WIP PATCH] Find routing islands

Reported by: taylor.smock Owned by: team
Priority: normal Milestone:
Component: Core validator Version:
Keywords: route, routing, island Cc:

Description

It would be nice to have a check for various routing issues (e.g., I can get into an area, but I cannot get out), and point that out to mappers.

The current patch does the following:

  • Point out areas where oneways cause routing islands

On the list of things TODO:

  • Account for turn restrictions (partially implemented)

Attachments (6)

18364.patch (29.3 KB) - added by taylor.smock 4 months ago.
Initial patch
18364.1.patch (37.2 KB) - added by taylor.smock 4 months ago.
Check different routing types (assumptions have been made about the default access values for highways)
18364.2.patch (65.6 KB) - added by taylor.smock 4 months ago.
Add missing Access class
18364.3.patch (69.5 KB) - added by taylor.smock 4 months ago.
Mark routable ways that are not connected to any other way (i.e., a single way) as an error, unless a node is outside the download area. This patch does not deal with Node.isOutSideDownloadArea returning false when the node is new/undeleted.
18364.isOutsideDownloadArea.patch (1.3 KB) - added by taylor.smock 4 months ago.
Possible method to determine if the node is outside of a download area while ignoring if it is new/undeleted
18364.4.patch (72.0 KB) - added by taylor.smock 3 months ago.
Fix an issue where running on a selection would be more likely to create a warning about a routing island (it still will create a warning when there is an incoming way and an outgoing way in the selection, but the rest of the ways are not connected)

Download all attachments as: .zip

Change History (14)

Changed 4 months ago by taylor.smock

Attachment: 18364.patch added

Initial patch

comment:1 Changed 4 months ago by skyper

See #8754 for similar thoughts.

Changed 4 months ago by taylor.smock

Attachment: 18364.1.patch added

Check different routing types (assumptions have been made about the default access values for highways)

comment:2 Changed 4 months ago by taylor.smock

@skyper: Should I move the patch over to #8754? It is very similar to what I wanted.

Note: The way the test currently works is it checks if a node has been uploaded and is outside the download area or connected to a specific amenity. This allows the test to determine if there are routing islands. It also checks to see if there are boundaries for the downloaded data. If not, then it doesn't run the checks (Overpass could lead to a lot of false positives)

Last edited 4 months ago by taylor.smock (previous) (diff)

comment:3 in reply to:  2 ; Changed 4 months ago by skyper

Replying to taylor.smock:

@skyper: Should I move the patch over to #8754? It is very similar to what I wanted.

I do not care about the ticket numbers. Closing #8754 as "duplicate" would be a solution.

Note: The way the test currently works is it checks if a node has been uploaded and is outside the download area or connected to a specific amenity. This allows the test to determine if there are routing islands. It also checks to see if there are boundaries for the downloaded data. If not, then it doesn't run the checks (Overpass could lead to a lot of false positives)

  • Why needs the node to be uploaded ?
    • What happens if islands are created on current session ?
  • Does it check for each transportation mode (vehicle, bicycle, foot etc.) or only car routing ?
  • On #8754 there was the request to extend this for waterways.

comment:4 in reply to:  3 ; Changed 4 months ago by taylor.smock

Replying to skyper:

Replying to taylor.smock:

@skyper: Should I move the patch over to #8754? It is very similar to what I wanted.

I do not care about the ticket numbers. Closing #8754 as "duplicate" would be a solution.

Note: The way the test currently works is it checks if a node has been uploaded and is outside the download area or connected to a specific amenity. This allows the test to determine if there are routing islands. It also checks to see if there are boundaries for the downloaded data. If not, then it doesn't run the checks (Overpass could lead to a lot of false positives)

  • Why needs the node to be uploaded ?
    • What happens if islands are created on current session ?

The issue for this lies in source:trunk/src/org/openstreetmap/josm/data/osm/Node.java@15583:387-395#L387 (Node.isOutsideDownloadArea due to the isNewOrUndeleted check). EDIT: I haven't decided what the best way to fix this problem is.

  • Does it check for each transportation mode (vehicle, bicycle, foot etc.) or only car routing ?

It checks every land transport mode. It looks like I didn't add my Access.java class to the last patch. I'll fix that, but it checks foot, ski, inline_skates, ice_skates, horse, vehicle, bicycle, carriage, trailer, caravan, motor_vehicle, motorcycle, moped, mofa, motorcar, motorhome, psv, bus, taxi, tourist_bus, goods, hgv, agricultural, atv, snowmobile, hgv_articulated, ski_nordic, ski_alpine, ski_telemark, coach, and golf_cart.

  • On #8754 there was the request to extend this for waterways.

I should be able to extend it to waterways. The only issue is how oneways work with waterways (do I assume that someone intended to indicate the direction of flow?)

Last edited 4 months ago by taylor.smock (previous) (diff)

Changed 4 months ago by taylor.smock

Attachment: 18364.2.patch added

Add missing Access class

comment:5 in reply to:  4 ; Changed 4 months ago by skyper

Replying to taylor.smock:

Replying to skyper:

Replying to taylor.smock:

  • On #8754 there was the request to extend this for waterways.

I should be able to extend it to waterways. The only issue is how oneways work with waterways (do I assume that someone intended to indicate the direction of flow?)

No, no, flow direction and access are completely different categories:

  • flow direction is taken from the direction of the way. If there is no clear direction or if it does change you need flow_direction=*, additionally.
  • oneway=* is an access tag and has nothing to do with flow direction.

comment:6 in reply to:  5 ; Changed 4 months ago by taylor.smock

Replying to skyper:

  • oneway=* is an access tag and has nothing to do with flow direction.

OK. I do know that the JOSM validator currently throws a warning if oneway is on a waterway (oneway is unnecessary for waterway).

The patch I'm uploading in a minute or two deals with water-based transport (and could probably be extended to railways, if necessary). I do need to expand the tests on it.

Also, I should figure out if I can improve performance in some other way besides using parallelStream.

That being said, in my limited testing, it is working fairly well (everything it has found is an actual routing issue, although some really don't matter -- e.g., service road only connected to a motorway will indicate that it is an island for foot and bicycle traffic).

Changed 4 months ago by taylor.smock

Attachment: 18364.3.patch added

Mark routable ways that are not connected to any other way (i.e., a single way) as an error, unless a node is outside the download area. This patch does not deal with Node.isOutSideDownloadArea returning false when the node is new/undeleted.

comment:7 in reply to:  6 Changed 4 months ago by skyper

Replying to taylor.smock:

Replying to skyper:

  • oneway=* is an access tag and has nothing to do with flow direction.

OK. I do know that the JOSM validator currently throws a warning if oneway is on a waterway (oneway is unnecessary for waterway).

This warning is not 100% true. There are some oneway waterways, mostly canals. Probably another ticket.

Last edited 4 months ago by skyper (previous) (diff)

Changed 4 months ago by taylor.smock

Possible method to determine if the node is outside of a download area while ignoring if it is new/undeleted

comment:8 Changed 4 months ago by skyper

Ticket #8754 has been marked as a duplicate of this ticket.

Changed 3 months ago by taylor.smock

Attachment: 18364.4.patch added

Fix an issue where running on a selection would be more likely to create a warning about a routing island (it still will create a warning when there is an incoming way and an outgoing way in the selection, but the rest of the ways are not connected)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to taylor.smock
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.