Performance Improvements#59
Merged
Merged
Conversation
…ites; union_all - gen_hexagons: dedup H3 cell IDs across (multi)polygon parts before materializing shapely Polygons, dropping the post-hoc drop_duplicates. Equal output for single-polygon cities; ~1.05x and avoids redundant Polygon construction on multi-part inputs. - osmnx_coefficient_computation: accumulate per-row stats into a list of dicts and assign once; replaces O(N*K) .loc[i, col] = ... that forced repeated reindex/upcasts. 917 ms -> 4.5 ms on 2000 rows x 4 stats (~200x). - merge_geom_downloads (+ plotting.choropleth_map): switch deprecated GeoSeries.unary_union to GeoSeries.union_all() (shapely 2 path). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…Dijkstra in isochrones - compute_osrm_dist_matrix: replace nested per-pair /route/ requests with a single OSRM /table/v1/ call; preserves the per-pair fallback for older OSRM builds. Local stub: 117 ms -> 1.6 ms for a 10x10 matrix (~74x). With a real OSRM server the gap widens further as the cost is dominated by N*M HTTP round-trips. - Add a module-level requests.Session with urllib3 Retry + connection pooling, reused by osrm_route, ors_api, isochrone_from_api. Removes per-call TCP/TLS handshake overhead and the ad-hoc time.sleep retry. - isochrone_from_graph: run nx.single_source_dijkstra_path_length once per center and threshold the node set for each trip_time, instead of re-running ego_graph for every (center, time) pair. Also swap the GeoSeries(...).unary_union.convex_hull round-trip for a direct shapely.MultiPoint hull. 572 ms -> 145 ms on a 2k-node graph (~3.9x). - type(x) == gpd.GeoSeries -> isinstance(x, gpd.GeoSeries). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- friction: accept array input via np.where; preserves scalar behavior. - hu_access_map: replace the two progress_apply(scalar friction(...), axis=1) passes with numpy distance computation on cached x/y arrays. Synthetic 200k-row distance frame: 2104 ms -> 8.9 ms (~237x). - travel_times: compute geometry.centroid once and reuse for both nn_search inputs and OSRM calls; pull nearest POI geometries up front instead of recomputing centroid + pois.iloc per row inside progress_apply. Centroid handling alone: 577 ms -> 18 ms on a 20k unit gdf (~31x); the remaining cost is the OSRM HTTP call itself (separate /table/ work in routing). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…elation polys - overpass_to_gdf: the per-key NaN fallback loop discarded the result of Series.fillna() (no assignment, no inplace=True), so secondary tag keys never populated 'poi_type'. Assign the result back. Also switch tag[k] if k in tag.keys() else np.NaN -> tag.get(k, np.nan) (faster lookup, no-op .keys() removed, np.NaN deprecated alias). - process_overpass_relations: replace three sequential .apply() passes (shell -> length filter -> Polygon) with a single Python loop, and call shapely.make_valid on the underlying array instead of per-row apply. 5k-way payload: 144 ms -> 111 ms (~1.3x); also avoids a transient 'shell' column and the deprecated GeoSeries-level apply. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- get_hdx_dataset: replace df.apply(lambda r: Point(...), axis=1) with gpd.points_from_xy on the filtered slice; also .copy() the slice to avoid SettingWithCopyWarning, and set crs="EPSG:4326" on the resulting GeoDataFrame. 400k rows: 5231 ms -> 97 ms (~54x). - overpass_pois: tag[k] if k in tag.keys() else np.NaN -> tag.get(k, np.nan) (faster, np.NaN is deprecated, redundant .keys() removed). - hdx_dataset: fix str.ends_with/starts_with -> .endswith/.startswith (the originals would AttributeError on the first call) and the unbound 'hdx_url = hdx_url' branch. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claudio9701
approved these changes
May 23, 2026
Collaborator
Claudio9701
left a comment
There was a problem hiding this comment.
LGTM. One comment for a follow up related to the OSRM the user should be able to set a custom port if needed. we have port 5000 hardcoded in many places
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Performance improvements with minimal changes across modules
Before moving further with publishing in conda and other improvements to the package in general, I think a general QOL update to our packages' performance was important. I went through our modules one by one with Claude Code to figure out where we can gain the most with the least changes. In total, five per-module commits on top of
masterwith the following breakdown:Benchmarks (synthetic data, best of 5)
accessibility.friction(200k rows)geom.osmnx_coefficient_computationwrite pattern (2k × 4)routing.compute_osrm_dist_matrix(10 × 10, local stub)download.get_hdx_datasetPoint construction (400k rows)accessibility.travel_timescentroid handling (20k units)routing.isochrone_from_graph(2k-node graph, 3 × 3)utils.process_overpass_relations(5k ways)geom.gen_hexagons(res-8 over 0.5°²)geom.merge_geom_downloads(6 overlapping gdfs)¹ Local in-process HTTP stub — with a real OSRM server the gap is dominated by N·M HTTP round-trips, so this is a lower bound.
² Bottleneck is
h3.geo_to_cells/cell_to_boundarythemselves; new code's main win is avoiding duplicate Polygon construction on multi-part inputs.³
union_allis the same speed asunary_union— change exists to clear the shapely 2 deprecation warning.Correctness fixes shipped alongside
utils.overpass_to_gdf: the per-key NaN fallback loop discarded the result ofSeries.fillna()(no assignment, noinplace=True), so secondary tag keys never populatedpoi_type. Now correctly fills from fallback keys.download.hdx_dataset: hadresource.ends_with(...)/resource.starts_with(...)(AttributeError on the first call) and an unboundhdx_url = hdx_urlbranch. Both fixed.np.NaN→np.nanandk in tag.keys()→tag.get(k, np.nan)acrossutils.pyanddownload.py.Per-module change detail
geom(commitd6aa459)gen_hexagons: deduplicate H3 cell IDs across (multi)polygon parts before materializing shapely Polygons, dropping the post-hocdrop_duplicates(). Equal output for single-polygon cities; avoids redundant Polygon construction on multi-part inputs.osmnx_coefficient_computation: accumulate per-row stats into a list of dicts and assign once. ReplacesO(N·K).loc[i, col] = ...writes that forced repeated reindex / dtype upcasts.merge_geom_downloads(+plotting.choropleth_map): switch deprecatedGeoSeries.unary_uniontoGeoSeries.union_all()(shapely 2 path).routing(commit0fd8f2c)compute_osrm_dist_matrix: replace nested per-pair/route/requests with a single OSRM/table/v1/call; preserves the per-pair fallback for older OSRM builds.requests.Sessionwith urllib3Retry+ connection pooling, reused byosrm_route,ors_api,isochrone_from_api. Removes per-call TCP/TLS handshake overhead and the ad-hoctime.sleepretry.isochrone_from_graph: runnx.single_source_dijkstra_path_lengthonce per center and threshold the node set for eachtrip_time, instead of re-runningego_graphfor every(center, time)pair. Also swap theGeoSeries(...).unary_union.convex_hullround-trip for a directshapely.MultiPointhull.type(x) == gpd.GeoSeries→isinstance(x, gpd.GeoSeries).accessibility(commit3f93ce2)friction: accept array input vianp.where; preserves scalar behavior.hu_access_map: replace the twoprogress_apply(scalar friction(...), axis=1)passes with numpy distance computation on cached x/y arrays.travel_times: computegeometry.centroidonce and reuse for bothnn_searchinputs and OSRM calls; pull nearest POI geometries up front instead of recomputing centroid +pois.ilocper row insideprogress_apply.utils(commitb30960b)overpass_to_gdf: assign the result ofSeries.fillna()back togdf["poi_type"]. Switchtag[k] if k in tag.keys() else np.NaN→tag.get(k, np.nan).process_overpass_relations: replace three sequential.apply()passes (shell → length filter → Polygon) with a single Python loop, and callshapely.make_validon the underlying array instead of per-row apply.download(commit54d6d90)get_hdx_dataset:df.apply(lambda r: Point(...), axis=1)→gpd.points_from_xyon the filtered slice;.copy()the slice; setcrs="EPSG:4326"on the result.overpass_pois: sametag.get(...)rewrite as utils.hdx_dataset:.ends_with/.starts_with→.endswith/.startswith; fix unboundhdx_url = hdx_urlbranch.Bench methodology
Each comparison runs the old and new implementations side-by-side in the same process against synthetic data sized to make per-call overhead visible (e.g. 200k / 400k / 2k rows). Reported numbers are
best of 5. Network-dependent paths (OSRM/route,/table, Overpass) are exercised against a local in-process HTTP stub so we measure code-side overhead — the real-world gap on OSRM/tablewill be substantially larger because the per-pair path pays N·M HTTP round-trips.