From 1cc8c3cbc6de88f17ea8ae72ffc68a34819adfad Mon Sep 17 00:00:00 2001 From: Thedore-Chatziioannou Date: Tue, 14 May 2024 10:50:57 +0000 Subject: [PATCH 1/7] facility link snapping --- CHANGELOG.md | 1 + src/pam/cli.py | 28 +++++++++++++ src/pam/operations/snap.py | 48 ++++++++++++++++++++++ tests/test_29_snap.py | 33 +++++++++++++++ tests/test_data/test_link_geometry.geojson | 8 ++++ 5 files changed, 118 insertions(+) create mode 100644 src/pam/operations/snap.py create mode 100644 tests/test_29_snap.py create mode 100644 tests/test_data/test_link_geometry.geojson diff --git a/CHANGELOG.md b/CHANGELOG.md index 7755dc9d..eec4f98e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### Added +* Facility link snapping. ### Changed diff --git a/src/pam/cli.py b/src/pam/cli.py index 6123c6f3..823b411e 100644 --- a/src/pam/cli.py +++ b/src/pam/cli.py @@ -9,6 +9,7 @@ from pam import read, write from pam.operations.combine import pop_combine from pam.operations.cropping import simplify_population +from pam.operations.snap import run_facility_link_snapping from pam.report.benchmarks import benchmarks as bms from pam.report.stringify import stringify_plans from pam.report.summary import pretty_print_summary, print_summary @@ -667,3 +668,30 @@ def plan_filter(plan): logger.info("Population wipe complete") logger.info(f"Output saved at {path_population_output}") + + + +@cli.command() +@click.argument("path_population_in", type=click.Path(exists=True)) +@click.argument("path_population_out", type=click.Path(exists=False, writable=True)) +@click.argument("path_network_geometry", type=click.Path(exists=True)) +@click.option( + "--link_id_field", + "-f", + type=str, + default="id", + help="The link ID field to use in the network shapefile. Defaults to 'id'.", +) +def snap_facilities( + path_population_in: str, + path_population_out: str, + path_network_geometry: int, + link_id_field: Optional[str], +): + """Snap facilities to a network geometry.""" + run_facility_link_snapping( + path_population_in=path_population_in, + path_population_out=path_population_out, + path_network_geometry=path_network_geometry, + link_id_field=link_id_field + ) \ No newline at end of file diff --git a/src/pam/operations/snap.py b/src/pam/operations/snap.py new file mode 100644 index 00000000..cabc694b --- /dev/null +++ b/src/pam/operations/snap.py @@ -0,0 +1,48 @@ +""" Methods for snapping elements to the network or facilities. """ + +import geopandas as gp + +from pam.core import Population +from pam.read import read_matsim +from pam.write import write_matsim + + +def snap_facilities_to_network( + population: Population, + network: gp.GeoDataFrame, + link_id_field: str = "id" +) -> None: + """Snaps activity facilities to a network geometry (in-place). + + Args: + population (Population): A PAM popualtion. + network (gp.GeoDataFrame): A network geometry shapefile. + link_id_field (str, optional): The link ID field to use in the network shapefile. Defaults to "id". + """ + link_ids = network[link_id_field] + for _, _, person in population.people(): + for act in person.activities: + link_id = link_ids[network.distance(act.location.loc).argmin()] + act.location.link = link_id + +def run_facility_link_snapping( + path_population_in: str, + path_population_out: str, + path_network_geometry: str, + link_id_field: str = "id" +)->None: + """Reads a population, snaps activity facilities to a network geometry, and saves the results. + + Args: + path_population_in (str): Path to a PAM population. + path_population_out (str): The path to save the output population. + path_network_geometry (str): Path to the network geometry file. + link_id_field (str, optional): The link ID field to use in the network shapefile. Defaults to "id". + """ + population = read_matsim(path_population_in) + if path_network_geometry.endswith(".parquet"): + network = gp.read_parquet(path_network_geometry) + else: + network = gp.read_file(path_network_geometry) + snap_facilities_to_network(population=population, network=network, link_id_field=link_id_field) + write_matsim(population=population, plans_path=path_population_out) \ No newline at end of file diff --git a/tests/test_29_snap.py b/tests/test_29_snap.py new file mode 100644 index 00000000..ff144c58 --- /dev/null +++ b/tests/test_29_snap.py @@ -0,0 +1,33 @@ +import os +from pathlib import Path + +import geopandas as gp +from pam.operations.snap import run_facility_link_snapping, snap_facilities_to_network +from pam.read import read_matsim + +TEST_DATA_DIR = Path(__file__).parent / "test_data" + + +def test_add_snapping_adds_link_attribute(population_heh): + network=gp.read_file(os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson")) + for _, _, person in population_heh.people(): + for act in person.activities: + assert act.location.link is None + + snap_facilities_to_network(population=population_heh, network=network) + for _, _, person in population_heh.people(): + for act in person.activities: + assert act.location.link is not None + +def test_links_resnapped(tmpdir): + path_out=os.path.join(tmpdir, "pop_snapped.xml") + run_facility_link_snapping( + path_population_in=os.path.join(TEST_DATA_DIR, "1.plans.xml"), + path_population_out=path_out, + path_network_geometry=os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson") + ) + assert os.path.exists(path_out) + pop_snapped = read_matsim(path_out) + for _, _, person in pop_snapped.people(): + for act in person.activities: + assert "link-" in act.location.link \ No newline at end of file diff --git a/tests/test_data/test_link_geometry.geojson b/tests/test_data/test_link_geometry.geojson new file mode 100644 index 00000000..652418f5 --- /dev/null +++ b/tests/test_data/test_link_geometry.geojson @@ -0,0 +1,8 @@ +{ +"type": "FeatureCollection", +"crs": { "type": "name", "properties": { "name": "urn:ogc:def:crs:EPSG::2157" } }, +"features": [ +{ "type": "Feature", "properties": { "id": "link-1" }, "geometry": { "type": "LineString", "coordinates": [ [ 10000.0, 5000.0 ], [ 10000.0, 10000.0 ] ] } }, +{ "type": "Feature", "properties": { "id": "link-2" }, "geometry": { "type": "LineString", "coordinates": [ [ 10000.0, 10000.0 ], [ 5000.0, 5000.0 ] ] } } +] +} From 51363ad9daca8e7ab5be76cf318d86756a2fdf55 Mon Sep 17 00:00:00 2001 From: Thedore-Chatziioannou Date: Tue, 14 May 2024 11:06:30 +0000 Subject: [PATCH 2/7] update dependencies --- requirements/dev.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/dev.txt b/requirements/dev.txt index 1dd240cf..2e9310f8 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,10 +1,10 @@ cruft >= 2, < 3 jupyter < 2 mike >= 2, < 3 -mkdocs < 2 +mkdocs < 1.6 mkdocs-material >= 9.4, < 10 -mkdocs-click < 0.7 -mkdocs-jupyter < 0.25 +mkdocs-click >= 0.6, < 0.7 +mkdocs-jupyter < 0.24.7 mkdocstrings-python < 2 nbmake >= 1.5.1, < 2 pre-commit < 4 From 172d8e6f717d10daf05560e2cec4b04c4d9bf2c9 Mon Sep 17 00:00:00 2001 From: Thedore-Chatziioannou Date: Tue, 14 May 2024 11:07:35 +0000 Subject: [PATCH 3/7] update cruft --- .cruft.json | 2 +- requirements/dev.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.cruft.json b/.cruft.json index 8b4583a6..f0ee6048 100644 --- a/.cruft.json +++ b/.cruft.json @@ -1,6 +1,6 @@ { "template": "https://github.com/arup-group/cookiecutter-pypackage.git", - "commit": "fdc37ca81ad3f267199b04856722ea68648b9109", + "commit": "b7724521f898ab28f541d492aff8e2be2ed76a01", "checkout": null, "context": { "cookiecutter": { diff --git a/requirements/dev.txt b/requirements/dev.txt index 2e9310f8..57383799 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -3,7 +3,7 @@ jupyter < 2 mike >= 2, < 3 mkdocs < 1.6 mkdocs-material >= 9.4, < 10 -mkdocs-click >= 0.6, < 0.7 +mkdocs-click < 0.7 mkdocs-jupyter < 0.24.7 mkdocstrings-python < 2 nbmake >= 1.5.1, < 2 From fd3e3e3ef116418c6533eb0ebc2e4ace03a7ae2d Mon Sep 17 00:00:00 2001 From: Thedore-Chatziioannou Date: Tue, 14 May 2024 11:23:23 +0000 Subject: [PATCH 4/7] linting --- src/pam/cli.py | 5 ++--- src/pam/operations/snap.py | 17 ++++++++--------- tests/test_29_snap.py | 13 +++++++------ 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/pam/cli.py b/src/pam/cli.py index 823b411e..1e270c5d 100644 --- a/src/pam/cli.py +++ b/src/pam/cli.py @@ -670,7 +670,6 @@ def plan_filter(plan): logger.info(f"Output saved at {path_population_output}") - @cli.command() @click.argument("path_population_in", type=click.Path(exists=True)) @click.argument("path_population_out", type=click.Path(exists=False, writable=True)) @@ -693,5 +692,5 @@ def snap_facilities( path_population_in=path_population_in, path_population_out=path_population_out, path_network_geometry=path_network_geometry, - link_id_field=link_id_field - ) \ No newline at end of file + link_id_field=link_id_field, + ) diff --git a/src/pam/operations/snap.py b/src/pam/operations/snap.py index cabc694b..439ab378 100644 --- a/src/pam/operations/snap.py +++ b/src/pam/operations/snap.py @@ -8,9 +8,7 @@ def snap_facilities_to_network( - population: Population, - network: gp.GeoDataFrame, - link_id_field: str = "id" + population: Population, network: gp.GeoDataFrame, link_id_field: str = "id" ) -> None: """Snaps activity facilities to a network geometry (in-place). @@ -25,12 +23,13 @@ def snap_facilities_to_network( link_id = link_ids[network.distance(act.location.loc).argmin()] act.location.link = link_id + def run_facility_link_snapping( - path_population_in: str, - path_population_out: str, - path_network_geometry: str, - link_id_field: str = "id" -)->None: + path_population_in: str, + path_population_out: str, + path_network_geometry: str, + link_id_field: str = "id", +) -> None: """Reads a population, snaps activity facilities to a network geometry, and saves the results. Args: @@ -45,4 +44,4 @@ def run_facility_link_snapping( else: network = gp.read_file(path_network_geometry) snap_facilities_to_network(population=population, network=network, link_id_field=link_id_field) - write_matsim(population=population, plans_path=path_population_out) \ No newline at end of file + write_matsim(population=population, plans_path=path_population_out) diff --git a/tests/test_29_snap.py b/tests/test_29_snap.py index ff144c58..72976d1e 100644 --- a/tests/test_29_snap.py +++ b/tests/test_29_snap.py @@ -9,7 +9,7 @@ def test_add_snapping_adds_link_attribute(population_heh): - network=gp.read_file(os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson")) + network = gp.read_file(os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson")) for _, _, person in population_heh.people(): for act in person.activities: assert act.location.link is None @@ -19,15 +19,16 @@ def test_add_snapping_adds_link_attribute(population_heh): for act in person.activities: assert act.location.link is not None + def test_links_resnapped(tmpdir): - path_out=os.path.join(tmpdir, "pop_snapped.xml") + path_out = os.path.join(tmpdir, "pop_snapped.xml") run_facility_link_snapping( - path_population_in=os.path.join(TEST_DATA_DIR, "1.plans.xml"), - path_population_out=path_out, - path_network_geometry=os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson") + path_population_in=os.path.join(TEST_DATA_DIR, "1.plans.xml"), + path_population_out=path_out, + path_network_geometry=os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson"), ) assert os.path.exists(path_out) pop_snapped = read_matsim(path_out) for _, _, person in pop_snapped.people(): for act in person.activities: - assert "link-" in act.location.link \ No newline at end of file + assert "link-" in act.location.link From 21c362b413a9ceeaac4f3205b1ca5a8bc83cdc12 Mon Sep 17 00:00:00 2001 From: Thedore-Chatziioannou Date: Wed, 15 May 2024 10:25:14 +0000 Subject: [PATCH 5/7] comments --- CHANGELOG.md | 2 +- src/pam/operations/snap.py | 6 ++++-- tests/test_29_snap.py | 19 +++++++++++++------ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eec4f98e..a7b4e56b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### Added -* Facility link snapping. +* Facility link snapping (#276). ### Changed diff --git a/src/pam/operations/snap.py b/src/pam/operations/snap.py index 439ab378..3ce39deb 100644 --- a/src/pam/operations/snap.py +++ b/src/pam/operations/snap.py @@ -1,5 +1,7 @@ """ Methods for snapping elements to the network or facilities. """ +from pathlib import Path + import geopandas as gp from pam.core import Population @@ -13,7 +15,7 @@ def snap_facilities_to_network( """Snaps activity facilities to a network geometry (in-place). Args: - population (Population): A PAM popualtion. + population (Population): A PAM population. network (gp.GeoDataFrame): A network geometry shapefile. link_id_field (str, optional): The link ID field to use in the network shapefile. Defaults to "id". """ @@ -39,7 +41,7 @@ def run_facility_link_snapping( link_id_field (str, optional): The link ID field to use in the network shapefile. Defaults to "id". """ population = read_matsim(path_population_in) - if path_network_geometry.endswith(".parquet"): + if ".parquet" in Path(path_network_geometry).suffixes: network = gp.read_parquet(path_network_geometry) else: network = gp.read_file(path_network_geometry) diff --git a/tests/test_29_snap.py b/tests/test_29_snap.py index 72976d1e..ff972806 100644 --- a/tests/test_29_snap.py +++ b/tests/test_29_snap.py @@ -1,15 +1,13 @@ import os -from pathlib import Path import geopandas as gp +import pytest from pam.operations.snap import run_facility_link_snapping, snap_facilities_to_network from pam.read import read_matsim -TEST_DATA_DIR = Path(__file__).parent / "test_data" - def test_add_snapping_adds_link_attribute(population_heh): - network = gp.read_file(os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson")) + network = gp.read_file(pytest.test_data_dir / "test_link_geometry.geojson") for _, _, person in population_heh.people(): for act in person.activities: assert act.location.link is None @@ -19,13 +17,22 @@ def test_add_snapping_adds_link_attribute(population_heh): for act in person.activities: assert act.location.link is not None + # check that the link is indeed the nearest one + link_distance = ( + network.set_index("id") + .loc[act.location.link, "geometry"] + .distance(act.location.loc) + ) + min_distance = network.distance(act.location.loc).min() + assert link_distance == min_distance + def test_links_resnapped(tmpdir): path_out = os.path.join(tmpdir, "pop_snapped.xml") run_facility_link_snapping( - path_population_in=os.path.join(TEST_DATA_DIR, "1.plans.xml"), + path_population_in=pytest.test_data_dir / "1.plans.xml", path_population_out=path_out, - path_network_geometry=os.path.join(TEST_DATA_DIR, "test_link_geometry.geojson"), + path_network_geometry=pytest.test_data_dir / "test_link_geometry.geojson", ) assert os.path.exists(path_out) pop_snapped = read_matsim(path_out) From baffc5e4cffd880b6e7e927273efe366b1b82189 Mon Sep 17 00:00:00 2001 From: syhwawa Date: Fri, 2 Aug 2024 15:55:32 +0100 Subject: [PATCH 6/7] Update the snap function using KDTree --- src/pam/operations/snap.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/pam/operations/snap.py b/src/pam/operations/snap.py index 3ce39deb..c9e03606 100644 --- a/src/pam/operations/snap.py +++ b/src/pam/operations/snap.py @@ -3,10 +3,12 @@ from pathlib import Path import geopandas as gp +import numpy as np from pam.core import Population from pam.read import read_matsim from pam.write import write_matsim +from scipy.spatial import cKDTree def snap_facilities_to_network( @@ -19,11 +21,29 @@ def snap_facilities_to_network( network (gp.GeoDataFrame): A network geometry shapefile. link_id_field (str, optional): The link ID field to use in the network shapefile. Defaults to "id". """ - link_ids = network[link_id_field] + if network.geometry.geom_type[0] == 'Point': + coordinates = np.array(list(zip(network.geometry.x, network.geometry.y))) + else: + coordinates = np.array(list(zip(network.geometry.centroid.x, network.geometry.centroid.y))) + + tree = cKDTree(coordinates) + link_ids = network[link_id_field].values + + activity_points = [] + activities_info = [] for _, _, person in population.people(): for act in person.activities: - link_id = link_ids[network.distance(act.location.loc).argmin()] - act.location.link = link_id + point = act.location.loc + if not hasattr(point, 'x') or not hasattr(point, 'y'): + point = point.centroid + activity_points.append((point.x, point.y)) + activities_info.append(act) + + activity_points = np.array(activity_points) + distances, indices = tree.query(activity_points) + + for act, index in zip(activities_info, indices): + act.location.link = link_ids[index] def run_facility_link_snapping( From fda1ccca16e9471dc025a67dda1ea35e4cdcc914 Mon Sep 17 00:00:00 2001 From: syhwawa Date: Tue, 6 Aug 2024 12:03:41 +0100 Subject: [PATCH 7/7] Update the code to address the comment --- src/pam/operations/snap.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/pam/operations/snap.py b/src/pam/operations/snap.py index c9e03606..d3bb340f 100644 --- a/src/pam/operations/snap.py +++ b/src/pam/operations/snap.py @@ -4,11 +4,11 @@ import geopandas as gp import numpy as np +from scipy.spatial import cKDTree from pam.core import Population from pam.read import read_matsim from pam.write import write_matsim -from scipy.spatial import cKDTree def snap_facilities_to_network( @@ -21,7 +21,7 @@ def snap_facilities_to_network( network (gp.GeoDataFrame): A network geometry shapefile. link_id_field (str, optional): The link ID field to use in the network shapefile. Defaults to "id". """ - if network.geometry.geom_type[0] == 'Point': + if network.geometry.geom_type[0] == "Point": coordinates = np.array(list(zip(network.geometry.x, network.geometry.y))) else: coordinates = np.array(list(zip(network.geometry.centroid.x, network.geometry.centroid.y))) @@ -29,21 +29,11 @@ def snap_facilities_to_network( tree = cKDTree(coordinates) link_ids = network[link_id_field].values - activity_points = [] - activities_info = [] for _, _, person in population.people(): for act in person.activities: point = act.location.loc - if not hasattr(point, 'x') or not hasattr(point, 'y'): - point = point.centroid - activity_points.append((point.x, point.y)) - activities_info.append(act) - - activity_points = np.array(activity_points) - distances, indices = tree.query(activity_points) - - for act, index in zip(activities_info, indices): - act.location.link = link_ids[index] + distance, index = tree.query([(point.x, point.y)]) + act.location.link = link_ids[index[0]] def run_facility_link_snapping(