Opened 7 years ago

Last modified 7 years ago

#4539 accepted defect

Endpoints of node networks are incorrectly detected

Reported by: JDub Owned by: JDub
Priority: trivial Milestone:
Component: potlatch2 Version:
Keywords: rcn, node, display name Cc: Polyglot

Description

This is in relation to node-based cycle and hiking networks, where the endpoints rather than the connecting routes have refs. Potlatch2 normally lists these routes with a name based on the refs of the endpoints. Sometimes this fails, however.

An example:

http://www.openstreetmap.org/?lat=50.86198&lon=4.681851&zoom=18&layers=M

This is the intersection containing node 75 (OID 872014261), from which two nameless rcn relations go on to nodes 73 and 74.

If I start Potlatch2 zoomed in, both routes are given the wrong name "-75" and "-60". If I zoom out to the point that node 74 is included in the loaded data, the first route's name is corrected to "74-75". I assume this is an inherent limitation, in that the endpoint can't be shown without first loading it?

This being the case, "-60" for relation 6115 is still incorrect, as this route actually ends on node 75. It seems that Potlatch2 is somehow detecting the rwn node 60 (OID 16652149) as the endpoint of the rcn route, despite that it's in the middle and that the route is marked as an "rcn" route. Any idea what might be causing this, and if it's a data or a software error?

Cheers, Jw

Attachments (1)

fix_relation_endpoint_naming.patch (3.1 KB) - added by JDub 7 years ago.
Patch to fix endpoint detection

Download all attachments as: .zip

Change History (8)

comment:1 Changed 7 years ago by Polyglot

Cc: Polyglot added

Those rcn and rwn relations are being meticulously entered. So I would be very surprised if it where a data error. I also ran my QC script on the rcn route relations 2 days ago. They are all correct.

I think Jw found the cause of the many routes shown as xx- or -yy. The data of the other side of the relation isn't loaded yet.

What should really be done to determine what should be displayed is to load the relation. Take the first and the last way of the route relation and find the rwn_ref or rcn_ref of their nodes, of course rwn for rwn routes and rcn for rcn routes. There are also rhn routes with numbered nodes, BTW. Not many yet, but they do exist.

If the previous algorithm produces 2 numbers, you are done. If not, more ways should be investigated, as there is some funny stuff going on in rcn relations. (Having to do with routes that end on 'cycle network nodes' that need to be represented by more than 1 OSM node, on different sides of a street/canal/roundabout/etc). This will only occur if those ways have forward or backward roles.

I hope this helps to resolve the issues.

Kind regards,

Jo

comment:2 in reply to:  1 Changed 7 years ago by JDub

Replying to Polyglot:

I think Jw found the cause of the many routes shown as xx- or -yy. The data of the other side of the relation isn't loaded yet.

Well, this may not always be doable, as if it's a long relation it may require too much memory. However, even if we accept that only one endpoint is shown as is now, relation 6115 is still being detected incorrectly despite that the endpoint *is* in the active data set.

comment:3 Changed 7 years ago by Richard

Priority: minortrivial

Jo is right, the issue is that the other end of the data isn't loaded yet. We absolutely can't load /relation/full for everything just to get naming right on this one edge case - that would kill the servers for the 99.9999% of other uses. We could potentially add a 'Load relation contents' option to the relation contextual menu, I guess.

I can't reproduce your error for 6115; that works fine for me as soon as the data is loaded into memory and you re-click on the way to update the display.

comment:4 Changed 7 years ago by Richard

(If you want to look at the existing code and maybe submit a patch if it's not doing what you want, it's at https://github.com/systemed/potlatch2/blob/master/net/systemeD/halcyon/connection/Relation.as#L132 .)

comment:5 Changed 7 years ago by JDub

How I reproduce the error:

  • Open my link above, then Edit -> Potlatch2
  • Without zooming, select way 73579617, the small connection between the road and the bike path.
  • Select advanced view. One relation is listed as using this way, namely "Route bycicle rcn -60", id 6115. The end points of this relation are 872014261 [rcn_ref=75] and (off-screen) 60457713 [network=rcn, rcn_ref=73, rwn_ref=69]
  • If I stay in simple view but try to add this way to a cycle route, I get three options in the resulting window: "13186: route bicycle rcn 75-", "6115: route bicycle rcn -60" and "6119: route bicycle rcn -75". All three of these meet in rcn node 75.

If I zoom out enough that rcn node 73 is in the picture, and wait for my computer to stop swapping (it's a laptop), relation 6115 is reported as "route bicycle rcn 73-60". This is despite that it correctly recognizes that short segment ending on rcn node 75 as the last one of the relation (6115/28 for 29 members)

It shows you that relation as "route bicycle rcn -75"?

Took a look at the code, and here's my understanding of what's going wrong:

Relation 6115 is loaded, as is its last member way 6115/28 and this way's two end nodes 872014261 and 16652149. Calling getDescription(6115) then calls getSignificantName(6115/28), which in turn first tries getSignificantName(16652149) and then, if this gives no result, getSignificantName(872014261) which should be "75".

However, node 16652149 is rwn node 60, so getSignificantName(16652149) returns "60". The problem is that getDescription takes the a node of last way but not necessarily the last node of the relation. Reversing the direction of the way would fix it for the -75 routes, but this might not fix other, busier intersections.

getSignificantName(<way>) either needs a flag indicating which end to check, or the relation needs to be able to directly find the first and last *nodes* and call getSignificantName (or maybe just the needed regex) on them directly.

I'm not familiar with ActionScript?, but what about something like this, replacing lines 149-151:

var firstName:String = '';
if (getFirstMember() is Way) {
    if (Way(getMember(0)).getFirstNode()==Way(getMember(1)).getFirstNode() || Way(getMember(0)).getFirstNode()==Way(getMember(1)).getLastNode()) {
        firstName = getSignificantName(Way(getMember(0)).getLastNode());
    } else {
        firstName = getSignificantName(Way(getMember(0)).getfirstNode());
    }
} else if (getFirstMember() is Node) {
    firstName = getSignificantName(getFirstMember());
}

var lastName:String = '';
if (getLastMember() is Way) {
    var len:Number = length() - 1;
    if (Way(getMember(len)).getFirstNode()==Way(getMemler(len-1)).getFirstNode() || Way(getMember(len)).getFirstNode()==Way(getMember(len-1)).getLastNode()) {
        lastName = getSignificantName(Way(getMember(len)).getLastNode());
    } else {
        lastName = getSignificantName(Way(getMember(len)).getfirstNode());
    }
} else if (getLastMember() is Node) {
    lastName = getSignificantName(getLastMember());
}

if ((firstName+lastName)!='') desc+=" "+firstName+"-"+lastName;

There's probably a more elegant way to code this, but this way should work at least.

comment:6 Changed 7 years ago by JDub

Owner: changed from potlatch-dev@… to JDub
Status: newaccepted

Hacked at it a bit and got it working properly on my machine. Is this the proper place to submit patches?

Changed 7 years ago by JDub

Patch to fix endpoint detection

comment:7 in reply to:  6 Changed 7 years ago by JDub

Replying to JDub:

Is this the proper place to submit patches?

I've created a pull request on GitHub?, so there's no need to use the attached patch.

Note: See TracTickets for help on using tickets.