Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intersection consolidation: geometry #62

Closed
dabreegster opened this issue May 19, 2021 · 14 comments
Closed

Intersection consolidation: geometry #62

dabreegster opened this issue May 19, 2021 · 14 comments

Comments

@dabreegster
Copy link
Contributor

There are some other issues already for merging short roads, but I want to dump some recent notes about the geometry problem in particular.

Current approach

Code at https://github.com/a-b-street/abstreet/blob/master/map_model/src/make/merge_intersections.rs

Screenshot from 2021-05-18 18-10-34

The RawMap produced from OSM looks something like this. The pink segments have been manually tagged by me as junction=intersection in OSM, which often requires splitting the way. It's not too hard to auto-detect these segments and avoid the tagging. The problem is that the current algorithm for consolidating these intersections breaks in most cases, so I'm using the tagging as a way to manually opt in a few cases that I've worked on and verified turn out reasonably.

The current algorithm will take each pink segment in a pretty arbitrary order and do the following. It'll delete the segment and one of the intersections (arbitrarily), preserving the other intersection. It takes all of the roads connected to the deleted intersection, and reassigns them to be connected to the surviving intersection. It does some work to preserve the various types of OSM turn restrictions, some of which are implemented, some not. But then it also modifies the polyline geometry of some of the roads, to account for the deleted segment. The animation explains best:

screencast

The top intersection was deleted, so those 3 connecting roads have their polyline extended to roughly cover the deleted segment.

Then we proceed with map generation as usual, and the intersection geometry algorithm described elsewhere sometimes produces something half-reasonable:
Screenshot from 2021-05-18 18-15-35

But often it does not:
Screenshot from 2021-05-18 15-25-35
Screenshot from 2021-05-18 15-25-24

New idea

Do we always want to extend the geometry of surviving roads by that one new point? Maybe we don't need to extend any polylines at all, or maybe we should only extend the ones of non-pink segments, aka, normal roads that'll survive this process repeating. Maybe we should move the last point instead of adding, or add all of the points of the short road removed.

I added 2 new enums to quickly explore all of these combinations. One example worked much better for the case above:
Screenshot from 2021-05-18 15-38-01
But applying that policy everywhere broke another case that was previously working:
Screenshot from 2021-05-18 15-39-43

So is there a way to identify when we should AddOnePoint and when we should ModifyWhichRoads::None?

Wild alternative number 1

I've probably tried some form of this in the past. Often the original intersection geometries turn out reasonably:
Screenshot from 2021-05-18 18-20-09
What if we used these to decide how much to trim back the surviving roads, then deleted the pink segments, did the turn restriction fixing, but didn't modify any geometry? And mark that these intersections were "pre-trimmed" and skip part of the intersection geometry algorithm, and just construct the final polygon.

If this even works, would it blow up with road widening?

Wild alternative number 2

https://wiki.openstreetmap.org/wiki/Tag:junction%3Dyes#How_to_use_on_an_area

There are a few different proposals for tagging intersections as areas in OSM. I haven't checked Overpass yet, but I'm pretty sure I've seen a few of these mapped before. This particular scheme is purely additive; it shouldn't break any existing OSM software. Maybe in these really hard cases, it'd be much faster to just draw the polygon by hand in OSM from satellite imagery, and make the importer understand these as overrides. Haven't thought through how that import would work. Also not sure what to do if abst infers very different road width than reality and winds up disagreeing strongly with the human-mapped polygon.

dabreegster referenced this issue in a-b-street/abstreet Jun 5, 2021
…#654

Problems at i558, i147 and the Mill/13th one still crashes
@dabreegster
Copy link
Contributor Author

dabreegster commented Jun 8, 2021

Alright this is pretty exciting. I got "Wild alternative number 1" pretty much working in https://github.com/a-b-street/abstreet/tree/consolidate_differently. There are some rough edges, but I'm convinced this is the way forward. I'm focusing on consolidating all the intersections in the Tempe map first, while not breaking anything in Seattle.

My plan:

  • Fix a few Tempe intersections upstream in OSM; no amount of code can work around some of the weird mapping there.
  • Just import that using the old/current code. There should be some improvements, maybe even a full day without gridlock.
  • Fix some more stuff with the consolidate_differently branch: restore U-turn handling, figure out how all of this interacts with road widening
  • Locally opt in more/all junctions in Tempe and try the old/new code. If the new code does much better, proceed.
  • Sanity check the new code doesn't badly break existing Seattle maps.
  • Regenerate everything with the new code, commit it.
  • Tag more junctions in Tempe in OSM, regenerate.

Then assuming everything's good, lots of exciting cleanup:

  • Improve the intersection geometry for the consolidated intersections; I'll describe the problem more in the upcoming PR, but there are visual "gaps" and the polygon's exterior isn't a proper Ring.
  • Make Tempe the "level 1" for the traffic signal challenge
  • Manually try to consolidate lots of intersections in some of the tricky Seattle maps, particularly downtown
  • Revive the effort to automatically consolidate intersections by looking for roads shorter than some threshold. Then we can stop manually tagging junctions in OSM, if indeed the new algorithm is robust enough.
  • Very cautious optimism here: if we can robustly consolidate intersections, then it's worth auditing all open issues and reviewing some design decisions / hacks that've been trying to work around this problem. Particularly some of the core sim logic is overly complicated to try to deal with short roads.
  • Do we need uber-turn logic in simulations anymore?

dabreegster referenced this issue in a-b-street/abstreet Jul 7, 2021
…swapped quickly. Using this for more rapidly testing intersection consolidation algorithms. #654
dabreegster referenced this issue in a-b-street/abstreet Jul 11, 2021
dabreegster referenced this issue in a-b-street/abstreet Jul 11, 2021
It maintains a JSON file of ways to merge that the importer also uses.
For maps fast to import, this is the nicest workflow. Unlike map_editor,
turns and trimmed roads can also be checked.
dabreegster referenced this issue in a-b-street/abstreet Jul 11, 2021
dabreegster referenced this issue in a-b-street/abstreet Jul 19, 2021
dabreegster referenced this issue in a-b-street/abstreet Jul 19, 2021
…gorithm... #654

Phinney gridlocks again; 68daa68 was
too ambitious. Otherwise all fine!
dabreegster referenced this issue in a-b-street/abstreet Jul 19, 2021
…gorithm... #654

Phinney gridlocks again; 68daa68 was
too ambitious. Otherwise all fine!
dabreegster referenced this issue in a-b-street/abstreet Jul 20, 2021
… map_editor's ability to preview one intersection with the debugging geometry. #654
dabreegster referenced this issue in a-b-street/abstreet Jul 20, 2021
dabreegster referenced this issue in a-b-street/abstreet Jul 20, 2021
intersection. This often happens with a group of 4 intersections (two
divided highways), and there may be many small segments embedded in the
middle for street car tracks and such.

Also bring in fresh OSM for Tempe, with one such intersection now
consolidated! #654, #672
dabreegster referenced this issue in a-b-street/abstreet Jul 21, 2021
…into merging! #654

Manually invoked heuristic for the moment, and has some issues, but a
start.
dabreegster referenced this issue in a-b-street/abstreet Jul 27, 2021
dabreegster referenced this issue in a-b-street/abstreet Jul 30, 2021
…eft-handed maps. #654

Not regenerating yet. (There shouldn't be many junctions tagged on
non-US maps, at least not by me.)
dabreegster referenced this issue in a-b-street/abstreet Feb 20, 2022
dabreegster referenced this issue in a-b-street/abstreet Feb 20, 2022
…n of short roads from transforming them. #654

And add a convenient script to diff changed maps. (Motivated by
preserving the fixpoint behavior in Montlake)
dabreegster referenced this issue in a-b-street/abstreet Feb 20, 2022
Borders now seem to be skipped correctly.

Having a hard time skipping dual carriageways based on angle.

Aiming to enable first for Montlake, then gradually rollout to
individual maps, solving problems very incrementally and without
regressions.
@dabreegster
Copy link
Contributor Author

dabreegster commented Feb 20, 2022

Making progress here. Approximate next steps:

@Robinlovelace
Copy link
Contributor

Just had a quick look at the issues and possible solutions above. No specific input but just wanted to say big general 👍 on progress. The hardest problems in software are often the last ones to be tackled but seems there could be benefits of 'grasping the nettle' on this one. If you want any input from me, or before/after tests, happy to help.

@dabreegster
Copy link
Contributor Author

I'm now enabling it for map, and aiming to gradually rollout to more! Been working on merging "dog-leg" junctions. Before:
Screenshot from 2022-02-20 16-32-45
After:
Screenshot from 2022-02-20 16-32-48

Before:
Screenshot from 2022-02-20 16-33-38
After:
Screenshot from 2022-02-20 16-33-40

Treating this as a single 4-way intersection instead of two 3-way intersections with a vanishingly tiny road in between is a vast win for things downstream: rendering, LTN blockfinding logic, simulation for vehicles to not get stuck, editing the traffic signal as one logical object, etc. I've made so many attempts at this kind of fix before, but the problem is that the fix works in some areas, and breaks badly in others. The difference this time around is that I've got a much smoother development workflow using the map_editor (with ~3s recompilation in release mode!!!) and a clear separation of "find a problematic area" and "fix it"

@dabreegster
Copy link
Contributor Author

The next problem:
Screenshot from 2022-02-20 16-45-24
The blue road is short, we should be able to merge it. But if we try, intersection geometry crashes:

Can't get_slice_starting_at: PolyLine::new(vec![     // length 193.1375m
  Pt2D::new(669.682, 2846.0703),
  Pt2D::new(668.5644, 3039.2046),    // -1.1176000000000386, 193.13430000000017 (+ 193.1375m @ Angle(90.33154622249592 degrees))
]) doesn't contain Pt2D(669.7136, 2843.3807)

If we first delete the yellow road, it works fine. If I make the PolyLine routine not panic when given a point not on that polyline and just skip the process of pre-trimming the road, it looks like it works:
Screenshot from 2022-02-20 16-49-32
The road that can't be pretrimmed is the long vertical one:
Screenshot from 2022-02-20 16-52-04
We're trying to pre-trim it to a point a few meters above where my cursor is. That point isn't on the polyline, so we can't pretrim there. What's going on here?

@Robinlovelace
Copy link
Contributor

Treating this as a single 4-way intersection instead of two 3-way intersections with a vanishingly tiny road in between is a vast win for things downstream: rendering, LTN blockfinding logic, simulation for vehicles to not get stuck, editing the traffic signal as one logical object, etc.

Win-win-win-win solution!

@Robinlovelace
Copy link
Contributor

We're trying to pre-trim it to a point a few meters above where my cursor is. That point isn't on the polyline, so we can't pretrim there. What's going on here?

Not sure, could spout some suggestions but don't feel well enough informed to do so at this point.

@dabreegster
Copy link
Contributor Author

I don't understand the pre-trimming crash at all, but I think I'm going to take the easy way out for now, avoid crashing, not pre-trim, and just plow forward. It works in this case:
Screenshot from 2022-02-20 16-55-45
and we've been doing it all along for editing roads near merged intersections: https://github.com/a-b-street/abstreet/blob/efe5f76bbe0ee804fcbd31dbee8de146e8e05b35/map_model/src/edits/mod.rs#L622

Now attempting to roll out the de-dog-leggification to all maps...

Also, another good before:
Screenshot from 2022-02-20 16-55-23
After:
Screenshot from 2022-02-20 16-55-25

@dabreegster
Copy link
Contributor Author

Not sure, could spout some suggestions but don't feel well enough informed to do so at this point.

Feel free to, but no pressure. https://a-b-street.github.io/docs/tech/map/geometry/index.html#a-solution-two-passes describes what's happening here. It takes me about 3 days of full concentration to shove all of this back into my head; I don't find it simple. :\ Which is partly why progress is so slow.

@Robinlovelace
Copy link
Contributor

Also, another good before:
Screenshot from 2022-02-20 16-55-23
After:
Screenshot from 2022-02-20 16-55-25

So good to see!

@Robinlovelace
Copy link
Contributor

Robinlovelace commented Feb 20, 2022

It's pretty intense computational geometry going on here! Over my head and above my pay grade. Could help drum up interest in a call out to computational geometers interested in programming on Twitter and other platforms if that becomes appropriated at any point. As with many domains of human interest, there's a surprisingly large community of them, not least the 100s of people involved, directly and indirectly, to CGAL: https://www.cgal.org/people.html and the source code: https://github.com/CGAL/cgal People love a challenge.

@dabreegster
Copy link
Contributor Author

More motivation why this has downstream effects. When the geometry is broken enough, the LTN tool can't trace blocks, and the whole idea just kind of keels over. This is a "base" problem I've been trying to solve more robustly since forever. Before:
Screenshot from 2022-02-20 17-10-00
After:
Screenshot from 2022-02-20 17-10-15
Note the line artifact exploding out.

Also, I declared victory too soon -- many map crashing trying to use this new code. I'll keep iterating...

Could help drum up interest in a call out to computational geometers interested in programming on Twitter and other platforms if that becomes appropriated at any point

That would be fantastic, but let me split out the code / problem first: #3. The first steps I took yesterday towards this were what let me make progress today.

@Robinlovelace
Copy link
Contributor

Awesome. Interested to see what happens when a call out to the computational geometry community is made, will let you take the lead on that but of course happy to help out where I can.

@dabreegster
Copy link
Contributor Author

Next thing to fix is merges near roads that loop, like https://www.openstreetmap.org/way/26815503 and https://www.openstreetmap.org/way/13868806

dabreegster referenced this issue in a-b-street/abstreet Feb 20, 2022
…ly not on a road's center line, just skip instead of crashing. This allows us to roll out the dog-leg intersection merging for many more maps and see pretty much universal improvement in map geometry! #654 [rebuild] [release]
@dabreegster dabreegster transferred this issue from a-b-street/abstreet Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants