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

ENH: reimplement tessellation using shapely #538

Closed
wants to merge 5 commits into from

Conversation

martinfleis
Copy link
Member

WIP but f****ing hell how the time has changed what is possible now.

Performance comparison on 10k buildings - newWall time: 35 s, old Wall time: 47.7 s

Still space for improvements I think. What is great is that GEOS gives us directly polygons which we know are topologically clean.

@martinfleis martinfleis added this to the 0.8.0 milestone Jan 19, 2024
@martinfleis martinfleis self-assigned this Jan 19, 2024
@martinfleis
Copy link
Member Author

Got it down to Wall time: 10.6 s in 2ca9832 :)

Copy link

codecov bot commented Jan 19, 2024

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (29c0e5d) 97.5% compared to head (9de5899) 97.1%.
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #538     +/-   ##
=======================================
- Coverage   97.5%   97.1%   -0.5%     
=======================================
  Files         26      27      +1     
  Lines       4189    4210     +21     
=======================================
  Hits        4086    4086             
- Misses       103     124     +21     
Files Coverage Δ
momepy/functional/_elements.py 0.0% <0.0%> (ø)

@martinfleis
Copy link
Member Author

Squeezed it under 10s to 9.77, 5x times faster than before with cleaner geometry (it resolves #537).

It is at the moment less robust in terms of input data. Original tessellation does not error on overlapping polygons and similar issues. It produces erroneous geometry but produces something. This fails on GEOSException. I am not sure which way is better but I tend to say that geometries should be cleaned before running tessellation. We can probably provide some tooling for that, maybe on top of @sjsrey's geoplanar.

Still missing support of LineString tessellation (edge points conflict) and enclosed tessellation.

@martinfleis
Copy link
Member Author

I'm thinking that since this is fairly generic polygon-based Voronoi tessellation it may be better to have it in libpysal.cg, no? Rather than hidden in momepy.

@martinfleis
Copy link
Member Author

When passing a limit, the main bottleneck becomes clip. See the profiling below. Might be worth looking in geopandas/geopandas#1803 (comment) but the issue is not union here but a ton of intersections that is essentially void since the whole geom is within the limit. I might need to look into clip in geopandas and try to fix this over there.

Screenshot 2024-01-20 at 17 20 58

@martinfleis
Copy link
Member Author

Will be superseded by pysal/libpysal#678

@martinfleis
Copy link
Member Author

Superseded by pysal/libpysal#678 and #559

@martinfleis martinfleis deleted the tess branch April 24, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant