oturns / geosnap

The Geospatial Neighborhood Analysis Package
https://oturns.github.io/geosnap-guide
BSD 3-Clause "New" or "Revised" License
241 stars 32 forks source link

shapely backend for isochrone #398

Closed knaaptime closed 6 months ago

knaaptime commented 6 months ago

this allows the use of concave hull from shapely in addition to alpha shapes from libpysal for isochrone generation. The concave hull is faster, and can be much tighter. Thats not a very fair comparison since we're using 'auto' alpha shapes, but the shapely's hull seems easier to tune than the manual auto shape (good results around ratio=0.2)

also resolves #399

knaaptime commented 6 months ago

i'm not in love with the algorithm={'alpha','hull'}, terminology since an alpha shape is one type of concave hull. Maybe it would be better to use backend={'libpysal', 'shapely'}?

knaaptime commented 6 months ago

@ljwolf two quick questions if you have a minute:

  1. being more engaged with the cg literature, do you have an opinion on the algorithm/backend/hull parameter discussion? At the moment I'm partial to hull as an argument with options for shapely and libpysal
  2. slightly off-topic, but one thing ive used this adj builder for recently is dumping quickly (and nicely) into a Graph, which works at the moment, but only because the Network is denser than the actual observations that get snapped to it. Once there are duplicate observations at each network node, we'll need something like the clique expander... do you think that would be easy to hook up here?
ljwolf commented 6 months ago

On 1, I think I would prefer to deprecate the libpysal.cg.alpha_shape functionality 😅 Now that it's present in shapely, I think we should move away from it for further implementation. The one nice thing about our implementation is that it can construct the binding circles, but that's not enough to really use it over shapely's.

On 2, yes, you'd need to convert the dict(zip()) to build a proper one OSMID-to-many OriginalID lookup table. Then, the clique expander would use that with the OSMID adjacency to create the OriginalID-to-ID adjacency.

I think the current implementation will silently drop any points if they are snapped to the same OSMID:

import numpy

# many origins have the same OSMID
osmids = list(numpy.arange(5).astype(str))*3
origins = numpy.arange(15)

# but only the last one enters the lookup
dict(zip(osmids, origins))

# what _induce_cliques() needs is like:
pandas.DataFrame.from_dict(
    dict(osm=osmids, origins=origins)
).groupby("osm").origins.agg(list).to_dict()
ljwolf commented 6 months ago

Oh, and _induce_cliques() should be able to correctly rename an input adjacency table even if there's a one-to-one mapping!

knaaptime commented 6 months ago

@ljwolf dope, thank you. that's straightforward

(on 1, i made shapely the default with ratio=0.2, which works nicely for this application. On the binding circles, I we should be able to grab that from geos now? though not sure exactly which options are exposed in shapely. Doubt its worth reimplementing either)