iD icon indicating copy to clipboard operation
iD copied to clipboard

iD damages relations when splitting ways

Open robhubi opened this issue 4 years ago • 20 comments

My System

Windows 10, 64 Bit Firefox 89.0.2, 64 Bit MS Edge 91.0.864.67, 64 Bit iD 2.19.6

Description

Splitting ways that are members of relations destroys the order of the members. See also Order matters.

Steps to Reproduce

Starting point The RabenfeldstraĂźe is, among other things, part of the Relation Weingarten-Tour. It has 58 members: iDbR_01

Change The "RabenfeldstraĂźe" is split with the scissors: iDbR_02

Actual Result

As expected, the relation Weingarten-Tour has 59 members afterwards, but the order is destroyed: iDbR_03

The effect is not always reproducible. On the "Panorama Tour (Eibiswald)" it no longer occurred the third time I wanted to take screenshots. With the "Weingarten Tour" I took the screenshots on the first repetition. Of course, I did not save the changes.

Expected Result

Unchanged order of members as done by JOSM.

Other, possibly relevant issues

https://github.com/openstreetmap/iD/issues/4978 https://github.com/openstreetmap/iD/issues/4876 https://github.com/openstreetmap/iD/issues/4861

PLEASE

Do not allow splitting of ways that are part of a relation as long as the issue is unresolved.

robhubi avatar Jul 13 '21 19:07 robhubi

Did you try with release version 2.20? Reading #8612 makes it look like iD started to validate and therefore fetch route relations in their entirety, which might be promising?

On the expected result: Of course order will be changed, there is a new member in town! Then, behaviour does not depend on how JOSM performs, the error is an error, regardless of that.

But most of all: How is order destroyed? Does only the new member end up in the wrong place? Or are members completely shuffled?

hungerburg avatar Aug 27 '21 08:08 hungerburg

Version 2.20.1 shows the error in the same way.

If the way "3" is divided, the relation list "... 2, 3, 4 ..." should turn into the sequence "... 2, 3a, 3b, 4 ...". This is what is meant by constant order. JOSM is mentioned because it could provide a blueprint for problem solving.

The issue documented above shows the effect of the error. Before the way splitting, the first 5 members of the relation are completely different from the first 5 members after the way splitting.

robhubi avatar Aug 27 '21 20:08 robhubi

Recently an iD user did split several streets around my home-base, afflicting lots of PSV route relations. The editor actually did a very good job at this. The change to the relation was minimal, order was perfectly retained. Shouldn't both PSV and cycle routes be similar?

hungerburg avatar Oct 05 '21 21:10 hungerburg

Interesting. Unfortunately, hiking and cycling routes are often affected by this issue. Which boundary conditions lead to this behaviour can probably only be judged by the software developer.

robhubi avatar Oct 06 '21 18:10 robhubi

You can do some black-box-testing. iD is only supported by volunteers (at the moment.)

hungerburg avatar Oct 06 '21 20:10 hungerburg

The RabenfeldstraĂźe is, among other things, part of the Relation Weingarten-Tour.

This route relation forms a loop with a short spur at the southwestern corner. Most of the reported cases of incorrect relation member ordering so far have involved routes that double back on themselves in a similar manner. As you point out, recreational routes are particularly prone to this issue because, unfortunately, it’s common for spurs and excursions to be added to the main route relation. By contrast, public transport routes (and increasingly road routes) rely on superrelations or route master relations to associate linear routes with each other.

There’s quite a bit of relevant technical discussion about the issue in #4876 and #4589. The relevant code is here:

https://github.com/openstreetmap/iD/blob/a018d276eb0d28fc414e390fbd8c66b54c910d4c/modules/actions/add_member.js#L52-L60

1ec5 avatar Oct 07 '21 06:10 1ec5

Many thanks for this hint! The explanation for the relative frequency of issues in the Recreational Routes seems very plausible to me. The SW excerpt shows me that the developers have considered many things. Now I also see that splitting ways works in principle also for relations members. However, it sometimes fails for unknown (random?) reasons. Examples:

Test Panorama Tour (Eibiswald) on 12 July 2021. Test repeated 5x within one hour (test case see issue above):

  • 1st test: order damaged
  • 2nd test: order damaged
  • 3rd test: order damaged
  • 4th test: order ok
  • 5th test: order ok

Even in later tests, the order was always maintained.

Test Weingarten Tour on 12 July 2012 The test was carried out 2 times in a row, the order was damaged each time.

Test Weingarten Tour, today The test was carried out 4 times in a row:

  • 1st test: order ok
  • 2nd test: order damaged
  • 3rd test: order ok
  • 4th test: order ok

Changed order after the 2nd test: WGT2erT

In principle, keeping order seems to work, but sometimes (by random?) it goes wrong.

robhubi avatar Oct 07 '21 21:10 robhubi

In principle, keeping order seems to work, but sometimes (by random?) it goes wrong.

I think I remember that it has to do with which parts of the relation have been downloaded at the time of the test. Is this something that might have changed in your test cases?

tordans avatar Oct 08 '21 05:10 tordans

Yes, I paid attention to which parts were downloaded.

  • Test case 2: Before splitting members manually NOT downloaded - order damaged
  • Test case 3: Before splitting members manually NOT downloaded - order ok

There seems to be no direct correlation with the issue.

robhubi avatar Oct 08 '21 17:10 robhubi

Test case 2 was in July, three months before case 3 and an iD release in between; see my post above, iD changed downloading touching stuff; which then did not change anything though?

hungerburg avatar Oct 08 '21 18:10 hungerburg

Sorry for the confusion. In this post

Yes, I paid attention to which parts were downloaded.

* Test case 2: Before splitting members manually NOT downloaded - order damaged

* Test case 3: Before splitting members manually NOT downloaded - order ok

There seems to be no direct correlation with the issue.

Test cases related to the last test "Test Weingarten Tour, today" of 7 Oct. 21. iD version was 2.20.1, tests were completed within 1 hour.

robhubi avatar Oct 08 '21 20:10 robhubi

By the way, you may find it more convenient to test iD without uploading your changes to the live OSM server: click the “Download osmChange file” link at the bottom of the upload screen, then manually inspect the file’s list of relation member IDs to see if the order is correct. One side of the split will have a negative value, and the other should be adjacent to it in the list.

1ec5 avatar Oct 09 '21 05:10 1ec5

Addendum for completeness This issue was the subject of a discussion on OSMF-Talk. A discussion about 23 mails ensued. A short summary can be found here.

robhubi avatar Feb 08 '22 22:02 robhubi

To bring here the information the OP supplied in the local forum: The case "Weingartentour" seems to be fixed with the new release, only the case "Panorama Tour" remains wanting - I guess that is this one https://www.openstreetmap.org/relation/3132692 ?

The Panorama route looks quite simple, one complete round, no spurs. Searching for differences - it goes thtough a roundabout, that is mapped in much detail - https://www.openstreetmap.org/relation/3132692#map=19/46.68934/15.24868 - Tough to sort, quite close to the RabenfeldstraĂźe https://www.openstreetmap.org/way/962203250 too.

PS: What is completeness meant to say?

hungerburg avatar Feb 12 '22 09:02 hungerburg

I use ID mostly for my edits and if I edit and an existing multipolygon (...Node Line Node Line Node Line Node Line Node...) and remove the center node in the multi polygon there is now a hole in the multiploygon. Save. When I save I get no errors that the multipolygon is now (broken). No warning no errors are being displayed.

Video evidence to the above description: https://youtu.be/0-9AMyahcGc

To recreate this select a large recently unchanged multipolygon. create three new nodes in a line cut middle one delete it. Save the data.

natfoot avatar Apr 28 '22 19:04 natfoot

@natfoot I guess, what you describe is worth an issue of its own. This here is only about sorting the order of relation members, not about them providing closed ways.

BTW: your example matches the label bug much more than this one.

hungerburg avatar Apr 28 '22 23:04 hungerburg

Just stumbled into this issue with https://www.openstreetmap.org/changeset/126323215 by splitting a simple way - fixed the damage in JOSM afterwards. Didn't happen on the split of another way, so this looks like the new ordering is just determined by a coin toss and not by considering the original order ;-)

Never had this problem when splitting ways in JOSM, just saying...

arminus avatar Sep 18 '22 10:09 arminus

Just stumbled into this issue with https://www.openstreetmap.org/changeset/126323215 by splitting a simple way

This is related to the roundabout case discussed above in that this route relation is nonlinear: it includes all of this roadway but also this pathway that intersects it midway. If this roadway had been split and the route were continuous, I suspect the split you performed would’ve worked without any problems. In fact, if the relation had been poorly sorted beforehand, iD would’ve fixed the sorting. The reason JOSM doesn’t have this problem is that it doesn’t automatically sort relation members in the same manner.

/ref https://github.com/openstreetmap/iD/issues/8578#issuecomment-937484354

1ec5 avatar Sep 18 '22 15:09 1ec5

Just stumbled into this issue with https://www.openstreetmap.org/changeset/126323215 by splitting a simple way

This is related to the roundabout case discussed above in that this route relation is nonlinear: it includes all of this roadway but also this pathway that intersects it midway. If this roadway had been split and the route were continuous,

that split broke 3 MTB relations, I can't be sure they were ok before the split, but all 3 of them already broken/non-continuous? I don't know... And the fix in JOSM was to shuffle up a couple of ways "left" of the split, that intersection you pointed out had nothing to do with it.

Be that as it may - I know from my own experience that relation sorting is tricky. I was unsuccessfully trying to do my own resort implementation for my via ferrata and cross country skiing trails map site elevation profiles for a long time and eventually gave up. But that was only for a data consumer. If a resorting approach is known to break properly sorted relations (even only in edge cases) in the actual data, I'd make that an option (with a possibility to visually verify) and not a default.

I frequently observe broken relations for the ferrata and cross country skiing trail relations I keep an eye on, that iD behavior could be an explanation.

arminus avatar Sep 19 '22 07:09 arminus

And the fix in JOSM was to shuffle up a couple of ways "left" of the split, that intersection you pointed out had nothing to do with it.

The sorting code operates on the whole relation without assuming that any members were previously sorted, so it’s possible for a loop or backtrack to affect members far away. MTB Prinzenweg-Rundtour and MTB Alpbachtal Rundtour both have a loop (a very large one) along with a “tail”, so splitting anything along either relation would definitely trigger #4876. I can’t explain MTB Almtour though – I was under the impression that this issue doesn’t affect routes that only consist of a loop. Maybe it affects them too, maybe nondeterministically as https://github.com/openstreetmap/iD/issues/8578#issuecomment-938178678 discovered.

If a resorting approach is known to break properly sorted relations (even only in edge cases) in the actual data, I'd make that an option (with a possibility to visually verify) and not a default.

At a minimum, disabling the automatic resorting by default would require a more robust UI for ordering relations manually. But I think it would probably uncover a lot of other issues that would be even more commonplace than this loop issue. If I remember correctly, automatic resorting was implemented in response to a lot of feedback that iD was mangling member order of perfectly linear relations, so those latent issues would need to be addressed at the same time.

As far as I can tell, the loop/discontinuity issue raised here so far doesn’t appear to be a fundamentally intractable shortcoming of the automatic resorting approach. Rather, it’s an edge case that wasn’t addressed originally, essentially a to-do item as I mentioned in https://github.com/openstreetmap/iD/issues/8578#issuecomment-937484354. Unfortunately, it has been tripping up certain kinds of route relations more frequently than originally expected.

It is possible, though not particularly convenient, to visually verify the order before saving: with the relation selected, hover the cursor over each member in the list. As you hover over each member, iD highlights it on the map. Verify that the members are highlighted in the order that you expect. You can drag members around to fix the order.

1ec5 avatar Sep 20 '22 05:09 1ec5

See also the issue #8415

Mahabarata avatar Jan 18 '24 14:01 Mahabarata