uscuni / simplification

Simplification of street network geometry
Other
6 stars 0 forks source link

all `code/utils.py` to `core/*` && `evaluate_h3cells` overhaul #69

Closed jGaboardi closed 1 week ago

jGaboardi commented 2 weeks ago

This is a hefty PR that:

martinfleis commented 1 week ago

I will let @anastassiavybornova review this one as it mostly touches her code.

jGaboardi commented 1 week ago

I will let @anastassiavybornova review this one as it mostly touches her code.

LOL Okay lazy bones.

jGaboardi commented 1 week ago

@martinfleis I think we'll actually want your eyes on a couple things for performance that @anastassiavybornova had mentioned when she made her original PR for this stuff. While I have reproduced her outputs (mostly) faithfully with some improved performance there is still the outstanding use of apply in cell 10 of evaluate_h3cells.ipynb as it relates to get_edge_stats() and get_edge_stats() in core/stats.py

martinfleis commented 1 week ago

It looks okay to me. Is there some performance issue? Does it take hours or anything?

jGaboardi commented 1 week ago

It looks okay to me. Is there some performance issue? Does it take hours or anything?

OK, cool. For Auckland that notebook cell takes ≈2 seconds to run. I wasn't sure if there was some easy adjustment or clever spatial join that could be done to cut that time. At any rate, I think good enough for now and we can revisit if it becomes a problem with larger AOIs.

martinfleis commented 1 week ago

If it took 20 minutes I would hesitate if it is worth any further attention. At 2s, you spend more reading this comment than what would you save in the end.