Modify

Opened 17 months ago

Last modified 23 hours 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 17 months ago.
Initial patch
18364.1.patch (37.2 KB) - added by taylor.smock 17 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 17 months ago.
Add missing Access class
18364.3.patch (69.5 KB) - added by taylor.smock 17 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 17 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 16 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 (18)

Changed 17 months ago by taylor.smock

Attachment: 18364.patch added

Initial patch

comment:1 Changed 17 months ago by skyper

See #8754 for similar thoughts.

Changed 17 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 17 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 17 months ago by taylor.smock (previous) (diff)

comment:3 in reply to:  2 ; Changed 17 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 17 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 17 months ago by taylor.smock (previous) (diff)

Changed 17 months ago by taylor.smock

Attachment: 18364.2.patch added

Add missing Access class

comment:5 in reply to:  4 ; Changed 17 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 17 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 17 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 17 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 17 months ago by skyper (previous) (diff)

Changed 17 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 17 months ago by skyper

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

Changed 16 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)

comment:9 Changed 2 months ago by skyper

I'd almost filled a ticket about this issue. Thought this had been fixed.

Ping.

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

comment:10 Changed 2 months ago by taylor.smock

I've got this implemented in the MapWithAI plugin (see https://gitlab.com/gokaart/JOSM_MapWithAI/-/blob/master/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/RoutingIslandsTest.java ).

I wasn't happy with the performance, and I haven't had any good ideas on how to fix that. So it has kind of just been stuck in the MapWithAI plugin (mostly since I want to avoid having people accidentally create routing islands). I'll see if I can update the patch in the next day or two.

I'll take a look at the patch again (its been a year or so, so I might have some new inspiration).

comment:11 Changed 24 hours ago by skyper

Follow up #20780?

comment:12 in reply to:  11 Changed 23 hours ago by taylor.smock

Replying to skyper:

Follow up #20780?

Partially. I'd like to have a dedicated plugin for some of my WIP validations. Hopefully other people will use the plugin for complex validations where a little more real world testing would be nice.

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.