Modify

Opened 14 years ago

Last modified 3 months ago

#4142 new defect

JOSM does not query API for referring relations when downloading primitives

Reported by: Nakor Owned by: team
Priority: critical Milestone:
Component: Core Version: latest
Keywords: Cc:

Description (last modified by simon04)

Download data using the following bounding box: http://www.openstreetmap.org/?lat=42.65924&lon=-83.3199&zoom=17

Download all members from the relation

Search for way 43418202. It is shown only as a member of relation 308,352 but if it is also member of another relation (see http://www.openstreetmap.org/browse/way/43418202).

This would cause inconsistencies if the way is split.

Attachments (0)

Change History (29)

comment:1 by mjulius, 14 years ago

Summary: Incomplete data when downloading all memebers of a relationJOSM does not query API for referring relations when downloading primitives

When downloading primitives from the API (with multi get) the API does not return relations the primitive is member of. There is a special API call for that:

GET /api/0.6/[node|way|relation]/#id/relations

(see http://wiki.openstreetmap.org/wiki/API_v0.6#Relations_for_Element:_GET_.2Fapi.2F0.6.2F.5Bnode.7Cway.7Crelation.5D.2F.23id.2Frelations)

JOSM needs to iterate over all downloaded primitives and check whether they are members of any relation.

Better would be if the API provided a call like

GET /api/0.6/[nodes|ways|relations]/#ids/complete

that returns the same kind of data like the map call.

comment:2 by bastiK, 14 years ago

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

comment:4 by Nakor, 14 years ago

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

comment:5 by stoecker, 14 years ago

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

comment:6 by stoecker, 14 years ago

Priority: majorcritical

comment:7 by simon04, 12 years ago

Description: modified (diff)

Any update on this critically prioritized ticket?

comment:8 by skyper, 12 years ago

Maybe a warning with an option to download parents before splitting could help as these ways are outside of the download area.

comment:9 by verdy_p, 10 years ago

Learn CTRL+ALT+D before splitting or merging nodes !
Or place the icon for the "Download referers" tool just beside the two icons on the toolbar for merging or splitting ways (use the preferences panel to add it to your toolbar).
This "Download referers" tool should be on the status bar by default !

comment:10 by verdy_p, 10 years ago

Note: not just referers of ways, but also referers of nodes should be downloaded to avoid conflicts.

But more critically, referers of relations can also include other relations, and this could recurse at deep levels;

  • However siblings found as members of referer relations do not need to be downloaded: only the fact that they are members of a known relation is enough. These members will also be known with a role in the referer relation, though the role is not strictly needed, we just need the member ID and type (node/way/relation).
  • Recursion will only occur if the referer relation is not alread loaded (at least partially). There should be an API that can recurse over parent relations but loading them with only their IDs (we know theu can only be relations). These relations would be loaded in "incomplete" state, without any of its tags and with partial lists of members with unknown roles (and in the partial list of members, not all of them would be loaded).

This would speed up the download, and would also save memory... provided that JOSM can accept such object state for relations. That "incomplete" relation is NOT locally modifiable (not even just to set new tags) as long as it has not been correctly loaded with its tags and the fll ist of members (but not necessarily their roles). We would continue to use "donwload incomplete" to completely load tags of the relation, compeltely load roles of existing members (with current "<unknown>" role, and get the full list of members as references (but not necessarily their own data, i.e. their own tags or geometry or submembers).

comment:11 by verdyp@…, 7 years ago

Note that frequently, the server cannot return all dependencies for some objects, notably when selecting ways that are part of several/many large relations:

The query fails (visible on the console with an HTTP error status), even when retrying it. Apparently the server has limited ressources for doing it.

It is probably not a bug of JOSM, but of the server trying to perform too costly requests (trying to build some costly temporary index for joining results: incorrect execution plan, or existing index estimated to be insufficiently selective). But...

Workaround: select a node one that way, zoom on it to view a very small area around it (to avoid loading too many unnecessary surrounding objects), and then load the tiny area around this node : it works instantly !

For doing the same thing on relations, you need to recurse down to get at least one node (this may require loading members if the relation is incomplete), and then use that node to perform the download by area.

Suggestion: possibly, JOSM could detect that failure and retry by downloading automatically with a query by area, using such tiny rectangle (it can be as small as a few centimeters around that node).

comment:12 by anonymous, 6 years ago

This or a variant of it (such as for when an Overpass API query only downloads certain objects and not all objects in an area, but it should to avoid editing conflicts when that object is later uploaded) may now be possible as of Overpass API 0.7.55:

https://wiki.openstreetmap.org/wiki/Overpass_API/Overpass_QL#The_block_statement_complete

It should probably be used both when downloading objects in a bounding box (by default), and at least be allowed as a user-selectable option when downloading objects through Overpass API (e.g. '[x] Also download object dependencies', enabled by default (See https://wiki.openstreetmap.org/wiki/Overpass_API/Sparse_Editing for the use case of that).

comment:14 by michael2402, 3 years ago

We have the same problem when downloading relations: A relation can have parent relations and can therefore be 'incomplete' by itself.

This is especially a problem for route relations (=> route master) or multipolygons (=> their corresponding route, stop_area, … relations)

Instead of trying to download all those relations and recurse into them, I would suggest to have a 'incompleteReferrers' flag. This flag can then be unset when JOSM is sure to have downloaded all referrers of that node/way/relation. That way, we can show a warning when modifying this incomplete data.

comment:15 by GerdP, 3 years ago

See also #18959

in reply to:  15 comment:16 by michael2402, 3 years ago

Replying to GerdP:

See also #18959

Thanks for the hint. This is why I could not find a open ticket for it.

Well, since JOSM does not provide a way to indicate that referrers were downloaded, it seems like I need to do the tracking in the PT plugin on my own then. Because for PT routes, the information if parent relations are downloaded is important (e.g. I don't want to present the user with a 'add to route master' or 'create route area group' dialog if that primitive is already inside one. Otherwise, we will have tons of duplicate relations - I already found those on several stops.

comment:17 by skyper, 3 years ago

The PT-Plugin validator rules need an update, too. At least for nodes and ways it should only warn about new objects or within downloaded area, see #15038 or PT: Stop_position is not part of a way for example.

comment:18 by alt.people.davidcalman@…, 3 months ago

My opinion is that this is not a bug. If you want to download the rest of the relation's members, do so by right-clicking on the relation or when the dialog box tells you to.

comment:19 by GerdP, 3 months ago

Yes, and I think a 14 year old ticket about an API call can be closed anyway. It's not even clear to me what problem this ticket is about.

comment:20 by michael2402 -, 3 months ago

It is not about an API call, but about a user interaction problem:

If you download a street / node / relation, JOSM won't neccessarely download parent relations. And won't know if any parent relations exist.

So the actual problem which seems to regularely destroy relations in actual OSM data (observed by me in public transport):

Download a way that is part of a PT route by other means than "Download Area", split the way, then the relation will only be on the old way and not on the newly created one.

comment:21 by GerdP, 3 months ago

Yes, I've made this mistake as well a few times when I downloaded only highways via overpass api. I think we have many newer tickets about this issue?
I think it would be good to warn at least when the nodes of ways with a positive id are changed and there is no download area.

in reply to:  21 comment:22 by anonymous, 3 months ago

Replying to GerdP:

Yes, I've made this mistake as well a few times when I downloaded only highways via overpass api. I think we have many newer tickets about this issue?
I think it would be good to warn at least when the nodes of ways with a positive id are changed and there is no download area.

This would cause many false-positives and not provide a "solution" to users.

My proposal in #18959 was:

  • Store if all parent relations were downloaded as flag on the node/way/relation.
  • We set this flag if a node/way was downloaded by "Download area" or if parent relations were downloaded automatically

That way, we can warn that the way does not have all parent relations downloaded and directly provide a solution to the user: Download those (=> then flag is set) and retry the operation. That way, we do not only provide a warnig but an automated solution and it will probably be not much harder than checking if a way is in a downloaded area (which is not that trivial with multiple areas, the way ways that extend over the download area are handled, ...)

comment:23 by GerdP, 3 months ago

This would cause many false-positives

Why? In what situation?

in reply to:  23 comment:24 by michael2402 -, 3 months ago

Replying to GerdP:

This would cause many false-positives

Why? In what situation?

I don't think the typical case where we need to warn is for people editing just an area. They will download the area and then edit it.

One use case for splitting ways with relations on them without just downloading an area is public transport. In this case, often areas are not downloaded, but instead one downloads only "Relation members". This is also the case described in this ticket. Imageine bus line 1 and 2 share a lot of the way. Typical workflow:

  • Download members of relation "Bus Line 1"
  • Then download referenced relations of that street
  • Then split the street, e.g. for inserting an end stop for one of the bus lines.

In this case, you would get a warning, wether you forgot to do step2 or not. Therefore, the warning won't help there. And those are the critical cases, where data editing is so "complicated" that such errors happen easily.

comment:25 by GerdP, 3 months ago

I would still prefer that JOSM recommends to download an area around the split node.
My work case was this:

  • Download all major highways in an area to set zone:traffic.
  • Note that some ways have a node with traffic_sign=city_limit which is not an end node but very close to that.
  • Merge the two nodes (without downloading relations first)

I ended up with many small gaps in route relations because I simply forgot that JOSM won't warn in that situation.

in reply to:  25 comment:26 by michael2402 -, 3 months ago

Replying to GerdP:

I would still prefer that JOSM recommends to download an area around the split node.

This sould be: Around the original position of the split node (in case the node was moved). And then this won't work if the way was already modified (e.g. merging the node as you mentioned: If the node is not part of the way on the server, downloading the area won't download the way)

And that's why I meant that adding a flag on the way (like "we are sure all relations are loaded") is much easier than handling all the corner cases for downloading the area.

comment:27 by taylor.smock, 3 months ago

Why not add a flag to the DataSource? If a way is not inside fully downloaded bounds, ask the user if we can get referring objects. We could add a flag to the way (and that is arguably the right place to put the flag), but that would increase the memory size of the IPrimitive hierarchy. For reference, AbstractPrimitive has a short flags field instead of booleans.

With that said, now I'm wondering if the short flags still saves memory. I assume it did once, but the JVM has advanced significantly since at least 2011 (when the short flags got moved from OsmPrimitive to AbstractPrimitive). Time for some heap dumps...

EDIT: Using short bits is still more space efficient. Tested with sixteen boolean fields in a class (both record and traditional), a class with a boolean[] field, and a class with a short field. 20k instances of the boolean field class was 640kb. 20k instances of the short field instance was 320kb. The boolean[] field array class was the worst at 960kb. All values were randomly generated to avoid any efficiency shortcuts from the JVM.

Just for the heck of it, I also took a look at JEP 401 value objects. I don't know what is going on, but (as of the Valhalla JDK 20 EA build), it was not as efficient as the short field (it was as efficient as the more standard classes). This should probably be double-checked using real data in production once we move to a JVM with value classes.

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

in reply to:  27 comment:28 by micahel2402, 3 months ago

Replying to taylor.smock:

Why not add a flag to the DataSource? If a way is not inside fully downloaded bounds, ask the user if we can get referring objects. We could add a flag to the way (and that is arguably the right place to put the flag), but that would increase the memory size of the IPrimitive hierarchy. For reference, AbstractPrimitive has a short flags field instead of booleans.

As I stated: The current position of the node and the current ways that belong to the node may have changed after download. And therefore the "inside fully download bounds" check does not reliabely indicate if the node was fully downloaded.

Yes, I mean such a flag on the AbstractPrimitive. I would make it universal: put it on every primitive. Set it when downloaded using "download area" and it was in the orignal bounds. Set it when related objects were downloaded. Read it during merge ways.

Name: "FLAG_PARENTS_COMPLETE". Which means: All ways (for nodes) + relations this primitive is a member of are fetched.

With that said, now I'm wondering if the short flags still saves memory. I assume it did once, but the JVM has advanced significantly since at least 2011 (when the short flags got moved from OsmPrimitive to AbstractPrimitive). Time for some heap dumps...

EDIT: Using short bits is still more space efficient. Tested with sixteen boolean fields in a class (both record and traditional), a class with a boolean[] field, and a class with a short field. 20k instances of the boolean field class was 640kb. 20k instances of the short field instance was 320kb. The boolean[] field array class was the worst at 960kb. All values were randomly generated to avoid any efficiency shortcuts from the JVM.

Just for the heck of it, I also took a look at JEP 401 value objects. I don't know what is going on, but (as of the Valhalla JDK 20 EA build), it was not as efficient as the short field (it was as efficient as the more standard classes). This should probably be double-checked using real data in production once we move to a JVM with value classes.

Object layout has not changed significantly. A boolean is still one byte and byte-aligned, e.g. for performance reasons and for some atomic write operations. Your example did not even get the full benefit, since shorts are still padded and that way your short took more than 16 bit ;-)

Memory layout of the AbstractPrimitive should be to my knowledge (have not checked by docs...):

12 byte object header
2 bytes flags
2 bytes mappaintCacheIdx
8 byte id
4 byte user-reference (8 bytes for large heap)
4 byte changesetId
4 byte timestamp
(potentially 4 byte padding, if no other fields are present in subclass)

comment:29 by skyper, 3 months ago

+1 for a flag

An incomplete list of ticket which would benefit: #10999, #16803, #17400, #19008, #19394, #21211, #22820, #22847

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to Nakor.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.