-
Notifications
You must be signed in to change notification settings - Fork 417
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
perf(fastisochrones): apply modifications from regular isochrones #1658
Conversation
8a81b37
to
56ce464
Compare
92357b4
to
2e2564d
Compare
Comprehensive assessment of the changes introduced by this PR:
To summarize, changes introduced to the preparation part (constructing cell geometries) doesn't seem to have much impact on the overall performance, neither the actual build times, nor the query times. Some small differences in the resulting geometries are observed after switching the algorithm from
each of which individually improves the performance by around 18% and 17%, respectively. The combined performance gain introduced by this PR is around 33%. Last but not least, there now seem to be a better overlap with the geometries produced by regular isochrones. |
Replace `ConcaveHullOpenSphere` with jts `ConcaveHull` algorithm.
Remove neighbour search when adding points.
…d concave isochrone builder
It is not necessary to split long edges before storing cell geometries. This is so because at query time all fully reachable cell geometries are being merged, and the resulting polygon's edges split afterwards. This optimization reduces the cell storage by around 30%.
It is not necessary to split long edges of active cells' concave hulls because the resulting polygons are being merged with the fully reachable cells, after which the splitting has to be performed anyways.
Avoids speed reduction on ways tagged with `access=destination`.
01b2283
to
c4e48bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor questions.
ors-engine/src/main/java/org/heigit/ors/isochrones/builders/fast/FastIsochroneMapBuilder.java
Show resolved
Hide resolved
ors-engine/src/main/java/org/heigit/ors/routing/graphhopper/extensions/ORSGraphHopper.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Pull Request Checklist
have been resolved.
[Unreleased] heading.
along with a short description of what it is for, and documented this in the Pull Request (below).
(at least Germany), and the graphs build without problems (i.e. no out-of-memory errors).
importer etc.), I have generated longer distance routes for the affected profiles with different options
(avoid features, max weight etc.) and compared these with the routes of the same parameters and start/end
points generated from the current live ORS.
If there are differences then the reasoning for these MUST be documented in the pull request.
and why the change was needed.
Information about the changes
Build- and query-time optimizations based on the previous work on regular isochrones in #1607:
Examples and reasons for differences between live ORS routes, and those generated from this pull request
Changes to isochrone geometries in API tests (blue is reference, red is new)
testIntersections
testPolygon
testSmoothing