prochitecture / blosm

GNU General Public License v3.0
11 stars 3 forks source link

street.id == 2 is not in manager.streets #106

Open vvoovv opened 3 months ago

vvoovv commented 3 months ago

manager.interStreets does not return the Street with street.id == 2

polarkernel commented 3 months ago

manager.interStreets does not return the Street with street.id == 2

The street with id==2 has a minor intersection at one of its ends. When extracted from waymap by iterStreetsFromWaymap(), it was merged into a (new) long street with id==516, combined with several additional object classes:

2 <class 'way.item.section.Section'>
225 <class 'way.item.intersection.Intersection'>
8 <class 'way.item.section.Section'>
431 <class 'way.item.intersection.Intersection'>
212 <class 'way.item.section.Section'>

The numbers are the ids of the object (the section got the same id==2 as the street, because a section originally defined a street). The street with id==2 still exists in waymap, because waymap is not altered by the extraction, but it is not in the container of extracted streets.

polarkernel commented 3 months ago

There is still a bug somewhere. In _/osm_extracts/streets/milano01.osm there is a street (S 169, blue arrow) in waymap that goes in the opposite direction than the neighboring streets S 168 and S 467:

The merge algorithm tries to follow from S 168 to the right, because there was a major intersection on the left, and keeps getting rejected back to the minor intersection (cyan), in an endless loop. I am currently investigating the source of this bug and trying to find a solution.

vvoovv commented 3 months ago

The same problem with street.id == 2 for _berlin_karl_marxallee.osm but from a different angle.

intersection.id == 19
street = intersection.startConnector.item
street.id == 2

But it must be street.id==516!

So the Street in the Intersection Connector was not updated.

But why are Streets needed in WayMap at all now? I thought long Streets should have been constructed in iterStreetsFromWaymap.

polarkernel commented 3 months ago

But it must be street.id==516!

OK, that's another bug, but at least I know where to fix it.

But why are Streets needed in WayMap at all now? I thought long Streets should have been constructed in iterStreetsFromWaymap.

All streets are extracted by iterStreetsFromWaymap. At the moment you are right, WayMap is not needed anymore, but it was needed before, and maybe it will be needed again, I don't know. Or was it your idea to extract only the long streets and keep the short ones in WayMap? My first idea was to keep only WayMap, remove the short streets used to build longer ones, and replace them with the long ones. It is not clear to me what you are trying to accomplish.

polarkernel commented 3 months ago

But it must be street.id==516!

This bug is fixed in the last commit.

polarkernel commented 3 months ago

I am currently investigating the source of this bug and trying to find a solution.

The real source of this bug is the OSM operator, which drew the street S 169 the wrong way. This case can be detected. I assume it is rare.

A solution might be to correct the direction of such a street. However, this is not easy. The streets and intersections on both sides have to be considered and eventually reversed too. Reversing a street means not only reversing the vertices of its segments, but may also affect some tags like turn:lanes or forward, backward and similar.

Since this is a rare case, I suggest that we detect it and keep such crossings as major intersections and not as minor ones. Then this case is no longer an issue.

polarkernel commented 3 months ago

A new commit includes the new version of createParallelStreets(), now processing also long streets. You can enable it by uncommenting the line 78 in _/action/generate_streets.py_. As already available before, the sets of parallel streets can be displayed individually by setting inBundles (line 350) to True.

Since this is a rare case, I suggest that we detect it and keep such crossings as major intersections and not as minor ones. Then this case is no longer an issue.

For now, I implemented this idea.

vvoovv commented 3 months ago

It is not clear to me what you are trying to accomplish.

I wonder if short Streets are needed at all now to construct long Streets. If the short Streets are not needed at all, then only Sections, Intersections, SideLanes and SymLanes can be stored in the WayMap.

polarkernel commented 3 months ago

I wonder if short Streets are needed at all now to construct long Streets. If the short Streets are not needed at all, then only Sections, Intersections, SideLanes and SymLanes can be stored in the WayMap.

Well, this idea could be realized. Even SideLanes and SymLanes are initially intersections (of order 2), but are then embedded in Streets, together with their leaving sections. Eventually, this could also be done in iterStreetsFromWaymap(), so that WayMap would contain only Sections and Intersections.

vvoovv commented 3 months ago

I think the short Streets is now a legacy concept which still consumes some calculation capacities and memory. I am in favor of getting rid of the short Streets.

polarkernel commented 3 months ago

I think the short Streets is now a legacy concept which still consumes some calculation capacities and memory. I am in favor of getting rid of the short Streets.

Then let's do it. Before I start rewriting, could you please describe, how and in what format you would like to get the results. At first only for Sections, Intersections and long Streets (including those with SideLanes and SymLanes). Later, I would be interested in methods for bundles and intersection clusters.

vvoovv commented 3 months ago

describe, how and in what format you would like to get the results.

I can't propose anything better than the existing format. I'll about the format for Bundles later.

polarkernel commented 2 months ago

I can't propose anything better than the existing format. I'll about the format for Bundles later.

Currently, all data except the normal intersections are provided as instances of Street by WayManager.iterStreets(). My question is about short Streets. Am I right, that finally the remaining single Sections are packed into (short) Streets, and stored there too, or do you see another way to transfer them?

vvoovv commented 2 months ago

Yes, a single Section is also packed into a Street to process all Streets in the same way.

polarkernel commented 2 months ago

I am having some problems defining style attributes, such as number of lanes or lane width, of a Section. Previously they were derived from the Street that contained the Section using styleStore. As these were based on the category of the Section, is there a way to do something similar directly for a Section?

polarkernel commented 2 months ago

I think the short Streets is now a legacy concept which still consumes some calculation capacities and memory. I am in favor of getting rid of the short Streets.

I have to come back to our decision. It is by far less efficient than what we already have. Now, most of the objects are packed and linked correctly the first time they are processed. Only the rare exceptions like minor intersections and long streets have to be built later.

With the new idea, all the connectors and links in the intersections can't be set before the Street instances are built, because they link to them. As you can see from my question above, we have to build style blocks first for Sections and later for Streets again. I found many such efficiency issues during my rewrite.

So I suggest we stick with the concept we already have.

vvoovv commented 2 months ago

Is it my understanding correct of what is going now?

First, each Section is placed in a separate Street, so each Street contains only one Section. Then longer Streets are created for SideLanes. Finally even longer Streets are created for minor Intersections.

On a separate note, I think minor Intersections are not rare for well mapped locations like Berlin.

polarkernel commented 2 months ago

Is it my understanding correct of what is going now?

Yes, this is the state today.

On a separate note, I think minor Intersections are not rare for well mapped locations like Berlin.

Well, Berlin, Karl-Marx-Allee is a small scene. Its proportions of major, minor and other intersections are 169 : 90 : 15. For the scene Moscow-Leningradsky-Prospect for example, they are 1219 : 234 : 45.

First, each Section is placed in a separate Street, so each Street contains only one Section. Then longer Streets are created for SideLanes. Finally even longer Streets are created for minor Intersections.

What if we use only sections at first. The edges of WayMap become naked Sections, without attributes like number of lanes or lane widths. Their nodes are Intersections without IntConnectors. Of course, it is possible to link IntConnectors to Sections, but later, they have to be changed to links to Streets. It also makes no sense to link the Sections to IntConnectors (pred, succ), because later on they have to be replaced, because the Streets have to be connected to IntConnectors.

The most complicated problem arises, when we have to populate the Intersections (major and minor). Either the IntConnectors have to be inserted at the correct position within the circular double-linked list, or, if they have been created using references to Sections, their correct position has to be found within the list first, before the link can be corrected.

My feeling is that it is somehow double the work. I don't see an advantage.

vvoovv commented 2 months ago

My feeling is that it is somehow double the work. I don't see an advantage.

Ok. I agree. Let's stick to the concept we already have.